Position and thickness of underline as text size changes
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Feb 2014 01:54:19 +0000 (01:54 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Feb 2014 01:54:19 +0000 (01:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=16768

Source/WebCore:

Reviewed by Dean Jackson.

This patch adopts the iOS codepath for underlines. It also reorganizes
drawLineForText to avoid a costly global state save & restore.

Test: fast/css3-text/css3-text-decoration/text-decoration-thickness.html

* platform/graphics/cg/GraphicsContextCG.cpp:
(WebCore::computeLineBoundsAndAntialiasingModeForText):
(WebCore::GraphicsContext::computeLineBoundsForText):
(WebCore::GraphicsContext::drawLineForText):
(WebCore::GraphicsContext::drawLinesForText):
* rendering/InlineTextBox.cpp:
(WebCore::InlineTextBox::paintDecoration):

LayoutTests:

This test draws underlined text at a very large font size. It then positions and clips
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.

Reviewed by Dean Jackson.

* fast/css3-text/css3-text-decoration/text-decoration-thickness-expected.html: Added.
* fast/css3-text/css3-text-decoration/text-decoration-thickness.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-thickness-expected.html [new file with mode: 0644]
LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-thickness.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp
Source/WebCore/rendering/InlineTextBox.cpp

index ba1c7254555bf33ca6d1c1d698b9e0a456283608..da1635f53f99493de4d8b5cd577d09a0982d82d0 100644 (file)
@@ -1,3 +1,17 @@
+2014-02-11  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Position and thickness of underline as text size changes
+        https://bugs.webkit.org/show_bug.cgi?id=16768
+
+        This test draws underlined text at a very large font size. It then positions and clips
+        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.
+
+        Reviewed by Dean Jackson.
+
+        * fast/css3-text/css3-text-decoration/text-decoration-thickness-expected.html: Added.
+        * fast/css3-text/css3-text-decoration/text-decoration-thickness.html: Added.
+
 2014-02-10  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Convert position:sticky and position:fixed properties to position:static and position:absolute upon copy
diff --git a/LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-thickness-expected.html b/LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-thickness-expected.html
new file mode 100644 (file)
index 0000000..1adf82d
--- /dev/null
@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+</style>
+</head>
+<body>
+This test draws underlined text at a very large font size. It then positions and clips
+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; background: #000">
+</div>
+</body>
+</html>
diff --git a/LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-thickness.html b/LayoutTests/fast/css3-text/css3-text-decoration/text-decoration-thickness.html
new file mode 100644 (file)
index 0000000..d773637
--- /dev/null
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+This test draws underlined text at a very large font size. It then positions and clips
+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>
+</body>
+</html>
index cd79739cf3b93c9d36172a8b20cef578e615beab..fbfebc2df8a2a2f5de3655262c53edec57ab63f0 100644 (file)
@@ -1,3 +1,23 @@
+2014-02-11  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Position and thickness of underline as text size changes
+        https://bugs.webkit.org/show_bug.cgi?id=16768
+
+        Reviewed by Dean Jackson.
+
+        This patch adopts the iOS codepath for underlines. It also reorganizes
+        drawLineForText to avoid a costly global state save & restore.
+
+        Test: fast/css3-text/css3-text-decoration/text-decoration-thickness.html
+
+        * platform/graphics/cg/GraphicsContextCG.cpp:
+        (WebCore::computeLineBoundsAndAntialiasingModeForText):
+        (WebCore::GraphicsContext::computeLineBoundsForText):
+        (WebCore::GraphicsContext::drawLineForText):
+        (WebCore::GraphicsContext::drawLinesForText):
+        * rendering/InlineTextBox.cpp:
+        (WebCore::InlineTextBox::paintDecoration):
+
 2014-02-11  Ryosuke Niwa  <rniwa@webkit.org>
 
         Frame::rectForSelection shouldn't instantiate FrameSelection
index 0c561f8edbb19527e5d130d54665ff4d625ba3fb..6a37dd06653e71668213bd7cbec553ba62bc2f49 100644 (file)
@@ -1346,9 +1346,6 @@ FloatRect GraphicsContext::roundToDevicePixels(const FloatRect& rect, RoundingMo
     return FloatRect(roundedOrigin, roundedLowerRight - roundedOrigin);
 }
 
-// FIXME: We should look to consolidate the iOS and non-iOS implementations of
-// computeLineBoundsAndAntialiasingModeForText() and computeLineBoundsForText().
-#if PLATFORM(IOS)
 static FloatRect computeLineBoundsAndAntialiasingModeForText(GraphicsContext& initialContext, const FloatPoint& point, float width, bool printing, bool& shouldAntialias, Color& color)
 {
     CGContextRef context = initialContext.platformContext();
@@ -1387,7 +1384,6 @@ static FloatRect computeLineBoundsAndAntialiasingModeForText(GraphicsContext& in
         CGPoint devicePoint = CGPointApplyAffineTransform(point, t);
         CGPoint deviceOrigin = CGPointMake(roundf(devicePoint.x), ceilf(devicePoint.y) + dy);
         origin = CGPointApplyAffineTransform(deviceOrigin, CGAffineTransformInvert(t));
-        thickness /= scale;
     }
     return FloatRect(origin.x, origin.y, width, thickness);
 }
@@ -1398,43 +1394,6 @@ FloatRect GraphicsContext::computeLineBoundsForText(const FloatPoint& point, flo
     Color dummyColor;
     return computeLineBoundsAndAntialiasingModeForText(*this, point, width, printing, dummyBool, dummyColor);
 }
-#else
-static FloatRect computeLineBoundsAndAntialiasingModeForText(GraphicsContext& context, const FloatPoint& point, float width, bool printing, bool& shouldAntialias)
-{
-    shouldAntialias = true;
-
-    if (width <= 0)
-        return FloatRect();
-
-    // Use a minimum thickness of 0.5 in user space.
-    // See http://bugs.webkit.org/show_bug.cgi?id=4255 for details of why 0.5 is the right minimum thickness to use.
-    FloatRect initialBounds(point, FloatSize(width, std::max(context.strokeThickness(), 0.5f)));
-
-    if (printing || context.paintingDisabled() || !context.getCTM(GraphicsContext::DefinitelyIncludeDeviceScale).preservesAxisAlignment())
-        return initialBounds;
-
-    // On screen, use a minimum thickness of 1.0 in user space (later rounded to an integral number in device space).
-    FloatRect adjustedBounds = initialBounds;
-    adjustedBounds.setHeight(std::max(initialBounds.height(), 1.0f));
-
-    // FIXME: This should be done a better way.
-    // We try to round all parameters to integer boundaries in device space. If rounding pixels in device space
-    // makes our thickness more than double, then there must be a shrinking-scale factor and rounding to pixels
-    // in device space will make the underlines too thick.
-    FloatRect lineRect = context.roundToDevicePixels(adjustedBounds, GraphicsContext::RoundAllSides);
-    if (lineRect.height() < initialBounds.height() * 2) {
-        shouldAntialias = false;
-        return lineRect;
-    }
-    return initialBounds;
-}
-
-FloatRect GraphicsContext::computeLineBoundsForText(const FloatPoint& point, float width, bool printing)
-{
-    bool dummy;
-    return computeLineBoundsAndAntialiasingModeForText(*this, point, width, printing, dummy);
-}
-#endif
 
 void GraphicsContext::drawLineForText(const FloatPoint& point, float width, bool printing)
 {
@@ -1444,41 +1403,25 @@ void GraphicsContext::drawLineForText(const FloatPoint& point, float width, bool
     if (width <= 0)
         return;
 
-#if !PLATFORM(IOS)
-    bool shouldAntialiasLine;
-    FloatRect bounds = computeLineBoundsAndAntialiasingModeForText(*this, point, width, printing, shouldAntialiasLine);
+    Color localStrokeColor(strokeColor());
 
-    bool savedShouldAntialias = shouldAntialias();
-    bool restoreAntialiasMode = savedShouldAntialias != shouldAntialiasLine;
+    bool shouldAntialiasLine;
+    FloatRect bounds = computeLineBoundsAndAntialiasingModeForText(*this, point, width, printing, shouldAntialiasLine, localStrokeColor);
+    bool fillColorIsNotEqualToStrokeColor = fillColor() != localStrokeColor;
 
-    if (restoreAntialiasMode)
-        CGContextSetShouldAntialias(platformContext(), shouldAntialiasLine);
+#if PLATFORM(IOS)
+    if (m_state.shouldUseContextColors)
+#endif
+        if (fillColorIsNotEqualToStrokeColor)
+            setCGFillColor(platformContext(), localStrokeColor, strokeColorSpace());
 
-    bool fillColorIsNotEqualToStrokeColor = fillColor() != strokeColor();
-    if (fillColorIsNotEqualToStrokeColor)
-        setCGFillColor(platformContext(), strokeColor(), strokeColorSpace());
     CGContextFillRect(platformContext(), bounds);
-    if (fillColorIsNotEqualToStrokeColor)
-        setCGFillColor(platformContext(), fillColor(), fillColorSpace());
-    CGContextSetShouldAntialias(platformContext(), savedShouldAntialias);
-
-    if (restoreAntialiasMode)
-        CGContextSetShouldAntialias(platformContext(), true);
-#else
-    CGContextRef context = platformContext();
-    CGContextSaveGState(context);
-
-    Color color(strokeColor());
-    
-    bool shouldAntialiasLine;
-    FloatRect rect = computeLineBoundsAndAntialiasingModeForText(*this, point, width, printing, shouldAntialiasLine, color);
 
+#if PLATFORM(IOS)
     if (m_state.shouldUseContextColors)
-        setCGFillColor(context, color, strokeColorSpace());
-    CGContextFillRect(context, rect);
-
-    CGContextRestoreGState(context);
 #endif
+        if (fillColorIsNotEqualToStrokeColor)
+            setCGFillColor(platformContext(), fillColor(), fillColorSpace());
 }
 
 void GraphicsContext::drawLinesForText(const FloatPoint& point, const DashArray& widths, bool printing)
@@ -1489,9 +1432,11 @@ void GraphicsContext::drawLinesForText(const FloatPoint& point, const DashArray&
     if (widths.size() <= 0)
         return;
 
-#if !PLATFORM(IOS)
+    Color localStrokeColor(strokeColor());
+
     bool shouldAntialiasLine;
-    FloatRect bounds = computeLineBoundsAndAntialiasingModeForText(*this, point, widths.last(), printing, shouldAntialiasLine);
+    FloatRect bounds = computeLineBoundsAndAntialiasingModeForText(*this, point, widths.last(), printing, shouldAntialiasLine, localStrokeColor);
+    bool fillColorIsNotEqualToStrokeColor = fillColor() != localStrokeColor;
     
     Vector<CGRect, 4> dashBounds;
     ASSERT(!(widths.size() % 2));
@@ -1499,43 +1444,19 @@ void GraphicsContext::drawLinesForText(const FloatPoint& point, const DashArray&
     for (size_t i = 0; i < widths.size(); i += 2)
         dashBounds.append(CGRectMake(bounds.x() + widths[i], bounds.y(), widths[i+1] - widths[i], bounds.height()));
 
-    bool savedShouldAntialias = shouldAntialias();
-    bool restoreAntialiasMode = savedShouldAntialias != shouldAntialiasLine;
-
-    if (restoreAntialiasMode)
-        CGContextSetShouldAntialias(platformContext(), shouldAntialiasLine);
+#if PLATFORM(IOS)
+    if (m_state.shouldUseContextColors)
+#endif
+        if (fillColorIsNotEqualToStrokeColor)
+            setCGFillColor(platformContext(), localStrokeColor, strokeColorSpace());
 
-    bool fillColorIsNotEqualToStrokeColor = fillColor() != strokeColor();
-    if (fillColorIsNotEqualToStrokeColor)
-        setCGFillColor(platformContext(), strokeColor(), strokeColorSpace());
     CGContextFillRects(platformContext(), dashBounds.data(), dashBounds.size());
-    if (fillColorIsNotEqualToStrokeColor)
-        setCGFillColor(platformContext(), fillColor(), fillColorSpace());
-    CGContextSetShouldAntialias(platformContext(), savedShouldAntialias);
-
-    if (restoreAntialiasMode)
-        CGContextSetShouldAntialias(platformContext(), true);
-#else
-    CGContextRef context = platformContext();
-    CGContextSaveGState(context);
-
-    Color color(strokeColor());
-    
-    bool shouldAntialiasLine;
-    FloatRect rect = computeLineBoundsAndAntialiasingModeForText(*this, point, widths.last(), printing, shouldAntialiasLine, color);
-    
-    Vector<CGRect, 4> dashBounds;
-    ASSERT(!(widths.size() % 2));
-    dashBounds.reserveInitialCapacity(dashBounds.size() / 2);
-    for (size_t i = 0; i < widths.size(); i += 2)
-        dashBounds.append(CGRectMake(rect.x() + widths[i], rect.y(), widths[i+1] - widths[i], rect.height()));
 
+#if PLATFORM(IOS)
     if (m_state.shouldUseContextColors)
-        setCGFillColor(context, color, strokeColorSpace());
-    CGContextFillRects(context, dashBounds.data(), dashBounds.size());
-
-    CGContextRestoreGState(context);
 #endif
+        if (fillColorIsNotEqualToStrokeColor)
+            setCGFillColor(platformContext(), fillColor(), fillColorSpace());
 }
 
 void GraphicsContext::setURLForRect(const URL& link, const IntRect& destRect)
index d6f8a3b3912e2bafba7904063627c90abc9dcf57..fb28d75f2850b6a9e7c5c2fdf5f8880fe16da300 100644 (file)
@@ -1059,9 +1059,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();
-#if !PLATFORM(IOS)
-    context.setStrokeThickness(textDecorationThickness);
-#else
+
     // On iOS we want to draw crisp decorations. The function drawLineForText takes the context's
     // strokeThickness and renders that at device pixel scale (i.e. a strokeThickness of 1 will
     // produce a 1 device pixel line, so thinner on retina than non-retina). We will also scale
@@ -1075,7 +1073,6 @@ void InlineTextBox::paintDecoration(GraphicsContext& context, const FloatPoint&
     float fontSizeScaling = renderer().style().fontSize() / textDecorationBaseFontSize;
     float strokeThickness = roundf(textDecorationThickness * fontSizeScaling * pageScale);
     context.setStrokeThickness(strokeThickness);
-#endif
 
     bool linesAreOpaque = !isPrinting && (!(decoration & TextDecorationUnderline) || underline.alpha() == 255) && (!(decoration & TextDecorationOverline) || overline.alpha() == 255) && (!(decoration & TextDecorationLineThrough) || linethrough.alpha() == 255);