allow createScalerContext to return null, and then have the GDI backend trigger that if we fail to …

src/ports/SkFontHost_win.cpp

 
 /*
  * Copyright 2006 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkAdvancedTypefaceMetrics.h"
 #include "SkBase64.h"
@@ skipping 412 matching lines @@
     enum {
         // will always trigger us to reset the color, since we
         // should only store 0 or 0x00FFFFFF or gray (0x007F7F7F)
         kInvalid_Color = 12345
     };
 };
 
 const void* HDCOffscreen::draw(const SkGlyph& glyph, bool isBW,
                                size_t* srcRBPtr) {
     if (0 == fDC) {
         fDC = CreateCompatibleDC(0);

[bungeman-skia, 2013/07/09 22:45:37]

We do check creation of this guy, and just return an empty glyph if we can't
create it. I think the idea of delay crating this was to avoid creating it until
we actually need the glyph masks. It may just be better to set this guy up in
the constructor and have just the one DC, since they both have the same font in
them.

[reed1, 2013/07/10 13:59:04]

Good optimization -- may do in a 2nd CL

[bungeman-skia, 2013/07/10 14:58:25]

Changing this so this guy is part of initialization failure (where failure
invokes a cache gc) isn't really an optimization (in some sense its a
de-optimization); without it we will sometimes just not draw (when we could have
drawn, if only we had done a cache gc). Same issue for the HBITMAP below.

         if (0 == fDC) {
             return NULL;
         }
         SetGraphicsMode(fDC, GM_ADVANCED);
         SetBkMode(fDC, TRANSPARENT);
         SetTextAlign(fDC, TA_LEFT | TA_BASELINE);
         SelectObject(fDC, fFont);
 
         COLORREF color = 0x00FFFFFF;
         SkDEBUGCODE(COLORREF prev =) SetTextColor(fDC, color);
@@ skipping 22 matching lines @@
         }
         info.bmiHeader.biSize = sizeof(info.bmiHeader);
         info.bmiHeader.biWidth = biWidth;
         info.bmiHeader.biHeight = fHeight;
         info.bmiHeader.biPlanes = 1;
         info.bmiHeader.biBitCount = isBW ? 1 : 32;
         info.bmiHeader.biCompression = BI_RGB;
         if (isBW) {
             info.bmiHeader.biClrUsed = 2;
         }
         fBM = CreateDIBSection(fDC, &info, DIB_RGB_COLORS, &fBits, 0, 0);

[bungeman-skia, 2013/07/09 22:45:37]

This is an HBITMAP, so this could fail too. This one is harder in that it is
actually something we resize at runtime. We could be squirrely and just TODO
this one. 

         if (0 == fBM) {
             return NULL;
         }
         SelectObject(fDC, fBM);
     }
 
     // erase
     size_t srcRB = isBW ? (biWidth >> 3) : (fWidth << 2);
     size_t size = fHeight * srcRB;
     memset(fBits, 0, size);
@@ skipping 14 matching lines @@
     return (const char*)fBits + (fHeight - glyph.fHeight) * srcRB;
 }
 
 //////////////////////////////////////////////////////////////////////////////
 
 class SkScalerContext_Windows : public SkScalerContext {
 public:
     SkScalerContext_Windows(SkTypeface*, const SkDescriptor* desc);
     virtual ~SkScalerContext_Windows();
 
+    // Returns true if the constructor was able to complete all of its
+    // initializations (which may include calling GDI).
+    bool isValid() const;
+
 protected:
     virtual unsigned generateGlyphCount() SK_OVERRIDE;
     virtual uint16_t generateCharToGlyph(SkUnichar uni) SK_OVERRIDE;
     virtual void generateAdvance(SkGlyph* glyph) SK_OVERRIDE;
     virtual void generateMetrics(SkGlyph* glyph) SK_OVERRIDE;
     virtual void generateImage(const SkGlyph& glyph) SK_OVERRIDE;
     virtual void generatePath(const SkGlyph& glyph, SkPath* path) SK_OVERRIDE;
     virtual void generateFontMetrics(SkPaint::FontMetrics* mX,
                                      SkPaint::FontMetrics* mY) SK_OVERRIDE;
 
@@ skipping 55 matching lines @@
         : SkScalerContext(rawTypeface, desc)
         , fDDC(0)
         , fFont(0)
         , fSavefont(0)
         , fSC(0)
         , fGlyphCount(-1)
 {
     LogFontTypeface* typeface = reinterpret_cast<LogFontTypeface*>(rawTypeface);
 
     fDDC = ::CreateCompatibleDC(NULL);
+    if (!fDDC) {
+        return;
+    }
+
     SetGraphicsMode(fDDC, GM_ADVANCED);
     SetBkMode(fDDC, TRANSPARENT);
 
     // Scaling by the DPI is inconsistent with how Skia draws elsewhere
     //SkScalar height = -(fRec.fTextSize * GetDeviceCaps(ddc, LOGPIXELSY) / 72);
     LOGFONT lf = typeface->fLogFont;
     lf.lfHeight = -gCanonicalTextSize;
     lf.lfQuality = compute_quality(fRec);
     fFont = CreateFontIndirect(&lf);
+    if (!fFont) {
+        return;
+    }
 
     // if we're rotated, or want fractional widths, create a hires font
     fHiResFont = 0;
     if (needHiResMetrics(fRec.fPost2x2)) {
         lf.lfHeight = -HIRES_TEXTSIZE;
         fHiResFont = CreateFontIndirect(&lf);

[bungeman-skia, 2013/07/09 22:45:37]

fHiResFont? I suppose things should still work without it, but that seems rather
fishy to have such a subtle change when we could probably just purge the cache
and give the user what they wanted.

[reed1, 2013/07/10 13:59:04]

Done.

 
         fMat22Identity.eM11 = fMat22Identity.eM22 = SkFixedToFIXED(SK_Fixed1);
         fMat22Identity.eM12 = fMat22Identity.eM21 = SkFixedToFIXED(0);
 
         // construct a matrix to go from HIRES logical units to our device units
         fRec.getSingleMatrix(&fHiResMatrix);
         SkScalar scale = SkScalarInvert(SkIntToScalar(HIRES_TEXTSIZE));
         fHiResMatrix.preScale(scale, scale);
     }
     fSavefont = (HFONT)SelectObject(fDDC, fFont);
@@ skipping 53 matching lines @@
         xform.eDy = 0.0f;
 
         // fPost2x2 is column-major, left handed (y down).
         // MAT2 is row major, right handed (y up).
         fMat22.eM11 = SkScalarToFIXED(fRec.fPost2x2[0][0]);
         fMat22.eM12 = SkScalarToFIXED(-fRec.fPost2x2[1][0]);
         fMat22.eM21 = SkScalarToFIXED(-fRec.fPost2x2[0][1]);
         fMat22.eM22 = SkScalarToFIXED(fRec.fPost2x2[1][1]);
 
         lf.lfHeight = -SkScalarCeilToInt(fRec.fTextSize);
         HFONT bitmapFont = CreateFontIndirect(&lf);

[bungeman-skia, 2013/07/09 22:45:37]

here (though this replaces fFont, so we could remove it from the fDDC, delete it
first to free up a handle, but this could fail anyway due to some other thread
snatching it up first)

[reed1, 2013/07/10 13:59:04]

I think this guy just works. If bitmapFont is 0, then we ultimately assign it to
fFont, and we will be marked as invalid.

         SelectObject(fDDC, bitmapFont);
         ::DeleteObject(fFont);
         fFont = bitmapFont;
 
         if (0 == GetTextMetrics(fDDC, &fTM)) {
             call_ensure_accessible(lf);
             //if the following fails, we'll just draw at gCanonicalTextSize.
             GetTextMetrics(fDDC, &fTM);
         }
     }
@@ skipping 10 matching lines @@
         ::DeleteObject(fFont);
     }
     if (fHiResFont) {
         ::DeleteObject(fHiResFont);
     }
     if (fSC) {
         ::ScriptFreeCache(&fSC);
     }
 }
 
+bool SkScalerContext_Windows::isValid() const {
+    return fDDC && fFont;
+}
+
 unsigned SkScalerContext_Windows::generateGlyphCount() {
     if (fGlyphCount < 0) {
         if (fType == SkScalerContext_Windows::kBitmap_Type) {
            return fTM.tmLastChar;
         }
         fGlyphCount = calculateOutlineGlyphCount(fDDC);
     }
     return fGlyphCount;
 }
 
@@ skipping 943 matching lines @@
     }
 
     SelectObject(hdc, savefont);
     DeleteObject(font);
     DeleteDC(hdc);
 
     return stream;
 }
 
 SkScalerContext* LogFontTypeface::onCreateScalerContext(const SkDescriptor* desc) const {
-    return SkNEW_ARGS(SkScalerContext_Windows, (const_cast<LogFontTypeface*>(this), desc));
+    SkScalerContext_Windows* ctx = SkNEW_ARGS(SkScalerContext_Windows,

[bungeman-skia, 2013/07/09 22:45:37]

It may be easier to understand the intent with something like:

SkAutoTDelete<SkScalerContext_Windows> ctx(SkNEW_ARGS(SkScalerContext_Windows,
(const_cast<LogFontTypeface*>(this), desc)));

return ctx->isValid() ? ctx.detach() : NULL;

[reed1, 2013/07/10 13:59:04]

Probably a matter of taste, as I find the current way clearer, but I'll make the
change.

+                                                (const_cast<LogFontTypeface*>(this), desc));
+    if (!ctx->isValid()) {
+        SkDELETE(ctx);
+        ctx = NULL;
+    }
+    return ctx;
 }
 
 /** Return the 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 familyFace.
  3) If both are null, return the default font that best matches style
  This MUST not return NULL.
  */
@@ skipping 239 matching lines @@
         return this->createFromStream(stream);
     }
 
 private:
     SkTDArray<ENUMLOGFONTEX> fLogFontArray;
 };
 
 SkFontMgr* SkFontMgr::Factory() {
     return SkNEW(SkFontMgrGDI);
 }