From dc8de001df23f2263bbe2e5e3935f85a2be2ef5e Mon Sep 17 00:00:00 2001 From: Seigo Nonaka Date: Tue, 25 Oct 2016 00:49:52 +0000 Subject: [PATCH] Revert "Clean Up: Removing unused interface GetTable from MinikinFont." This reverts commit 7b02f5e95c4386c250f6f83db1ff61b69177140e. This causes a crash on Android Auto. Bug: 32374752 Change-Id: Ia2ff77bf9a12351c6949f79ef6fa2d8016e3022d --- .../src/flutter/include/minikin/MinikinFont.h | 2 + .../src/flutter/libs/minikin/HbFontCache.cpp | 31 ++++++++--- .../unittest/FontCollectionItemizeTest.cpp | 8 +-- .../tests/unittest/FontCollectionTest.cpp | 2 +- .../flutter/tests/unittest/FontFamilyTest.cpp | 4 +- .../tests/unittest/HbFontCacheTest.cpp | 8 +-- .../src/flutter/tests/util/FontTestUtils.cpp | 5 +- .../flutter/tests/util/MinikinFontForTest.cpp | 54 ++++++++++++------- .../flutter/tests/util/MinikinFontForTest.h | 18 +++---- 9 files changed, 85 insertions(+), 47 deletions(-) diff --git a/engine/src/flutter/include/minikin/MinikinFont.h b/engine/src/flutter/include/minikin/MinikinFont.h index 253160204e..ac9235be8d 100644 --- a/engine/src/flutter/include/minikin/MinikinFont.h +++ b/engine/src/flutter/include/minikin/MinikinFont.h @@ -109,6 +109,8 @@ public: virtual void GetBounds(MinikinRect* bounds, uint32_t glyph_id, const MinikinPaint &paint) const = 0; + virtual const void* GetTable(uint32_t tag, size_t* size, MinikinDestroyFunc* destroy) = 0; + // Override if font can provide access to raw data virtual const void* GetFontData() const { return nullptr; diff --git a/engine/src/flutter/libs/minikin/HbFontCache.cpp b/engine/src/flutter/libs/minikin/HbFontCache.cpp index 3c6619d10a..45c3f5807b 100644 --- a/engine/src/flutter/libs/minikin/HbFontCache.cpp +++ b/engine/src/flutter/libs/minikin/HbFontCache.cpp @@ -28,6 +28,22 @@ namespace minikin { +static hb_blob_t* referenceTable(hb_face_t* /* face */, hb_tag_t tag, void* userData) { + MinikinFont* font = reinterpret_cast(userData); + MinikinDestroyFunc destroy = 0; + size_t size = 0; + const void* buffer = font->GetTable(tag, &size, &destroy); + if (buffer == nullptr) { + return nullptr; + } +#ifdef VERBOSE_DEBUG + ALOGD("referenceTable %c%c%c%c length=%zd", + (tag >>24)&0xff, (tag>>16)&0xff, (tag>>8)&0xff, tag&0xff, size); +#endif + return hb_blob_create(reinterpret_cast(buffer), size, + HB_MEMORY_MODE_READONLY, const_cast(buffer), destroy); +} + class HbFontCache : private android::OnEntryRemoved { public: HbFontCache() : mCache(kMaxEntries) { @@ -103,12 +119,15 @@ hb_font_t* getHbFontLocked(MinikinFont* minikinFont) { hb_face_t* face; const void* buf = minikinFont->GetFontData(); - size_t size = minikinFont->GetFontSize(); - hb_blob_t* blob = hb_blob_create(reinterpret_cast(buf), size, - HB_MEMORY_MODE_READONLY, nullptr, nullptr); - face = hb_face_create(blob, minikinFont->GetFontIndex()); - hb_blob_destroy(blob); - + if (buf == nullptr) { + face = hb_face_create_for_tables(referenceTable, minikinFont, nullptr); + } else { + size_t size = minikinFont->GetFontSize(); + hb_blob_t* blob = hb_blob_create(reinterpret_cast(buf), size, + HB_MEMORY_MODE_READONLY, nullptr, nullptr); + face = hb_face_create(blob, minikinFont->GetFontIndex()); + hb_blob_destroy(blob); + } hb_font_t* parent_font = hb_font_create(face); hb_ot_font_set_funcs(parent_font); diff --git a/engine/src/flutter/tests/unittest/FontCollectionItemizeTest.cpp b/engine/src/flutter/tests/unittest/FontCollectionItemizeTest.cpp index 0d2a7cbfa6..367739683d 100644 --- a/engine/src/flutter/tests/unittest/FontCollectionItemizeTest.cpp +++ b/engine/src/flutter/tests/unittest/FontCollectionItemizeTest.cpp @@ -666,12 +666,12 @@ TEST_F(FontCollectionItemizeTest, itemize_vs_sequence_but_no_base_char) { std::vector families; FontFamily* family1 = new FontFamily(VARIANT_DEFAULT); - MinikinAutoUnref font(new MinikinFontForTest(kLatinFont)); + MinikinAutoUnref font(MinikinFontForTest::createFromFile(kLatinFont)); family1->addFont(font.get()); families.push_back(family1); FontFamily* family2 = new FontFamily(VARIANT_DEFAULT); - MinikinAutoUnref font2(new MinikinFontForTest(kVSTestFont)); + MinikinAutoUnref font2(MinikinFontForTest::createFromFile(kVSTestFont)); family2->addFont(font2.get()); families.push_back(family2); @@ -797,7 +797,7 @@ TEST_F(FontCollectionItemizeTest, itemize_LanguageScore) { FontFamily* firstFamily = new FontFamily( FontStyle::registerLanguageList("und"), 0 /* variant */); MinikinAutoUnref firstFamilyMinikinFont( - new MinikinFontForTest(kNoGlyphFont)); + MinikinFontForTest::createFromFile(kNoGlyphFont)); firstFamily->addFont(firstFamilyMinikinFont.get()); families.push_back(firstFamily); @@ -809,7 +809,7 @@ TEST_F(FontCollectionItemizeTest, itemize_LanguageScore) { for (size_t i = 0; i < testCase.fontLanguages.size(); ++i) { FontFamily* family = new FontFamily( FontStyle::registerLanguageList(testCase.fontLanguages[i]), 0 /* variant */); - MinikinAutoUnref minikin_font(new MinikinFontForTest(kJAFont)); + MinikinAutoUnref minikin_font(MinikinFontForTest::createFromFile(kJAFont)); family->addFont(minikin_font.get()); families.push_back(family); fontLangIdxMap.insert(std::make_pair(minikin_font.get(), i)); diff --git a/engine/src/flutter/tests/unittest/FontCollectionTest.cpp b/engine/src/flutter/tests/unittest/FontCollectionTest.cpp index 62d2f022fa..ef2da66b2b 100644 --- a/engine/src/flutter/tests/unittest/FontCollectionTest.cpp +++ b/engine/src/flutter/tests/unittest/FontCollectionTest.cpp @@ -58,7 +58,7 @@ void expectVSGlyphs(const FontCollection* fc, uint32_t codepoint, const std::set TEST(FontCollectionTest, hasVariationSelectorTest) { MinikinAutoUnref family(new FontFamily()); - MinikinAutoUnref font(new MinikinFontForTest(kVsTestFont)); + MinikinAutoUnref font(MinikinFontForTest::createFromFile(kVsTestFont)); family->addFont(font.get()); std::vector families({family.get()}); MinikinAutoUnref fc(new FontCollection(families)); diff --git a/engine/src/flutter/tests/unittest/FontFamilyTest.cpp b/engine/src/flutter/tests/unittest/FontFamilyTest.cpp index 4aaa601a23..69fef23da8 100644 --- a/engine/src/flutter/tests/unittest/FontFamilyTest.cpp +++ b/engine/src/flutter/tests/unittest/FontFamilyTest.cpp @@ -351,7 +351,7 @@ void expectVSGlyphs(FontFamily* family, uint32_t codepoint, const std::set - minikinFont(new MinikinFontForTest(kVsTestFont)); + minikinFont(MinikinFontForTest::createFromFile(kVsTestFont)); MinikinAutoUnref family(new FontFamily); family->addFont(minikinFont.get()); @@ -405,7 +405,7 @@ TEST_F(FontFamilyTest, hasVSTableTest) { "Font " + testCase.fontPath + " shouldn't have a variation sequence table."); MinikinAutoUnref minikinFont( - new MinikinFontForTest(testCase.fontPath)); + MinikinFontForTest::createFromFile(testCase.fontPath)); MinikinAutoUnref family(new FontFamily); family->addFont(minikinFont.get()); android::AutoMutex _l(gMinikinLock); diff --git a/engine/src/flutter/tests/unittest/HbFontCacheTest.cpp b/engine/src/flutter/tests/unittest/HbFontCacheTest.cpp index 5e560430c2..aa4cb34c05 100644 --- a/engine/src/flutter/tests/unittest/HbFontCacheTest.cpp +++ b/engine/src/flutter/tests/unittest/HbFontCacheTest.cpp @@ -38,13 +38,13 @@ public: TEST_F(HbFontCacheTest, getHbFontLockedTest) { MinikinAutoUnref fontA( - new MinikinFontForTest(kTestFontDir "Regular.ttf")); + MinikinFontForTest::createFromFile(kTestFontDir "Regular.ttf")); MinikinAutoUnref fontB( - new MinikinFontForTest(kTestFontDir "Bold.ttf")); + MinikinFontForTest::createFromFile(kTestFontDir "Bold.ttf")); MinikinAutoUnref fontC( - new MinikinFontForTest(kTestFontDir "BoldItalic.ttf")); + MinikinFontForTest::createFromFile(kTestFontDir "BoldItalic.ttf")); android::AutoMutex _l(gMinikinLock); // Never return NULL. @@ -66,7 +66,7 @@ TEST_F(HbFontCacheTest, getHbFontLockedTest) { TEST_F(HbFontCacheTest, purgeCacheTest) { MinikinAutoUnref minikinFont( - new MinikinFontForTest(kTestFontDir "Regular.ttf")); + MinikinFontForTest::createFromFile(kTestFontDir "Regular.ttf")); android::AutoMutex _l(gMinikinLock); hb_font_t* font = getHbFontLocked(minikinFont.get()); diff --git a/engine/src/flutter/tests/util/FontTestUtils.cpp b/engine/src/flutter/tests/util/FontTestUtils.cpp index b675620e25..246c87231a 100644 --- a/engine/src/flutter/tests/util/FontTestUtils.cpp +++ b/engine/src/flutter/tests/util/FontTestUtils.cpp @@ -77,11 +77,12 @@ FontCollection* getFontCollection(const char* fontDir, const char* fontXml) { if (index == nullptr) { MinikinAutoUnref - minikinFont(new MinikinFontForTest(fontPath)); + minikinFont(MinikinFontForTest::createFromFile(fontPath)); family->addFont(minikinFont.get(), FontStyle(weight, italic)); } else { MinikinAutoUnref - minikinFont(new MinikinFontForTest(fontPath, atoi((const char*)index))); + minikinFont(MinikinFontForTest::createFromFileWithIndex(fontPath, + atoi((const char*)index))); family->addFont(minikinFont.get(), FontStyle(weight, italic)); } } diff --git a/engine/src/flutter/tests/util/MinikinFontForTest.cpp b/engine/src/flutter/tests/util/MinikinFontForTest.cpp index db4b8333f8..807b234296 100644 --- a/engine/src/flutter/tests/util/MinikinFontForTest.cpp +++ b/engine/src/flutter/tests/util/MinikinFontForTest.cpp @@ -18,31 +18,31 @@ #include +#include + #include -#include -#include -#include namespace minikin { -static int uniqueId = 0; // TODO: make thread safe if necessary. - -MinikinFontForTest::MinikinFontForTest(const std::string& font_path, int index) : - MinikinFont(uniqueId++), - mFontPath(font_path), - mFontIndex(index) { - int fd = open(font_path.c_str(), O_RDONLY); - LOG_ALWAYS_FATAL_IF(fd == -1); - struct stat st = {}; - LOG_ALWAYS_FATAL_IF(fstat(fd, &st) != 0); - mFontSize = st.st_size; - mFontData = mmap(NULL, mFontSize, PROT_READ, MAP_SHARED, fd, 0); - LOG_ALWAYS_FATAL_IF(mFontData == nullptr); - close(fd); +// static +MinikinFontForTest* MinikinFontForTest::createFromFile(const std::string& font_path) { + sk_sp typeface = SkTypeface::MakeFromFile(font_path.c_str()); + MinikinFontForTest* font = new MinikinFontForTest(font_path, std::move(typeface)); + return font; } -MinikinFontForTest::~MinikinFontForTest() { - munmap(mFontData, mFontSize); +// static +MinikinFontForTest* MinikinFontForTest::createFromFileWithIndex(const std::string& font_path, + int index) { + sk_sp typeface = SkTypeface::MakeFromFile(font_path.c_str(), index); + MinikinFontForTest* font = new MinikinFontForTest(font_path, std::move(typeface)); + return font; +} + +MinikinFontForTest::MinikinFontForTest(const std::string& font_path, sk_sp typeface) : + MinikinFont(typeface->uniqueID()), + mTypeface(std::move(typeface)), + mFontPath(font_path) { } float MinikinFontForTest::GetHorizontalAdvance(uint32_t /* glyph_id */, @@ -56,4 +56,20 @@ void MinikinFontForTest::GetBounds(MinikinRect* /* bounds */, uint32_t /* glyph_ LOG_ALWAYS_FATAL("MinikinFontForTest::GetBounds is not yet implemented"); } +const void* MinikinFontForTest::GetTable(uint32_t tag, size_t* size, + MinikinDestroyFunc* destroy) { + const size_t tableSize = mTypeface->getTableSize(tag); + *size = tableSize; + if (tableSize == 0) { + return nullptr; + } + void* buf = malloc(tableSize); + if (buf == nullptr) { + return nullptr; + } + mTypeface->getTableData(tag, 0, tableSize, buf); + *destroy = free; + return buf; +} + } // namespace minikin diff --git a/engine/src/flutter/tests/util/MinikinFontForTest.h b/engine/src/flutter/tests/util/MinikinFontForTest.h index ee0eadbe0c..423792f8d5 100644 --- a/engine/src/flutter/tests/util/MinikinFontForTest.h +++ b/engine/src/flutter/tests/util/MinikinFontForTest.h @@ -18,6 +18,7 @@ #define MINIKIN_TEST_MINIKIN_FONT_FOR_TEST_H #include +#include class SkTypeface; @@ -25,28 +26,27 @@ namespace minikin { class MinikinFontForTest : public MinikinFont { public: - MinikinFontForTest(const std::string& font_path, int index); - MinikinFontForTest(const std::string& font_path) : MinikinFontForTest(font_path, 0) {} - virtual ~MinikinFontForTest(); + MinikinFontForTest(const std::string& font_path, sk_sp typeface); + + // Helper function for creating MinikinFontForTest instance from font file. + // Calller need to unref returned object. + static MinikinFontForTest* createFromFile(const std::string& font_path); + static MinikinFontForTest* createFromFileWithIndex(const std::string& font_path, int index); // MinikinFont overrides. float GetHorizontalAdvance(uint32_t glyph_id, const MinikinPaint &paint) const; void GetBounds(MinikinRect* bounds, uint32_t glyph_id, const MinikinPaint& paint) const; + const void* GetTable(uint32_t tag, size_t* size, MinikinDestroyFunc* destroy); const std::string& fontPath() const { return mFontPath; } - const void* GetFontData() const { return mFontData; } - size_t GetFontSize() const { return mFontSize; } - int GetFontIndex() const { return mFontIndex; } private: MinikinFontForTest() = delete; MinikinFontForTest(const MinikinFontForTest&) = delete; MinikinFontForTest& operator=(MinikinFontForTest&) = delete; + sk_sp mTypeface; const std::string mFontPath; - const int mFontIndex; - void* mFontData; - size_t mFontSize; }; } // namespace minikin