Bug 1587094 - Create a pref to control whether we use DirectWrite's bold simulation or multi-strike synthetic bold; default to multi-strike for webfonts. r=lsalzman

This is designed to mitigate the problem of third-party fonts that render poorly
under DirectWrite's bold simulation, by using multi-strike synthetic bold (like
we use on macOS) instead.

The behavior is controlled by a pref, so that we can readily switch between
using DWrite's bold simulation for all fonts (pref=2, our current behavior);
using it only for installed fonts and falling back to multi-strike for webfonts
(pref=1, new behavior); or never using the DWrite simulation (pref=0).

Differential Revision: https://phabricator.services.mozilla.com/D137584

accessible/base/TextAttrs.cpp

@@ -556,10 +556,9 @@ FontWeight TextAttrsMgr::FontWeightTextAttr::GetFontWeight(nsIFrame* aFrame) {
 
   // When there doesn't exist a bold font in the family and so the rendering of
   // a non-bold font face is changed so that the user sees what looks like a
-  // bold font, i.e. synthetic bolding is used. IsSyntheticBold method is only
-  // needed on Mac, but it is "safe" to use on all platforms.  (For non-Mac
-  // platforms it always return false.)
-  if (font->IsSyntheticBold()) {
+  // bold font, i.e. synthetic bolding is used. (Simply returns false on any
+  // platforms that don't use the multi-strike synthetic bolding.)
+  if (font->ApplySyntheticBold()) {
     return FontWeight::Bold();
   }
 

gfx/2d/2D.h

@@ -2045,7 +2045,7 @@ class GFX2D_API Factory {
   static already_AddRefed<ScaledFont> CreateScaledFontForDWriteFont(
       IDWriteFontFace* aFontFace, const gfxFontStyle* aStyle,
       const RefPtr<UnscaledFont>& aUnscaledFont, Float aSize,
-      bool aUseEmbeddedBitmap, bool aGDIForced);
+      bool aUseEmbeddedBitmap, bool aUseMultistrikeBold, bool aGDIForced);
 
   static already_AddRefed<ScaledFont> CreateScaledFontForGDIFont(
       const void* aLogFont, const RefPtr<UnscaledFont>& aUnscaledFont,

gfx/2d/Factory.cpp

@@ -929,9 +929,10 @@ void Factory::D2DCleanup() {
 already_AddRefed<ScaledFont> Factory::CreateScaledFontForDWriteFont(
     IDWriteFontFace* aFontFace, const gfxFontStyle* aStyle,
     const RefPtr<UnscaledFont>& aUnscaledFont, float aSize,
-    bool aUseEmbeddedBitmap, bool aGDIForced) {
+    bool aUseEmbeddedBitmap, bool aUseMultistrikeBold, bool aGDIForced) {
   return MakeAndAddRef<ScaledFontDWrite>(
-      aFontFace, aUnscaledFont, aSize, aUseEmbeddedBitmap, aGDIForced, aStyle);
+      aFontFace, aUnscaledFont, aSize, aUseEmbeddedBitmap, aUseMultistrikeBold,
+      aGDIForced, aStyle);
 }
 
 already_AddRefed<ScaledFont> Factory::CreateScaledFontForGDIFont(

gfx/2d/ScaledFontDWrite.cpp

@@ -121,10 +121,12 @@ static inline DWRITE_FONT_STRETCH DWriteFontStretchFromStretch(
 ScaledFontDWrite::ScaledFontDWrite(IDWriteFontFace* aFontFace,
                                    const RefPtr<UnscaledFont>& aUnscaledFont,
                                    Float aSize, bool aUseEmbeddedBitmap,
-                                   bool aGDIForced, const gfxFontStyle* aStyle)
+                                   bool aUseMultistrikeBold, bool aGDIForced,
+                                   const gfxFontStyle* aStyle)
     : ScaledFontBase(aUnscaledFont, aSize),
       mFontFace(aFontFace),
       mUseEmbeddedBitmap(aUseEmbeddedBitmap),
+      mUseMultistrikeBold(aUseMultistrikeBold),
       mGDIForced(aGDIForced) {
   if (aStyle) {
     mStyle = SkFontStyle(aStyle->weight.ToIntRounded(),
@@ -391,14 +393,16 @@ bool UnscaledFontDWrite::GetFontDescriptor(FontDescriptorOutput aCb,
 
 ScaledFontDWrite::InstanceData::InstanceData(
     const wr::FontInstanceOptions* aOptions,
-    const wr::FontInstancePlatformOptions* aPlatformOptions)
-    : mUseEmbeddedBitmap(false), mApplySyntheticBold(false) {
+    const wr::FontInstancePlatformOptions* aPlatformOptions) {
   if (aOptions) {
     if (aOptions->flags & wr::FontInstanceFlags::EMBEDDED_BITMAPS) {
       mUseEmbeddedBitmap = true;
     }
     if (aOptions->flags & wr::FontInstanceFlags::SYNTHETIC_BOLD) {
-      mApplySyntheticBold = true;
+      mUseBoldSimulation = true;
+    }
+    if (aOptions->flags & wr::FontInstanceFlags::MULTISTRIKE_BOLD) {
+      mUseMultistrikeBold = true;
     }
     if (aOptions->flags & wr::FontInstanceFlags::FORCE_GDI) {
       mGDIForced = true;
@@ -466,9 +470,12 @@ bool ScaledFontDWrite::GetWRFontInstanceOptions(
   wr::FontInstanceOptions options;
   options.render_mode = wr::ToFontRenderMode(GetDefaultAAMode());
   options.flags = wr::FontInstanceFlags{0};
-  if (HasSyntheticBold()) {
+  if (HasBoldSimulation()) {
     options.flags |= wr::FontInstanceFlags::SYNTHETIC_BOLD;
   }
+  if (UseMultistrikeBold()) {
+    options.flags |= wr::FontInstanceFlags::MULTISTRIKE_BOLD;
+  }
   if (UseEmbeddedBitmaps()) {
     options.flags |= wr::FontInstanceFlags::EMBEDDED_BITMAPS;
   }
@@ -614,7 +621,7 @@ already_AddRefed<ScaledFont> UnscaledFontDWrite::CreateScaledFont(
       *reinterpret_cast<const ScaledFontDWrite::InstanceData*>(aInstanceData);
 
   IDWriteFontFace* face = mFontFace;
-  if (instanceData.mApplySyntheticBold) {
+  if (instanceData.mUseBoldSimulation) {
     if (!InitBold()) {
       gfxWarning() << "Failed creating bold IDWriteFontFace.";
       return nullptr;
@@ -636,9 +643,9 @@ already_AddRefed<ScaledFont> UnscaledFontDWrite::CreateScaledFont(
     }
   }
 
-  return MakeAndAddRef<ScaledFontDWrite>(face, this, aGlyphSize,
-                                         instanceData.mUseEmbeddedBitmap,
-                                         instanceData.mGDIForced);
+  return MakeAndAddRef<ScaledFontDWrite>(
+      face, this, aGlyphSize, instanceData.mUseEmbeddedBitmap,
+      instanceData.mUseMultistrikeBold, instanceData.mGDIForced, nullptr);
 }
 
 already_AddRefed<ScaledFont> UnscaledFontDWrite::CreateScaledFontFromWRFont(

gfx/2d/ScaledFontDWrite.h

@@ -25,8 +25,8 @@ class ScaledFontDWrite final : public ScaledFontBase {
   MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(ScaledFontDWrite, override)
   ScaledFontDWrite(IDWriteFontFace* aFontFace,
                    const RefPtr<UnscaledFont>& aUnscaledFont, Float aSize,
-                   bool aUseEmbeddedBitmap, bool aGDIForced,
-                   const gfxFontStyle* aStyle = nullptr);
+                   bool aUseEmbeddedBitmap, bool aUseMultistrikeBold,
+                   bool aGDIForced, const gfxFontStyle* aStyle);
 
   FontType GetType() const override { return FontType::DWRITE; }
 
@@ -52,9 +52,10 @@ class ScaledFontDWrite final : public ScaledFontBase {
   AntialiasMode GetDefaultAAMode() override;
 
   bool UseEmbeddedBitmaps() const { return mUseEmbeddedBitmap; }
+  bool UseMultistrikeBold() const { return mUseMultistrikeBold; }
   bool ForceGDIMode() const { return mGDIForced; }
 
-  bool HasSyntheticBold() const {
+  bool HasBoldSimulation() const {
     return (mFontFace->GetSimulations() & DWRITE_FONT_SIMULATIONS_BOLD) != 0;
   }
 
@@ -64,6 +65,7 @@ class ScaledFontDWrite final : public ScaledFontBase {
 
   RefPtr<IDWriteFontFace> mFontFace;
   bool mUseEmbeddedBitmap;
+  bool mUseMultistrikeBold = false;
   bool mGDIForced = false;
 
   cairo_font_face_t* CreateCairoFontFace(
@@ -77,14 +79,16 @@ class ScaledFontDWrite final : public ScaledFontBase {
   struct InstanceData {
     explicit InstanceData(ScaledFontDWrite* aScaledFont)
         : mUseEmbeddedBitmap(aScaledFont->mUseEmbeddedBitmap),
-          mApplySyntheticBold(aScaledFont->HasSyntheticBold()),
+          mUseBoldSimulation(aScaledFont->HasBoldSimulation()),
+          mUseMultistrikeBold(aScaledFont->UseMultistrikeBold()),
           mGDIForced(aScaledFont->mGDIForced) {}
 
     InstanceData(const wr::FontInstanceOptions* aOptions,
                  const wr::FontInstancePlatformOptions* aPlatformOptions);
 
-    bool mUseEmbeddedBitmap;
-    bool mApplySyntheticBold;
+    bool mUseEmbeddedBitmap = false;
+    bool mUseBoldSimulation = false;
+    bool mUseMultistrikeBold = false;
     bool mGDIForced = false;
   };
 };

gfx/thebes/gfxDWriteFontList.cpp

@@ -651,11 +651,25 @@ void gfxDWriteFontEntry::GetVariationInstances(
 
 gfxFont* gfxDWriteFontEntry::CreateFontInstance(
     const gfxFontStyle* aFontStyle) {
-  bool needsBold = aFontStyle->NeedsSyntheticBold(this);
+  // We use the DirectWrite bold simulation for installed fonts, but NOT for
+  // webfonts; those will use multi-strike synthetic bold instead.
+  bool useBoldSim = false;
+  if (aFontStyle->NeedsSyntheticBold(this)) {
+    switch (StaticPrefs::gfx_font_rendering_directwrite_bold_simulation()) {
+      case 0:  // never use the DWrite simulation
+        break;
+      case 1:  // use DWrite simulation for installed fonts but not webfonts
+        useBoldSim = !mIsDataUserFont;
+        break;
+      default:  // always use DWrite bold simulation
+        useBoldSim = true;
+        break;
+    }
+  }
   DWRITE_FONT_SIMULATIONS sims =
-      needsBold ? DWRITE_FONT_SIMULATIONS_BOLD : DWRITE_FONT_SIMULATIONS_NONE;
+      useBoldSim ? DWRITE_FONT_SIMULATIONS_BOLD : DWRITE_FONT_SIMULATIONS_NONE;
   ThreadSafeWeakPtr<UnscaledFontDWrite>& unscaledFontPtr =
-      needsBold ? mUnscaledFontBold : mUnscaledFont;
+      useBoldSim ? mUnscaledFontBold : mUnscaledFont;
   RefPtr<UnscaledFontDWrite> unscaledFont(unscaledFontPtr);
   if (!unscaledFont) {
     RefPtr<IDWriteFontFace> fontFace;

gfx/thebes/gfxDWriteFonts.cpp

@@ -89,7 +89,21 @@ gfxDWriteFont::gfxDWriteFont(const RefPtr<UnscaledFontDWrite>& aUnscaledFont,
   // faster glyph width retrieval.
   mFontFace->QueryInterface(__uuidof(IDWriteFontFace1),
                             (void**)getter_AddRefs(mFontFace1));
-
+  // If a fake-bold effect is needed, determine whether we're using DWrite's
+  // "simulation" or applying our multi-strike "synthetic bold".
+  if (aFontStyle->NeedsSyntheticBold(aFontEntry)) {
+    switch (StaticPrefs::gfx_font_rendering_directwrite_bold_simulation()) {
+      case 0:  // never use the DWrite simulation
+        mApplySyntheticBold = true;
+        break;
+      case 1:  // use DWrite simulation for installed fonts but not webfonts
+        mApplySyntheticBold = aFontEntry->mIsDataUserFont;
+        break;
+      default:  // always use DWrite bold simulation
+        // the flag is initialized to false in gfxFont
+        break;
+    }
+  }
   ComputeMetrics(anAAOption);
 }
 
@@ -450,6 +464,19 @@ void gfxDWriteFont::ComputeMetrics(AntialiasOption anAAOption) {
 
   SanitizeMetrics(mMetrics, GetFontEntry()->mIsBadUnderlineFont);
 
+  if (ApplySyntheticBold()) {
+    auto delta = GetSyntheticBoldOffset();
+    mMetrics->spaceWidth += delta;
+    mMetrics->aveCharWidth += delta;
+    mMetrics->maxAdvance += delta;
+    if (mMetrics->zeroWidth > 0) {
+      mMetrics->zeroWidth += delta;
+    }
+    if (mMetrics->ideographicWidth > 0) {
+      mMetrics->ideographicWidth += delta;
+    }
+  }
+
 #if 0
     printf("Font: %p (%s) size: %f\n", this,
            NS_ConvertUTF16toUTF8(GetName()).get(), mStyle.size);
@@ -770,7 +797,7 @@ already_AddRefed<ScaledFont> gfxDWriteFont::GetScaledFont(
     const gfxFontStyle* fontStyle = GetStyle();
     azureScaledFont = Factory::CreateScaledFontForDWriteFont(
         mFontFace, fontStyle, GetUnscaledFont(), GetAdjustedSize(),
-        useEmbeddedBitmap, forceGDI);
+        useEmbeddedBitmap, ApplySyntheticBold(), forceGDI);
     if (!azureScaledFont) {
       return nullptr;
     }

gfx/thebes/gfxFont.cpp

@@ -2259,11 +2259,12 @@ void gfxFont::Draw(const gfxTextRun* aTextRun, uint32_t aStart, uint32_t aEnd,
     fontParams.contextPaint = contextPaint.get();
   }
 
-  // Synthetic-bold strikes are each offset one device pixel in run direction.
-  // (these values are only needed if IsSyntheticBold() is true)
-  // WebRender handles synthetic bold independently via FontInstanceFlags,
-  // so just ignore requests in that case.
-  if (IsSyntheticBold() && !textDrawer) {
+  // Synthetic-bold strikes are each offset one device pixel in run direction
+  // (these values are only needed if ApplySyntheticBold() is true).
+  // If drawing via webrender, it will do multistrike internally so we don't
+  // need to handle it here.
+  bool doMultistrikeBold = ApplySyntheticBold() && !textDrawer;
+  if (doMultistrikeBold) {
     gfx::Float xscale = CalcXScale(aRunParams.context->GetDrawTarget());
     fontParams.synBoldOnePixelOffset = aRunParams.direction * xscale;
     if (xscale != 0.0) {
@@ -2946,7 +2947,7 @@ bool gfxFont::ShapeText(DrawTarget* aDrawTarget, const char16_t* aText,
 void gfxFont::PostShapingFixup(DrawTarget* aDrawTarget, const char16_t* aText,
                                uint32_t aOffset, uint32_t aLength,
                                bool aVertical, gfxShapedText* aShapedText) {
-  if (IsSyntheticBold()) {
+  if (ApplySyntheticBold()) {
     const Metrics& metrics = GetMetrics(aVertical ? nsFontMetrics::eVertical
                                                   : nsFontMetrics::eHorizontal);
     if (metrics.maxAdvance > metrics.aveCharWidth) {

gfx/thebes/gfxFont.h

@@ -1783,7 +1783,7 @@ class gfxFont {
 
   virtual bool AllowSubpixelAA() { return true; }
 
-  bool IsSyntheticBold() const { return mApplySyntheticBold; }
+  bool ApplySyntheticBold() const { return mApplySyntheticBold; }
 
   float AngleForSyntheticOblique() const;
   float SkewForSyntheticOblique() const;

gfx/thebes/gfxGDIFont.cpp

@@ -366,9 +366,17 @@ void gfxGDIFont::Initialize() {
     mFUnitsConvFactor = 0.0;  // zero-sized font: all values scale to zero
   }
 
-  if (IsSyntheticBold()) {
-    mMetrics->aveCharWidth += GetSyntheticBoldOffset();
-    mMetrics->maxAdvance += GetSyntheticBoldOffset();
+  if (ApplySyntheticBold()) {
+    auto delta = GetSyntheticBoldOffset();
+    mMetrics->spaceWidth += delta;
+    mMetrics->aveCharWidth += delta;
+    mMetrics->maxAdvance += delta;
+    if (mMetrics->zeroWidth > 0) {
+      mMetrics->zeroWidth += delta;
+    }
+    if (mMetrics->ideographicWidth > 0) {
+      mMetrics->ideographicWidth += delta;
+    }
   }
 
 #if 0

gfx/thebes/gfxMacFont.cpp

@@ -382,19 +382,23 @@ void gfxMacFont::InitMetrics() {
     mMetrics.ideographicWidth = -1.0;
   }
 
-  if (IsSyntheticBold()) {
-    mMetrics.spaceWidth += GetSyntheticBoldOffset();
-    mMetrics.aveCharWidth += GetSyntheticBoldOffset();
-    mMetrics.maxAdvance += GetSyntheticBoldOffset();
-    if (mMetrics.zeroWidth > 0) {
-      mMetrics.zeroWidth += GetSyntheticBoldOffset();
-    }
-  }
-
   CalculateDerivedMetrics(mMetrics);
 
   SanitizeMetrics(&mMetrics, mFontEntry->mIsBadUnderlineFont);
 
+  if (ApplySyntheticBold()) {
+    auto delta = GetSyntheticBoldOffset();
+    mMetrics.spaceWidth += delta;
+    mMetrics.aveCharWidth += delta;
+    mMetrics.maxAdvance += delta;
+    if (mMetrics.zeroWidth > 0) {
+      mMetrics.zeroWidth += delta;
+    }
+    if (mMetrics.ideographicWidth > 0) {
+      mMetrics.ideographicWidth += delta;
+    }
+  }
+
 #if 0
     fprintf (stderr, "Font: %p (%s) size: %f\n", this,
              NS_ConvertUTF16toUTF8(GetName()).get(), mStyle.size);
@@ -583,7 +587,7 @@ already_AddRefed<ScaledFont> gfxMacFont::GetScaledFont(
     mAzureScaledFont = Factory::CreateScaledFontForMacFont(
         GetCGFontRef(), GetUnscaledFont(), GetAdjustedSize(),
         ToDeviceColor(mFontSmoothingBackgroundColor),
-        !mStyle.useGrayscaleAntialiasing, IsSyntheticBold(), hasColorGlyphs);
+        !mStyle.useGrayscaleAntialiasing, ApplySyntheticBold(), hasColorGlyphs);
     if (!mAzureScaledFont) {
       return nullptr;
     }

gfx/thebes/gfxTextRun.cpp

@@ -517,7 +517,7 @@ void gfxTextRun::DrawPartialLigature(gfxFont* aFont, Range aRange,
 // check whether the text run needs to be explicitly composited in order to
 // support opacity.
 static bool HasSyntheticBoldOrColor(gfxFont* aFont) {
-  if (aFont->IsSyntheticBold()) {
+  if (aFont->ApplySyntheticBold()) {
     return true;
   }
   gfxFontEntry* fe = aFont->GetFontEntry();

gfx/wr/webrender/src/platform/windows/font.rs

@@ -587,7 +587,7 @@ impl FontContext {
             let (bold_pixels, bold_width) = apply_multistrike_bold(
                 &bgra_pixels,
                 (width + padding * 2) as usize,
-                height as usize,
+                (height + padding * 2) as usize,
                 is_subpixel,
                 extra_strikes,
                 pixel_step,

modules/libpref/init/StaticPrefList.yaml

@@ -5398,6 +5398,21 @@
   value: true
   mirror: always
 
+#if defined(XP_WIN)
+# Whether the DirectWrite bold simulation should be used when a bold font-weight
+# is requested but no bold face available in the family. This renders poorly with
+# some third-party fonts, so by default we disable it for webfonts and allow it
+# only with locally-installed fonts.
+# Values:
+#   0 - never use DWrite bold simulation; always multi-strike instead
+#   1 - use DWrite bold for installed fonts, multi-strike for webfont resources
+#   2 - use DWrite bold for all fonts
+- name: gfx.font_rendering.directwrite.bold_simulation
+  type: uint32_t
+  value: 1
+  mirror: always
+#endif
+
 # The level of logging:
 # - 0: no logging;
 # - 1: adds errors;