Bug 282126 - Part 1: Allow getting zero glyph width from nsFontMetrics without falling back to average glyph width. r?jfkthame draft
authorCameron McCormack <cam@mcc.id.au>
Wed, 20 Jun 2018 16:48:35 +1000
changeset 809792 f23e38381ed47990c4fe95823f0ce66cec9908ef
parent 808588 65821feb6358421ec9592320d5f739161c55f86c
child 809793 97c95948d8adb360217413b5d1d0374611918b9a
push id113820
push userbmo:cam@mcc.id.au
push dateSat, 23 Jun 2018 03:01:03 +0000
reviewersjfkthame
bugs282126
milestone62.0a1
Bug 282126 - Part 1: Allow getting zero glyph width from nsFontMetrics without falling back to average glyph width. r?jfkthame While we're here, fix the measurement of ' ' and 'x' so that we don't measure the .notdef glyph if those glyphs aren't present. MozReview-Commit-ID: Kb7ndaKBruu
gfx/thebes/gfxDWriteFonts.cpp
gfx/thebes/gfxFT2FontBase.cpp
gfx/thebes/gfxFont.cpp
gfx/thebes/gfxFont.h
gfx/thebes/gfxGDIFont.cpp
gfx/thebes/gfxMacFont.cpp
layout/style/ServoBindings.cpp
--- a/gfx/thebes/gfxDWriteFonts.cpp
+++ b/gfx/thebes/gfxDWriteFonts.cpp
@@ -228,22 +228,21 @@ gfxDWriteFont::ComputeMetrics(AntialiasO
         }
     }
 
     mMetrics->internalLeading = std::max(mMetrics->maxHeight - mMetrics->emHeight, 0.0);
     mMetrics->externalLeading = ceil(fontMetrics.lineGap * mFUnitsConvFactor);
 
     UINT32 ucs = L' ';
     UINT16 glyph;
-    HRESULT hr = mFontFace->GetGlyphIndicesW(&ucs, 1, &glyph);
-    if (FAILED(hr)) {
-        mMetrics->spaceWidth = 0;
-    } else {
+    if (SUCCEEDED(mFontFace->GetGlyphIndicesW(&ucs, 1, &glyph)) && glyph != 0) {
         mSpaceGlyph = glyph;
         mMetrics->spaceWidth = MeasureGlyphWidth(glyph);
+    } else {
+        mMetrics->spaceWidth = 0;
     }
 
     // try to get aveCharWidth from the OS/2 table, fall back to measuring 'x'
     // if the table is not available or if using hinted/pixel-snapped widths
     if (mUseSubpixelPositions) {
         mMetrics->aveCharWidth = 0;
         gfxFontEntry::AutoTable os2Table(GetFontEntry(),
                                          TRUETYPE_TAG('O','S','/','2'));
@@ -258,31 +257,31 @@ gfxDWriteFont::ComputeMetrics(AntialiasO
                 mMetrics->aveCharWidth =
                     int16_t(os2->xAvgCharWidth) * mFUnitsConvFactor;
             }
         }
     }
 
     if (mMetrics->aveCharWidth < 1) {
         ucs = L'x';
-        if (SUCCEEDED(mFontFace->GetGlyphIndicesW(&ucs, 1, &glyph))) {
+        if (SUCCEEDED(mFontFace->GetGlyphIndicesW(&ucs, 1, &glyph) &&
+            glyph != 0)) {
             mMetrics->aveCharWidth = MeasureGlyphWidth(glyph);
         }
         if (mMetrics->aveCharWidth < 1) {
             // Let's just assume the X is square.
             mMetrics->aveCharWidth = fontMetrics.xHeight * mFUnitsConvFactor;
         }
     }
 
     ucs = L'0';
-    if (SUCCEEDED(mFontFace->GetGlyphIndicesW(&ucs, 1, &glyph))) {
-        mMetrics->zeroOrAveCharWidth = MeasureGlyphWidth(glyph);
-    }
-    if (mMetrics->zeroOrAveCharWidth < 1) {
-        mMetrics->zeroOrAveCharWidth = mMetrics->aveCharWidth;
+    if (SUCCEEDED(mFontFace->GetGlyphIndicesW(&ucs, 1, &glyph) && glyph != 0)) {
+        mMetrics->zeroWidth = MeasureGlyphWidth(glyph);
+    } else {
+        mMetrics->zeroWidth = -1.0;
     }
 
     mMetrics->underlineOffset =
         fontMetrics.underlinePosition * mFUnitsConvFactor;
     mMetrics->underlineSize = 
         fontMetrics.underlineThickness * mFUnitsConvFactor;
     mMetrics->strikeoutOffset =
         fontMetrics.strikethroughPosition * mFUnitsConvFactor;
@@ -292,18 +291,18 @@ gfxDWriteFont::ComputeMetrics(AntialiasO
     SanitizeMetrics(mMetrics, GetFontEntry()->mIsBadUnderlineFont);
 
 #if 0
     printf("Font: %p (%s) size: %f\n", this,
            NS_ConvertUTF16toUTF8(GetName()).get(), mStyle.size);
     printf("    emHeight: %f emAscent: %f emDescent: %f\n", mMetrics->emHeight, mMetrics->emAscent, mMetrics->emDescent);
     printf("    maxAscent: %f maxDescent: %f maxAdvance: %f\n", mMetrics->maxAscent, mMetrics->maxDescent, mMetrics->maxAdvance);
     printf("    internalLeading: %f externalLeading: %f\n", mMetrics->internalLeading, mMetrics->externalLeading);
-    printf("    spaceWidth: %f aveCharWidth: %f zeroOrAve: %f\n",
-           mMetrics->spaceWidth, mMetrics->aveCharWidth, mMetrics->zeroOrAveCharWidth);
+    printf("    spaceWidth: %f aveCharWidth: %f zeroWidth: %f\n",
+           mMetrics->spaceWidth, mMetrics->aveCharWidth, mMetrics->zeroWidth);
     printf("    xHeight: %f capHeight: %f\n", mMetrics->xHeight, mMetrics->capHeight);
     printf("    uOff: %f uSize: %f stOff: %f stSize: %f\n",
            mMetrics->underlineOffset, mMetrics->underlineSize, mMetrics->strikeoutOffset, mMetrics->strikeoutSize);
 #endif
 }
 
 using namespace mozilla; // for AutoSwap_* types
 
@@ -635,17 +634,17 @@ gfxDWriteFont::MeasureGlyphWidth(uint16_
                       FLOAT(mAdjustedSize), 1.0f, nullptr,
                       GetMeasuringMode() == DWRITE_MEASURING_MODE_GDI_NATURAL,
                       &aGlyph, 1, &metrics, FALSE);
             if (SUCCEEDED(hr)) {
                 return NS_lround(metrics.advanceWidth * mFUnitsConvFactor);
             }
         }
     }
-    return 0;
+    return 0.0;
 }
 
 void
 gfxDWriteFont::AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
                                       FontCacheSizes* aSizes) const
 {
     gfxFont::AddSizeOfExcludingThis(aMallocSizeOf, aSizes);
     aSizes->mFontInstances += aMallocSizeOf(mMetrics);
--- a/gfx/thebes/gfxFT2FontBase.cpp
+++ b/gfx/thebes/gfxFT2FontBase.cpp
@@ -210,17 +210,17 @@ gfxFT2FontBase::InitMetrics()
         mMetrics.maxDescent = mMetrics.emDescent = 0.2 * emHeight;
         mMetrics.maxHeight = emHeight;
         mMetrics.internalLeading = 0.0;
         mMetrics.externalLeading = 0.2 * emHeight;
         const gfxFloat spaceWidth = 0.5 * emHeight;
         mMetrics.spaceWidth = spaceWidth;
         mMetrics.maxAdvance = spaceWidth;
         mMetrics.aveCharWidth = spaceWidth;
-        mMetrics.zeroOrAveCharWidth = spaceWidth;
+        mMetrics.zeroWidth = spaceWidth;
         const gfxFloat xHeight = 0.5 * emHeight;
         mMetrics.xHeight = xHeight;
         mMetrics.capHeight = mMetrics.maxAscent;
         const gfxFloat underlineSize = emHeight / 14.0;
         mMetrics.underlineSize = underlineSize;
         mMetrics.underlineOffset = -underlineSize;
         mMetrics.strikeoutOffset = 0.25 * emHeight;
         mMetrics.strikeoutSize = underlineSize;
@@ -405,19 +405,19 @@ gfxFT2FontBase::InitMetrics()
     mSpaceGlyph = GetCharWidth(' ', &width);
     if (mSpaceGlyph) {
         mMetrics.spaceWidth = width;
     } else {
         mMetrics.spaceWidth = mMetrics.maxAdvance; // guess
     }
 
     if (GetCharWidth('0', &width)) {
-        mMetrics.zeroOrAveCharWidth = width;
+        mMetrics.zeroWidth = width;
     } else {
-        mMetrics.zeroOrAveCharWidth = 0.0;
+        mMetrics.zeroWidth = -1.0;  // indicates not found
     }
 
     // Prefering a measured x over sxHeight because sxHeight doesn't consider
     // hinting, but maybe the x extents are not quite right in some fancy
     // script fonts.  CSS 2.1 suggests possibly using the height of an "o",
     // which would have a more consistent glyph across fonts.
     cairo_text_extents_t extents;
     if (GetCharExtents('x', &extents) && extents.y_bearing < 0.0) {
@@ -426,23 +426,20 @@ gfxFT2FontBase::InitMetrics()
             std::max(mMetrics.aveCharWidth, extents.x_advance);
     }
 
     if (GetCharExtents('H', &extents) && extents.y_bearing < 0.0) {
         mMetrics.capHeight = -extents.y_bearing;
     }
 
     mMetrics.aveCharWidth =
-        std::max(mMetrics.aveCharWidth, mMetrics.zeroOrAveCharWidth);
+        std::max(mMetrics.aveCharWidth, mMetrics.zeroWidth);
     if (mMetrics.aveCharWidth == 0.0) {
         mMetrics.aveCharWidth = mMetrics.spaceWidth;
     }
-    if (mMetrics.zeroOrAveCharWidth == 0.0) {
-        mMetrics.zeroOrAveCharWidth = mMetrics.aveCharWidth;
-    }
     // Apparently hinting can mean that max_advance is not always accurate.
     mMetrics.maxAdvance =
         std::max(mMetrics.maxAdvance, mMetrics.aveCharWidth);
 
     mMetrics.maxHeight = mMetrics.maxAscent + mMetrics.maxDescent;
 
     // Make the line height an integer number of pixels so that lines will be
     // equally spaced (rather than just being snapped to pixels, some up and
--- a/gfx/thebes/gfxFont.cpp
+++ b/gfx/thebes/gfxFont.cpp
@@ -4065,17 +4065,17 @@ gfxFont::CreateVerticalMetrics()
     metrics->underlineSize = std::max(1.0, metrics->underlineSize);
     metrics->underlineOffset = - metrics->maxDescent - metrics->underlineSize;
 
     metrics->strikeoutSize = std::max(1.0, metrics->strikeoutSize);
     metrics->strikeoutOffset = - 0.5 * metrics->strikeoutSize;
 
     // Somewhat arbitrary values for now, subject to future refinement...
     metrics->spaceWidth = metrics->aveCharWidth;
-    metrics->zeroOrAveCharWidth = metrics->aveCharWidth;
+    metrics->zeroWidth = metrics->aveCharWidth;
     metrics->maxHeight = metrics->maxAscent + metrics->maxDescent;
     metrics->xHeight = metrics->emHeight / 2;
     metrics->capHeight = metrics->maxAscent;
 
     return std::move(metrics);
 }
 
 gfxFloat
@@ -4087,17 +4087,17 @@ gfxFont::SynthesizeSpaceWidth(uint32_t a
     switch (aCh) {
     case 0x2000:                                 // en quad
     case 0x2002: return GetAdjustedSize() / 2;   // en space
     case 0x2001:                                 // em quad
     case 0x2003: return GetAdjustedSize();       // em space
     case 0x2004: return GetAdjustedSize() / 3;   // three-per-em space
     case 0x2005: return GetAdjustedSize() / 4;   // four-per-em space
     case 0x2006: return GetAdjustedSize() / 6;   // six-per-em space
-    case 0x2007: return GetMetrics(eHorizontal).zeroOrAveCharWidth; // figure space
+    case 0x2007: return GetMetrics(eHorizontal).ZeroOrAveCharWidth(); // figure space
     case 0x2008: return GetMetrics(eHorizontal).spaceWidth; // punctuation space
     case 0x2009: return GetAdjustedSize() / 5;   // thin space
     case 0x200a: return GetAdjustedSize() / 10;  // hair space
     case 0x202f: return GetAdjustedSize() / 5;   // narrow no-break space
     default: return -1.0;
     }
 }
 
--- a/gfx/thebes/gfxFont.h
+++ b/gfx/thebes/gfxFont.h
@@ -1666,19 +1666,22 @@ public:
         gfxFloat emDescent;
         gfxFloat maxHeight;
         gfxFloat maxAscent;
         gfxFloat maxDescent;
         gfxFloat maxAdvance;
 
         gfxFloat aveCharWidth;
         gfxFloat spaceWidth;
-        gfxFloat zeroOrAveCharWidth;  // width of '0', or if there is
-                                      // no '0' glyph in this font,
-                                      // equal to .aveCharWidth
+        gfxFloat zeroWidth;  // -1 if there was no zero glyph
+
+        gfxFloat ZeroOrAveCharWidth() const
+        {
+          return zeroWidth >= 0 ? zeroWidth : aveCharWidth;
+        }
     };
 
     enum Orientation {
         eHorizontal,
         eVertical
     };
 
     const Metrics& GetMetrics(Orientation aOrientation)
--- a/gfx/thebes/gfxGDIFont.cpp
+++ b/gfx/thebes/gfxGDIFont.cpp
@@ -377,19 +377,19 @@ gfxGDIFont::Initialize()
             mMetrics->spaceWidth = mMetrics->aveCharWidth;
         }
 
         // Cache the width of digit zero, if available.
         ret = GetGlyphIndicesW(dc.GetDC(), L"0", 1, &glyph,
                                GGI_MARK_NONEXISTING_GLYPHS);
         if (ret != GDI_ERROR && glyph != 0xFFFF) {
             GetTextExtentPoint32W(dc.GetDC(), L"0", 1, &size);
-            mMetrics->zeroOrAveCharWidth = ROUND(size.cx);
+            mMetrics->zeroWidth = ROUND(size.cx);
         } else {
-            mMetrics->zeroOrAveCharWidth = mMetrics->aveCharWidth;
+            mMetrics->zeroWidth = -1.0;  // indicates not found
         }
 
         SanitizeMetrics(mMetrics, GetFontEntry()->mIsBadUnderlineFont);
     } else {
         mFUnitsConvFactor = 0.0; // zero-sized font: all values scale to zero
     }
 
     if (IsSyntheticBold()) {
--- a/gfx/thebes/gfxMacFont.cpp
+++ b/gfx/thebes/gfxMacFont.cpp
@@ -403,20 +403,19 @@ gfxMacFont::InitMetrics()
 
     mMetrics.spaceWidth = GetCharWidth(cmap, ' ', &glyphID, cgConvFactor);
     if (glyphID == 0) {
         // no space glyph?!
         mMetrics.spaceWidth = mMetrics.aveCharWidth;
     }
     mSpaceGlyph = glyphID;
 
-    mMetrics.zeroOrAveCharWidth = GetCharWidth(cmap, '0', &glyphID,
-                                               cgConvFactor);
+    mMetrics.zeroWidth = GetCharWidth(cmap, '0', &glyphID, cgConvFactor);
     if (glyphID == 0) {
-        mMetrics.zeroOrAveCharWidth = mMetrics.aveCharWidth;
+        mMetrics.zeroWidth = -1.0;  // indicates not found
     }
 
     if (cmap) {
         ::CFRelease(cmap);
     }
 
     CalculateDerivedMetrics(mMetrics);
 
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -2574,17 +2574,17 @@ Gecko_GetFontMetrics(RawGeckoPresContext
   nsPresContext* presContext = const_cast<nsPresContext*>(aPresContext);
   presContext->SetUsesExChUnits(true);
   RefPtr<nsFontMetrics> fm = nsLayoutUtils::GetMetricsFor(
       presContext, aIsVertical, aFont, aFontSize, aUseUserFontSet,
       nsLayoutUtils::FlushUserFontSet::No);
 
   ret.mXSize = fm->XHeight();
   gfxFloat zeroWidth = fm->GetThebesFontGroup()->GetFirstValidFont()->
-                           GetMetrics(fm->Orientation()).zeroOrAveCharWidth;
+                           GetMetrics(fm->Orientation()).ZeroOrAveCharWidth();
   ret.mChSize = ceil(aPresContext->AppUnitsPerDevPixel() * zeroWidth);
   return ret;
 }
 
 int32_t
 Gecko_GetAppUnitsPerPhysicalInch(RawGeckoPresContextBorrowed aPresContext)
 {
   nsPresContext* presContext = const_cast<nsPresContext*>(aPresContext);