From a1c5d47ba67f772498d6b6c94d7903437221dd3a Mon Sep 17 00:00:00 2001 From: Jonathan Kew Date: Wed, 9 Feb 2022 12:07:05 +0000 Subject: [PATCH] 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 | 7 ++--- gfx/2d/2D.h | 2 +- gfx/2d/Factory.cpp | 5 +-- gfx/2d/ScaledFontDWrite.cpp | 25 +++++++++------ gfx/2d/ScaledFontDWrite.h | 16 ++++++---- gfx/thebes/gfxDWriteFontList.cpp | 20 ++++++++++-- gfx/thebes/gfxDWriteFonts.cpp | 31 +++++++++++++++++-- gfx/thebes/gfxFont.cpp | 13 ++++---- gfx/thebes/gfxFont.h | 2 +- gfx/thebes/gfxGDIFont.cpp | 14 +++++++-- gfx/thebes/gfxMacFont.cpp | 24 ++++++++------ gfx/thebes/gfxTextRun.cpp | 2 +- gfx/wr/webrender/src/platform/windows/font.rs | 2 +- modules/libpref/init/StaticPrefList.yaml | 15 +++++++++ 14 files changed, 129 insertions(+), 49 deletions(-) diff --git a/accessible/base/TextAttrs.cpp b/accessible/base/TextAttrs.cpp index af06d3b49e96..325de804cc41 100644 --- a/accessible/base/TextAttrs.cpp +++ b/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(); } diff --git a/gfx/2d/2D.h b/gfx/2d/2D.h index 8fe2b5128ed0..37ce1a3e1738 100644 --- a/gfx/2d/2D.h +++ b/gfx/2d/2D.h @@ -2045,7 +2045,7 @@ class GFX2D_API Factory { static already_AddRefed CreateScaledFontForDWriteFont( IDWriteFontFace* aFontFace, const gfxFontStyle* aStyle, const RefPtr& aUnscaledFont, Float aSize, - bool aUseEmbeddedBitmap, bool aGDIForced); + bool aUseEmbeddedBitmap, bool aUseMultistrikeBold, bool aGDIForced); static already_AddRefed CreateScaledFontForGDIFont( const void* aLogFont, const RefPtr& aUnscaledFont, diff --git a/gfx/2d/Factory.cpp b/gfx/2d/Factory.cpp index d7087c886764..e8ad6fb12186 100644 --- a/gfx/2d/Factory.cpp +++ b/gfx/2d/Factory.cpp @@ -929,9 +929,10 @@ void Factory::D2DCleanup() { already_AddRefed Factory::CreateScaledFontForDWriteFont( IDWriteFontFace* aFontFace, const gfxFontStyle* aStyle, const RefPtr& aUnscaledFont, float aSize, - bool aUseEmbeddedBitmap, bool aGDIForced) { + bool aUseEmbeddedBitmap, bool aUseMultistrikeBold, bool aGDIForced) { return MakeAndAddRef( - aFontFace, aUnscaledFont, aSize, aUseEmbeddedBitmap, aGDIForced, aStyle); + aFontFace, aUnscaledFont, aSize, aUseEmbeddedBitmap, aUseMultistrikeBold, + aGDIForced, aStyle); } already_AddRefed Factory::CreateScaledFontForGDIFont( diff --git a/gfx/2d/ScaledFontDWrite.cpp b/gfx/2d/ScaledFontDWrite.cpp index f4f73a4c99e8..f0846ce13f85 100644 --- a/gfx/2d/ScaledFontDWrite.cpp +++ b/gfx/2d/ScaledFontDWrite.cpp @@ -121,10 +121,12 @@ static inline DWRITE_FONT_STRETCH DWriteFontStretchFromStretch( ScaledFontDWrite::ScaledFontDWrite(IDWriteFontFace* aFontFace, const RefPtr& 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 UnscaledFontDWrite::CreateScaledFont( *reinterpret_cast(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 UnscaledFontDWrite::CreateScaledFont( } } - return MakeAndAddRef(face, this, aGlyphSize, - instanceData.mUseEmbeddedBitmap, - instanceData.mGDIForced); + return MakeAndAddRef( + face, this, aGlyphSize, instanceData.mUseEmbeddedBitmap, + instanceData.mUseMultistrikeBold, instanceData.mGDIForced, nullptr); } already_AddRefed UnscaledFontDWrite::CreateScaledFontFromWRFont( diff --git a/gfx/2d/ScaledFontDWrite.h b/gfx/2d/ScaledFontDWrite.h index 0022a7bae0eb..c1f1d7ca84b6 100644 --- a/gfx/2d/ScaledFontDWrite.h +++ b/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& 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 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; }; }; diff --git a/gfx/thebes/gfxDWriteFontList.cpp b/gfx/thebes/gfxDWriteFontList.cpp index 29690ba2c32b..1259e25a2533 100644 --- a/gfx/thebes/gfxDWriteFontList.cpp +++ b/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& unscaledFontPtr = - needsBold ? mUnscaledFontBold : mUnscaledFont; + useBoldSim ? mUnscaledFontBold : mUnscaledFont; RefPtr unscaledFont(unscaledFontPtr); if (!unscaledFont) { RefPtr fontFace; diff --git a/gfx/thebes/gfxDWriteFonts.cpp b/gfx/thebes/gfxDWriteFonts.cpp index f502c09f6def..c57725b7cbb7 100644 --- a/gfx/thebes/gfxDWriteFonts.cpp +++ b/gfx/thebes/gfxDWriteFonts.cpp @@ -89,7 +89,21 @@ gfxDWriteFont::gfxDWriteFont(const RefPtr& 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 gfxDWriteFont::GetScaledFont( const gfxFontStyle* fontStyle = GetStyle(); azureScaledFont = Factory::CreateScaledFontForDWriteFont( mFontFace, fontStyle, GetUnscaledFont(), GetAdjustedSize(), - useEmbeddedBitmap, forceGDI); + useEmbeddedBitmap, ApplySyntheticBold(), forceGDI); if (!azureScaledFont) { return nullptr; } diff --git a/gfx/thebes/gfxFont.cpp b/gfx/thebes/gfxFont.cpp index eb721b3e6a8f..a9ddaaa6d981 100644 --- a/gfx/thebes/gfxFont.cpp +++ b/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) { diff --git a/gfx/thebes/gfxFont.h b/gfx/thebes/gfxFont.h index b7c69586bb08..bc97d03f76ba 100644 --- a/gfx/thebes/gfxFont.h +++ b/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; diff --git a/gfx/thebes/gfxGDIFont.cpp b/gfx/thebes/gfxGDIFont.cpp index c8d327229e29..aa7ec382b0f3 100644 --- a/gfx/thebes/gfxGDIFont.cpp +++ b/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 diff --git a/gfx/thebes/gfxMacFont.cpp b/gfx/thebes/gfxMacFont.cpp index a83d2fa27bca..3cd6d316d4d3 100644 --- a/gfx/thebes/gfxMacFont.cpp +++ b/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 gfxMacFont::GetScaledFont( mAzureScaledFont = Factory::CreateScaledFontForMacFont( GetCGFontRef(), GetUnscaledFont(), GetAdjustedSize(), ToDeviceColor(mFontSmoothingBackgroundColor), - !mStyle.useGrayscaleAntialiasing, IsSyntheticBold(), hasColorGlyphs); + !mStyle.useGrayscaleAntialiasing, ApplySyntheticBold(), hasColorGlyphs); if (!mAzureScaledFont) { return nullptr; } diff --git a/gfx/thebes/gfxTextRun.cpp b/gfx/thebes/gfxTextRun.cpp index bf02812bddc1..6d1488825069 100644 --- a/gfx/thebes/gfxTextRun.cpp +++ b/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(); diff --git a/gfx/wr/webrender/src/platform/windows/font.rs b/gfx/wr/webrender/src/platform/windows/font.rs index 782a865e15ab..a2fea8cbd526 100644 --- a/gfx/wr/webrender/src/platform/windows/font.rs +++ b/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, diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index a9cb70ed8bec..ac86db192bd4 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/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: RelaxedAtomicUint32 + value: 1 + mirror: always +#endif + # The level of logging: # - 0: no logging; # - 1: adds errors;