2011-01-22 Nikolas Zimmermann <nzimmermann@rim.com>
authorzimmermann@webkit.org <zimmermann@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 22 Jan 2011 13:46:31 +0000 (13:46 +0000)
committerzimmermann@webkit.org <zimmermann@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 22 Jan 2011 13:46:31 +0000 (13:46 +0000)
        Reviewed by Dirk Schulze.

        REGRESSION: Vertical line metrics incorrect
        https://bugs.webkit.org/show_bug.cgi?id=52960

        SimpleFontDataMac.mm contains a hack to modifiy lineGap/descent for the 'Hiragino' font.
        That didn't influence the lineSpacing so far, but does now, causing regressions.

        Restore old line spacing behaviour to fix the regression.
        Covered by existing fast/blockflow, fast/repaint and fast/text/international test cases.

        * platform/graphics/FontMetrics.h:
        (WebCore::FontMetrics::FontMetrics):
        (WebCore::FontMetrics::floatLineSpacing):
        (WebCore::FontMetrics::setLineSpacing):
        (WebCore::FontMetrics::lineSpacing):
        (WebCore::FontMetrics::reset):
        * platform/graphics/SimpleFontData.cpp:
        (WebCore::SimpleFontData::SimpleFontData):
        * platform/graphics/chromium/SimpleFontDataChromiumWin.cpp:
        (WebCore::SimpleFontData::platformInit):
        * platform/graphics/chromium/SimpleFontDataLinux.cpp:
        (WebCore::SimpleFontData::platformInit):
        * platform/graphics/freetype/SimpleFontDataFreeType.cpp:
        (WebCore::SimpleFontData::platformInit):
        * platform/graphics/haiku/SimpleFontDataHaiku.cpp:
        (WebCore::SimpleFontData::platformInit):
        * platform/graphics/mac/SimpleFontDataMac.mm:
        (WebCore::SimpleFontData::platformInit):
        * platform/graphics/pango/SimpleFontDataPango.cpp:
        (WebCore::SimpleFontData::platformInit):
        * platform/graphics/qt/SimpleFontDataQt.cpp:
        (WebCore::SimpleFontData::platformInit):
        * platform/graphics/win/SimpleFontDataCGWin.cpp:
        (WebCore::SimpleFontData::platformInit):
        * platform/graphics/win/SimpleFontDataCairoWin.cpp:
        (WebCore::SimpleFontData::platformInit):
        * platform/graphics/win/SimpleFontDataWin.cpp:
        (WebCore::SimpleFontData::initGDIFont):
        * platform/graphics/wince/SimpleFontDataWinCE.cpp:
        (WebCore::SimpleFontData::platformInit):
        * platform/graphics/wx/SimpleFontDataWx.cpp:
        (WebCore::SimpleFontData::platformInit):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@76445 268f45cc-cd09-0410-ab3c-d52691b4dbfc

15 files changed:
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/FontMetrics.h
Source/WebCore/platform/graphics/SimpleFontData.cpp
Source/WebCore/platform/graphics/chromium/SimpleFontDataChromiumWin.cpp
Source/WebCore/platform/graphics/chromium/SimpleFontDataLinux.cpp
Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp
Source/WebCore/platform/graphics/haiku/SimpleFontDataHaiku.cpp
Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm
Source/WebCore/platform/graphics/pango/SimpleFontDataPango.cpp
Source/WebCore/platform/graphics/qt/SimpleFontDataQt.cpp
Source/WebCore/platform/graphics/win/SimpleFontDataCGWin.cpp
Source/WebCore/platform/graphics/win/SimpleFontDataCairoWin.cpp
Source/WebCore/platform/graphics/win/SimpleFontDataWin.cpp
Source/WebCore/platform/graphics/wince/SimpleFontDataWinCE.cpp
Source/WebCore/platform/graphics/wx/SimpleFontDataWx.cpp

index 7ffa608c799bb5d5b6c0480cdf8b482a99331e96..2b04b8d288b68888951694d1aacfc7499227b4a1 100644 (file)
@@ -1,3 +1,49 @@
+2011-01-22  Nikolas Zimmermann  <nzimmermann@rim.com>
+
+        Reviewed by Dirk Schulze.
+
+        REGRESSION: Vertical line metrics incorrect
+        https://bugs.webkit.org/show_bug.cgi?id=52960
+
+        SimpleFontDataMac.mm contains a hack to modifiy lineGap/descent for the 'Hiragino' font.
+        That didn't influence the lineSpacing so far, but does now, causing regressions.
+
+        Restore old line spacing behaviour to fix the regression.
+        Covered by existing fast/blockflow, fast/repaint and fast/text/international test cases.
+
+        * platform/graphics/FontMetrics.h:
+        (WebCore::FontMetrics::FontMetrics):
+        (WebCore::FontMetrics::floatLineSpacing):
+        (WebCore::FontMetrics::setLineSpacing):
+        (WebCore::FontMetrics::lineSpacing):
+        (WebCore::FontMetrics::reset):
+        * platform/graphics/SimpleFontData.cpp:
+        (WebCore::SimpleFontData::SimpleFontData):
+        * platform/graphics/chromium/SimpleFontDataChromiumWin.cpp:
+        (WebCore::SimpleFontData::platformInit):
+        * platform/graphics/chromium/SimpleFontDataLinux.cpp:
+        (WebCore::SimpleFontData::platformInit):
+        * platform/graphics/freetype/SimpleFontDataFreeType.cpp:
+        (WebCore::SimpleFontData::platformInit):
+        * platform/graphics/haiku/SimpleFontDataHaiku.cpp:
+        (WebCore::SimpleFontData::platformInit):
+        * platform/graphics/mac/SimpleFontDataMac.mm:
+        (WebCore::SimpleFontData::platformInit):
+        * platform/graphics/pango/SimpleFontDataPango.cpp:
+        (WebCore::SimpleFontData::platformInit):
+        * platform/graphics/qt/SimpleFontDataQt.cpp:
+        (WebCore::SimpleFontData::platformInit):
+        * platform/graphics/win/SimpleFontDataCGWin.cpp:
+        (WebCore::SimpleFontData::platformInit):
+        * platform/graphics/win/SimpleFontDataCairoWin.cpp:
+        (WebCore::SimpleFontData::platformInit):
+        * platform/graphics/win/SimpleFontDataWin.cpp:
+        (WebCore::SimpleFontData::initGDIFont):
+        * platform/graphics/wince/SimpleFontDataWinCE.cpp:
+        (WebCore::SimpleFontData::platformInit):
+        * platform/graphics/wx/SimpleFontDataWx.cpp:
+        (WebCore::SimpleFontData::platformInit):
+
 2011-01-22  Andreas Kling  <kling@webkit.org>
 
         Reviewed by Kenneth Rohde Christiansen.
index a101cbc00126697fd70de4b3070a7c7c46cd442b..89c55455d99e1d66cb0391059f77d5568bf15923 100644 (file)
@@ -33,6 +33,7 @@ public:
         , m_ascent(0)
         , m_descent(0)
         , m_lineGap(0)
+        , m_lineSpacing(0)
         , m_xHeight(0)
     {
     }
@@ -66,7 +67,8 @@ public:
     float floatLineGap() const { return m_lineGap; }
     void setLineGap(float lineGap) { m_lineGap = lineGap; }
 
-    float floatLineSpacing() const { return m_ascent + m_descent + m_lineGap; }
+    float floatLineSpacing() const { return m_lineSpacing; }
+    void setLineSpacing(float lineSpacing) { m_lineSpacing = lineSpacing; }
 
     float xHeight() const { return m_xHeight; }
     void setXHeight(float xHeight) { m_xHeight = xHeight; }
@@ -92,12 +94,7 @@ public:
     }
 
     int lineGap() const { return lroundf(m_lineGap); }
-
-    int lineSpacing() const
-    {
-        // This mimics the old WebCore definition of lineSpacing. Changing to lroundf(m_ascent + m_descent + m_lineGap) causes lots of 1px height differences in the DRT dumps
-        return lroundf(m_ascent) + lroundf(m_descent) + lroundf(m_lineGap);
-    }
+    int lineSpacing() const { return lroundf(m_lineSpacing); }
 
 private:
     friend class SimpleFontData;
@@ -108,6 +105,7 @@ private:
         m_ascent = 0;
         m_descent = 0;
         m_lineGap = 0;
+        m_lineSpacing = 0;
         m_xHeight = 0;
     }
 
@@ -115,6 +113,7 @@ private:
     float m_ascent;
     float m_descent;
     float m_lineGap;
+    float m_lineSpacing;
     float m_xHeight;
 };
 
index 81b54f240a90ee08e541b7b5f054a60e4ecc4331..19e2d4a81fb68b043d1b7bb38cb37c16303ec403 100644 (file)
@@ -88,6 +88,7 @@ SimpleFontData::SimpleFontData(PassOwnPtr<SVGFontData> svgFontData, int size, bo
     m_fontMetrics.setAscent(ascent);
     m_fontMetrics.setDescent(descent);
     m_fontMetrics.setLineGap(lineGap);
+    m_fontMetrics.setLineSpacing(ascent + descent + lineGap);
     m_fontMetrics.setXHeight(xHeight);
 
     SVGFontElement* associatedFontElement = svgFontFaceElement->associatedFontElement();
index 750a31733358c090b0686ec61fab00db27b8c04f..1450c5acdf0261746c11fa422a41a08a60caf48d 100644 (file)
@@ -97,6 +97,7 @@ void SimpleFontData::platformInit()
     m_fontMetrics.setDescent(descent);
     m_fontMetrics.setLineGap(lineGap);
     m_fontMetrics.setXHeight(xHeight);
+    m_fontMetrics.setLineSpacing(ascent + descent + lineGap);
 
     SelectObject(dc, oldFont);
     ReleaseDC(0, dc);
index b995a4a01f948bc57e9e30c58d56194cd456a37a..787d3be0d07711af8a46e2c2c79a9fb1ec38b3b5 100644 (file)
@@ -109,8 +109,10 @@ void SimpleFontData::platformInit()
         xHeight = ascent * 0.56f;
     }
 
-    m_fontMetrics.setLineGap(SkScalarToFloat(metrics.fLeading));
+    float lineGap = SkScalarToFloat(metrics.fLeading);
+    m_fontMetrics.setLineGap(lineGap);
     m_fontMetrics.setXHeight(xHeight);
+    m_fontMetrics.setLineSpacing(lroundf(ascent) + lroundf(descent) + lroundf(lineGap));
 
     if (m_orientation == Vertical) {
         static const uint32_t vheaTag = SkSetFourByteTag('v', 'h', 'e', 'a');
index fcbf54ba7e7a87583b8ffb07562a76c23926aa21..6290eeb349d16e54ddc8427d875e1e5248555e57 100644 (file)
@@ -62,6 +62,8 @@ void SimpleFontData::platformInit()
     float lineSpacing = font_extents.height;
     if (lineSpacing < font_extents.ascent + font_extents.descent)
         lineSpacing = font_extents.ascent + font_extents.descent;
+
+    m_fontMetrics.setLineSpacing(lroundf(lineSpacing));
     m_fontMetrics.setLineGap(lineSpacing - font_extents.ascent - font_extents.descent);
 
     cairo_scaled_font_text_extents(m_platformData.scaledFont(), "x", &text_extents);
index 468b0004aee783d9cc604c96bb8c4cc58c912723..4ded761d38a141d6dcd002e8a9c9301d9be27a33 100644 (file)
@@ -52,6 +52,7 @@ void SimpleFontData::platformInit()
     m_fontMetrics.setDescent(height.descent);
     m_fontMetrics.setXHeight(height.ascent * 0.56f); // Hack taken from the win port.
     m_fontMetrics.setLineGap(height.leading);
+    m_fontMetrics.setLineSpacing(lroundf(height.ascent) + lroundf(height.descent) + lroundf(height.leading));
 }
 
 void SimpleFontData::platformCharWidthInit()
index 2b58e92a6044168e766a0f80bc623d3e52de2191..29fb9a07b44ab1d0246dcbac27bc90188c7bfb4e 100644 (file)
@@ -249,6 +249,9 @@ void SimpleFontData::platformInit()
         descent *= 2.f;
     }
 
+    // Compute and store line spacing, before the line metrics hacks are applied.
+    m_fontMetrics.setLineSpacing(lroundf(ascent) + lroundf(descent) + lroundf(lineGap));
+
     // Hack Hiragino line metrics to allow room for marked text underlines.
     // <rdar://problem/5386183>
     if (descent < 3 && lineGap >= 3 && [familyName hasPrefix:@"Hiragino"]) {
index aa37255cdad78b6108490e223b686b84d5ba39bc..3fe15b3673666eded89c9ff1f0c2c0fc9cf9f9c8 100644 (file)
@@ -61,6 +61,7 @@ void SimpleFontData::platformInit()
     float lineSpacing = font_extents.height;
     if (lineSpacing < font_extents.ascent + font_extents.descent)
         lineSpacing = font_extents.ascent + font_extents.descent;
+    m_fontMetrics.setLineSpacing(lroundf(lineSpacing));
     m_fontMetrics.setLineGap(lineSpacing - font_extents.ascent - font_extents.descent);
 
     cairo_scaled_font_text_extents(m_platformData.m_scaledFont, "x", &text_extents);
index ac8fe395533f9c6771c735e3adee1f3985c4c217..9e4355863ef518848fed7f12d71415e8b8c8684b 100644 (file)
@@ -52,6 +52,7 @@ void SimpleFontData::platformInit()
     m_fontMetrics.setDescent(fm.descent());
     m_fontMetrics.setXHeight(fm.xHeight());
     m_fontMetrics.setLineGap(fm.leading());
+    m_fontMetrics.setLineSpacing(fm.lineSpacing());
     m_spaceWidth = fm.width(QLatin1Char(' '));
 }
 
index 21f25333283f8f24e0121db64dde9c04d83c6b2e..e8f066f10387eb4067f4f69d3fdb4c34a56d9b50 100644 (file)
@@ -96,6 +96,7 @@ void SimpleFontData::platformInit()
     m_fontMetrics.setAscent(fAscent);
     m_fontMetrics.setDescent(fDescent);
     m_fontMetrics.setLineGap(fLineGap);
+    m_fontMetrics.setLineSpacing(lroundf(fAscent) + lroundf(fDescent) + lroundf(fLineGap));
 
     // Measure the actual character "x", because AppKit synthesizes X height rather than getting it from the font.
     // Unfortunately, NSFont will round this for us so we don't quite get the right value.
index 452f8195e4a575afd6fc76d79c12d1df789b0b37..eea10df33a6bfbee66d1e23f6ec8a8911a69c77e 100644 (file)
@@ -70,6 +70,7 @@ void SimpleFontData::platformInit()
     m_fontMetrics.setAscent(ascent);
     m_fontMetrics.setDescent(descent);
     m_fontMetrics.setLineGap(lineGap);
+    m_fontMetrics.setLineSpacing(lroundf(ascent) + lroundf(descent) + lroundf(lineGap));
     m_avgCharWidth = textMetrics.tmAveCharWidth * metricsMultiplier;
     m_maxCharWidth = textMetrics.tmMaxCharWidth * metricsMultiplier;
 
index d31e0403faae81613f1e314d29439bb1af851475..d466c29227ab7713bb6e9a7998d1247c16986bad 100644 (file)
@@ -83,6 +83,7 @@ void SimpleFontData::initGDIFont()
      m_fontMetrics.setAscent(ascent);
      m_fontMetrics.setDescent(descent);
      m_fontMetrics.setLineGap(lineGap);
+     m_fontMetrics.setLineSpacing(lroundf(ascent) + lroundf(descent) + lroundf(lineGap));
      m_avgCharWidth = textMetrics.tmAveCharWidth;
      m_maxCharWidth = textMetrics.tmMaxCharWidth;
      float xHeight = ascent * 0.56f; // Best guess for xHeight if no x glyph is present.
index 0c1be21dc863be9a987500a68365f30d9d4e2eee..f3e511c9a45cd901d90b965c2abd2a860b7d1e05 100644 (file)
@@ -57,6 +57,7 @@ void SimpleFontData::platformInit()
     m_fontMetrics.setAscent(ascent);
     m_fontMetrics.setDescent(descent);
     m_fontMetrics.setLineGap(lineGap);
+    m_fontMetrics.setLineSpacing(lroundf(ascent) + lroundf(descent) + lroundf(lineGap));
     m_fontMetrics.setXHeight(ascent * 0.56f);
 }
 
index cbabc2957b47d0e5250556e127628dcf1a39e064..4f24e4c25dec6da824de90ceaa153661fadaa890 100644 (file)
@@ -58,6 +58,7 @@ void SimpleFontData::platformInit()
         m_fontMetrics.setXHeight(props.GetXHeight());
         m_fontMetrics.setUnitsPerEm(1); // FIXME!
         m_fontMetrics.setLineGap(props.GetLineGap());
+        m_fontMetrics.setLineSpacing(props.GetLineSpacing());
     }
 
     m_syntheticBoldOffset = 0.0f;