Unreviewed, rolling out r176263 and r176273.
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Nov 2014 01:19:43 +0000 (01:19 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Nov 2014 01:19:43 +0000 (01:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=138854

Underlines are hideous. (Requested by litherum on #webkit).

Reverted changesets:

"Use underlining metrics from the font file"
https://bugs.webkit.org/show_bug.cgi?id=138762
http://trac.webkit.org/changeset/176263

"iOS build fix."
http://trac.webkit.org/changeset/176273

Patch by Commit Queue <commit-queue@webkit.org> on 2014-11-18# Please enter the commit message for your changes. Lines starting

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

16 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-thickness-expected.html [moved from LayoutTests/platform/mac/fast/css3-text/css3-text-decoration/text-decoration-thickness-expected.html with 100% similarity]
LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-thickness.html [moved from LayoutTests/platform/mac/fast/css3-text/css3-text-decoration/text-decoration-thickness.html with 86% similarity]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/FontMetrics.h
Source/WebCore/platform/graphics/SimpleFontData.h
Source/WebCore/platform/graphics/freetype/SimpleFontDataFreeType.cpp
Source/WebCore/platform/graphics/ios/SimpleFontDataIOS.mm
Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm
Source/WebCore/platform/graphics/win/SimpleFontDataCGWin.cpp
Source/WebCore/platform/graphics/win/SimpleFontDataCairoWin.cpp
Source/WebCore/platform/graphics/win/SimpleFontDataWin.cpp
Source/WebCore/rendering/InlineTextBox.cpp
Source/WebCore/style/InlineTextBoxStyle.cpp
Source/WebCore/style/InlineTextBoxStyle.h
Source/WebCore/svg/SVGFontData.cpp

index a801c47ecb025b89b4a2a7f74c93e4f900849f18..7577ccaa5373f127bb33f6f22c05a713d0271ccd 100644 (file)
@@ -1,3 +1,19 @@
+2014-11-18  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r176263 and r176273.
+        https://bugs.webkit.org/show_bug.cgi?id=138854
+
+        Underlines are hideous. (Requested by litherum on #webkit).
+
+        Reverted changesets:
+
+        "Use underlining metrics from the font file"
+        https://bugs.webkit.org/show_bug.cgi?id=138762
+        http://trac.webkit.org/changeset/176263
+
+        "iOS build fix."
+        http://trac.webkit.org/changeset/176273
+
 2014-11-18  David Hyatt  <hyatt@apple.com>
 
         REGRESSION (r167210): Invalid cast in WebCore::RenderBlock::blockSelectionGaps
similarity index 86%
rename from LayoutTests/platform/mac/fast/css3-text/css3-text-decoration/text-decoration-thickness.html
rename to LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-thickness.html
index e37b7f57067c49ac0e5a1340131c522adb81e9b8..dfdd648e42556d088734c9b082711ab0a6da1011 100644 (file)
@@ -7,7 +7,7 @@ This test draws underlined text at a very large font size. It then positions and
 the text so that the underline should fill a box if the underline grows in proportion
 to text size. The comparison is to a box that has its background color set.
 <div style="position: relative; width: 600px; height: 600px; overflow: hidden;">
-<div style="font-family: Ahem; text-decoration: underline; font-size: 31000px; position: absolute; left: 0px; top: -28925px;">&nbsp;</div>
+<div style="font-family: Ahem; text-decoration: underline; font-size: 10000px; position: absolute; left: 0px; top: -8350px;">&nbsp;</div>
 </div>
 </body>
 </html>
index 66542ed849be7e0e483032abef7ef5e4dda158d2..87a0282a2d0c6c412e89e0a54f24d1b3aa682b84 100644 (file)
@@ -1,3 +1,19 @@
+2014-11-18  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r176263 and r176273.
+        https://bugs.webkit.org/show_bug.cgi?id=138854
+
+        Underlines are hideous. (Requested by litherum on #webkit).
+
+        Reverted changesets:
+
+        "Use underlining metrics from the font file"
+        https://bugs.webkit.org/show_bug.cgi?id=138762
+        http://trac.webkit.org/changeset/176263
+
+        "iOS build fix."
+        http://trac.webkit.org/changeset/176273
+
 2014-11-18  Chris Dumez  <cdumez@apple.com>
 
         Add a setting to toggle DOMTimer throttling support
index c23831aa93bc8df30960948b37adc4b3818f429d..a1eac37463148b5b8faac6ba6b17e7dd2d80e335 100644 (file)
@@ -37,8 +37,6 @@ public:
         , m_lineSpacing(0)
         , m_xHeight(0)
         , m_zeroWidth(0)
-        , m_decorationThickness(1)
-        , m_underlinePosition(1)
         , m_hasXHeight(false)
         , m_hasZeroWidth(false)
     {
@@ -134,12 +132,6 @@ public:
     bool hasZeroWidth() const { return m_hasZeroWidth; }
     void setHasZeroWidth(bool hasZeroWidth) { m_hasZeroWidth = hasZeroWidth; }
 
-    float decorationThickness() const { return m_decorationThickness; }
-    void setDecorationThickness(float decorationThickness) { m_decorationThickness = decorationThickness; }
-
-    float underlinePosition() const { return m_underlinePosition; }
-    void setUnderlinePosition(float underlinePosition) { m_underlinePosition = underlinePosition; }
-
 private:
     friend class SimpleFontData;
 
@@ -162,8 +154,6 @@ private:
     float m_xHeight;
     float m_capHeight;
     float m_zeroWidth;
-    float m_decorationThickness;
-    float m_underlinePosition;
     bool m_hasXHeight;
     bool m_hasCapHeight;
     bool m_hasZeroWidth;
index 1eb4b488f742129c274f610f97101d802ec44bf5..e0b3740d3724bbe3d877799f1c6dccae56098c10 100644 (file)
@@ -362,14 +362,6 @@ ALWAYS_INLINE float SimpleFontData::widthForGlyph(Glyph glyph) const
     return width;
 }
 
-static inline void populateDecorationMetrics(float fontSize, float& decorationThickness, float& underlinePosition)
-{
-    // Decoration underlines should be proportional to the font size
-    decorationThickness = fontSize / 16.0f;
-    // An amount to lower the underline below the baseline
-    underlinePosition = std::max(1.0f, ceilf(decorationThickness / 2.0));
-}
-
 } // namespace WebCore
 
 SPECIALIZE_TYPE_TRAITS_BEGIN(WebCore::SimpleFontData)
index 54f2711a575f92dd6aec0d428faee651dfa730c2..0bb34334174018bd9af26a853275fc2c5bc507c2 100644 (file)
@@ -65,15 +65,10 @@ void SimpleFontData::platformInit()
     float descent = narrowPrecisionToFloat(fontExtents.descent);
     float capHeight = narrowPrecisionToFloat(fontExtents.height);
     float lineGap = narrowPrecisionToFloat(fontExtents.height - fontExtents.ascent - fontExtents.descent);
-    float decorationThickness;
-    float underlinePosition;
-    populateDecorationMetrics(m_platformData.m_size, decorationThickness, underlinePosition);
 
     m_fontMetrics.setAscent(ascent);
     m_fontMetrics.setDescent(descent);
     m_fontMetrics.setCapHeight(capHeight);
-    m_fontMetrics.setDecorationThickness(decorationThickness);
-    m_fontMetrics.setUnderlinePosition(underlinePosition);
 
 #if PLATFORM(EFL)
     m_fontMetrics.setLineSpacing(ascent + descent + lineGap);
index 5444800a6b6f75c0a0f610f2b8a9f037c8b412dd..d0c080068876729a4ed5b97115a1baf268752569 100644 (file)
@@ -79,8 +79,6 @@ void SimpleFontData::platformInit()
     float lineGap;
     float lineSpacing;
     float xHeight;
-    float decorationThickness;
-    float underlinePosition;
     RetainPtr<CFStringRef> familyName;
     if (CTFontRef ctFont = m_platformData.font()) {
         FontServicesIOS fontService(ctFont);
@@ -89,8 +87,6 @@ void SimpleFontData::platformInit()
         lineSpacing = fontService.lineSpacing();
         lineGap = fontService.lineGap();
         xHeight = fontService.xHeight();
-        decorationThickness = CTFontGetUnderlineThickness(ctFont);
-        underlinePosition = -CTFontGetUnderlinePosition(ctFont);
         capHeight = fontService.capHeight();
         unitsPerEm = fontService.unitsPerEm();
         familyName = adoptCF(CTFontCopyFamilyName(ctFont));
@@ -105,7 +101,6 @@ void SimpleFontData::platformInit()
         lineGap = lroundf(scaleEmToUnits(CGFontGetLeading(cgFont), unitsPerEm) * pointSize);
         xHeight = scaleEmToUnits(CGFontGetXHeight(cgFont), unitsPerEm) * pointSize;
         capHeight = scaleEmToUnits(CGFontGetCapHeight(cgFont), unitsPerEm) * pointSize;
-        populateDecorationMetrics(m_platformData.size(), decorationThickness, underlinePosition);
 
         lineSpacing = ascent + descent + lineGap;
         familyName = adoptCF(CGFontCopyFamilyName(cgFont));
@@ -118,8 +113,6 @@ void SimpleFontData::platformInit()
     m_fontMetrics.setLineSpacing(lineSpacing);
     m_fontMetrics.setXHeight(xHeight);
     m_fontMetrics.setCapHeight(capHeight);
-    m_fontMetrics.setDecorationThickness(decorationThickness);
-    m_fontMetrics.setUnderlinePosition(underlinePosition);
     m_shouldNotBeUsedForArabic = fontFamilyShouldNotBeUsedForArabic(familyName.get());
 
     if (platformData().orientation() == Vertical && !isTextOrientationFallback())
index d75ea0c5de3b5b6e05350847dde8228a2ed16aae..cabddce89f2b3fd9c4a2895fa9a5d31b28d03e1e 100644 (file)
@@ -193,8 +193,6 @@ void SimpleFontData::platformInit()
     float capHeight = scaleEmToUnits(iCapHeight, unitsPerEm) * pointSize;
     
     float lineGap = scaleEmToUnits(iLineGap, unitsPerEm) * pointSize;
-    float decorationThickness = CTFontGetUnderlineThickness(m_platformData.ctFont());
-    float underlinePosition = -CTFontGetUnderlinePosition(m_platformData.ctFont());
 
     // We need to adjust Times, Helvetica, and Courier to closely match the
     // vertical metrics of their Microsoft counterparts that are the de facto
@@ -238,8 +236,6 @@ void SimpleFontData::platformInit()
     m_fontMetrics.setCapHeight(capHeight);
     m_fontMetrics.setLineGap(lineGap);
     m_fontMetrics.setXHeight(xHeight);
-    m_fontMetrics.setDecorationThickness(decorationThickness);
-    m_fontMetrics.setUnderlinePosition(underlinePosition);
 }
 
 static CFDataRef copyFontTableForTag(FontPlatformData& platformData, FourCharCode tableName)
index e532e7d4725f223030efbd29c166ca23bfa75b72..7a781c84435e6520183bb53fa5aed782754d5f71 100644 (file)
@@ -69,9 +69,6 @@ void SimpleFontData::platformInit()
     float fDescent = -scaleEmToUnits(iDescent, unitsPerEm) * pointSize;
     float fCapHeight = scaleEmToUnits(iCapHeight, unitsPerEm) * pointSize;
     float fLineGap = scaleEmToUnits(iLineGap, unitsPerEm) * pointSize;
-    float decorationThickness;
-    float underlinePosition;
-    populateDecorationMetrics(m_platformData.size(), decorationThickness, underlinePosition);
 
     if (!isCustomFont()) {
         HWndDC dc(0);
@@ -90,8 +87,6 @@ void SimpleFontData::platformInit()
     m_fontMetrics.setCapHeight(fCapHeight);
     m_fontMetrics.setLineGap(fLineGap);
     m_fontMetrics.setLineSpacing(lroundf(fAscent) + lroundf(fDescent) + lroundf(fLineGap));
-    m_fontMetrics.setDecorationThickness(decorationThickness);
-    m_fontMetrics.setUnderlinePosition(underlinePosition);
 
     GlyphPage* glyphPageZero = GlyphPageTreeNode::getRootChild(this, 0)->page();
     Glyph xGlyph = glyphPageZero ? glyphPageZero->glyphDataForCharacter('x').glyph : 0;
index f6e0613062f49b35f1eb1a3cff917be73c5fccc5..5ecdeac9ccd78b3120507fbdd17c80a6431be996 100644 (file)
@@ -76,9 +76,6 @@ void SimpleFontData::platformInit()
     float descent = textMetrics.tmDescent * metricsMultiplier;
     float xHeight = ascent * 0.56f; // Best guess for xHeight for non-Truetype fonts.
     float lineGap = textMetrics.tmExternalLeading * metricsMultiplier;
-    float decorationThickness;
-    float underlinePosition;
-    populateDecorationMetrics(m_platformData.size(), decorationThickness, underlinePosition);
 
     int faceLength = ::GetTextFace(dc, 0, 0);
     Vector<WCHAR> faceName(faceLength);
@@ -91,8 +88,6 @@ void SimpleFontData::platformInit()
     m_fontMetrics.setDescent(descent);
     m_fontMetrics.setLineGap(lineGap);
     m_fontMetrics.setLineSpacing(lroundf(ascent) + lroundf(descent) + lroundf(lineGap));
-    m_fontMetrics.setDecorationThickness(decorationThickness);
-    m_fontMetrics.setUnderlinePosition(underlinePosition);
     m_avgCharWidth = textMetrics.tmAveCharWidth * metricsMultiplier;
     m_maxCharWidth = textMetrics.tmMaxCharWidth * metricsMultiplier;
 
index 2c8043f3ac2a3e788481d9f35beada879eca3928..60e7ee786f14eed8a8846f32606080a74ee73491 100644 (file)
@@ -89,15 +89,10 @@ void SimpleFontData::initGDIFont()
     float ascent = textMetrics.tmAscent;
     float descent = textMetrics.tmDescent;
     float lineGap = textMetrics.tmExternalLeading;
-    float decorationThickness;
-    float underlinePosition;
-    populateDecorationMetrics(m_platformData.size(), decorationThickness, underlinePosition);
     m_fontMetrics.setAscent(ascent);
     m_fontMetrics.setDescent(descent);
     m_fontMetrics.setLineGap(lineGap);
     m_fontMetrics.setLineSpacing(lroundf(ascent) + lroundf(descent) + lroundf(lineGap));
-    m_fontMetrics.setDecorationThickness(decorationThickness);
-    m_fontMetrics.setUnderlinePosition(underlinePosition);
     m_avgCharWidth = textMetrics.tmAveCharWidth;
     m_maxCharWidth = textMetrics.tmMaxCharWidth;
     float xHeight = ascent * 0.56f; // Best guess for xHeight if no x glyph is present.
index 346c594d4ba5627334870db8361aed8b90253682..0c01ca93432e5a59aff816b07134591ba0c5e0ed 100644 (file)
@@ -925,7 +925,7 @@ void InlineTextBox::paintDecoration(GraphicsContext& context, const FloatPoint&
     // Use a special function for underlines to get the positioning exactly right.
     bool isPrinting = renderer().document().printing();
 
-    float textDecorationThickness = renderer().style().fontMetrics().decorationThickness();
+    float textDecorationThickness = textDecorationStrokeThickness(renderer().style().fontSize());
     context.setStrokeThickness(textDecorationThickness);
 
     bool linesAreOpaque = !isPrinting && (!(decoration & TextDecorationUnderline) || underline.alpha() == 255) && (!(decoration & TextDecorationOverline) || overline.alpha() == 255) && (!(decoration & TextDecorationLineThrough) || linethrough.alpha() == 255);
@@ -977,7 +977,7 @@ void InlineTextBox::paintDecoration(GraphicsContext& context, const FloatPoint&
         // These decorations should match the visual overflows computed in visualOverflowForDecorations()
         if (decoration & TextDecorationUnderline) {
             context.setStrokeColor(underline, colorSpace);
-            const int underlineOffset = computeUnderlineOffset(lineStyle.textUnderlinePosition(), lineStyle.fontMetrics(), this);
+            const int underlineOffset = computeUnderlineOffset(lineStyle.textUnderlinePosition(), lineStyle.fontMetrics(), this, textDecorationThickness);
 
             switch (decorationStyle) {
             case TextDecorationStyleWavy: {
index 34e54c28e4ab45799d1f213ca3b7660e9201a4d7..0f3acde51c778e19adad857e813f96a003bcb83d 100644 (file)
 
 namespace WebCore {
     
-int computeUnderlineOffset(TextUnderlinePosition underlinePosition, const FontMetrics& fontMetrics, InlineTextBox* inlineTextBox)
+int computeUnderlineOffset(TextUnderlinePosition underlinePosition, const FontMetrics& fontMetrics, InlineTextBox* inlineTextBox, int textDecorationThickness)
 {
     // This represents the gap between the baseline and the closest edge of the underline.
-    float gap = fontMetrics.underlinePosition();
+    int gap = std::max<int>(1, ceilf(textDecorationThickness / 2.0));
 
     // According to the specification TextUnderlinePositionAuto should default to 'alphabetic' for horizontal text
     // and to 'under Left' for vertical text (e.g. japanese). We support only horizontal text for now.
@@ -83,7 +83,7 @@ GlyphOverflow visualOverflowForDecorations(const RenderStyle& lineStyle, InlineT
     if (decoration == TextDecorationNone)
         return GlyphOverflow();
     
-    float strokeThickness = lineStyle.fontMetrics().decorationThickness();
+    float strokeThickness = textDecorationStrokeThickness(lineStyle.fontSize());
     float controlPointDistance;
     float step;
     float wavyOffset;
@@ -101,7 +101,7 @@ GlyphOverflow visualOverflowForDecorations(const RenderStyle& lineStyle, InlineT
 
     // These metrics must match where underlines get drawn.
     if (decoration & TextDecorationUnderline) {
-        float underlineOffset = computeUnderlineOffset(lineStyle.textUnderlinePosition(), lineStyle.fontMetrics(), inlineTextBox);
+        float underlineOffset = computeUnderlineOffset(lineStyle.textUnderlinePosition(), lineStyle.fontMetrics(), inlineTextBox, strokeThickness);
         if (decorationStyle == TextDecorationStyleWavy) {
             extendIntToFloat(overflowResult.bottom, underlineOffset + wavyOffset + controlPointDistance + strokeThickness - height);
             extendIntToFloat(overflowResult.top, -(underlineOffset + wavyOffset - controlPointDistance - strokeThickness));
index 9639c2242cad234c1bb450b48fdb06e3e5a496c0..9689341a56d0b4c42d8eb2afdc0d7769a3309876 100644 (file)
@@ -33,6 +33,12 @@ namespace WebCore {
     
 class InlineTextBox;
 
+inline float textDecorationStrokeThickness(float fontSize)
+{
+    const float textDecorationBaseFontSize = 16;
+    return fontSize / textDecorationBaseFontSize;
+}
+
 inline float wavyOffsetFromDecoration()
 {
     return 2;
@@ -40,7 +46,7 @@ inline float wavyOffsetFromDecoration()
 
 GlyphOverflow visualOverflowForDecorations(const RenderStyle& lineStyle, InlineTextBox*);
 void getWavyStrokeParameters(float strokeThickness, float& controlPointDistance, float& step);
-int computeUnderlineOffset(TextUnderlinePosition, const FontMetrics&, InlineTextBox*);
+int computeUnderlineOffset(TextUnderlinePosition, const FontMetrics&, InlineTextBox*, int textDecorationThickness);
     
 }
 
index a16bd81b44fb916701b40dbabf79a695ca359c97..0b8fd038fdc1c90326abab8d5177e89f1baa14c6 100644 (file)
@@ -79,9 +79,6 @@ void SVGFontData::initializeFontData(SimpleFontData* fontData, float fontSize)
     float ascent = svgFontFaceElement->ascent() * scale;
     float descent = svgFontFaceElement->descent() * scale;
     float lineGap = 0.1f * fontSize;
-    float decorationThickness;
-    float underlinePosition;
-    populateDecorationMetrics(fontSize, decorationThickness, underlinePosition);
 
     GlyphPage* glyphPageZero = GlyphPageTreeNode::getRootChild(fontData, 0)->page();
 
@@ -98,8 +95,6 @@ void SVGFontData::initializeFontData(SimpleFontData* fontData, float fontSize)
     fontMetrics.setLineGap(lineGap);
     fontMetrics.setLineSpacing(roundf(ascent) + roundf(descent) + roundf(lineGap));
     fontMetrics.setXHeight(xHeight);
-    fontMetrics.setDecorationThickness(decorationThickness);
-    fontMetrics.setUnderlinePosition(underlinePosition);
 
     if (!glyphPageZero) {
         fontData->setSpaceGlyph(0);