Add a mutex to protect the DWrite calls.

src/ports/SkScalerContext_win_dw.cpp

Index: src/ports/SkScalerContext_win_dw.cpp
diff --git a/src/ports/SkScalerContext_win_dw.cpp b/src/ports/SkScalerContext_win_dw.cpp
index e259479fcb7d320487eb36d70a5559203b705098..5f4e0f1d8eb71d80edff484bfb91fc1caf3f2c49 100644
--- a/src/ports/SkScalerContext_win_dw.cpp
+++ b/src/ports/SkScalerContext_win_dw.cpp
@@ -15,6 +15,7 @@
 #include "SkHRESULT.h"
 #include "SkMaskGamma.h"
 #include "SkMatrix22.h"
+#include "SkMutex.h"
 #include "SkOTTable_EBLC.h"
 #include "SkOTTable_EBSC.h"
 #include "SkOTTable_gasp.h"
@@ -30,11 +31,30 @@
 #  include <dwrite_1.h>
 #endif
 
+/* Note:
+ * In versions 8 and 8.1 of Windows, some calls in DWrite are not thread safe.
+ * The DWriteFactoryMutex protects the calls that are problematic.
+ */
+SK_DECLARE_STATIC_MUTEX(DWriteFactoryMutex);
+
+class Exclusive {

[bungeman-skia, 2015/10/21 14:44:31]

What is this providing us that SkAutoMutexAcquire isn't already? Is the idea to
make this more like FCLocker in src/ports/SkFontMgr_fontconfig.cpp?

[herb_g, 2015/10/21 14:54:41]

SkAutoMutexAcquire is heavier weight because it always check to see if the mutex
is NULL (this is used in some use cases in the code). Since the mutex can not be
NULL, then this wrapper can be lighter weight and results in fewer if
statements. This pattern has been used in other parts of the code.

[bungeman-skia, 2015/10/21 15:27:13]

The extra check against NULL is pretty much nothing against the rest of the
mutex. In addition, if this is being done in other parts of the code it needs to
be factored out instead of adding yet another name for this.

+public:
+    Exclusive(SkBaseMutex& mutex) : fMutex(mutex) {
+        fMutex.acquire();
+    }
+    ~Exclusive() {
+        fMutex.release();
+    }
+private:
+    SkBaseMutex& fMutex;
+};
+
 static bool isLCD(const SkScalerContext::Rec& rec) {
     return SkMask::kLCD16_Format == rec.fMaskFormat;
 }
 
 static bool is_hinted_without_gasp(DWriteFontTypeface* typeface) {
+    Exclusive l(DWriteFactoryMutex);
     AutoTDWriteTable<SkOTTableMaximumProfile> maxp(typeface->fDWriteFontFace.get());
     if (!maxp.fExists) {
         return false;
@@ -104,6 +124,7 @@ static void expand_range_if_gridfit_only(DWriteFontTypeface* typeface, int size,
 }
 
 static bool has_bitmap_strike(DWriteFontTypeface* typeface, PPEMRange range) {
+    Exclusive l(DWriteFactoryMutex);
     {
         AutoTDWriteTable<SkOTTableEmbeddedBitmapLocation> eblc(typeface->fDWriteFontFace.get());
         if (!eblc.fExists) {
@@ -337,6 +358,7 @@ void SkScalerContext_DW::generateAdvance(SkGlyph* glyph) {
     if (DWRITE_MEASURING_MODE_GDI_CLASSIC == fMeasuringMode ||
         DWRITE_MEASURING_MODE_GDI_NATURAL == fMeasuringMode)
     {
+        Exclusive l(DWriteFactoryMutex);
         HRVM(fTypeface->fDWriteFontFace->GetGdiCompatibleGlyphMetrics(
                  fTextSizeMeasure,
                  1.0f, // pixelsPerDip
@@ -346,12 +368,16 @@ void SkScalerContext_DW::generateAdvance(SkGlyph* glyph) {
                  &gm),
              "Could not get gdi compatible glyph metrics.");
     } else {
+        Exclusive l(DWriteFactoryMutex);
         HRVM(fTypeface->fDWriteFontFace->GetDesignGlyphMetrics(&glyphId, 1, &gm),
              "Could not get design metrics.");
     }
 
     DWRITE_FONT_METRICS dwfm;
-    fTypeface->fDWriteFontFace->GetMetrics(&dwfm);
+    {
+        Exclusive l(DWriteFactoryMutex);
+        fTypeface->fDWriteFontFace->GetMetrics(&dwfm);
+    }
     SkScalar advanceX = SkScalarMulDiv(fTextSizeMeasure,
                                        SkIntToScalar(gm.advanceWidth),
                                        SkIntToScalar(dwfm.designUnitsPerEm));
@@ -398,9 +424,10 @@ HRESULT SkScalerContext_DW::getBoundingBox(SkGlyph* glyph,
     run.glyphIndices = &glyphId;
     run.isSideways = FALSE;
     run.glyphOffsets = &offset;
-
-    SkTScopedComPtr<IDWriteGlyphRunAnalysis> glyphRunAnalysis;
-    HRM(fTypeface->fFactory->CreateGlyphRunAnalysis(
+    {
+        Exclusive l(DWriteFactoryMutex);
+        SkTScopedComPtr<IDWriteGlyphRunAnalysis> glyphRunAnalysis;
+        HRM(fTypeface->fFactory->CreateGlyphRunAnalysis(
             &run,
             1.0f, // pixelsPerDip,
             &fXform,
@@ -409,11 +436,11 @@ HRESULT SkScalerContext_DW::getBoundingBox(SkGlyph* glyph,
             0.0f, // baselineOriginX,
             0.0f, // baselineOriginY,
             &glyphRunAnalysis),
-        "Could not create glyph run analysis.");
-
-    HRM(glyphRunAnalysis->GetAlphaTextureBounds(textureType, bbox),
-        "Could not get texture bounds.");
+            "Could not create glyph run analysis.");
 
+        HRM(glyphRunAnalysis->GetAlphaTextureBounds(textureType, bbox),
+            "Could not get texture bounds.");
+    }
     return S_OK;
 }
 
@@ -473,6 +500,7 @@ void SkScalerContext_DW::generateFontMetrics(SkPaint::FontMetrics* metrics) {
 
     sk_bzero(metrics, sizeof(*metrics));
 
+

[bungeman-skia, 2015/10/21 14:44:31]

Stray extra line?

[herb_g, 2015/10/21 14:54:41]

Done.

     DWRITE_FONT_METRICS dwfm;
     if (DWRITE_MEASURING_MODE_GDI_CLASSIC == fMeasuringMode ||
         DWRITE_MEASURING_MODE_GDI_NATURAL == fMeasuringMode)
@@ -651,30 +679,32 @@ const void* SkScalerContext_DW::drawDWMask(const SkGlyph& glyph,
     run.glyphIndices = &index;
     run.isSideways = FALSE;
     run.glyphOffsets = &offset;
-
-    SkTScopedComPtr<IDWriteGlyphRunAnalysis> glyphRunAnalysis;
-    HRNM(fTypeface->fFactory->CreateGlyphRunAnalysis(&run,
-                                          1.0f, // pixelsPerDip,
-                                          &fXform,
-                                          renderingMode,
-                                          fMeasuringMode,
-                                          0.0f, // baselineOriginX,
-                                          0.0f, // baselineOriginY,
-                                          &glyphRunAnalysis),
-         "Could not create glyph run analysis.");
-
-    //NOTE: this assumes that the glyph has already been measured
-    //with an exact same glyph run analysis.
-    RECT bbox;
-    bbox.left = glyph.fLeft;
-    bbox.top = glyph.fTop;
-    bbox.right = glyph.fLeft + glyph.fWidth;
-    bbox.bottom = glyph.fTop + glyph.fHeight;
-    HRNM(glyphRunAnalysis->CreateAlphaTexture(textureType,
-                                              &bbox,
-                                              fBits.begin(),
-                                              sizeNeeded),
-         "Could not draw mask.");
+    {
+        Exclusive l(DWriteFactoryMutex);
+        SkTScopedComPtr<IDWriteGlyphRunAnalysis> glyphRunAnalysis;
+        HRNM(fTypeface->fFactory->CreateGlyphRunAnalysis(&run,
+            1.0f, // pixelsPerDip,
+            &fXform,
+            renderingMode,
+            fMeasuringMode,
+            0.0f, // baselineOriginX,
+            0.0f, // baselineOriginY,
+            &glyphRunAnalysis),
+            "Could not create glyph run analysis.");
+
+        //NOTE: this assumes that the glyph has already been measured
+        //with an exact same glyph run analysis.
+        RECT bbox;
+        bbox.left = glyph.fLeft;
+        bbox.top = glyph.fTop;
+        bbox.right = glyph.fLeft + glyph.fWidth;
+        bbox.bottom = glyph.fTop + glyph.fHeight;
+        HRNM(glyphRunAnalysis->CreateAlphaTexture(textureType,
+            &bbox,
+            fBits.begin(),
+            sizeNeeded),
+            "Could not draw mask.");
+    }
     return fBits.begin();
 }
 
@@ -730,17 +760,20 @@ void SkScalerContext_DW::generatePath(const SkGlyph& glyph, SkPath* path) {
     HRVM(SkDWriteGeometrySink::Create(path, &geometryToPath),
          "Could not create geometry to path converter.");
     uint16_t glyphId = glyph.getGlyphID();
-    //TODO: convert to<->from DIUs? This would make a difference if hinting.
-    //It may not be needed, it appears that DirectWrite only hints at em size.
-    HRVM(fTypeface->fDWriteFontFace->GetGlyphRunOutline(SkScalarToFloat(fTextSizeRender),
-                                       &glyphId,
-                                       nullptr, //advances
-                                       nullptr, //offsets
-                                       1, //num glyphs
-                                       FALSE, //sideways
-                                       FALSE, //rtl
-                                       geometryToPath.get()),
-         "Could not create glyph outline.");
+    {
+        Exclusive l(DWriteFactoryMutex);
+        //TODO: convert to<->from DIUs? This would make a difference if hinting.
+        //It may not be needed, it appears that DirectWrite only hints at em size.
+        HRVM(fTypeface->fDWriteFontFace->GetGlyphRunOutline(SkScalarToFloat(fTextSizeRender),
+            &glyphId,
+            nullptr, //advances
+            nullptr, //offsets
+            1, //num glyphs
+            FALSE, //sideways
+            FALSE, //rtl
+            geometryToPath.get()),
+            "Could not create glyph outline.");
+    }
 
     path->transform(fSkXform);
 }