Reland "Reland "Split FreeType & FontConfig, step #1""
This reverts commit 33342f41f9b920a46c9d8e8975589e96fc089ef8.
Reason for revert: Fixiing the embolden issue with font config
> Revert "Reland "Split FreeType & FontConfig, step #1""
>
> This reverts commit 9b3146d7c3f42d08d68f9ecf840eb1678186edce.
>
> Reason for revert: Breaking Google3 tests
>
> > Reland "Split FreeType & FontConfig, step #1"
> >
> > This reverts commit e37b6b19801693c54b47bfe42c1cb2f904fe65ef.
> >
> > Reason for revert: Fixing the broken tests
> > Landed "RemoteStrike should use typeface Id from StrikeSpec"
> > https://skia.googlesource.com/skia.git/+/46d8175e88150ad4e870914c135a3baa72801b48
> >
> > Original change's description:
> > > Revert "Split FreeType & FontConfig, step #1"
> > >
> > > This reverts commit f5f3648c25ecd00f3d4ce31e60611d8e372c1429.
> > >
> > > Reason for revert: Broke some tests
> > >
> > > > Split FreeType & FontConfig, step #1
> > > >
> > > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/894476
> >
> > Reviewed-on: https://skia-review.googlesource.com/c/skia/+/894776
>
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/894418
Change-Id: I8fba80b919356bff23cdd0b5120bf795f7ca83fe
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/895610
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Florin Malita <fmalita@google.com>
Commit-Queue: Julia Lavrova <jlavrova@google.com>
diff --git a/include/core/SkTypeface.h b/include/core/SkTypeface.h
index e112992..5495bdf 100644
--- a/include/core/SkTypeface.h
+++ b/include/core/SkTypeface.h
@@ -352,6 +352,10 @@
// Must return a valid scaler context. It can not return nullptr.
virtual std::unique_ptr<SkScalerContext> onCreateScalerContext(const SkScalerContextEffects&,
const SkDescriptor*) const = 0;
+ virtual std::unique_ptr<SkScalerContext> onCreateScalerContextAsProxyTypeface
+ (const SkScalerContextEffects&,
+ const SkDescriptor*,
+ sk_sp<SkTypeface>) const;
virtual void onFilterRec(SkScalerContextRec*) const = 0;
friend class SkScalerContext; // onFilterRec
@@ -420,6 +424,7 @@
std::unique_ptr<SkAdvancedTypefaceMetrics> getAdvancedMetrics() const;
friend class SkRandomTypeface; // getAdvancedMetrics
friend class SkPDFFont; // getAdvancedMetrics
+ friend class SkTypeface_fontconfig;
friend class SkFontPriv; // getGlyphToUnicodeMap
diff --git a/src/core/SkScalerContext.h b/src/core/SkScalerContext.h
index df42cfa..7737607 100644
--- a/src/core/SkScalerContext.h
+++ b/src/core/SkScalerContext.h
@@ -450,6 +450,7 @@
friend class PathText; // For debug purposes
friend class PathTextBench; // For debug purposes
friend class RandomScalerContext; // For debug purposes
+ friend class SkScalerContext_fontconfig;
static SkScalerContextRec PreprocessRec(const SkTypeface&,
const SkScalerContextEffects&,
diff --git a/src/core/SkTypeface.cpp b/src/core/SkTypeface.cpp
index 24d2748..4abadec 100644
--- a/src/core/SkTypeface.cpp
+++ b/src/core/SkTypeface.cpp
@@ -354,6 +354,13 @@
return scalerContext;
}
+std::unique_ptr<SkScalerContext> SkTypeface::onCreateScalerContextAsProxyTypeface
+ (const SkScalerContextEffects&,
+ const SkDescriptor*,
+ sk_sp<SkTypeface>) const {
+ SK_ABORT("Not implemented.");
+}
+
void SkTypeface::unicharsToGlyphs(const SkUnichar uni[], int count, SkGlyphID glyphs[]) const {
if (count > 0 && glyphs && uni) {
this->onCharsToGlyphs(uni, count, glyphs);
diff --git a/src/ports/SkFontHost_FreeType.cpp b/src/ports/SkFontHost_FreeType.cpp
index f6f26c9..9a62181 100644
--- a/src/ports/SkFontHost_FreeType.cpp
+++ b/src/ports/SkFontHost_FreeType.cpp
@@ -441,7 +441,8 @@
public:
SkScalerContext_FreeType(sk_sp<SkTypeface_FreeType>,
const SkScalerContextEffects&,
- const SkDescriptor* desc);
+ const SkDescriptor* desc,
+ sk_sp<SkTypeface>);
~SkScalerContext_FreeType() override;
bool success() const {
@@ -690,10 +691,19 @@
}
std::unique_ptr<SkScalerContext> SkTypeface_FreeType::onCreateScalerContext(
- const SkScalerContextEffects& effects, const SkDescriptor* desc) const
-{
+ const SkScalerContextEffects& effects, const SkDescriptor* desc) const {
+ return this->onCreateScalerContextAsProxyTypeface(effects, desc, nullptr);
+}
+
+std::unique_ptr<SkScalerContext> SkTypeface_FreeType::onCreateScalerContextAsProxyTypeface(
+ const SkScalerContextEffects& effects,
+ const SkDescriptor* desc,
+ sk_sp<SkTypeface> realTypeface) const {
auto c = std::make_unique<SkScalerContext_FreeType>(
- sk_ref_sp(const_cast<SkTypeface_FreeType*>(this)), effects, desc);
+ sk_ref_sp(const_cast<SkTypeface_FreeType*>(this)),
+ effects,
+ desc,
+ realTypeface ? realTypeface : sk_ref_sp(const_cast<SkTypeface_FreeType*>(this)));
if (c->success()) {
return c;
}
@@ -892,16 +902,17 @@
return chosenStrikeIndex;
}
-SkScalerContext_FreeType::SkScalerContext_FreeType(sk_sp<SkTypeface_FreeType> typeface,
+SkScalerContext_FreeType::SkScalerContext_FreeType(sk_sp<SkTypeface_FreeType> proxyTypeface,
const SkScalerContextEffects& effects,
- const SkDescriptor* desc)
- : SkScalerContext(std::move(typeface), effects, desc)
+ const SkDescriptor* desc,
+ sk_sp<SkTypeface> realTypeface)
+ : SkScalerContext(realTypeface, effects, desc)
, fFace(nullptr)
, fFTSize(nullptr)
, fStrikeIndex(-1)
{
SkAutoMutexExclusive ac(f_t_mutex());
- fFaceRec = static_cast<SkTypeface_FreeType*>(this->getTypeface())->getFaceRec();
+ fFaceRec = proxyTypeface->getFaceRec();
// load the font file
if (nullptr == fFaceRec) {
diff --git a/src/ports/SkFontMgr_fontconfig.cpp b/src/ports/SkFontMgr_fontconfig.cpp
index 2cca01d..fef28e7 100644
--- a/src/ports/SkFontMgr_fontconfig.cpp
+++ b/src/ports/SkFontMgr_fontconfig.cpp
@@ -411,7 +411,8 @@
FcPatternAddInteger(pattern, FC_SLANT , slant);
}
-class SkTypeface_fontconfig : public SkTypeface_FreeType {
+class SkScalerContext_fontconfig;
+class SkTypeface_fontconfig : public SkTypeface {
public:
static sk_sp<SkTypeface_fontconfig> Make(SkAutoFcPattern pattern, SkString sysroot) {
return sk_sp<SkTypeface_fontconfig>(new SkTypeface_fontconfig(std::move(pattern),
@@ -423,7 +424,6 @@
void onGetFamilyName(SkString* familyName) const override {
*familyName = get_string(fPattern, FC_FAMILY);
}
-
void onGetFontDescriptor(SkFontDescriptor* desc, bool* serialize) const override {
// TODO: need to serialize FC_MATRIX and FC_EMBOLDEN
FCLocker lock;
@@ -431,26 +431,12 @@
desc->setFullName(get_string(fPattern, FC_FULLNAME));
desc->setPostscriptName(get_string(fPattern, FC_POSTSCRIPT_NAME));
desc->setStyle(this->fontStyle());
- desc->setFactoryId(SkTypeface_FreeType::FactoryId);
+ desc->setFactoryId(SkTypeface_FreeType::FactoryId); // TODO: Fix the Id
*serialize = false;
}
-
std::unique_ptr<SkStreamAsset> onOpenStream(int* ttcIndex) const override {
- FCLocker lock;
- *ttcIndex = get_int(fPattern, FC_INDEX, 0);
- const char* filename = get_string(fPattern, FC_FILE);
- // See FontAccessible for note on searching sysroot then non-sysroot path.
- SkString resolvedFilename;
- if (!fSysroot.isEmpty()) {
- resolvedFilename = fSysroot;
- resolvedFilename += filename;
- if (sk_exists(resolvedFilename.c_str(), kRead_SkFILE_Flag)) {
- filename = resolvedFilename.c_str();
- }
- }
- return SkStream::MakeFromFile(filename);
+ return fProxy->onOpenStream(ttcIndex);
}
-
void onFilterRec(SkScalerContextRec* rec) const override {
// FontConfig provides 10-scale-bitmap-fonts.conf which applies an inverse "pixelsize"
// matrix. It is not known if this .conf is active or not, so it is not clear if
@@ -478,44 +464,58 @@
if (get_bool(fPattern, FC_EMBOLDEN)) {
rec->fFlags |= SkScalerContext::kEmbolden_Flag;
}
- this->INHERITED::onFilterRec(rec);
+
+ fProxy->filterRec(rec);
}
-
std::unique_ptr<SkAdvancedTypefaceMetrics> onGetAdvancedMetrics() const override {
- std::unique_ptr<SkAdvancedTypefaceMetrics> info =
- this->INHERITED::onGetAdvancedMetrics();
-
+ std::unique_ptr<SkAdvancedTypefaceMetrics> info = fProxy->onGetAdvancedMetrics();
// Simulated fonts shouldn't be considered to be of the type of their data.
if (get_matrix(fPattern, FC_MATRIX) || get_bool(fPattern, FC_EMBOLDEN)) {
info->fType = SkAdvancedTypefaceMetrics::kOther_Font;
}
return info;
}
-
sk_sp<SkTypeface> onMakeClone(const SkFontArguments& args) const override {
- SkFontStyle style = this->fontStyle();
- std::unique_ptr<SkFontData> data = this->cloneFontData(args, &style);
- if (!data) {
- return nullptr;
- }
-
- // TODO: need to clone FC_MATRIX and FC_EMBOLDEN
- SkString familyName;
- this->getFamilyName(&familyName);
- return sk_make_sp<SkTypeface_FreeTypeStream>(
- std::move(data), familyName, style, this->isFixedPitch());
+ // TODO: need to clone FC_MATRIX and FC_EMBOLDEN by wrapping this
+ return fProxy->onMakeClone(args);
}
- std::unique_ptr<SkFontData> onMakeFontData() const override {
- int index;
- std::unique_ptr<SkStreamAsset> stream(this->onOpenStream(&index));
- if (!stream) {
- return nullptr;
- }
- // TODO: FC_VARIABLE and FC_FONT_VARIATIONS
- return std::make_unique<SkFontData>(std::move(stream), index, 0, nullptr, 0, nullptr, 0);
- }
+ std::unique_ptr<SkScalerContext> onCreateScalerContext(const SkScalerContextEffects& effects, const SkDescriptor* descr) const override;
+ void getGlyphToUnicodeMap(SkUnichar* glyphToUnicode) const override {
+ fProxy->getGlyphToUnicodeMap(glyphToUnicode);
+ }
+ void onCharsToGlyphs(const SkUnichar* chars, int count, SkGlyphID glyphs[]) const override {
+ fProxy->unicharsToGlyphs(chars, count, glyphs);
+ }
+ int onCountGlyphs() const override { return fProxy->countGlyphs(); }
+ int onGetUPEM() const override { return fProxy->getUnitsPerEm(); }
+ bool onGetPostScriptName(SkString* postScriptName) const override {
+ return fProxy->getPostScriptName(postScriptName);
+ }
+ SkTypeface::LocalizedStrings* onCreateFamilyNameIterator() const override {
+ return fProxy->createFamilyNameIterator();
+ }
+ void getPostScriptGlyphNames(SkString* names) const override {
+ return fProxy->getPostScriptGlyphNames(names);
+ }
+ bool onGlyphMaskNeedsCurrentColor() const override {
+ return fProxy->glyphMaskNeedsCurrentColor();
+ }
+ int onGetVariationDesignPosition(SkFontArguments::VariationPosition::Coordinate coordinates[],
+ int coordinateCount) const override {
+ return fProxy->onGetVariationDesignPosition(coordinates, coordinateCount);
+ }
+ int onGetVariationDesignParameters(SkFontParameters::Variation::Axis parameters[],
+ int parameterCount) const override {
+ return fProxy->onGetVariationDesignParameters(parameters, parameterCount);
+ }
+ int onGetTableTags(SkFontTableTag tags[]) const override {
+ return fProxy->getTableTags(tags);
+ }
+ size_t onGetTableData(SkFontTableTag tag, size_t offset, size_t length, void* data) const override {
+ return fProxy->getTableData(tag, offset, length, data);
+ }
~SkTypeface_fontconfig() override {
// Hold the lock while unrefing the pattern.
FCLocker lock;
@@ -524,15 +524,76 @@
private:
SkTypeface_fontconfig(SkAutoFcPattern pattern, SkString sysroot)
- : INHERITED(skfontstyle_from_fcpattern(pattern),
+ : SkTypeface(skfontstyle_from_fcpattern(pattern),
FC_PROPORTIONAL != get_int(pattern, FC_SPACING, FC_PROPORTIONAL))
, fPattern(std::move(pattern))
- , fSysroot(std::move(sysroot))
+ , fSysroot(std::move(sysroot)) {
+ // TODO: for now only FreeTypeStream
+ SkString resolvedFilename;
+ FCLocker lock;
+ const char* filename = get_string(fPattern, FC_FILE);
+ // See FontAccessible for note on searching sysroot then non-sysroot path.
+ if (!fSysroot.isEmpty()) {
+ resolvedFilename = fSysroot;
+ resolvedFilename += filename;
+ if (sk_exists(resolvedFilename.c_str(), kRead_SkFILE_Flag)) {
+ filename = resolvedFilename.c_str();
+ }
+ }
+ // TODO: FC_VARIABLE and FC_FONT_VARIATIONS in arguments
+ auto ttcIndex = get_int(fPattern, FC_INDEX, 0);
+ fProxy = SkTypeface_FreeType::MakeFromStream(
+ SkStream::MakeFromFile(filename), SkFontArguments().setCollectionIndex(ttcIndex));
+ }
+ sk_sp<SkTypeface> fProxy;
+};
+
+class SkScalerContext_fontconfig : public SkScalerContext {
+private:
+ std::unique_ptr<SkScalerContext> fProxy;
+public:
+ SkScalerContext_fontconfig(std::unique_ptr<SkScalerContext> proxy,
+ sk_sp<SkTypeface_fontconfig> typeface,
+ const SkScalerContextEffects& effects,
+ const SkDescriptor* desc)
+ : SkScalerContext(std::move(typeface), effects, desc)
+ , fProxy(std::move(proxy))
{ }
- using INHERITED = SkTypeface_FreeType;
+ ~SkScalerContext_fontconfig() override = default;
+protected:
+ SkScalerContext::GlyphMetrics generateMetrics(const SkGlyph& glyph, SkArenaAlloc* alloc) override {
+ return fProxy->generateMetrics(glyph, alloc);
+ }
+
+ void generateImage(const SkGlyph& glyph, void* imageBuffer) override {
+ fProxy->generateImage(glyph, imageBuffer);
+ }
+
+ bool generatePath(const SkGlyph& glyph, SkPath* path, bool* modified) override {
+ return fProxy->generatePath(glyph, path, modified);
+ }
+
+ sk_sp<SkDrawable> generateDrawable(const SkGlyph& glyph) override {
+ return fProxy->generateDrawable(glyph);
+ }
+
+ void generateFontMetrics(SkFontMetrics* metrics) override {
+ fProxy->generateFontMetrics(metrics);
+ }
};
+std::unique_ptr<SkScalerContext> SkTypeface_fontconfig::onCreateScalerContext(
+ const SkScalerContextEffects& effects, const SkDescriptor* descr) const {
+ auto proxyScaler = fProxy->onCreateScalerContextAsProxyTypeface(effects, descr, sk_ref_sp(this));
+ auto scaler = std::make_unique<SkScalerContext_fontconfig>(
+ std::move(proxyScaler),
+ sk_ref_sp(const_cast<SkTypeface_fontconfig*>(this)),
+ effects,
+ descr);
+ return scaler;
+}
+
class SkFontMgr_fontconfig : public SkFontMgr {
mutable SkAutoFcConfig fFC; // Only mutable to avoid const cast when passed to FontConfig API.
const SkString fSysroot;
diff --git a/src/ports/SkTypeface_FreeType.h b/src/ports/SkTypeface_FreeType.h
index 13c7131..23ca216 100644
--- a/src/ports/SkTypeface_FreeType.h
+++ b/src/ports/SkTypeface_FreeType.h
@@ -49,6 +49,10 @@
std::unique_ptr<SkFontData> cloneFontData(const SkFontArguments&, SkFontStyle* style) const;
std::unique_ptr<SkScalerContext> onCreateScalerContext(const SkScalerContextEffects&,
const SkDescriptor*) const override;
+ std::unique_ptr<SkScalerContext> onCreateScalerContextAsProxyTypeface(
+ const SkScalerContextEffects&,
+ const SkDescriptor*,
+ sk_sp<SkTypeface>) const override;
void onFilterRec(SkScalerContextRec*) const override;
void getGlyphToUnicodeMap(SkUnichar*) const override;
std::unique_ptr<SkAdvancedTypefaceMetrics> onGetAdvancedMetrics() const override;
diff --git a/tests/FontMgrFontConfigTest.cpp b/tests/FontMgrFontConfigTest.cpp
index b8657c8..b2e6296 100644
--- a/tests/FontMgrFontConfigTest.cpp
+++ b/tests/FontMgrFontConfigTest.cpp
@@ -33,16 +33,18 @@
namespace {
bool bitmap_compare(const SkBitmap& ref, const SkBitmap& test) {
+ auto count = 0;
for (int y = 0; y < test.height(); ++y) {
for (int x = 0; x < test.width(); ++x) {
SkColor testColor = test.getColor(x, y);
SkColor refColor = ref.getColor(x, y);
if (refColor != testColor) {
- return false;
+ ++count;
+ SkDebugf("%d: (%d,%d) ", count, x, y);
}
}
}
- return true;
+ return (count == 0);
}
FcConfig* build_fontconfig_with_fontfile(const char* fontFilename) {
@@ -65,14 +67,13 @@
SkString content;
content.append("<?xml version=\"1.0\" encoding=\"utf-8\"?>"
"<!DOCTYPE fontconfig SYSTEM \"fonts.dtd\">"
- "<fontconfig>\n");
- //content.appendf(" <cachedir>/fonts</cachedir\n>");
- content.appendf(" <dir>/fonts</dir>\n");
- content.appendf(" <match target=\"font\">\n"
- " <edit name=\"embolden\" mode=\"assign\">\n"
- " <bool>true</bool>\n"
- " </edit>\n"
- " </match>");
+ "<fontconfig>\n"
+ "<dir>/fonts</dir>\n");
+ content.append("<match target=\"font\">\n"
+ " <edit name=\"embolden\" mode=\"assign\">\n"
+ " <bool>true</bool>\n"
+ " </edit>\n"
+ "</match>");
content.append("</fontconfig>\n");
FcConfig* fc_config = FcConfigCreate();
FcConfigSetSysRoot(fc_config, reinterpret_cast<const FcChar8*>(path.c_str()));
@@ -122,7 +123,7 @@
constexpr float kTextSize = 20;
std::unique_ptr<SkStreamAsset> distortableStream(
- GetResourceAsStream("fonts/Distortable.ttf"));
+ GetResourceAsStream("fonts/Distortable.ttf"));
if (!distortableStream) {
return;
}
@@ -169,7 +170,7 @@
constexpr float kTextSize = 20;
constexpr char text[] = "abc";
- SkString filePath = GetResourcePath("/fonts/Roboto-Regular.ttf");
+ SkString filePath = GetResourcePath("fonts/Roboto-Regular.ttf");
sk_sp<SkTypeface> dataTypeface(fontMgr->makeFromFile(filePath.c_str(), 0));
if (!dataTypeface) {
ERRORF(reporter, "Could not find data typeface. FcVersion: %d", FcGetVersion());