Reland "Pin FcConfig for FCIDirect at creation."
This reverts commit d7f7cc87910800232c5900ebb64c0c00781cd0c2.
SkFontConfigInterfaceDirect class methods used the FontConfig library
static global "current" FcConfig (implicitly through the use of
nullptr). This was pinned down once per call to each method which used
it (to avoid the "current" FcConfig from being changed out from under it
while running). However, the use of global state as a matter of course
makes it very difficult to reliably test.
Modify SkFontConfigInterface to optionally take an FcConfig on
contruction. If nullptr is provided it is equivelent to the old behavior
so that existing users are unaffected. SkFontConfigInterface takes
ownership of any passed FcConfig and will release it on destruction.
Bug: skia:12916
Change-Id: I20a3cd9405ad40f28b394c713c0514aaa3b08cd0
Revert-Reviewed-on: https://skia-review.googlesource.com/c/skia/+/504776
Revert-Change-Id: I812547bf27371ab716b7a167d7e975f7538d37fb
Revert-Reason: google3 roll failure due to memory leak
Original-Reviewed-on: https://skia-review.googlesource.com/c/skia/+/504477
Origianl-Change-Id: Ie3573403a95c6bf627ce5ff7f2eb5617c9cd162d
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/505136
Reviewed-by: Herb Derby <herb@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
diff --git a/src/ports/SkFontConfigInterface_direct.cpp b/src/ports/SkFontConfigInterface_direct.cpp
index 0cfc932..c16b2c1 100644
--- a/src/ports/SkFontConfigInterface_direct.cpp
+++ b/src/ports/SkFontConfigInterface_direct.cpp
@@ -493,11 +493,15 @@
const char* kFontFormatCFF = "CFF";
#endif
-SkFontConfigInterfaceDirect::SkFontConfigInterfaceDirect() {
+SkFontConfigInterfaceDirect::SkFontConfigInterfaceDirect(FcConfig* fc) : fFC(fc)
+{
SkDEBUGCODE(fontconfiginterface_unittest();)
}
SkFontConfigInterfaceDirect::~SkFontConfigInterfaceDirect() {
+ if (fFC) {
+ FcConfigDestroy(fFC);
+ }
}
bool SkFontConfigInterfaceDirect::isAccessible(const char* filename) {
@@ -523,8 +527,15 @@
if (!c_filename) {
return false;
}
- UniqueFCConfig fcConfig(FcConfigReference(nullptr));
- const char* sysroot = (const char*)FcConfigGetSysRoot(fcConfig.get());
+
+ FcConfig* fc = fFC;
+ UniqueFCConfig fcStorage;
+ if (!fc) {
+ fcStorage.reset(FcConfigReference(nullptr));
+ fc = fcStorage.get();
+ }
+
+ const char* sysroot = (const char*)FcConfigGetSysRoot(fc);
SkString resolvedFilename;
if (sysroot) {
resolvedFilename = sysroot;
@@ -584,8 +595,14 @@
return false;
}
+ FcConfig* fc = fFC;
+ UniqueFCConfig fcStorage;
+ if (!fc) {
+ fcStorage.reset(FcConfigReference(nullptr));
+ fc = fcStorage.get();
+ }
+
FCLocker lock;
- UniqueFCConfig fcConfig(FcConfigReference(nullptr));
FcPattern* pattern = FcPatternCreate();
if (familyName) {
@@ -595,7 +612,7 @@
FcPatternAddBool(pattern, FC_SCALABLE, FcTrue);
- FcConfigSubstitute(fcConfig.get(), pattern, FcMatchPattern);
+ FcConfigSubstitute(fc, pattern, FcMatchPattern);
FcDefaultSubstitute(pattern);
// Font matching:
@@ -634,7 +651,7 @@
}
FcResult result;
- FcFontSet* font_set = FcFontSort(fcConfig.get(), pattern, 0, nullptr, &result);
+ FcFontSet* font_set = FcFontSort(fc, pattern, 0, nullptr, &result);
if (!font_set) {
FcPatternDestroy(pattern);
return false;
@@ -662,7 +679,7 @@
FcFontSetDestroy(font_set);
return false;
}
- const char* sysroot = (const char*)FcConfigGetSysRoot(fcConfig.get());
+ const char* sysroot = (const char*)FcConfigGetSysRoot(fc);
SkString resolvedFilename;
if (sysroot) {
resolvedFilename = sysroot;
diff --git a/src/ports/SkFontConfigInterface_direct.h b/src/ports/SkFontConfigInterface_direct.h
index 8293bfd..e0f3127 100644
--- a/src/ports/SkFontConfigInterface_direct.h
+++ b/src/ports/SkFontConfigInterface_direct.h
@@ -15,7 +15,11 @@
class SkFontConfigInterfaceDirect : public SkFontConfigInterface {
public:
- SkFontConfigInterfaceDirect();
+ /** Create around a FontConfig instance.
+ * If 'fc' is nullptr, each method call will use the current config.
+ * Takes ownership of 'fc' and will call FcConfigDestroy on it.
+ */
+ SkFontConfigInterfaceDirect(FcConfig* fc);
~SkFontConfigInterfaceDirect() override;
bool matchFamilyName(const char familyName[],
@@ -30,6 +34,7 @@
virtual bool isAccessible(const char* filename);
private:
+ FcConfig * const fFC;
bool isValidPattern(FcPattern* pattern);
FcPattern* MatchFont(FcFontSet* font_set, const char* post_config_family,
const SkString& family);
diff --git a/src/ports/SkFontConfigInterface_direct_factory.cpp b/src/ports/SkFontConfigInterface_direct_factory.cpp
index 7b70b64..f580bd5 100644
--- a/src/ports/SkFontConfigInterface_direct_factory.cpp
+++ b/src/ports/SkFontConfigInterface_direct_factory.cpp
@@ -11,6 +11,6 @@
SkFontConfigInterface* SkFontConfigInterface::GetSingletonDirectInterface() {
static SkFontConfigInterface* singleton;
static SkOnce once;
- once([]{ singleton = new SkFontConfigInterfaceDirect(); });
+ once([]{ singleton = new SkFontConfigInterfaceDirect(nullptr); });
return singleton;
}
diff --git a/tests/FontMgrFontConfigTest.cpp b/tests/FontMgrFontConfigTest.cpp
index ddda539..fe7e4d6 100644
--- a/tests/FontMgrFontConfigTest.cpp
+++ b/tests/FontMgrFontConfigTest.cpp
@@ -134,8 +134,7 @@
}
FcConfig* config = build_fontconfig_with_fontfile("/fonts/NotoSansCJK-VF-subset.otf.ttc");
- FcConfigSetCurrent(config);
- sk_sp<SkFontConfigInterfaceDirect> fciDirect(new SkFontConfigInterfaceDirect());
+ sk_sp<SkFontConfigInterfaceDirect> fciDirect(new SkFontConfigInterfaceDirect(config));
std::vector<std::string> family_names{{"Noto Sans CJK JP",
"Noto Sans CJK HK",
