Remove a layer of indirection and code from SkFontHost. (issue 105223006 by bungeman@google.com)

5 views
Skip to first unread message

bung...@google.com

unread,
Sep 10, 2014, 6:05:45 PM9/10/14
to mtk...@google.com, tomh...@google.com, skia-...@googlegroups.com, djso...@google.com
Reviewers: mtklein, tomhudson,

Message:
Now that Android is on SkFontMgr this can finally land. This essentially
moves
the implementation of some private SkFontHost methods into their only
callers.

Description:
Remove a layer of indirection and code from SkFontHost.

Please review this at https://codereview.chromium.org/105223006/

SVN Base: http://skia.googlecode.com/svn/trunk/

Affected files (+18, -72 lines):
M include/core/SkFontHost.h
M src/core/SkFontHost.cpp
M src/core/SkTypeface.cpp


Index: include/core/SkFontHost.h
diff --git a/include/core/SkFontHost.h b/include/core/SkFontHost.h
index
4c5013fe4a044d7c0cc2cfc91950e30d89804719..a2cc04bc709726e0c993d5e9da37408bdea87089
100644
--- a/include/core/SkFontHost.h
+++ b/include/core/SkFontHost.h
@@ -90,42 +90,6 @@ public:
static void SetSubpixelOrder(LCDOrder order);
/** @deprecated get from Device. */
static LCDOrder GetSubpixelOrder();
-
-private:
- /** Return a new, closest matching typeface given either an existing
family
- (specified by a typeface in that family) or by a familyName and a
- requested style.
- 1) If familyFace is null, use familyName.
- 2) If familyName is null, use data (UTF-16 to cover).
- 3) If all are null, return the default font that best matches style
- */
- static SkTypeface* CreateTypeface(const SkTypeface* familyFace,
- const char familyName[],
- SkTypeface::Style style);
-
- /** Return a new typeface given the data buffer. If the data does not
- represent a valid font, returns null.
-
- If a typeface instance is returned, the caller is responsible for
- calling unref() on the typeface when they are finished with it.
-
- The returned typeface may or may not have called ref() on the
stream
- parameter. If the typeface has not called ref(), then it may have
made
- a copy of the releveant data. In either case, the caller is still
- responsible for its refcnt ownership of the stream.
- */
- static SkTypeface* CreateTypefaceFromStream(SkStream*);
-
- /** Return a new typeface from the specified file path. If the file
does not
- represent a valid font, this returns null. If a typeface is
returned,
- the caller is responsible for calling unref() when it is no longer
used.
- */
- static SkTypeface* CreateTypefaceFromFile(const char path[]);
-
-
///////////////////////////////////////////////////////////////////////////
-
- friend class SkScalerContext;
- friend class SkTypeface;
};

#endif
Index: src/core/SkFontHost.cpp
diff --git a/src/core/SkFontHost.cpp b/src/core/SkFontHost.cpp
index
a4055a1d8a10a1cbd57cbb54a74a98711a33d017..c582ba5bfd4ca1ba11619c766f8ee53cc710a4e3
100644
--- a/src/core/SkFontHost.cpp
+++ b/src/core/SkFontHost.cpp
@@ -207,33 +207,3 @@ SkFontMgr* SkFontMgr::RefDefault() {
SK_DECLARE_STATIC_LAZY_PTR(SkFontMgr, singleton, CreateDefault);
return SkRef(singleton.get());
}
-
-//////////////////////////////////////////////////////////////////////////
-
-SkTypeface* SkFontHost::CreateTypeface(const SkTypeface* familyFace,
- const char familyName[],
- SkTypeface::Style style) {
- SkAutoTUnref<SkFontMgr> fm(SkFontMgr::RefDefault());
- if (familyFace) {
- bool bold = style & SkTypeface::kBold;
- bool italic = style & SkTypeface::kItalic;
- SkFontStyle newStyle = SkFontStyle(bold ? SkFontStyle::kBold_Weight
- :
SkFontStyle::kNormal_Weight,
- SkFontStyle::kNormal_Width,
- italic ?
SkFontStyle::kItalic_Slant
- :
SkFontStyle::kUpright_Slant);
- return fm->matchFaceStyle(familyFace, newStyle);
- } else {
- return fm->legacyCreateTypeface(familyName, style);
- }
-}
-
-SkTypeface* SkFontHost::CreateTypefaceFromFile(const char path[]) {
- SkAutoTUnref<SkFontMgr> fm(SkFontMgr::RefDefault());
- return fm->createFromFile(path);
-}
-
-SkTypeface* SkFontHost::CreateTypefaceFromStream(SkStream* stream) {
- SkAutoTUnref<SkFontMgr> fm(SkFontMgr::RefDefault());
- return fm->createFromStream(stream);
-}
Index: src/core/SkTypeface.cpp
diff --git a/src/core/SkTypeface.cpp b/src/core/SkTypeface.cpp
index
01b534c95fa4adb28f216b4818e4ef9c42b957cd..fcb2b8e2b13ae7d7430cb9863ff47a9f4a0bf650
100644
--- a/src/core/SkTypeface.cpp
+++ b/src/core/SkTypeface.cpp
@@ -8,7 +8,7 @@
#include "SkAdvancedTypefaceMetrics.h"
#include "SkEndian.h"
#include "SkFontDescriptor.h"
-#include "SkFontHost.h"
+#include "SkFontMgr.h"
#include "SkLazyPtr.h"
#include "SkOTTable_OS_2.h"
#include "SkStream.h"
@@ -84,7 +84,8 @@ SkTypeface* SkTypeface::CreateDefault(int style) {
// TODO(bungeman, mtklein): This is sad. Make our fontconfig code
safe?
SkAutoMutexAcquire lock(&gCreateDefaultMutex);

- SkTypeface* t = SkFontHost::CreateTypeface(NULL, NULL, (Style)style);
+ SkAutoTUnref<SkFontMgr> fm(SkFontMgr::RefDefault());
+ SkTypeface* t = fm->legacyCreateTypeface(NULL, style);;
return t ? t : SkEmptyTypeface::Create();
}

@@ -123,7 +124,8 @@ SkTypeface* SkTypeface::CreateFromName(const char
name[], Style style) {
if (NULL == name) {
return RefDefault(style);
}
- return SkFontHost::CreateTypeface(NULL, name, style);
+ SkAutoTUnref<SkFontMgr> fm(SkFontMgr::RefDefault());
+ return fm->legacyCreateTypeface(name, style);
}

SkTypeface* SkTypeface::CreateFromTypeface(const SkTypeface* family, Style
s) {
@@ -131,15 +133,25 @@ SkTypeface* SkTypeface::CreateFromTypeface(const
SkTypeface* family, Style s) {
family->ref();
return const_cast<SkTypeface*>(family);
}
- return SkFontHost::CreateTypeface(family, NULL, s);
+ SkAutoTUnref<SkFontMgr> fm(SkFontMgr::RefDefault());
+ bool bold = s & SkTypeface::kBold;
+ bool italic = s & SkTypeface::kItalic;
+ SkFontStyle newStyle = SkFontStyle(bold ? SkFontStyle::kBold_Weight
+ : SkFontStyle::kNormal_Weight,
+ SkFontStyle::kNormal_Width,
+ italic ? SkFontStyle::kItalic_Slant
+ :
SkFontStyle::kUpright_Slant);
+ return fm->matchFaceStyle(family, newStyle);
}

SkTypeface* SkTypeface::CreateFromStream(SkStream* stream) {
- return SkFontHost::CreateTypefaceFromStream(stream);
+ SkAutoTUnref<SkFontMgr> fm(SkFontMgr::RefDefault());
+ return fm->createFromStream(stream);
}

SkTypeface* SkTypeface::CreateFromFile(const char path[]) {
- return SkFontHost::CreateTypefaceFromFile(path);
+ SkAutoTUnref<SkFontMgr> fm(SkFontMgr::RefDefault());
+ return fm->createFromFile(path);
}


///////////////////////////////////////////////////////////////////////////////


bung...@google.com

unread,
Sep 10, 2014, 6:55:33 PM9/10/14
to mtk...@google.com, tomh...@google.com, djso...@google.com, skia-...@googlegroups.com
On 2014/09/10 22:49:55, I haz the power (commit-bot) wrote:
> Committed patchset #2 (id:20001) as
> f91c47d91d72a1d85e2d6701864b8d7accc81647

I didn't actually mean to commit this, just run the CQ bots. See
https://code.google.com/p/skia/issues/detail?id=2927 for details. Feel free
to
consider yourselves TBRed!

https://codereview.chromium.org/105223006/

mtk...@google.com

unread,
Sep 10, 2014, 7:30:40 PM9/10/14
to bung...@google.com, tomh...@google.com, djso...@google.com, skia-...@googlegroups.com

tomh...@google.com

unread,
Sep 11, 2014, 8:16:31 AM9/11/14
to bung...@google.com, mtk...@google.com, djso...@google.com, skia-...@googlegroups.com
Moot LGTM.
Any chance of getting rid of legacyCreateTypeface() too?

https://codereview.chromium.org/105223006/
Reply all
Reply to author
Forward
0 new messages