De-generalize create_typeface on Windows.

src/ports/SkFontHost_win_dw.cpp

Index: src/ports/SkFontHost_win_dw.cpp
===================================================================
--- src/ports/SkFontHost_win_dw.cpp	(revision 12308)
+++ src/ports/SkFontHost_win_dw.cpp	(working copy)
@@ -1724,43 +1724,6 @@
     return info;
 }
 
-static SkTypeface* create_typeface(const SkTypeface* familyFace,
-                                   const char familyName[],
-                                   unsigned style,
-                                   SkFontMgr_DirectWrite* fontMgr) {
-    HRESULT hr;
-    SkTScopedComPtr<IDWriteFontFamily> fontFamily;
-    if (familyFace) {
-        const DWriteFontTypeface* face = static_cast<const DWriteFontTypeface*>(familyFace);
-        *(&fontFamily) = SkRefComPtr(face->fDWriteFontFamily.get());
-
-    } else if (familyName) {
-        hr = get_by_family_name(familyName, &fontFamily);
-    }
-
-    if (NULL == fontFamily.get()) {
-        //No good family found, go with default.
-        SkTScopedComPtr<IDWriteFont> font;
-        hr = get_default_font(&font);
-        hr = font->GetFontFamily(&fontFamily);
-    }
-
-    SkTScopedComPtr<IDWriteFont> font;
-    DWRITE_FONT_WEIGHT weight = (style & SkTypeface::kBold)
-                                 ? DWRITE_FONT_WEIGHT_BOLD
-                                 : DWRITE_FONT_WEIGHT_NORMAL;
-    DWRITE_FONT_STRETCH stretch = DWRITE_FONT_STRETCH_UNDEFINED;
-    DWRITE_FONT_STYLE italic = (style & SkTypeface::kItalic)
-                                ? DWRITE_FONT_STYLE_ITALIC
-                                : DWRITE_FONT_STYLE_NORMAL;
-    hr = fontFamily->GetFirstMatchingFont(weight, stretch, italic, &font);
-
-    SkTScopedComPtr<IDWriteFontFace> fontFace;
-    hr = font->CreateFontFace(&fontFace);
-
-    return fontMgr->createTypefaceFromDWriteFont(fontFace.get(), font.get(), fontFamily.get());
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 
 static void get_locale_string(IDWriteLocalizedStrings* names, const WCHAR* preferedLocale,
@@ -1869,7 +1832,33 @@
 
 SkTypeface* SkFontMgr_DirectWrite::onLegacyCreateTypeface(const char familyName[],
                                                           unsigned styleBits) {
-    return create_typeface(NULL, familyName, styleBits, this);
+    HRESULT hr;
+    SkTScopedComPtr<IDWriteFontFamily> fontFamily;
+    if (familyName) {
+        hr = get_by_family_name(familyName, &fontFamily);

[caryclark, 2013/11/19 21:56:39]

if 'hr' is always effectively discarded, would

  (void) get_by_family_name(

serve as well? As is, I keep scanning to see if hr is ever checked.

[bungeman-skia, 2013/11/19 22:27:34]

Yes, well, it's here because it's a big red flag that error checking hasn't
actually been thought about yet. If I remember correctly, this call wasn't
allowed to ever return NULL, but that was difficult. I believe this is now
allowed to return NULL if all sorts of things fail, so I will update
appropriately.

+    }
+
+    if (NULL == fontFamily.get()) {
+        //No good family found, go with default.
+        SkTScopedComPtr<IDWriteFont> font;
+        hr = get_default_font(&font);

[caryclark, 2013/11/19 21:56:39]

I see that 'font' here is not the same as 'font' below, but I'd feel better if
the two variables didn't share the same name.

[bungeman-skia, 2013/11/19 22:27:34]

Done. Yes, this is a bit confusing, renamed this one to defaultFont.

+        hr = font->GetFontFamily(&fontFamily);
+    }
+
+    SkTScopedComPtr<IDWriteFont> font;
+    DWRITE_FONT_WEIGHT weight = (styleBits & SkTypeface::kBold)
+                              ? DWRITE_FONT_WEIGHT_BOLD
+                              : DWRITE_FONT_WEIGHT_NORMAL;
+    DWRITE_FONT_STRETCH stretch = DWRITE_FONT_STRETCH_UNDEFINED;
+    DWRITE_FONT_STYLE italic = (styleBits & SkTypeface::kItalic)
+                             ? DWRITE_FONT_STYLE_ITALIC
+                             : DWRITE_FONT_STYLE_NORMAL;
+    hr = fontFamily->GetFirstMatchingFont(weight, stretch, italic, &font);
+
+    SkTScopedComPtr<IDWriteFontFace> fontFace;
+    hr = font->CreateFontFace(&fontFace);
+
+    return this->createTypefaceFromDWriteFont(fontFace.get(), font.get(), fontFamily.get());
 }
 
 ///////////////////////////////////////////////////////////////////////////////