bug 624310 - get glyph widths via directwrite rather than font tables when using simulated bold. r=bas a=joe
authorJonathan Kew <jfkthame@gmail.com>
Fri, 21 Jan 2011 10:35:21 +0000
changeset 61073 02b92b61f5cdaa8efd8dfa5c8cb86d4a0f25aa01
parent 61072 6dca94a53967cb3d0a12a0d1316615fe8e9a50f7
child 61074 e0c9841ac3ddec2a86b8cbee70cba5039f71e5d1
push idunknown
push userunknown
push dateunknown
reviewersbas, joe
bugs624310
milestone2.0b10pre
bug 624310 - get glyph widths via directwrite rather than font tables when using simulated bold. r=bas a=joe
gfx/thebes/gfxDWriteFonts.cpp
gfx/thebes/gfxDWriteFonts.h
gfx/thebes/gfxDWriteShaper.cpp
gfx/thebes/gfxFT2FontBase.cpp
gfx/thebes/gfxFT2FontBase.h
gfx/thebes/gfxFont.h
gfx/thebes/gfxGDIFont.cpp
gfx/thebes/gfxGDIFont.h
gfx/thebes/gfxHarfBuzzShaper.cpp
gfx/thebes/gfxHarfBuzzShaper.h
layout/reftests/text/reftest.list
--- a/gfx/thebes/gfxDWriteFonts.cpp
+++ b/gfx/thebes/gfxDWriteFonts.cpp
@@ -476,32 +476,42 @@ gfxDWriteFont::GetFontTable(PRUint32 aTa
             return blob;
         }
     }
 
     return nsnull;
 }
 
 PRInt32
-gfxDWriteFont::GetHintedGlyphWidth(gfxContext *aCtx, PRUint16 aGID)
+gfxDWriteFont::GetGlyphWidth(gfxContext *aCtx, PRUint16 aGID)
 {
     if (!mGlyphWidths.IsInitialized()) {
         mGlyphWidths.Init(200);
     }
 
-    PRInt32 width;
+    PRInt32 width = -1;
     if (mGlyphWidths.Get(aGID, &width)) {
         return width;
     }
 
     DWRITE_GLYPH_METRICS glyphMetrics;
-    HRESULT hr = mFontFace->GetGdiCompatibleGlyphMetrics(
+    HRESULT hr;
+    if (mUsingClearType) {
+        hr = mFontFace->GetDesignGlyphMetrics(
+                  &aGID, 1, &glyphMetrics, FALSE);
+        if (SUCCEEDED(hr)) {
+            width =
+                NS_lround(glyphMetrics.advanceWidth * mFUnitsConvFactor *
+                          65536.0);
+        }
+    } else {
+        hr = mFontFace->GetGdiCompatibleGlyphMetrics(
                   GetAdjustedSize(), 1.0f, nsnull, FALSE,
                   &aGID, 1, &glyphMetrics, FALSE);
-
-    if (NS_SUCCEEDED(hr)) {
-        width = NS_lround(glyphMetrics.advanceWidth * mFUnitsConvFactor) << 16;
-        mGlyphWidths.Put(aGID, width);
-        return width;
+        if (SUCCEEDED(hr)) {
+            width =
+                NS_lround(glyphMetrics.advanceWidth * mFUnitsConvFactor) << 16;
+        }
     }
 
-    return -1;
+    mGlyphWidths.Put(aGID, width);
+    return width;
 }
--- a/gfx/thebes/gfxDWriteFonts.h
+++ b/gfx/thebes/gfxDWriteFonts.h
@@ -74,23 +74,26 @@ public:
     gfxFloat GetAdjustedSize() const { return mAdjustedSize; }
 
     IDWriteFontFace *GetFontFace() { return mFontFace.get(); }
 
     // override gfxFont table access function to bypass gfxFontEntry cache,
     // use DWrite API to get direct access to system font data
     virtual hb_blob_t *GetFontTable(PRUint32 aTag);
 
-    virtual PRBool ProvidesHintedWidths() const {
-        return !mUsingClearType;
+    virtual PRBool ProvidesGlyphWidths() const {
+        return !mUsingClearType ||
+               (mFontFace->GetSimulations() & DWRITE_FONT_SIMULATIONS_BOLD);
     }
 
-    virtual PRInt32 GetHintedGlyphWidth(gfxContext *aCtx, PRUint16 aGID);
+    virtual PRInt32 GetGlyphWidth(gfxContext *aCtx, PRUint16 aGID);
 
 protected:
+    friend class gfxDWriteShaper;
+
     virtual void CreatePlatformShaper();
 
     void ComputeMetrics();
 
     cairo_font_face_t *CairoFontFace();
 
     cairo_scaled_font_t *CairoScaledFont();
 
--- a/gfx/thebes/gfxDWriteShaper.cpp
+++ b/gfx/thebes/gfxDWriteShaper.cpp
@@ -170,17 +170,17 @@ trymoreglyphs:
         WORD gID = indices[0];
         nsAutoTArray<FLOAT, 400> advances;
         nsAutoTArray<DWRITE_GLYPH_OFFSET, 400> glyphOffsets;
         if (!advances.SetLength(actualGlyphs) || 
             !glyphOffsets.SetLength(actualGlyphs)) {
             continue;
         }
 
-        if (mFont->ProvidesHintedWidths()) {
+        if (!static_cast<gfxDWriteFont*>(mFont)->mUsingClearType) {
             hr = analyzer->GetGdiCompatibleGlyphPlacements(
                                               aString + range.start,
                                               clusters.Elements(),
                                               textProperties.Elements(),
                                               range.Length(),
                                               indices.Elements(),
                                               glyphProperties.Elements(),
                                               actualGlyphs,
--- a/gfx/thebes/gfxFT2FontBase.cpp
+++ b/gfx/thebes/gfxFT2FontBase.cpp
@@ -211,17 +211,17 @@ gfxFT2FontBase::GetGlyph(PRUint32 unicod
         if (id)
             return id;
     }
 
     return GetGlyph(unicode);
 }
 
 PRInt32
-gfxFT2FontBase::GetHintedGlyphWidth(gfxContext *aCtx, PRUint16 aGID)
+gfxFT2FontBase::GetGlyphWidth(gfxContext *aCtx, PRUint16 aGID)
 {
     cairo_text_extents_t extents;
     GetGlyphExtents(aGID, &extents);
     // convert to 16.16 fixed point
     return NS_lround(0x10000 * extents.x_advance);
 }
 
 PRBool
--- a/gfx/thebes/gfxFT2FontBase.h
+++ b/gfx/thebes/gfxFT2FontBase.h
@@ -58,18 +58,18 @@ public:
     void GetGlyphExtents(PRUint32 aGlyph,
                          cairo_text_extents_t* aExtents);
     virtual const gfxFont::Metrics& GetMetrics();
     virtual nsString GetUniqueName();
     virtual PRUint32 GetSpaceGlyph();
     virtual hb_blob_t *GetFontTable(PRUint32 aTag);
     virtual PRBool ProvidesGetGlyph() const { return PR_TRUE; }
     virtual PRUint32 GetGlyph(PRUint32 unicode, PRUint32 variation_selector);
-    virtual PRBool ProvidesHintedWidths() const { return PR_TRUE; }
-    virtual PRInt32 GetHintedGlyphWidth(gfxContext *aCtx, PRUint16 aGID);
+    virtual PRBool ProvidesGlyphWidths() const { return PR_TRUE; }
+    virtual PRInt32 GetGlyphWidth(gfxContext *aCtx, PRUint16 aGID);
 
     cairo_scaled_font_t *CairoScaledFont() { return mScaledFont; };
     virtual PRBool SetupCairoFont(gfxContext *aContext);
 
 protected:
     cairo_scaled_font_t *mScaledFont;
     PRUint32 mSpaceGlyph;
     PRBool mHasMetrics;
--- a/gfx/thebes/gfxFont.h
+++ b/gfx/thebes/gfxFont.h
@@ -1004,26 +1004,26 @@ public:
         return PR_FALSE;
     }
     // Map unicode character to glyph ID.
     // Only used if ProvidesGetGlyph() returns PR_TRUE.
     virtual PRUint32 GetGlyph(PRUint32 unicode, PRUint32 variation_selector) {
         return 0;
     }
 
-    // subclasses may provide hinted glyph widths (in font units);
+    // subclasses may provide (possibly hinted) glyph widths (in font units);
     // if they do not override this, harfbuzz will use unhinted widths
     // derived from the font tables
-    virtual PRBool ProvidesHintedWidths() const {
+    virtual PRBool ProvidesGlyphWidths() const {
         return PR_FALSE;
     }
 
     // The return value is interpreted as a horizontal advance in 16.16 fixed
     // point format.
-    virtual PRInt32 GetHintedGlyphWidth(gfxContext *aCtx, PRUint16 aGID) {
+    virtual PRInt32 GetGlyphWidth(gfxContext *aCtx, PRUint16 aGID) {
         return -1;
     }
 
     // Font metrics
     struct Metrics {
         gfxFloat xHeight;
         gfxFloat superscriptOffset;
         gfxFloat subscriptOffset;
--- a/gfx/thebes/gfxGDIFont.cpp
+++ b/gfx/thebes/gfxGDIFont.cpp
@@ -471,17 +471,17 @@ gfxGDIFont::FillLogFont(LOGFONTW& aLogFo
         }
     }
 
     fe->FillLogFont(&aLogFont, italic, weight, aSize, 
                     (mAntialiasOption == kAntialiasSubpixel) ? PR_TRUE : PR_FALSE);
 }
 
 PRInt32
-gfxGDIFont::GetHintedGlyphWidth(gfxContext *aCtx, PRUint16 aGID)
+gfxGDIFont::GetGlyphWidth(gfxContext *aCtx, PRUint16 aGID)
 {
     if (!mGlyphWidths.IsInitialized()) {
         mGlyphWidths.Init(200);
     }
 
     PRInt32 width;
     if (mGlyphWidths.Get(aGID, &width)) {
         return width;
--- a/gfx/thebes/gfxGDIFont.h
+++ b/gfx/thebes/gfxGDIFont.h
@@ -71,20 +71,20 @@ public:
 
     virtual PRUint32 GetSpaceGlyph();
 
     virtual PRBool SetupCairoFont(gfxContext *aContext);
 
     /* required for MathML to suppress effects of ClearType "padding" */
     virtual gfxFont* CopyWithAntialiasOption(AntialiasOption anAAOption);
 
-    virtual PRBool ProvidesHintedWidths() const { return PR_TRUE; }
+    virtual PRBool ProvidesGlyphWidths() const { return PR_TRUE; }
 
     // get hinted glyph width in pixels as 16.16 fixed-point value
-    virtual PRInt32 GetHintedGlyphWidth(gfxContext *aCtx, PRUint16 aGID);
+    virtual PRInt32 GetGlyphWidth(gfxContext *aCtx, PRUint16 aGID);
 
 protected:
     virtual void CreatePlatformShaper();
 
     /* override to check for uniscribe failure and fall back to GDI */
     virtual PRBool InitTextRun(gfxContext *aContext,
                                gfxTextRun *aTextRun,
                                const PRUnichar *aString,
--- a/gfx/thebes/gfxHarfBuzzShaper.cpp
+++ b/gfx/thebes/gfxHarfBuzzShaper.cpp
@@ -78,17 +78,17 @@ gfxHarfBuzzShaper::gfxHarfBuzzShaper(gfx
       mKernTable(nsnull),
       mHmtxTable(nsnull),
       mNumLongMetrics(0),
       mCmapTable(nsnull),
       mCmapFormat(-1),
       mSubtableOffset(0),
       mUVSTableOffset(0),
       mUseFontGetGlyph(aFont->ProvidesGetGlyph()),
-      mUseHintedWidths(aFont->ProvidesHintedWidths())
+      mUseFontGlyphWidths(aFont->ProvidesGlyphWidths())
 {
 }
 
 gfxHarfBuzzShaper::~gfxHarfBuzzShaper()
 {
     hb_blob_destroy(mCmapTable);
     hb_blob_destroy(mHmtxTable);
     hb_blob_destroy(mKernTable);
@@ -210,18 +210,18 @@ struct HMetrics {
 };
 
 void
 gfxHarfBuzzShaper::GetGlyphAdvance(gfxContext *aContext,
                                    hb_codepoint_t glyph,
                                    hb_position_t *x_advance,
                                    hb_position_t *y_advance) const
 {
-    if (mUseHintedWidths) {
-        *x_advance = mFont->GetHintedGlyphWidth(aContext, glyph);
+    if (mUseFontGlyphWidths) {
+        *x_advance = mFont->GetGlyphWidth(aContext, glyph);
         return;
     }
 
     // font did not implement GetHintedGlyphWidth, so get an unhinted value
     // directly from the font tables
 
     NS_ASSERTION((mNumLongMetrics > 0) && mHmtxTable != nsnull,
                  "font is lacking metrics, we shouldn't be here");
@@ -747,18 +747,18 @@ gfxHarfBuzzShaper::InitTextRun(gfxContex
             PRBool symbol;
             mCmapFormat = gfxFontUtils::
                 FindPreferredSubtable(data, hb_blob_get_length(mCmapTable),
                                       &mSubtableOffset, &mUVSTableOffset,
                                       &symbol);
             hb_blob_unlock(mCmapTable);
         }
 
-        if (!mUseHintedWidths) {
-            // if font doesn't implement hinted widths, we will be reading
+        if (!mUseFontGlyphWidths) {
+            // if font doesn't implement GetGlyphWidth, we will be reading
             // the hmtx table directly;
             // read mNumLongMetrics from hhea table without caching its blob,
             // and preload/cache the hmtx table
             hb_blob_t *hheaTable =
                 mFont->GetFontTable(TRUETYPE_TAG('h','h','e','a'));
             if (hheaTable &&
                 hb_blob_get_length(hheaTable) >= sizeof(HMetricsHeader)) {
                 const HMetricsHeader* hhea =
@@ -783,17 +783,17 @@ gfxHarfBuzzShaper::InitTextRun(gfxContex
                     }
                 }
             }
             hb_blob_destroy(hheaTable);
         }
     }
 
     if ((!mUseFontGetGlyph && mCmapFormat <= 0) ||
-        (!mUseHintedWidths && !mHmtxTable)) {
+        (!mUseFontGlyphWidths && !mHmtxTable)) {
         // unable to shape with this font
         return PR_FALSE;
     }
 
     FontCallbackData fcd(this, aContext);
     hb_font_t *font = hb_font_create();
     hb_font_set_funcs(font, sHBFontFuncs, nsnull, &fcd);
     hb_font_set_ppem(font, mFont->GetAdjustedSize(), mFont->GetAdjustedSize());
--- a/gfx/thebes/gfxHarfBuzzShaper.h
+++ b/gfx/thebes/gfxHarfBuzzShaper.h
@@ -113,14 +113,14 @@ protected:
     mutable hb_blob_t *mCmapTable;
     mutable PRInt32    mCmapFormat;
     mutable PRUint32   mSubtableOffset;
     mutable PRUint32   mUVSTableOffset;
 
     // Whether the font implements GetGlyph, or we should read tables
     // directly
     PRPackedBool mUseFontGetGlyph;
-    // Whether the font implements hinted widths, or we should read tables
+    // Whether the font implements GetGlyphWidth, or we should read tables
     // directly to get ideal widths
-    PRPackedBool mUseHintedWidths;
+    PRPackedBool mUseFontGlyphWidths;
 };
 
 #endif /* GFX_HARFBUZZSHAPER_H */
--- a/layout/reftests/text/reftest.list
+++ b/layout/reftests/text/reftest.list
@@ -36,19 +36,17 @@ skip-if(d2d||cocoaWidget) == subpixel-gl
 # design, but that is considered more tolerable because they are subpixel
 # inconsistencies.  On those platforms we just test that glyph positions are
 # subpixel.
 skip-if(!(d2d||cocoaWidget)) != subpixel-glyphs-x-2a.html subpixel-glyphs-x-2b.html
 # No platforms do subpixel positioning vertically
 == subpixel-glyphs-y-1a.html subpixel-glyphs-y-1b.html
 == subpixel-lineheight-1a.html subpixel-lineheight-1b.html
 == swash-1.html swash-1-ref.html
-# Test for bug 593564, synthetic bold should cause glyph metrics to increase
-# Random on Windows - fails on tinderbox Opt reftest, passes elsewhere - see bug 597300
-random-if(winWidget) HTTP(..) != synthetic-bold-metrics-01.html synthetic-bold-metrics-01-notref.html
+HTTP(..) != synthetic-bold-metrics-01.html synthetic-bold-metrics-01-notref.html
 == variation-selector-unsupported-1.html variation-selector-unsupported-1-ref.html
 == white-space-1a.html white-space-1-ref.html
 == white-space-1b.html white-space-1-ref.html
 == white-space-2.html white-space-2-ref.html
 == wordwrap-01.html wordwrap-01-ref.html
 HTTP(..) == wordwrap-02.html wordwrap-02-ref.html
 HTTP(..) == wordwrap-03.html wordwrap-03-ref.html
 == wordwrap-04.html wordwrap-04-ref.html