[Mac] [iOS] Underlines are too low
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 11 May 2014 17:35:14 +0000 (17:35 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 11 May 2014 17:35:14 +0000 (17:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=132770

Reviewed by Darin Adler.

Source/WebCore:
computeUnderlineOffset() inside InlineTextBox.cpp lowers underlines from text
baseline by a value that is proportional to the font size. However, this
lowering was done a second time in
GraphicsContext::computeLineBoundsAndAntialiasingModeForText(). This patch
removes this second, platform-dependent lowering.

This duplication was caused by merging iOS into open source, where iOS used
the GraphicsContext approach and open source used the InlineTextBox approach.

Covered by fast/css3-text/css3-text-decoration/text-decoration-thickness.html.

* platform/graphics/GraphicsContext.cpp:
(WebCore::GraphicsContext::computeLineBoundsAndAntialiasingModeForText): Remove
redundant lowering code
* rendering/InlineTextBox.cpp:
(WebCore::InlineTextBox::paintDecoration): Clean up textDecorationThickness
variable

LayoutTests:
See per-file descriptions.

* fast/css3-text/css3-text-decoration/text-decoration-style-double-space-scales.html: Made
test more robust so it does not barely clip underlines, but rather gives them a couple
pixels of wiggle room.
* fast/css3-text/css3-text-decoration/text-decoration-thickness.html: Not only does this test
underline thickness, but it also tests underline position. Updated this test to not expect
incorrect results.

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

LayoutTests/ChangeLog
LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-style-double-space-scales.html
LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-thickness.html
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/GraphicsContext.cpp
Source/WebCore/rendering/InlineTextBox.cpp

index dbc5728716e6744ec97f277afda82f2f25d30a22..cbfb127a64d7dfd9975c87ba10a2274717c76999 100644 (file)
@@ -1,3 +1,19 @@
+2014-05-09  Myles C. Maxfield  <litherum@gmail.com>
+
+        [Mac] [iOS] Underlines are too low
+        https://bugs.webkit.org/show_bug.cgi?id=132770
+
+        Reviewed by Darin Adler.
+
+        See per-file descriptions.
+
+        * fast/css3-text/css3-text-decoration/text-decoration-style-double-space-scales.html: Made
+        test more robust so it does not barely clip underlines, but rather gives them a couple
+        pixels of wiggle room.
+        * fast/css3-text/css3-text-decoration/text-decoration-thickness.html: Not only does this test
+        underline thickness, but it also tests underline position. Updated this test to not expect
+        incorrect results.
+
 2014-05-11  Antti Koivisto  <antti@apple.com>
 
         Text with simple line layout not getting pushed below float when there is not enough space for it
index 06d3ecf833380048068afcc981f2103a66f20dd3..56ef1288b16998b82e1c84e55f5d5a1aae080dd4 100644 (file)
@@ -7,7 +7,7 @@ This test renders large text with a double underline, but then barely clips off
 using overflow: hidden. It makes sure that this is exactly the same as a single underline. If the
 space between the two underlines does not scale with font size, it will appear as though there is a
 single thick underline (because they will be drawn on top of each other) and will thus fail this test.
-<div style="width: 100px; height: 99px; overflow: hidden;">
+<div style="width: 100px; height: 95px; overflow: hidden;">
 <div style="text-decoration: underline; font-size: 100px; -webkit-text-decoration-style: double; font-family: Ahem;">&nbsp;</div>
 </div>
 </body>
index d77363707089175f75c90d42f2697311f921ceab..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: 10000px; position: absolute; left: 0px; top: -8650px;">&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 3cc09b131dd621e418e0c204d8bdc2532b15f9a4..2a56232a1160420be3a57553b30abc5302e47bca 100644 (file)
@@ -1,3 +1,28 @@
+2014-05-09  Myles C. Maxfield  <litherum@gmail.com>
+
+        [Mac] [iOS] Underlines are too low
+        https://bugs.webkit.org/show_bug.cgi?id=132770
+
+        Reviewed by Darin Adler.
+
+        computeUnderlineOffset() inside InlineTextBox.cpp lowers underlines from text
+        baseline by a value that is proportional to the font size. However, this
+        lowering was done a second time in
+        GraphicsContext::computeLineBoundsAndAntialiasingModeForText(). This patch
+        removes this second, platform-dependent lowering.
+
+        This duplication was caused by merging iOS into open source, where iOS used
+        the GraphicsContext approach and open source used the InlineTextBox approach.
+
+        Covered by fast/css3-text/css3-text-decoration/text-decoration-thickness.html.
+
+        * platform/graphics/GraphicsContext.cpp:
+        (WebCore::GraphicsContext::computeLineBoundsAndAntialiasingModeForText): Remove
+        redundant lowering code
+        * rendering/InlineTextBox.cpp:
+        (WebCore::InlineTextBox::paintDecoration): Clean up textDecorationThickness
+        variable
+
 2014-05-11  Antti Koivisto  <antti@apple.com>
 
         Text with simple line layout not getting pushed below float when there is not enough space for it
index e736d6f45f29e9f52853e65391202dff1e9c0f78..225e2fe5ef9f36ddcbcd0f0c4a06565a0a98e51a 100644 (file)
@@ -977,16 +977,8 @@ FloatRect GraphicsContext::computeLineBoundsAndAntialiasingModeForText(const Flo
             color = Color(color.red(), color.green(), color.blue(), alpha);
         }
 
-        // Don't offset line from bottom of text if scale is less than offsetUnderLineScale.
-        static const float offsetUnderlineScale = 0.4f;
-        float dy = scale < offsetUnderlineScale ? 0 : 1;
-
-        // If we've increased the thickness of the line, make sure to move the location too.
-        if (thickness > 1)
-            dy += roundf(thickness) - 1;
-
         FloatPoint devicePoint = transform.mapPoint(point);
-        FloatPoint deviceOrigin = FloatPoint(roundf(devicePoint.x()), ceilf(devicePoint.y()) + dy);
+        FloatPoint deviceOrigin = FloatPoint(roundf(devicePoint.x()), ceilf(devicePoint.y()));
         origin = transform.inverse().mapPoint(deviceOrigin);
     }
     return FloatRect(origin.x(), origin.y(), width, thickness);
index 326d1155efb16df696e9b1d983fe0083005c39fa..42fc17a07a0b3a1c5efa6d34d30ab23a9ebddee6 100644 (file)
@@ -979,9 +979,6 @@ void InlineTextBox::paintDecoration(GraphicsContext& context, const FloatPoint&
     UNUSED_PARAM(textPainter);
 #endif
 
-    // FIXME: We should improve this rule and not always just assume 1.
-    const float textDecorationThickness = 1.f;
-
     if (m_truncation == cFullTruncation)
         return;
 
@@ -1004,9 +1001,8 @@ void InlineTextBox::paintDecoration(GraphicsContext& context, const FloatPoint&
     bool isPrinting = renderer().document().printing();
 
     const float textDecorationBaseFontSize = 16;
-    float fontSizeScaling = renderer().style().fontSize() / textDecorationBaseFontSize;
-    float strokeThickness = roundf(textDecorationThickness * fontSizeScaling);
-    context.setStrokeThickness(strokeThickness);
+    float textDecorationThickness = renderer().style().fontSize() / textDecorationBaseFontSize;
+    context.setStrokeThickness(textDecorationThickness);
 
     bool linesAreOpaque = !isPrinting && (!(decoration & TextDecorationUnderline) || underline.alpha() == 255) && (!(decoration & TextDecorationOverline) || overline.alpha() == 255) && (!(decoration & TextDecorationLineThrough) || linethrough.alpha() == 255);