Bug 1533546 - disable Skia's global DWrite lock on Windows 10. r=jrmuizel
authorLee Salzman <lsalzman@mozilla.com>
Wed, 15 May 2019 21:23:23 +0000
changeset 532828 6440419b9d330aa54135832c910187b63d1649e2
parent 532827 1274c074caef1fb327fad2d59bda256e5029e5e2
child 532829 14e4705002fef497e1d7d9f92bac75360b81945b
push id11272
push userapavel@mozilla.com
push dateThu, 16 May 2019 15:28:22 +0000
treeherdermozilla-beta@2265bfc5920d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel
bugs1533546
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1533546 - disable Skia's global DWrite lock on Windows 10. r=jrmuizel Differential Revision: https://phabricator.services.mozilla.com/D31328
gfx/skia/skia/src/ports/SkScalerContext_win_dw.cpp
gfx/skia/skia/src/ports/SkScalerContext_win_dw.h
--- a/gfx/skia/skia/src/ports/SkScalerContext_win_dw.cpp
+++ b/gfx/skia/skia/src/ports/SkScalerContext_win_dw.cpp
@@ -37,27 +37,56 @@
 
 #include <dwrite.h>
 #include <dwrite_1.h>
 #include <dwrite_3.h>
 
 /* 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.
+ *
+ * On DWrite 3 or above, which is only available on Windows 10, we don't enable
+ * the locking to avoid thread contention.
  */
 static SkSharedMutex DWriteFactoryMutex;
 
-typedef SkAutoSharedMutexShared Shared;
+struct MaybeExclusive {
+    MaybeExclusive(SkScalerContext_DW* ctx) : fEnabled(!ctx->isDWrite3()) {
+        if (fEnabled) {
+            DWriteFactoryMutex.acquire();
+        }
+    }
+    ~MaybeExclusive() {
+        if (fEnabled) {
+            DWriteFactoryMutex.release();
+        }
+    }
+    bool fEnabled;
+};
+
+struct MaybeShared {
+    MaybeShared(SkScalerContext_DW* ctx) : fEnabled(!ctx->isDWrite3()) {
+        if (fEnabled) {
+            DWriteFactoryMutex.acquireShared();
+        }
+    }
+    ~MaybeShared() {
+        if (fEnabled) {
+            DWriteFactoryMutex.releaseShared();
+        }
+    }
+    bool fEnabled;
+};
 
 static bool isLCD(const SkScalerContextRec& rec) {
     return SkMask::kLCD16_Format == rec.fMaskFormat;
 }
 
-static bool is_hinted(DWriteFontTypeface* typeface) {
-    SkAutoExclusive l(DWriteFactoryMutex);
+static bool is_hinted(SkScalerContext_DW* ctx, DWriteFontTypeface* typeface) {
+    MaybeExclusive l(ctx);
     AutoTDWriteTable<SkOTTableMaximumProfile> maxp(typeface->fDWriteFontFace.get());
     if (!maxp.fExists) {
         return false;
     }
     if (maxp.fSize < sizeof(SkOTTableMaximumProfile::Version::TT)) {
         return false;
     }
     if (maxp->version.version != SkOTTableMaximumProfile::Version::TT::VERSION) {
@@ -117,18 +146,18 @@ bool get_gasp_range(DWriteFontTypeface* 
 }
 /** If the rendering mode for the specified 'size' is gridfit, then place
  *  the gridfit range into 'range'. Otherwise, leave 'range' alone.
  */
 static bool is_gridfit_only(GaspRange::Behavior flags) {
     return flags.raw.value == GaspRange::Behavior::Raw::GridfitMask;
 }
 
-static bool has_bitmap_strike(DWriteFontTypeface* typeface, GaspRange range) {
-    SkAutoExclusive l(DWriteFactoryMutex);
+static bool has_bitmap_strike(SkScalerContext_DW* ctx, DWriteFontTypeface* typeface, GaspRange range) {
+    MaybeExclusive l(ctx);
     {
         AutoTDWriteTable<SkOTTableEmbeddedBitmapLocation> eblc(typeface->fDWriteFontFace.get());
         if (!eblc.fExists) {
             return false;
         }
         if (eblc.fSize < sizeof(SkOTTableEmbeddedBitmapLocation)) {
             return false;
         }
@@ -263,17 +292,17 @@ SkScalerContext_DW::SkScalerContext_DW(s
         // a bitmap strike if the range is gridfit only and contains a bitmap.
         int bitmapPPEM = SkScalarTruncToInt(gdiTextSize);
         GaspRange range(bitmapPPEM, bitmapPPEM, 0, GaspRange::Behavior());
         if (get_gasp_range(typeface, bitmapPPEM, &range)) {
             if (!is_gridfit_only(range.fFlags)) {
                 range = GaspRange(bitmapPPEM, bitmapPPEM, 0, GaspRange::Behavior());
             }
         }
-        treatLikeBitmap = has_bitmap_strike(typeface, range);
+        treatLikeBitmap = has_bitmap_strike(this, typeface, range);
 
         axisAlignedBitmap = is_axis_aligned(fRec);
     }
 
     GaspRange range(0, 0xFFFF, 0, GaspRange::Behavior());
 
     // If the user requested aliased, do so with aliased compatible metrics.
     if (SkMask::kBW_Format == fRec.fMaskFormat) {
@@ -299,17 +328,17 @@ SkScalerContext_DW::SkScalerContext_DW(s
         fRenderingMode = DWRITE_RENDERING_MODE_NATURAL_SYMMETRIC;
         fTextureType = DWRITE_TEXTURE_CLEARTYPE_3x1;
         fTextSizeMeasure = gdiTextSize;
         fMeasuringMode = DWRITE_MEASURING_MODE_GDI_CLASSIC;
 
     // If the font has a gasp table version 1, use it to determine symmetric rendering.
     } else if ((get_gasp_range(typeface, SkScalarRoundToInt(gdiTextSize), &range) &&
                 range.fVersion >= 1) ||
-               realTextSize > SkIntToScalar(20) || !is_hinted(typeface)) {
+               realTextSize > SkIntToScalar(20) || !is_hinted(this, typeface)) {
         fTextSizeRender = realTextSize;
         fTextureType = DWRITE_TEXTURE_CLEARTYPE_3x1;
         fTextSizeMeasure = realTextSize;
         fMeasuringMode = DWRITE_MEASURING_MODE_NATURAL;
 
         IDWriteRenderingParams* params = sk_get_dwrite_default_rendering_params();
         DWriteFontTypeface* typeface = static_cast<DWriteFontTypeface*>(getTypeface());
         if (params &&
@@ -392,36 +421,36 @@ bool SkScalerContext_DW::generateAdvance
     glyph->fAdvanceY = 0;
 
     uint16_t glyphId = glyph->getGlyphID();
     DWRITE_GLYPH_METRICS gm;
 
     if (DWRITE_MEASURING_MODE_GDI_CLASSIC == fMeasuringMode ||
         DWRITE_MEASURING_MODE_GDI_NATURAL == fMeasuringMode)
     {
-        SkAutoExclusive l(DWriteFactoryMutex);
+        MaybeExclusive l(this);
         HRBM(this->getDWriteTypeface()->fDWriteFontFace->GetGdiCompatibleGlyphMetrics(
                  fTextSizeMeasure,
                  1.0f, // pixelsPerDip
                  // This parameter does not act like the lpmat2 parameter to GetGlyphOutlineW.
                  // If it did then GsA here and G_inv below to mapVectors.
                  nullptr,
                  DWRITE_MEASURING_MODE_GDI_NATURAL == fMeasuringMode,
                  &glyphId, 1,
                  &gm),
              "Could not get gdi compatible glyph metrics.");
     } else {
-        SkAutoExclusive l(DWriteFactoryMutex);
+        MaybeExclusive l(this);
         HRBM(this->getDWriteTypeface()->fDWriteFontFace->GetDesignGlyphMetrics(&glyphId, 1, &gm),
              "Could not get design metrics.");
     }
 
     DWRITE_FONT_METRICS dwfm;
     {
-        Shared l(DWriteFactoryMutex);
+        MaybeShared l(this);
         this->getDWriteTypeface()->fDWriteFontFace->GetMetrics(&dwfm);
     }
     SkScalar advanceX = fTextSizeMeasure * gm.advanceWidth / dwfm.designUnitsPerEm;
 
     SkVector advance = { advanceX, 0 };
     if (DWRITE_MEASURING_MODE_GDI_CLASSIC == fMeasuringMode ||
         DWRITE_MEASURING_MODE_GDI_NATURAL == fMeasuringMode)
     {
@@ -460,17 +489,17 @@ HRESULT SkScalerContext_DW::getBoundingB
     run.fontEmSize = SkScalarToFloat(fTextSizeRender);
     run.bidiLevel = 0;
     run.glyphIndices = &glyphId;
     run.isSideways = FALSE;
     run.glyphOffsets = &offset;
 
     SkTScopedComPtr<IDWriteGlyphRunAnalysis> glyphRunAnalysis;
     {
-        SkAutoExclusive l(DWriteFactoryMutex);
+        MaybeExclusive l(this);
         // IDWriteFactory2::CreateGlyphRunAnalysis is very bad at aliased glyphs.
         if (this->getDWriteTypeface()->fFactory2 &&
                 (fGridFitMode == DWRITE_GRID_FIT_MODE_DISABLED ||
                  fAntiAliasMode == DWRITE_TEXT_ANTIALIAS_MODE_GRAYSCALE))
         {
             HRM(this->getDWriteTypeface()->fFactory2->CreateGlyphRunAnalysis(
                     &run,
                     &fXform,
@@ -490,17 +519,17 @@ HRESULT SkScalerContext_DW::getBoundingB
                     fMeasuringMode,
                     0.0f, // baselineOriginX,
                     0.0f, // baselineOriginY,
                     &glyphRunAnalysis),
                 "Could not create glyph run analysis.");
         }
     }
     {
-        Shared l(DWriteFactoryMutex);
+        MaybeShared l(this);
         HRM(glyphRunAnalysis->GetAlphaTextureBounds(textureType, bbox),
             "Could not get texture bounds.");
     }
     return S_OK;
 }
 
 /** GetAlphaTextureBounds succeeds but sometimes returns empty bounds like
  *  { 0x80000000, 0x80000000, 0x80000000, 0x80000000 }
@@ -583,17 +612,17 @@ void SkScalerContext_DW::generateColorMe
         const DWRITE_COLOR_GLYPH_RUN* colorGlyph;
         HRVM(colorLayers->GetCurrentRun(&colorGlyph), "Could not get current color glyph run");
 
         SkPath path;
         SkTScopedComPtr<IDWriteGeometrySink> geometryToPath;
         HRVM(SkDWriteGeometrySink::Create(&path, &geometryToPath),
             "Could not create geometry to path converter.");
         {
-            SkAutoExclusive l(DWriteFactoryMutex);
+            MaybeExclusive l(this);
             HRVM(colorGlyph->glyphRun.fontFace->GetGlyphRunOutline(
                     colorGlyph->glyphRun.fontEmSize,
                     colorGlyph->glyphRun.glyphIndices,
                     colorGlyph->glyphRun.glyphAdvances,
                     colorGlyph->glyphRun.glyphOffsets,
                     colorGlyph->glyphRun.glyphCount,
                     colorGlyph->glyphRun.isSideways,
                     colorGlyph->glyphRun.bidiLevel % 2, //rtl
@@ -942,17 +971,17 @@ const void* SkScalerContext_DW::drawDWMa
     run.fontEmSize = SkScalarToFloat(fTextSizeRender);
     run.bidiLevel = 0;
     run.glyphIndices = &index;
     run.isSideways = FALSE;
     run.glyphOffsets = &offset;
     {
         SkTScopedComPtr<IDWriteGlyphRunAnalysis> glyphRunAnalysis;
         {
-            SkAutoExclusive l(DWriteFactoryMutex);
+            MaybeExclusive l(this);
             // IDWriteFactory2::CreateGlyphRunAnalysis is very bad at aliased glyphs.
             if (this->getDWriteTypeface()->fFactory2 &&
                     (fGridFitMode == DWRITE_GRID_FIT_MODE_DISABLED ||
                      fAntiAliasMode == DWRITE_TEXT_ANTIALIAS_MODE_GRAYSCALE))
             {
                 HRNM(this->getDWriteTypeface()->fFactory2->CreateGlyphRunAnalysis(&run,
                          &fXform,
                          renderingMode,
@@ -978,17 +1007,17 @@ const void* SkScalerContext_DW::drawDWMa
         //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;
         {
-            Shared l(DWriteFactoryMutex);
+            MaybeShared l(this);
             HRNM(glyphRunAnalysis->CreateAlphaTexture(textureType,
                     &bbox,
                     fBits.begin(),
                     sizeNeeded),
                 "Could not draw mask.");
         }
     }
     return fBits.begin();
@@ -1042,17 +1071,17 @@ void SkScalerContext_DW::generateColorGl
         }
         paint.setColor(color);
 
         SkPath path;
         SkTScopedComPtr<IDWriteGeometrySink> geometryToPath;
         HRVM(SkDWriteGeometrySink::Create(&path, &geometryToPath),
              "Could not create geometry to path converter.");
         {
-            SkAutoExclusive l(DWriteFactoryMutex);
+            MaybeExclusive l(this);
             HRVM(colorGlyph->glyphRun.fontFace->GetGlyphRunOutline(
                 colorGlyph->glyphRun.fontEmSize,
                 colorGlyph->glyphRun.glyphIndices,
                 colorGlyph->glyphRun.glyphAdvances,
                 colorGlyph->glyphRun.glyphOffsets,
                 colorGlyph->glyphRun.glyphCount,
                 colorGlyph->glyphRun.isSideways,
                 colorGlyph->glyphRun.bidiLevel % 2, //rtl
@@ -1181,17 +1210,17 @@ bool SkScalerContext_DW::generatePath(Sk
 
     path->reset();
 
     SkTScopedComPtr<IDWriteGeometrySink> geometryToPath;
     HRBM(SkDWriteGeometrySink::Create(path, &geometryToPath),
          "Could not create geometry to path converter.");
     UINT16 glyphId = SkTo<UINT16>(glyph);
     {
-        SkAutoExclusive l(DWriteFactoryMutex);
+        MaybeExclusive l(this);
         //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.
         HRBM(this->getDWriteTypeface()->fDWriteFontFace->GetGlyphRunOutline(
              SkScalarToFloat(fTextSizeRender),
              &glyphId,
              nullptr, //advances
              nullptr, //offsets
              1, //num glyphs
--- a/gfx/skia/skia/src/ports/SkScalerContext_win_dw.h
+++ b/gfx/skia/skia/src/ports/SkScalerContext_win_dw.h
@@ -21,16 +21,20 @@ class SkDescriptor;
 
 class SkScalerContext_DW : public SkScalerContext {
 public:
     SkScalerContext_DW(sk_sp<DWriteFontTypeface>,
                        const SkScalerContextEffects&,
                        const SkDescriptor*);
     ~SkScalerContext_DW() override;
 
+    // The IDWriteFontFace4 interface is only available in DWrite 3,
+    // so checking if it was found is sufficient to detect DWrite 3.
+    bool isDWrite3() { return bool(getDWriteTypeface()->fDWriteFontFace4); }
+
 protected:
     unsigned generateGlyphCount() override;
     uint16_t generateCharToGlyph(SkUnichar uni) override;
     bool generateAdvance(SkGlyph* glyph) override;
     void generateMetrics(SkGlyph* glyph) override;
     void generateImage(const SkGlyph& glyph) override;
     bool generatePath(SkGlyphID glyph, SkPath* path) override;
     void generateFontMetrics(SkFontMetrics*) override;