De-generalize create_typeface on Windows.

src/ports/SkFontHost_win_dw.cpp

 /*
  * Copyright 2011 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkTypes.h"
 #undef GetGlyphIndices
 
@@ skipping 1706 matching lines @@
                                glyphCount,
                                glyphIDs,
                                glyphIDsCount,
                                getWidthAdvance));
         }
     }
 
     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,
                               SkString* skname) {
     UINT32 nameIndex = 0;
     if (preferedLocale) {
         // Ignore any errors and continue with index 0 if there is a problem.
         BOOL nameExists;
         names->FindLocaleName(preferedLocale, &nameIndex, &nameExists);
         if (!nameExists) {
@@ skipping 88 matching lines @@
     return this->createFromStream(stream, ttcIndex);
 }
 
 SkTypeface* SkFontMgr_DirectWrite::onCreateFromFile(const char path[], int ttcIndex) {
     SkAutoTUnref<SkStream> stream(SkStream::NewFromFile(path));
     return this->createFromStream(stream, ttcIndex);
 }
 
 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());
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 int SkFontStyleSet_DirectWrite::count() {
     return fFontFamily->GetFontCount();
 }
 
 SkTypeface* SkFontStyleSet_DirectWrite::createTypeface(int index) {
     SkTScopedComPtr<IDWriteFont> font;
@@ skipping 97 matching lines @@
         SK_TRACEHR(hr, "Could not get GetUserDefaultLocaleName.");
     } else {
         localeNameLen = getUserDefaultLocaleNameProc(localeNameStorage, LOCALE_NAME_MAX_LENGTH);
         if (localeNameLen) {
             localeName = localeNameStorage;
         };
     }
 
     return SkNEW_ARGS(SkFontMgr_DirectWrite, (sysFontCollection.get(), localeName, localeNameLen));
 }