Revert "Remove HbFontCache"
This causes a regression for some characters.
This reverts commit 6c6954b40f7a6bd6dcf8a3d01c78600437c2e011.
Bug: 73054061
Change-Id: I010754474eebd55e43666893c4f7af9de12b22f3
(cherry picked from commit 4de86391218f9fa2d1ba15d78cd80514fb5ce43d)
diff --git a/include/minikin/FontFamily.h b/include/minikin/FontFamily.h
index 7605dd9..6cc672c 100644
--- a/include/minikin/FontFamily.h
+++ b/include/minikin/FontFamily.h
@@ -23,13 +23,10 @@
#include <vector>
#include "minikin/FontStyle.h"
-#include "minikin/HbUtils.h"
-#include "minikin/Macros.h"
#include "minikin/SparseBitSet.h"
namespace minikin {
-class Font;
class MinikinFont;
// attributes representing transforms (fake bold, fake italic) to match styles
@@ -48,73 +45,25 @@
struct FakedFont {
// ownership is the enclosing FontCollection
- const Font* font;
+ MinikinFont* font;
FontFakery fakery;
};
typedef uint32_t AxisTag;
-// Represents a single font file.
-class Font {
-public:
- class Builder {
- public:
- Builder(const std::shared_ptr<MinikinFont>& typeface) : mTypeface(typeface) {}
-
- // Override the font style. If not called, info from OS/2 table is used.
- Builder& setStyle(FontStyle style) {
- mWeight = style.weight();
- mSlant = style.slant();
- mIsWeightSet = mIsSlantSet = true;
- return *this;
- }
-
- // Override the font weight. If not called, info from OS/2 table is used.
- Builder& setWeight(uint16_t weight) {
- mWeight = weight;
- mIsWeightSet = true;
- return *this;
- }
-
- // Override the font slant. If not called, info from OS/2 table is used.
- Builder& setSlant(FontStyle::Slant slant) {
- mSlant = slant;
- mIsSlantSet = true;
- return *this;
- }
-
- Font build();
-
- private:
- std::shared_ptr<MinikinFont> mTypeface;
- uint16_t mWeight = static_cast<uint16_t>(FontStyle::Weight::NORMAL);
- FontStyle::Slant mSlant = FontStyle::Slant::UPRIGHT;
- bool mIsWeightSet = false;
- bool mIsSlantSet = false;
- };
+struct Font {
+ Font(const std::shared_ptr<MinikinFont>& typeface, FontStyle style);
Font(Font&& o) = default;
Font& operator=(Font&& o) = default;
+ // prevent copy constructor and assign operator.
+ Font(const Font& o) = delete;
+ Font& operator=(const Font& o) = delete;
- inline const std::shared_ptr<MinikinFont>& typeface() const { return mTypeface; }
- inline FontStyle style() const { return mStyle; }
- inline const HbFontUniquePtr& baseFont() const { return mBaseFont; }
+ std::shared_ptr<MinikinFont> typeface;
+ FontStyle style;
- std::unordered_set<AxisTag> getSupportedAxes() const;
-
-private:
- // Use Builder instead.
- Font(std::shared_ptr<MinikinFont>&& typeface, FontStyle style, HbFontUniquePtr&& baseFont)
- : mTypeface(std::move(typeface)), mStyle(style), mBaseFont(std::move(baseFont)) {}
-
- static HbFontUniquePtr prepareFont(const std::shared_ptr<MinikinFont>& typeface);
- static FontStyle analyzeStyle(const HbFontUniquePtr& font);
-
- std::shared_ptr<MinikinFont> mTypeface;
- FontStyle mStyle;
- HbFontUniquePtr mBaseFont;
-
- MINIKIN_PREVENT_COPY_AND_ASSIGN(Font);
+ std::unordered_set<AxisTag> getSupportedAxesLocked() const;
};
struct FontVariation {
@@ -136,6 +85,9 @@
FontFamily(Variant variant, std::vector<Font>&& fonts);
FontFamily(uint32_t localeListId, Variant variant, std::vector<Font>&& fonts);
+ // TODO: Good to expose FontUtil.h.
+ static bool analyzeStyle(const std::shared_ptr<MinikinFont>& typeface, int* weight,
+ bool* italic);
FakedFont getClosestMatch(FontStyle style) const;
uint32_t localeListId() const { return mLocaleListId; }
@@ -143,8 +95,10 @@
// API's for enumerating the fonts in a family. These don't guarantee any particular order
size_t getNumFonts() const { return mFonts.size(); }
- const Font* getFont(size_t index) const { return &mFonts[index]; }
- FontStyle getStyle(size_t index) const { return mFonts[index].style(); }
+ const std::shared_ptr<MinikinFont>& getFont(size_t index) const {
+ return mFonts[index].typeface;
+ }
+ FontStyle getStyle(size_t index) const { return mFonts[index].style; }
bool isColorEmojiFamily() const;
const std::unordered_set<AxisTag>& supportedAxes() const { return mSupportedAxes; }
@@ -174,7 +128,9 @@
SparseBitSet mCoverage;
std::vector<std::unique_ptr<SparseBitSet>> mCmapFmt14Coverage;
- MINIKIN_PREVENT_COPY_AND_ASSIGN(FontFamily);
+ // Forbid copying and assignment.
+ FontFamily(const FontFamily&) = delete;
+ void operator=(const FontFamily&) = delete;
};
} // namespace minikin
diff --git a/include/minikin/HbUtils.h b/include/minikin/HbUtils.h
deleted file mode 100644
index 1cf375d..0000000
--- a/include/minikin/HbUtils.h
+++ /dev/null
@@ -1,42 +0,0 @@
-/*
- * Copyright (C) 2018 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#ifndef MINIKIN_HB_UTILS_H
-#define MINIKIN_HB_UTILS_H
-
-#include <hb.h>
-#include <memory>
-
-namespace minikin {
-
-struct HbBlobDeleter {
- void operator()(hb_blob_t* v) { hb_blob_destroy(v); }
-};
-
-struct HbFaceDeleter {
- void operator()(hb_face_t* v) { hb_face_destroy(v); }
-};
-
-struct HbFontDeleter {
- void operator()(hb_font_t* v) { hb_font_destroy(v); }
-};
-using HbBlobUniquePtr = std::unique_ptr<hb_blob_t, HbBlobDeleter>;
-using HbFaceUniquePtr = std::unique_ptr<hb_face_t, HbFaceDeleter>;
-using HbFontUniquePtr = std::unique_ptr<hb_font_t, HbFontDeleter>;
-
-} // namespace minikin
-
-#endif // MINIKIN_HB_UTILS_H
diff --git a/include/minikin/LineBreaker.h b/include/minikin/LineBreaker.h
index 7d59c63..d97965a 100644
--- a/include/minikin/LineBreaker.h
+++ b/include/minikin/LineBreaker.h
@@ -115,7 +115,7 @@
}
private:
- MINIKIN_PREVENT_COPY_AND_ASSIGN(LineBreakResult);
+ PREVENT_COPY_AND_ASSIGN(LineBreakResult);
};
LineBreakResult breakIntoLines(const U16StringPiece& textBuffer, BreakStrategy strategy,
diff --git a/include/minikin/Macros.h b/include/minikin/Macros.h
index 6791ce0..1eeaa8c 100644
--- a/include/minikin/Macros.h
+++ b/include/minikin/Macros.h
@@ -16,13 +16,13 @@
#ifndef MINIKIN_MACROS_H
#define MINIKIN_MACROS_H
-#define MINIKIN_PREVENT_COPY_AND_ASSIGN(Type) \
- Type(const Type&) = delete; \
+#define PREVENT_COPY_AND_ASSIGN(Type) \
+ Type(const Type&) = delete; \
Type& operator=(const Type&) = delete
-#define MINIKIN_PREVENT_COPY_ASSIGN_AND_MOVE(Type) \
- Type(const Type&) = delete; \
- Type& operator=(const Type&) = delete; \
- Type(Type&&) = delete; \
+#define PREVENT_COPY_ASSIGN_AND_MOVE(Type) \
+ Type(const Type&) = delete; \
+ Type& operator=(const Type&) = delete; \
+ Type(Type&&) = delete; \
Type& operator=(Type&&) = delete
#endif // MINIKIN_MACROS_H
diff --git a/include/minikin/MeasuredText.h b/include/minikin/MeasuredText.h
index 83b774f..5da37eb 100644
--- a/include/minikin/MeasuredText.h
+++ b/include/minikin/MeasuredText.h
@@ -175,7 +175,7 @@
MeasuredText(MeasuredText&&) = default;
MeasuredText& operator=(MeasuredText&&) = default;
- MINIKIN_PREVENT_COPY_AND_ASSIGN(MeasuredText);
+ PREVENT_COPY_AND_ASSIGN(MeasuredText);
private:
friend class MeasuredTextBuilder;
@@ -215,7 +215,7 @@
new MeasuredText(textBuf, std::move(mRuns), computeHyphenation, computeLayout));
}
- MINIKIN_PREVENT_COPY_ASSIGN_AND_MOVE(MeasuredTextBuilder);
+ PREVENT_COPY_ASSIGN_AND_MOVE(MeasuredTextBuilder);
private:
std::vector<std::unique_ptr<Run>> mRuns;
diff --git a/include/minikin/MinikinFont.h b/include/minikin/MinikinFont.h
index 6c53583..74a3811 100644
--- a/include/minikin/MinikinFont.h
+++ b/include/minikin/MinikinFont.h
@@ -117,7 +117,7 @@
public:
explicit MinikinFont(int32_t uniqueId) : mUniqueId(uniqueId) {}
- virtual ~MinikinFont() {}
+ virtual ~MinikinFont();
virtual float GetHorizontalAdvance(uint32_t glyph_id, const MinikinPaint& paint,
const FontFakery& fakery) const = 0;
diff --git a/libs/minikin/Android.bp b/libs/minikin/Android.bp
index 0f9d003..b1c8fa5 100644
--- a/libs/minikin/Android.bp
+++ b/libs/minikin/Android.bp
@@ -36,6 +36,7 @@
"FontUtils.cpp",
"GraphemeBreak.cpp",
"GreedyLineBreaker.cpp",
+ "HbFontCache.cpp",
"HyphenatorMap.cpp",
"Layout.cpp",
"LayoutUtils.cpp",
@@ -45,6 +46,7 @@
"LocaleListCache.cpp",
"MeasuredText.cpp",
"Measurement.cpp",
+ "MinikinFont.cpp",
"MinikinInternal.cpp",
"OptimalLineBreaker.cpp",
"SparseBitSet.cpp",
diff --git a/libs/minikin/FontFamily.cpp b/libs/minikin/FontFamily.cpp
index 1c89960..5291540 100644
--- a/libs/minikin/FontFamily.cpp
+++ b/libs/minikin/FontFamily.cpp
@@ -21,13 +21,10 @@
#include <cstdint>
#include <vector>
-#include <hb-ot.h>
-#include <hb.h>
#include <log/log.h>
#include <utils/JenkinsHash.h>
#include "minikin/CmapCoverage.h"
-#include "minikin/HbUtils.h"
#include "minikin/MinikinFont.h"
#include "FontUtils.h"
@@ -37,73 +34,22 @@
namespace minikin {
-Font Font::Builder::build() {
- if (mIsWeightSet && mIsSlantSet) {
- // No need to read OS/2 header of the font file.
- return Font(std::move(mTypeface), FontStyle(mWeight, mSlant), prepareFont(mTypeface));
- }
+Font::Font(const std::shared_ptr<MinikinFont>& typeface, FontStyle style)
+ : typeface(typeface), style(style) {}
- HbFontUniquePtr font = prepareFont(mTypeface);
- FontStyle styleFromFont = analyzeStyle(font);
- if (!mIsWeightSet) {
- mWeight = styleFromFont.weight();
- }
- if (!mIsSlantSet) {
- mSlant = styleFromFont.slant();
- }
- return Font(std::move(mTypeface), FontStyle(mWeight, mSlant), std::move(font));
-}
-
-// static
-HbFontUniquePtr Font::prepareFont(const std::shared_ptr<MinikinFont>& typeface) {
- const char* buf = reinterpret_cast<const char*>(typeface->GetFontData());
- size_t size = typeface->GetFontSize();
- uint32_t ttcIndex = typeface->GetFontIndex();
-
- HbBlobUniquePtr blob(hb_blob_create(buf, size, HB_MEMORY_MODE_READONLY, nullptr, nullptr));
- HbFaceUniquePtr face(hb_face_create(blob.get(), ttcIndex));
- HbFontUniquePtr parent(hb_font_create(face.get()));
- hb_ot_font_set_funcs(parent.get());
-
- uint32_t upem = hb_face_get_upem(face.get());
- hb_font_set_scale(parent.get(), upem, upem);
-
- HbFontUniquePtr font(hb_font_create_sub_font(parent.get()));
- std::vector<hb_variation_t> variations;
- variations.reserve(typeface->GetAxes().size());
- for (const FontVariation& variation : typeface->GetAxes()) {
- variations.push_back({variation.axisTag, variation.value});
- }
- hb_font_set_variations(font.get(), variations.data(), variations.size());
- return font;
-}
-
-// static
-FontStyle Font::analyzeStyle(const HbFontUniquePtr& font) {
- HbBlob os2Table(font, MinikinFont::MakeTag('O', 'S', '/', '2'));
- if (!os2Table) {
- return FontStyle();
- }
-
- int weight;
- bool italic;
- if (!::minikin::analyzeStyle(os2Table.get(), os2Table.size(), &weight, &italic)) {
- return FontStyle();
- }
- // TODO: Update weight/italic based on fvar value.
- return FontStyle(static_cast<uint16_t>(weight), static_cast<FontStyle::Slant>(italic));
-}
-
-std::unordered_set<AxisTag> Font::getSupportedAxes() const {
- HbBlob fvarTable(mBaseFont, MinikinFont::MakeTag('f', 'v', 'a', 'r'));
- if (!fvarTable) {
+std::unordered_set<AxisTag> Font::getSupportedAxesLocked() const {
+ const uint32_t fvarTag = MinikinFont::MakeTag('f', 'v', 'a', 'r');
+ HbBlob fvarTable(getFontTable(typeface.get(), fvarTag));
+ if (fvarTable.size() == 0) {
return std::unordered_set<AxisTag>();
}
+
std::unordered_set<AxisTag> supportedAxes;
analyzeAxes(fvarTable.get(), fvarTable.size(), &supportedAxes);
return supportedAxes;
}
+// static
FontFamily::FontFamily(std::vector<Font>&& fonts)
: FontFamily(Variant::DEFAULT, std::move(fonts)) {}
@@ -116,6 +62,15 @@
computeCoverage();
}
+bool FontFamily::analyzeStyle(const std::shared_ptr<MinikinFont>& typeface, int* weight,
+ bool* italic) {
+ android::AutoMutex _l(gMinikinLock);
+ const uint32_t os2Tag = MinikinFont::MakeTag('O', 'S', '/', '2');
+ HbBlob os2Table(getFontTable(typeface.get(), os2Tag));
+ if (os2Table.get() == nullptr) return false;
+ return ::minikin::analyzeStyle(os2Table.get(), os2Table.size(), weight, italic);
+}
+
// Compute a matching metric between two styles - 0 is an exact match
static int computeMatch(FontStyle style1, FontStyle style2) {
if (style1 == style2) return 0;
@@ -138,16 +93,16 @@
FakedFont FontFamily::getClosestMatch(FontStyle style) const {
const Font* bestFont = &mFonts[0];
- int bestMatch = computeMatch(bestFont->style(), style);
+ int bestMatch = computeMatch(bestFont->style, style);
for (size_t i = 1; i < mFonts.size(); i++) {
const Font& font = mFonts[i];
- int match = computeMatch(font.style(), style);
+ int match = computeMatch(font.style, style);
if (i == 0 || match < bestMatch) {
bestFont = &font;
bestMatch = match;
}
}
- return FakedFont{bestFont, computeFakery(style, bestFont->style())};
+ return FakedFont{bestFont->typeface.get(), computeFakery(style, bestFont->style)};
}
bool FontFamily::isColorEmojiFamily() const {
@@ -161,17 +116,19 @@
}
void FontFamily::computeCoverage() {
- const Font* font = getClosestMatch(FontStyle()).font;
- HbBlob cmapTable(font->baseFont(), MinikinFont::MakeTag('c', 'm', 'a', 'p'));
+ android::AutoMutex _l(gMinikinLock);
+ const FontStyle defaultStyle;
+ const MinikinFont* typeface = getClosestMatch(defaultStyle).font;
+ const uint32_t cmapTag = MinikinFont::MakeTag('c', 'm', 'a', 'p');
+ HbBlob cmapTable(getFontTable(typeface, cmapTag));
if (cmapTable.get() == nullptr) {
ALOGE("Could not get cmap table size!\n");
return;
}
-
mCoverage = CmapCoverage::getCoverage(cmapTable.get(), cmapTable.size(), &mCmapFmt14Coverage);
for (size_t i = 0; i < mFonts.size(); ++i) {
- std::unordered_set<AxisTag> supportedAxes = mFonts[i].getSupportedAxes();
+ std::unordered_set<AxisTag> supportedAxes = mFonts[i].getSupportedAxesLocked();
mSupportedAxes.insert(supportedAxes.begin(), supportedAxes.end());
}
}
@@ -222,7 +179,8 @@
std::vector<Font> fonts;
for (const Font& font : mFonts) {
bool supportedVariations = false;
- std::unordered_set<AxisTag> supportedAxes = font.getSupportedAxes();
+ android::AutoMutex _l(gMinikinLock);
+ std::unordered_set<AxisTag> supportedAxes = font.getSupportedAxesLocked();
if (!supportedAxes.empty()) {
for (const FontVariation& variation : variations) {
if (supportedAxes.find(variation.axisTag) != supportedAxes.end()) {
@@ -233,12 +191,12 @@
}
std::shared_ptr<MinikinFont> minikinFont;
if (supportedVariations) {
- minikinFont = font.typeface()->createFontWithVariation(variations);
+ minikinFont = font.typeface->createFontWithVariation(variations);
}
if (minikinFont == nullptr) {
- minikinFont = font.typeface();
+ minikinFont = font.typeface;
}
- fonts.push_back(Font::Builder(minikinFont).setStyle(font.style()).build());
+ fonts.push_back(Font(std::move(minikinFont), font.style));
}
return std::shared_ptr<FontFamily>(new FontFamily(mLocaleListId, mVariant, std::move(fonts)));
diff --git a/libs/minikin/GreedyLineBreaker.cpp b/libs/minikin/GreedyLineBreaker.cpp
index 6d82373..9336f28 100644
--- a/libs/minikin/GreedyLineBreaker.cpp
+++ b/libs/minikin/GreedyLineBreaker.cpp
@@ -124,7 +124,7 @@
// The result of line breaking.
std::vector<BreakPoint> mBreakPoints;
- MINIKIN_PREVENT_COPY_ASSIGN_AND_MOVE(GreedyLineBreaker);
+ PREVENT_COPY_ASSIGN_AND_MOVE(GreedyLineBreaker);
};
void GreedyLineBreaker::breakLineAt(uint32_t offset, float lineWidth, float remainingNextLineWidth,
diff --git a/libs/minikin/HbFontCache.cpp b/libs/minikin/HbFontCache.cpp
new file mode 100644
index 0000000..4f80c37
--- /dev/null
+++ b/libs/minikin/HbFontCache.cpp
@@ -0,0 +1,111 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "HbFontCache.h"
+
+#include <hb-ot.h>
+#include <hb.h>
+#include <utils/LruCache.h>
+
+#include "minikin/MinikinFont.h"
+
+#include "MinikinInternal.h"
+
+namespace minikin {
+
+class HbFontCache : private android::OnEntryRemoved<int32_t, hb_font_t*> {
+public:
+ HbFontCache() : mCache(kMaxEntries) { mCache.setOnEntryRemovedListener(this); }
+
+ // callback for OnEntryRemoved
+ void operator()(int32_t& /* key */, hb_font_t*& value) { hb_font_destroy(value); }
+
+ hb_font_t* get(int32_t fontId) { return mCache.get(fontId); }
+
+ void put(int32_t fontId, hb_font_t* font) { mCache.put(fontId, font); }
+
+ void clear() { mCache.clear(); }
+
+ void remove(int32_t fontId) { mCache.remove(fontId); }
+
+private:
+ static const size_t kMaxEntries = 100;
+
+ android::LruCache<int32_t, hb_font_t*> mCache;
+};
+
+HbFontCache* getFontCacheLocked() {
+ assertMinikinLocked();
+ static HbFontCache* cache = nullptr;
+ if (cache == nullptr) {
+ cache = new HbFontCache();
+ }
+ return cache;
+}
+
+void purgeHbFontLocked(const MinikinFont* minikinFont) {
+ assertMinikinLocked();
+ const int32_t fontId = minikinFont->GetUniqueId();
+ getFontCacheLocked()->remove(fontId);
+}
+
+// Returns a new reference to a hb_font_t object, caller is
+// responsible for calling hb_font_destroy() on it.
+hb_font_t* getHbFontLocked(const MinikinFont* minikinFont) {
+ assertMinikinLocked();
+ // TODO: get rid of nullFaceFont
+ static hb_font_t* nullFaceFont = nullptr;
+ if (minikinFont == nullptr) {
+ if (nullFaceFont == nullptr) {
+ nullFaceFont = hb_font_create(nullptr);
+ }
+ return hb_font_reference(nullFaceFont);
+ }
+
+ HbFontCache* fontCache = getFontCacheLocked();
+ const int32_t fontId = minikinFont->GetUniqueId();
+ hb_font_t* font = fontCache->get(fontId);
+ if (font != nullptr) {
+ return hb_font_reference(font);
+ }
+
+ hb_face_t* face;
+ const void* buf = minikinFont->GetFontData();
+ size_t size = minikinFont->GetFontSize();
+ hb_blob_t* blob = hb_blob_create(reinterpret_cast<const char*>(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);
+
+ unsigned int upem = hb_face_get_upem(face);
+ hb_font_set_scale(parent_font, upem, upem);
+
+ font = hb_font_create_sub_font(parent_font);
+ std::vector<hb_variation_t> variations;
+ for (const FontVariation& variation : minikinFont->GetAxes()) {
+ variations.push_back({variation.axisTag, variation.value});
+ }
+ hb_font_set_variations(font, variations.data(), variations.size());
+ hb_font_destroy(parent_font);
+ hb_face_destroy(face);
+ fontCache->put(fontId, font);
+ return hb_font_reference(font);
+}
+
+} // namespace minikin
diff --git a/libs/minikin/Layout.cpp b/libs/minikin/Layout.cpp
index 1a413f8..ba6ca83 100644
--- a/libs/minikin/Layout.cpp
+++ b/libs/minikin/Layout.cpp
@@ -33,8 +33,8 @@
#include <utils/Singleton.h>
#include "minikin/Emoji.h"
-#include "minikin/HbUtils.h"
+#include "HbFontCache.h"
#include "LayoutUtils.h"
#include "LocaleListCache.h"
#include "MinikinInternal.h"
@@ -73,8 +73,18 @@
struct LayoutContext {
LayoutContext(const MinikinPaint& paint) : paint(paint) {}
+
const MinikinPaint& paint;
- std::vector<HbFontUniquePtr> hbFonts; // parallel to mFaces
+
+ std::vector<hb_font_t*> hbFonts; // parallel to mFaces
+
+ void clearHbFonts() {
+ for (size_t i = 0; i < hbFonts.size(); i++) {
+ hb_font_set_funcs(hbFonts[i], nullptr, nullptr, nullptr);
+ hb_font_destroy(hbFonts[i]);
+ }
+ hbFonts.clear();
+ }
};
// Layout cache datatypes
@@ -117,6 +127,7 @@
void doLayout(Layout* layout, LayoutContext* ctx) const {
layout->mAdvances.resize(mCount, 0);
layout->mExtents.resize(mCount);
+ ctx->clearHbFonts();
layout->doLayoutRun(mChars, mStart, mCount, mNchars, mIsRtl, ctx, mStartHyphen, mEndHyphen);
}
@@ -309,9 +320,10 @@
return *funcs;
}
-static bool isColorBitmapFont(const HbFontUniquePtr& font) {
- HbBlob cbdt(font, HB_TAG('C', 'B', 'D', 'T'));
- return cbdt;
+static bool isColorBitmapFont(hb_font_t* font) {
+ hb_face_t* face = hb_font_get_face(font);
+ HbBlob cbdt(hb_face_reference_table(face, HB_TAG('C', 'B', 'D', 'T')));
+ return cbdt.size() > 0;
}
static float HBFixedToFloat(hb_position_t v) {
@@ -341,14 +353,11 @@
// Note: ctx == nullptr means we're copying from the cache, no need to create corresponding
// hb_font object.
if (ctx != nullptr) {
- // We override some functions which are not thread safe.
- // Create new hb_font_t from base font.
- HbFontUniquePtr font(hb_font_create_sub_font(face.font->baseFont().get()));
- hb_font_set_funcs(
- font.get(), getHbFontFuncs(isColorBitmapFont(font)),
- new SkiaArguments({face.font->typeface().get(), &ctx->paint, face.fakery}),
- [](void* data) { delete reinterpret_cast<SkiaArguments*>(data); });
- ctx->hbFonts.push_back(std::move(font));
+ hb_font_t* font = getHbFontLocked(face.font);
+ hb_font_set_funcs(font, getHbFontFuncs(isColorBitmapFont(font)),
+ new SkiaArguments({face.font, &ctx->paint, face.fakery}),
+ [](void* data) { delete reinterpret_cast<SkiaArguments*>(data); });
+ ctx->hbFonts.push_back(font);
}
return ix;
}
@@ -588,6 +597,7 @@
runInfo.mIsRtl, &ctx, range.getStart(), startHyphen, endHyphen, this,
nullptr, nullptr, nullptr);
}
+ ctx.clearHbFonts();
}
// static
@@ -609,6 +619,8 @@
nullptr, // extents. Not used.
out);
}
+
+ ctx.clearHbFonts();
}
float Layout::measureText(const U16StringPiece& textBuf, const Range& range, Bidi bidiFlags,
@@ -627,6 +639,8 @@
textBuf.size(), runInfo.mIsRtl, &ctx, 0, startHyphen,
endHyphen, NULL, advancesForRun, extentsForRun, nullptr);
}
+
+ ctx.clearHbFonts();
return advance;
}
@@ -768,13 +782,13 @@
}
template <typename HyphenEdit>
-static inline void addHyphenToHbBuffer(hb_buffer_t* buffer, const HbFontUniquePtr& font,
- HyphenEdit hyphen, uint32_t cluster) {
+static inline void addHyphenToHbBuffer(hb_buffer_t* buffer, hb_font_t* font, HyphenEdit hyphen,
+ uint32_t cluster) {
const uint32_t* chars;
size_t size;
std::tie(chars, size) = getHyphenString(hyphen);
for (size_t i = 0; i < size; i++) {
- hb_buffer_add(buffer, determineHyphenChar(chars[i], font.get()), cluster);
+ hb_buffer_add(buffer, determineHyphenChar(chars[i], font), cluster);
}
}
@@ -783,7 +797,7 @@
static inline uint32_t addToHbBuffer(hb_buffer_t* buffer, const uint16_t* buf, size_t start,
size_t count, size_t bufSize, ssize_t scriptRunStart,
ssize_t scriptRunEnd, StartHyphenEdit inStartHyphen,
- EndHyphenEdit inEndHyphen, const HbFontUniquePtr& hbFont) {
+ EndHyphenEdit inEndHyphen, hb_font_t* hbFont) {
// Only hyphenate the very first script run for starting hyphens.
const StartHyphenEdit startHyphen =
(scriptRunStart == 0) ? inStartHyphen : StartHyphenEdit::NO_EDIT;
@@ -907,15 +921,19 @@
isRtl ? --run_ix : ++run_ix) {
FontCollection::Run& run = items[run_ix];
const FakedFont& fakedFont = run.fakedFont;
+ if (fakedFont.font == NULL) {
+ ALOGE("no font for run starting u+%04x length %d", buf[run.start], run.end - run.start);
+ continue;
+ }
const int font_ix = findOrPushBackFace(fakedFont, ctx);
- const HbFontUniquePtr& hbFont = ctx->hbFonts[font_ix];
+ hb_font_t* hbFont = ctx->hbFonts[font_ix];
MinikinExtent verticalExtent;
- fakedFont.font->typeface()->GetFontExtent(&verticalExtent, ctx->paint, fakedFont.fakery);
+ fakedFont.font->GetFontExtent(&verticalExtent, ctx->paint, fakedFont.fakery);
std::fill(&mExtents[run.start], &mExtents[run.end], verticalExtent);
- hb_font_set_ppem(hbFont.get(), size * scaleX, size);
- hb_font_set_scale(hbFont.get(), HBFloatToFixed(size * scaleX), HBFloatToFixed(size));
+ hb_font_set_ppem(hbFont, size * scaleX, size);
+ hb_font_set_scale(hbFont, HBFloatToFixed(size * scaleX), HBFloatToFixed(size));
const bool is_color_bitmap_font = isColorBitmapFont(hbFont);
@@ -971,7 +989,7 @@
addToHbBuffer(buffer, buf, start, count, bufSize, scriptRunStart, scriptRunEnd,
startHyphen, endHyphen, hbFont);
- hb_shape(hbFont.get(), buffer, features.empty() ? NULL : &features[0], features.size());
+ hb_shape(hbFont, buffer, features.empty() ? NULL : &features[0], features.size());
unsigned int numGlyphs;
hb_glyph_info_t* info = hb_buffer_get_glyph_infos(buffer, &numGlyphs);
hb_glyph_position_t* positions = hb_buffer_get_glyph_positions(buffer, NULL);
@@ -1008,8 +1026,7 @@
}
MinikinRect glyphBounds;
hb_glyph_extents_t extents = {};
- if (is_color_bitmap_font &&
- hb_font_get_glyph_extents(hbFont.get(), glyph_ix, &extents)) {
+ if (is_color_bitmap_font && hb_font_get_glyph_extents(hbFont, glyph_ix, &extents)) {
// Note that it is technically possible for a TrueType font to have outline and
// embedded bitmap at the same time. We ignore modified bbox of hinted outline
// glyphs in that case.
@@ -1019,8 +1036,7 @@
glyphBounds.mBottom =
roundf(HBFixedToFloat(-extents.y_bearing - extents.height));
} else {
- fakedFont.font->typeface()->GetBounds(&glyphBounds, glyph_ix, ctx->paint,
- fakedFont.fakery);
+ fakedFont.font->GetBounds(&glyphBounds, glyph_ix, ctx->paint, fakedFont.fakery);
}
glyphBounds.offset(xoff, yoff);
@@ -1088,7 +1104,7 @@
const MinikinFont* Layout::getFont(int i) const {
const LayoutGlyph& glyph = mGlyphs[i];
- return mFaces[glyph.font_ix].font->typeface().get();
+ return mFaces[glyph.font_ix].font;
}
FontFakery Layout::getFakery(int i) const {
diff --git a/libs/minikin/MinikinInternal.cpp b/libs/minikin/MinikinInternal.cpp
index 0ed667d..a5702e5 100644
--- a/libs/minikin/MinikinInternal.cpp
+++ b/libs/minikin/MinikinInternal.cpp
@@ -19,6 +19,7 @@
#include <log/log.h>
+#include "HbFontCache.h"
#include "MinikinInternal.h"
namespace minikin {
@@ -31,6 +32,15 @@
#endif
}
+hb_blob_t* getFontTable(const MinikinFont* minikinFont, uint32_t tag) {
+ assertMinikinLocked();
+ hb_font_t* font = getHbFontLocked(minikinFont);
+ hb_face_t* face = hb_font_get_face(font);
+ hb_blob_t* blob = hb_face_reference_table(face, tag);
+ hb_font_destroy(font);
+ return blob;
+}
+
inline static bool isBMPVariationSelector(uint32_t codePoint) {
return VS1 <= codePoint && codePoint <= VS16;
}
diff --git a/libs/minikin/MinikinInternal.h b/libs/minikin/MinikinInternal.h
index 05dcb9c..bffda70 100644
--- a/libs/minikin/MinikinInternal.h
+++ b/libs/minikin/MinikinInternal.h
@@ -23,7 +23,6 @@
#include <utils/Log.h>
#include <utils/Mutex.h>
-#include "minikin/HbUtils.h"
#include "minikin/MinikinFont.h"
namespace minikin {
@@ -37,6 +36,8 @@
// Aborts if gMinikinLock is not acquired. Do nothing on the release build.
void assertMinikinLocked();
+hb_blob_t* getFontTable(const MinikinFont* minikinFont, uint32_t tag);
+
#ifdef ENABLE_ASSERTION
#define MINIKIN_ASSERT(cond, ...) LOG_ALWAYS_FATAL_IF(!(cond), __VA_ARGS__)
#else
@@ -64,24 +65,24 @@
// Note that this function returns false for Mongolian free variation selectors.
bool isVariationSelector(uint32_t codePoint);
-// An RAII accessor for hb_blob_t
+// An RAII wrapper for hb_blob_t
class HbBlob {
public:
- HbBlob(const HbFaceUniquePtr& face, uint32_t tag)
- : mBlob(hb_face_reference_table(face.get(), tag)) {}
- HbBlob(const HbFontUniquePtr& font, uint32_t tag)
- : mBlob(hb_face_reference_table(hb_font_get_face(font.get()), tag)) {}
+ // Takes ownership of hb_blob_t object, caller is no longer
+ // responsible for calling hb_blob_destroy().
+ explicit HbBlob(hb_blob_t* blob) : mBlob(blob) {}
- inline const uint8_t* get() const {
- return reinterpret_cast<const uint8_t*>(hb_blob_get_data(mBlob.get(), nullptr));
+ ~HbBlob() { hb_blob_destroy(mBlob); }
+
+ const uint8_t* get() const {
+ const char* data = hb_blob_get_data(mBlob, nullptr);
+ return reinterpret_cast<const uint8_t*>(data);
}
- inline size_t size() const { return (size_t)hb_blob_get_length(mBlob.get()); }
-
- inline operator bool() const { return size() > 0; }
+ size_t size() const { return (size_t)hb_blob_get_length(mBlob); }
private:
- HbBlobUniquePtr mBlob;
+ hb_blob_t* mBlob;
};
} // namespace minikin
diff --git a/tests/stresstest/FontFamilyTest.cpp b/tests/stresstest/FontFamilyTest.cpp
index f7d8b8e..a7eb07b 100644
--- a/tests/stresstest/FontFamilyTest.cpp
+++ b/tests/stresstest/FontFamilyTest.cpp
@@ -17,11 +17,11 @@
#include "minikin/FontFamily.h"
#include <gtest/gtest.h>
-#include <hb.h>
#include "minikin/FontCollection.h"
#include "FontTestUtils.h"
+#include "HbFontCache.h"
#include "MinikinInternal.h"
namespace minikin {
@@ -37,7 +37,7 @@
std::shared_ptr<FontFamily> family = buildFontFamily(fontPath);
android::AutoMutex _l(gMinikinLock);
- hb_font_t* hbFont = family->getFont(0)->baseFont().get();
+ hb_font_t* hbFont = getHbFontLocked(family->getFont(0).get());
for (uint32_t codePoint = 0; codePoint < MAX_UNICODE_CODE_POINT; ++codePoint) {
uint32_t unusedGlyph;
diff --git a/tests/unittest/Android.bp b/tests/unittest/Android.bp
index 9ded52e..34f789e 100644
--- a/tests/unittest/Android.bp
+++ b/tests/unittest/Android.bp
@@ -45,6 +45,7 @@
"FontCollectionItemizeTest.cpp",
"FontFamilyTest.cpp",
"FontLanguageListCacheTest.cpp",
+ "HbFontCacheTest.cpp",
"HyphenatorMapTest.cpp",
"HyphenatorTest.cpp",
"GraphemeBreakTests.cpp",
diff --git a/tests/unittest/FontCollectionItemizeTest.cpp b/tests/unittest/FontCollectionItemizeTest.cpp
index c130f6d..2ab33f6 100644
--- a/tests/unittest/FontCollectionItemizeTest.cpp
+++ b/tests/unittest/FontCollectionItemizeTest.cpp
@@ -91,7 +91,7 @@
// Utility function to obtain font path associated with run.
const std::string& getFontPath(const FontCollection::Run& run) {
EXPECT_NE(nullptr, run.fakedFont.font);
- return ((FreeTypeMinikinFontForTest*)run.fakedFont.font->typeface().get())->fontPath();
+ return ((FreeTypeMinikinFontForTest*)run.fakedFont.font)->fontPath();
}
// Utility function to obtain LocaleList from string.
@@ -945,7 +945,7 @@
// Prepare first font which doesn't supports U+9AA8
auto firstFamilyMinikinFont = std::make_shared<FreeTypeMinikinFontForTest>(kNoGlyphFont);
std::vector<Font> fonts;
- fonts.push_back(Font::Builder(firstFamilyMinikinFont).build());
+ fonts.push_back(Font(firstFamilyMinikinFont, FontStyle()));
auto firstFamily = std::make_shared<FontFamily>(
registerLocaleList("und"), FontFamily::Variant::DEFAULT, std::move(fonts));
families.push_back(firstFamily);
@@ -958,7 +958,7 @@
for (size_t i = 0; i < testCase.fontLocales.size(); ++i) {
auto minikinFont = std::make_shared<FreeTypeMinikinFontForTest>(kJAFont);
std::vector<Font> fonts;
- fonts.push_back(Font::Builder(minikinFont).build());
+ fonts.push_back(Font(minikinFont, FontStyle()));
auto family =
std::make_shared<FontFamily>(registerLocaleList(testCase.fontLocales[i]),
FontFamily::Variant::DEFAULT, std::move(fonts));
@@ -974,10 +974,10 @@
// First family doesn't support U+9AA8 and others support it, so the first font should not
// be selected.
- EXPECT_NE(firstFamilyMinikinFont.get(), runs[0].fakedFont.font->typeface().get());
+ EXPECT_NE(firstFamilyMinikinFont.get(), runs[0].fakedFont.font);
// Lookup used font family by MinikinFont*.
- const int usedLocaleIndex = fontLocaleIdxMap[runs[0].fakedFont.font->typeface().get()];
+ const int usedLocaleIndex = fontLocaleIdxMap[runs[0].fakedFont.font];
EXPECT_EQ(testCase.selectedFontIndex, usedLocaleIndex);
}
}
@@ -1545,10 +1545,10 @@
// selected.
std::vector<FontCollection::Run> runs;
itemize(collection, "U+35A8 U+E0100", &runs);
- EXPECT_EQ(familyA->getFont(0), runs[0].fakedFont.font);
+ EXPECT_EQ(familyA->getFont(0).get(), runs[0].fakedFont.font);
itemize(reversedCollection, "U+35A8 U+E0100", &runs);
- EXPECT_EQ(familyB->getFont(0), runs[0].fakedFont.font);
+ EXPECT_EQ(familyB->getFont(0).get(), runs[0].fakedFont.font);
}
// For b/29585939
@@ -1569,10 +1569,10 @@
// The first font should be selected.
std::vector<FontCollection::Run> runs;
itemize(collection, "U+5380 U+E0100", &runs);
- EXPECT_EQ(hasCmapFormat14Family->getFont(0), runs[0].fakedFont.font);
+ EXPECT_EQ(hasCmapFormat14Family->getFont(0).get(), runs[0].fakedFont.font);
itemize(reversedCollection, "U+5380 U+E0100", &runs);
- EXPECT_EQ(noCmapFormat14Family->getFont(0), runs[0].fakedFont.font);
+ EXPECT_EQ(noCmapFormat14Family->getFont(0).get(), runs[0].fakedFont.font);
}
} // namespace minikin
diff --git a/tests/unittest/FontFamilyTest.cpp b/tests/unittest/FontFamilyTest.cpp
index 402f26b..a0ed9be 100644
--- a/tests/unittest/FontFamilyTest.cpp
+++ b/tests/unittest/FontFamilyTest.cpp
@@ -725,7 +725,7 @@
for (auto familyStyle : testCase.familyStyles) {
std::shared_ptr<MinikinFont> dummyFont(new FreeTypeMinikinFontForTest(ROBOTO));
dummyFonts.push_back(dummyFont);
- fonts.push_back(Font::Builder(dummyFont).setStyle(familyStyle).build());
+ fonts.push_back(Font(dummyFont, familyStyle));
}
FontFamily family(std::move(fonts));
@@ -733,7 +733,7 @@
size_t idx = dummyFonts.size();
for (size_t i = 0; i < dummyFonts.size(); i++) {
- if (dummyFonts[i].get() == closest.font->typeface().get()) {
+ if (dummyFonts[i].get() == closest.font) {
idx = i;
break;
}
diff --git a/tests/unittest/HbFontCacheTest.cpp b/tests/unittest/HbFontCacheTest.cpp
new file mode 100644
index 0000000..2c88077
--- /dev/null
+++ b/tests/unittest/HbFontCacheTest.cpp
@@ -0,0 +1,61 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "HbFontCache.h"
+
+#include <memory>
+
+#include <gtest/gtest.h>
+#include <hb.h>
+
+#include "minikin/MinikinFont.h"
+
+#include "FreeTypeMinikinFontForTest.h"
+#include "MinikinInternal.h"
+
+namespace minikin {
+
+class HbFontCacheTest : public testing::Test {
+public:
+ virtual void TearDown() {
+ android::AutoMutex _l(gMinikinLock);
+ }
+};
+
+TEST_F(HbFontCacheTest, getHbFontLockedTest) {
+ auto fontA = std::make_shared<FreeTypeMinikinFontForTest>(kTestFontDir "Regular.ttf");
+ auto fontB = std::make_shared<FreeTypeMinikinFontForTest>(kTestFontDir "Bold.ttf");
+ auto fontC = std::make_shared<FreeTypeMinikinFontForTest>(kTestFontDir "BoldItalic.ttf");
+
+ android::AutoMutex _l(gMinikinLock);
+ // Never return NULL.
+ EXPECT_NE(nullptr, getHbFontLocked(fontA.get()));
+ EXPECT_NE(nullptr, getHbFontLocked(fontB.get()));
+ EXPECT_NE(nullptr, getHbFontLocked(fontC.get()));
+
+ EXPECT_NE(nullptr, getHbFontLocked(nullptr));
+
+ // Must return same object if same font object is passed.
+ EXPECT_EQ(getHbFontLocked(fontA.get()), getHbFontLocked(fontA.get()));
+ EXPECT_EQ(getHbFontLocked(fontB.get()), getHbFontLocked(fontB.get()));
+ EXPECT_EQ(getHbFontLocked(fontC.get()), getHbFontLocked(fontC.get()));
+
+ // Different object must be returned if the passed minikinFont has different ID.
+ EXPECT_NE(getHbFontLocked(fontA.get()), getHbFontLocked(fontB.get()));
+ EXPECT_NE(getHbFontLocked(fontA.get()), getHbFontLocked(fontC.get()));
+}
+
+} // namespace minikin
diff --git a/tests/util/FontTestUtils.cpp b/tests/util/FontTestUtils.cpp
index 699d53b..77199ac 100644
--- a/tests/util/FontTestUtils.cpp
+++ b/tests/util/FontTestUtils.cpp
@@ -88,16 +88,15 @@
continue;
}
- FontStyle style(weight, italic);
if (index == nullptr) {
std::shared_ptr<MinikinFont> minikinFont =
std::make_shared<FreeTypeMinikinFontForTest>(fontPath);
- fonts.push_back(Font::Builder(minikinFont).setStyle(style).build());
+ fonts.push_back(Font(minikinFont, FontStyle(weight, italic)));
} else {
std::shared_ptr<MinikinFont> minikinFont =
std::make_shared<FreeTypeMinikinFontForTest>(fontPath,
atoi((const char*)index));
- fonts.push_back(Font::Builder(minikinFont).setStyle(style).build());
+ fonts.push_back(Font(minikinFont, FontStyle(weight, italic)));
}
}
@@ -126,7 +125,7 @@
std::shared_ptr<FontFamily> buildFontFamily(const std::string& filePath) {
auto font = std::make_shared<FreeTypeMinikinFontForTest>(filePath);
std::vector<Font> fonts;
- fonts.push_back(Font::Builder(font).build());
+ fonts.push_back(Font(font, FontStyle()));
return std::make_shared<FontFamily>(std::move(fonts));
}
diff --git a/tests/util/FreeTypeMinikinFontForTest.h b/tests/util/FreeTypeMinikinFontForTest.h
index d63b2bf..f9e0e9f 100644
--- a/tests/util/FreeTypeMinikinFontForTest.h
+++ b/tests/util/FreeTypeMinikinFontForTest.h
@@ -58,7 +58,7 @@
FT_Library mFtLibrary;
FT_Face mFtFace;
- MINIKIN_PREVENT_COPY_AND_ASSIGN(FreeTypeMinikinFontForTest);
+ PREVENT_COPY_AND_ASSIGN(FreeTypeMinikinFontForTest);
};
} // namespace minikin
