Backed out changeset 6440419b9d33 (Bug 1533546). r=jrmuizel a=jcristau
authorLee Salzman <lsalzman@mozilla.com>
Wed, 22 May 2019 18:20:09 +0000
changeset 536409 290e613b8b2211e41209565318a63cafe0671082
parent 536408 42d37e2229abc02684df909b0a8c29f6a31b6f6b
child 536410 ddc0c7609e57ad43e1a28888ffffcbdf02131a53
push id2082
push userffxbld-merge
push dateMon, 01 Jul 2019 08:34:18 +0000
treeherdermozilla-release@2fb19d0466d2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjrmuizel, jcristau
bugs1533546
milestone68.0
backs out6440419b9d330aa54135832c910187b63d1649e2
Backed out changeset 6440419b9d33 (Bug 1533546). r=jrmuizel a=jcristau Differential Revision: https://phabricator.services.mozilla.com/D32180
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,56 +37,27 @@
 
 #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;
 
-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;
-};
+typedef SkAutoSharedMutexShared Shared;
 
 static bool isLCD(const SkScalerContextRec& rec) {
     return SkMask::kLCD16_Format == rec.fMaskFormat;
 }
 
-static bool is_hinted(SkScalerContext_DW* ctx, DWriteFontTypeface* typeface) {
-    MaybeExclusive l(ctx);
+static bool is_hinted(DWriteFontTypeface* typeface) {
+    SkAutoExclusive l(DWriteFactoryMutex);
     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) {
@@ -146,18 +117,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(SkScalerContext_DW* ctx, DWriteFontTypeface* typeface, GaspRange range) {
-    MaybeExclusive l(ctx);
+static bool has_bitmap_strike(DWriteFontTypeface* typeface, GaspRange range) {
+    SkAutoExclusive l(DWriteFactoryMutex);
     {
         AutoTDWriteTable<SkOTTableEmbeddedBitmapLocation> eblc(typeface->fDWriteFontFace.get());
         if (!eblc.fExists) {
             return false;
         }
         if (eblc.fSize < sizeof(SkOTTableEmbeddedBitmapLocation)) {
             return false;
         }
@@ -292,17 +263,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(this, typeface, range);
+        treatLikeBitmap = has_bitmap_strike(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) {
@@ -328,17 +299,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(this, typeface)) {
+               realTextSize > SkIntToScalar(20) || !is_hinted(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 &&
@@ -421,36 +392,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)
     {
-        MaybeExclusive l(this);
+        SkAutoExclusive l(DWriteFactoryMutex);
         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 {
-        MaybeExclusive l(this);
+        SkAutoExclusive l(DWriteFactoryMutex);
         HRBM(this->getDWriteTypeface()->fDWriteFontFace->GetDesignGlyphMetrics(&glyphId, 1, &gm),
              "Could not get design metrics.");
     }
 
     DWRITE_FONT_METRICS dwfm;
     {
-        MaybeShared l(this);
+        Shared l(DWriteFactoryMutex);
         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)
     {
@@ -489,17 +460,17 @@ HRESULT SkScalerContext_DW::getBoundingB
     run.fontEmSize = SkScalarToFloat(fTextSizeRender);
     run.bidiLevel = 0;
     run.glyphIndices = &glyphId;
     run.isSideways = FALSE;
     run.glyphOffsets = &offset;
 
     SkTScopedComPtr<IDWriteGlyphRunAnalysis> glyphRunAnalysis;
     {
-        MaybeExclusive l(this);
+        SkAutoExclusive l(DWriteFactoryMutex);
         // 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,
@@ -519,17 +490,17 @@ HRESULT SkScalerContext_DW::getBoundingB
                     fMeasuringMode,
                     0.0f, // baselineOriginX,
                     0.0f, // baselineOriginY,
                     &glyphRunAnalysis),
                 "Could not create glyph run analysis.");
         }
     }
     {
-        MaybeShared l(this);
+        Shared l(DWriteFactoryMutex);
         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 }
@@ -612,17 +583,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.");
         {
-            MaybeExclusive l(this);
+            SkAutoExclusive l(DWriteFactoryMutex);
             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
@@ -971,17 +942,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;
         {
-            MaybeExclusive l(this);
+            SkAutoExclusive l(DWriteFactoryMutex);
             // 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,
@@ -1007,17 +978,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;
         {
-            MaybeShared l(this);
+            Shared l(DWriteFactoryMutex);
             HRNM(glyphRunAnalysis->CreateAlphaTexture(textureType,
                     &bbox,
                     fBits.begin(),
                     sizeNeeded),
                 "Could not draw mask.");
         }
     }
     return fBits.begin();
@@ -1071,17 +1042,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.");
         {
-            MaybeExclusive l(this);
+            SkAutoExclusive l(DWriteFactoryMutex);
             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
@@ -1210,17 +1181,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);
     {
-        MaybeExclusive l(this);
+        SkAutoExclusive 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.
         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,20 +21,16 @@ 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;