Underline bounds cannot be queried before underline itself is drawn
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 Oct 2013 01:51:17 +0000 (01:51 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 Oct 2013 01:51:17 +0000 (01:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=123310

Patch by Myles C. Maxfield <mmaxfield@apple.com> on 2013-10-29
Reviewed by Simon Fraser

GraphicsContext's drawLineForText function is used to draw underlines,
strikethroughs, and overlines. Before drawing the line, this function
modifies the bounds given to it in order to make underlines crisp. However,
this means that it is impossible to know where an underline will be drawn
before drawing it. This patch pulls out this adjustment computation into
InlineTextBox, then passes the result to drawLineForText.

Because there should be no observable difference, no tests need to be updated.

* platform/graphics/GraphicsContext.h: Changing the signature of drawLineForText
so it can accept bounds from our helper function
* platform/graphics/blackberry/PathBlackBerry.cpp:
(WebCore::GraphicsContext::drawLineForText): Update to work with new
signature of drawLineForText
* platform/graphics/cairo/GraphicsContextCairo.cpp:
(WebCore::GraphicsContext::drawLineForText): Ditto
* platform/graphics/cg/GraphicsContextCG.cpp:
(WebCore::GraphicsContext::drawLineForText): Ditto
* platform/graphics/wince/GraphicsContextWinCE.cpp:
(WebCore::GraphicsContext::drawLineForText): Ditto
* platform/win/WebCoreTextRenderer.cpp:
(WebCore::doDrawTextAtPoint): Update the last call site of drawLineForText
* rendering/InlineTextBox.cpp:
(WebCore::computeBoundsForUnderline): Pure function that computes the adjusted
bounds of the underline about to be drawn
(WebCore::drawLineForText): calls computeBoundsForUnderline and then
GraphicsContext::drawLineForText
(WebCore::InlineTextBox::paintDecoration): Use new drawLineForText function
(WebCore::InlineTextBox::paintCompositionUnderline): Ditto

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

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/GraphicsContext.h
Source/WebCore/platform/graphics/blackberry/PathBlackBerry.cpp
Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp
Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp
Source/WebCore/platform/graphics/wince/GraphicsContextWinCE.cpp
Source/WebCore/platform/win/WebCoreTextRenderer.cpp
Source/WebCore/rendering/InlineTextBox.cpp

index 274b3c2..3d0d7cf 100644 (file)
@@ -1,3 +1,40 @@
+2013-10-29  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        Underline bounds cannot be queried before underline itself is drawn
+        https://bugs.webkit.org/show_bug.cgi?id=123310
+
+        Reviewed by Simon Fraser
+
+        GraphicsContext's drawLineForText function is used to draw underlines,
+        strikethroughs, and overlines. Before drawing the line, this function
+        modifies the bounds given to it in order to make underlines crisp. However,  
+        this means that it is impossible to know where an underline will be drawn
+        before drawing it. This patch pulls out this adjustment computation into 
+        InlineTextBox, then passes the result to drawLineForText.
+
+        Because there should be no observable difference, no tests need to be updated.
+
+        * platform/graphics/GraphicsContext.h: Changing the signature of drawLineForText
+        so it can accept bounds from our helper function
+        * platform/graphics/blackberry/PathBlackBerry.cpp:
+        (WebCore::GraphicsContext::drawLineForText): Update to work with new
+        signature of drawLineForText
+        * platform/graphics/cairo/GraphicsContextCairo.cpp:
+        (WebCore::GraphicsContext::drawLineForText): Ditto
+        * platform/graphics/cg/GraphicsContextCG.cpp:
+        (WebCore::GraphicsContext::drawLineForText): Ditto
+        * platform/graphics/wince/GraphicsContextWinCE.cpp:
+        (WebCore::GraphicsContext::drawLineForText): Ditto
+        * platform/win/WebCoreTextRenderer.cpp:
+        (WebCore::doDrawTextAtPoint): Update the last call site of drawLineForText
+        * rendering/InlineTextBox.cpp:
+        (WebCore::computeBoundsForUnderline): Pure function that computes the adjusted
+        bounds of the underline about to be drawn
+        (WebCore::drawLineForText): calls computeBoundsForUnderline and then
+        GraphicsContext::drawLineForText
+        (WebCore::InlineTextBox::paintDecoration): Use new drawLineForText function
+        (WebCore::InlineTextBox::paintCompositionUnderline): Ditto
+
 2013-10-29  Alexey Proskuryakov  <ap@apple.com>
 
         Beef up CryptoKey
index c308cbd..039eeff 100644 (file)
@@ -330,7 +330,7 @@ namespace WebCore {
         };
         FloatRect roundToDevicePixels(const FloatRect&, RoundingMode = RoundAllSides);
 
-        void drawLineForText(const FloatPoint&, float width, bool printing);
+        void drawLineForText(const FloatRect& bounds, bool printing);
         enum DocumentMarkerLineStyle {
             DocumentMarkerSpellingLineStyle,
             DocumentMarkerGrammarLineStyle,
index ac6d6ba..faec68b 100644 (file)
@@ -263,12 +263,12 @@ void GraphicsContext::drawLineForDocumentMarker(const FloatPoint& pt, float widt
     platformContext()->addDrawLineForDocumentMarker(pt, width, (BlackBerry::Platform::Graphics::DocumentMarkerLineStyle)style);
 }
 
-void GraphicsContext::drawLineForText(const FloatPoint& pt, float width, bool printing)
+void GraphicsContext::drawLineForText(const FloatRect& bounds, bool printing)
 {
     if (paintingDisabled())
         return;
 
-    platformContext()->addDrawLineForText(pt, width, printing);
+    platformContext()->addDrawLineForText(bounds.location, bounds.width(), printing);
 }
 
 // FIXME: don't ignore the winding rule. https://bugs.webkit.org/show_bug.cgi?id=107064
index 70360ed..a71b145 100644 (file)
@@ -614,7 +614,7 @@ void GraphicsContext::drawFocusRing(const Vector<IntRect>& rects, int width, int
     cairo_restore(cr);
 }
 
-void GraphicsContext::drawLineForText(const FloatPoint& origin, float width, bool)
+void GraphicsContext::drawLineForText(const FloatRect& bounds, bool)
 {
     if (paintingDisabled())
         return;
@@ -623,16 +623,16 @@ void GraphicsContext::drawLineForText(const FloatPoint& origin, float width, boo
     cairo_save(cairoContext);
 
     // This bumping of <1 stroke thicknesses matches the one in drawLineOnCairoContext.
-    FloatPoint endPoint(origin + IntSize(width, 0));
-    FloatRect lineExtents(origin, FloatSize(width, strokeThickness()));
+    FloatPoint endPoint(bounds.location() + IntSize(bounds.width(), 0));
+    FloatRect lineExtents(bounds.location(), FloatSize(bounds.width(), strokeThickness()));
 
     ShadowBlur& shadow = platformContext()->shadowBlur();
     if (GraphicsContext* shadowContext = shadow.beginShadowLayer(this, lineExtents)) {
-        drawLineOnCairoContext(this, shadowContext->platformContext()->cr(), origin, endPoint);
+        drawLineOnCairoContext(this, shadowContext->platformContext()->cr(), bounds.location(), endPoint);
         shadow.endShadowLayer(this);
     }
 
-    drawLineOnCairoContext(this, cairoContext, origin, endPoint);
+    drawLineOnCairoContext(this, cairoContext, bounds.location(), endPoint);
     cairo_restore(cairoContext);
 }
 
index 1a9c905..e75293d 100644 (file)
@@ -1235,53 +1235,16 @@ FloatRect GraphicsContext::roundToDevicePixels(const FloatRect& rect, RoundingMo
     return FloatRect(roundedOrigin, roundedLowerRight - roundedOrigin);
 }
 
-void GraphicsContext::drawLineForText(const FloatPoint& point, float width, bool printing)
+void GraphicsContext::drawLineForText(const FloatRect& bounds, bool)
 {
     if (paintingDisabled())
         return;
 
-    if (width <= 0)
-        return;
-
-    float x = point.x();
-    float y = point.y();
-    float lineLength = width;
-
-    // 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.
-    float thickness = max(strokeThickness(), 0.5f);
-
-    bool restoreAntialiasMode = false;
-
-    if (!printing && getCTM(GraphicsContext::DefinitelyIncludeDeviceScale).preservesAxisAlignment()) {
-        // On screen, use a minimum thickness of 1.0 in user space (later rounded to an integral number in device space).
-        float adjustedThickness = max(thickness, 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.
-        CGRect lineRect = roundToDevicePixels(FloatRect(x, y, lineLength, adjustedThickness), RoundAllSides);
-        if (lineRect.size.height < thickness * 2.0) {
-            x = lineRect.origin.x;
-            y = lineRect.origin.y;
-            lineLength = lineRect.size.width;
-            thickness = lineRect.size.height;
-            if (shouldAntialias()) {
-                CGContextSetShouldAntialias(platformContext(), false);
-                restoreAntialiasMode = true;
-            }
-        }
-    }
-
     if (fillColor() != strokeColor())
         setCGFillColor(platformContext(), strokeColor(), strokeColorSpace());
-    CGContextFillRect(platformContext(), CGRectMake(x, y, lineLength, thickness));
+    CGContextFillRect(platformContext(), CGRectMake(bounds.location().x(), bounds.location().y(), bounds.width(), bounds.height()));
     if (fillColor() != strokeColor())
         setCGFillColor(platformContext(), fillColor(), fillColorSpace());
-
-    if (restoreAntialiasMode)
-        CGContextSetShouldAntialias(platformContext(), true);
 }
 
 void GraphicsContext::setURLForRect(const URL& link, const IntRect& destRect)
index 1ff3fae..157fae4 100644 (file)
@@ -950,14 +950,14 @@ void GraphicsContext::drawFocusRing(const Vector<IntRect>& rects, int width, int
     DrawFocusRect(dc, &rect);
 }
 
-void GraphicsContext::drawLineForText(const FloatPoint& origin, float width, bool printing)
+void GraphicsContext::drawLineForText(const FloatRect& bounds, bool)
 {
     if (paintingDisabled())
         return;
 
     StrokeStyle oldStyle = strokeStyle();
     setStrokeStyle(SolidStroke);
-    drawLine(roundedIntPoint(origin), roundedIntPoint(origin + FloatSize(width, 0)));
+    drawLine(roundedIntPoint(bounds.location()), roundedIntPoint(bounds.location() + FloatSize(bounds.width(), 0)));
     setStrokeStyle(oldStyle);
 }
 
index f540ffb..3d007cf 100644 (file)
@@ -76,7 +76,8 @@ static void doDrawTextAtPoint(GraphicsContext& context, const String& text, cons
         underlinePoint.move(beforeWidth, 1);
 
         context.setStrokeColor(color, ColorSpaceDeviceRGB);
-        context.drawLineForText(underlinePoint, underlinedWidth, false);
+        FloatRect bounds(underlinePoint, FloatSize(underlinedWidth, context.strokeThickness()));
+        context.drawLineForText(bounds, false);
     }
 }
 
index 95a7cae..9f7ad9b 100644 (file)
@@ -67,6 +67,49 @@ COMPILE_ASSERT(sizeof(InlineTextBox) == sizeof(SameSizeAsInlineTextBox), InlineT
 typedef WTF::HashMap<const InlineTextBox*, LayoutRect> InlineTextBoxOverflowMap;
 static InlineTextBoxOverflowMap* gTextBoxesWithOverflow;
 
+static FloatRect computeBoundsForUnderline(GraphicsContext& context, const FloatPoint& topleft, float length, bool printing, bool& shouldAntialias)
+{
+    float thickness = std::max(context.strokeThickness(), 0.5f);
+    
+    FloatRect bounds(topleft, FloatSize(length, thickness));
+
+    shouldAntialias = true;
+    
+    if (printing || context.paintingDisabled() || !context.getCTM(GraphicsContext::DefinitelyIncludeDeviceScale).preservesAxisAlignment())
+        return bounds;
+
+    // On screen, use a minimum thickness of 1.0 in user space (later rounded to an integral number in device space).
+    FloatRect adjustedBounds = bounds;
+    adjustedBounds.setHeight(std::max(thickness, 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() < thickness * 2.0) {
+        shouldAntialias = false;
+        return lineRect;
+    }
+
+    return bounds;
+}
+
+static void drawLineForText(GraphicsContext& context, const FloatPoint& topleft, float width, bool printing)
+{
+    if (width <= 0)
+        return;
+    
+    bool shouldAntialias;
+    
+    FloatRect underlineBounds = computeBoundsForUnderline(context, topleft, width, printing, shouldAntialias);
+    
+    bool wasAntialiasing = context.shouldAntialias();
+    context.setShouldAntialias(shouldAntialias);
+    context.drawLineForText(underlineBounds, printing);
+    context.setShouldAntialias(wasAntialiasing);
+}
+
 void InlineTextBox::destroy(RenderArena& arena)
 {
     if (!knownToHaveNoOverflow() && gTextBoxesWithOverflow)
@@ -1004,14 +1047,14 @@ void InlineTextBox::paintDecoration(GraphicsContext* context, const FloatPoint&
                 break;
             }
             default:
-                context->drawLineForText(FloatPoint(localOrigin.x(), localOrigin.y() + underlineOffset), width, isPrinting);
+                drawLineForText(*context, FloatPoint(localOrigin.x(), localOrigin.y() + underlineOffset), width, isPrinting);
 
                 if (decorationStyle == TextDecorationStyleDouble)
-                    context->drawLineForText(FloatPoint(localOrigin.x(), localOrigin.y() + underlineOffset + doubleOffset), width, isPrinting);
+                    drawLineForText(*context, FloatPoint(localOrigin.x(), localOrigin.y() + underlineOffset + doubleOffset), width, isPrinting);
             }
 #else
             // Leave one pixel of white between the baseline and the underline.
-            context->drawLineForText(FloatPoint(localOrigin.x(), localOrigin.y() + baseline + 1), width, isPrinting);
+            drawLineForText(*context, FloatPoint(localOrigin.x(), localOrigin.y() + baseline + 1), width, isPrinting);
 #endif // CSS3_TEXT
         }
         if (deco & TextDecorationOverline) {
@@ -1026,10 +1069,10 @@ void InlineTextBox::paintDecoration(GraphicsContext* context, const FloatPoint&
             }
             default:
 #endif // CSS3_TEXT
-                context->drawLineForText(localOrigin, width, isPrinting);
+                drawLineForText(*context, localOrigin, width, isPrinting);
 #if ENABLE(CSS3_TEXT)
                 if (decorationStyle == TextDecorationStyleDouble)
-                    context->drawLineForText(FloatPoint(localOrigin.x(), localOrigin.y() - doubleOffset), width, isPrinting);
+                    drawLineForText(*context, FloatPoint(localOrigin.x(), localOrigin.y() - doubleOffset), width, isPrinting);
             }
 #endif // CSS3_TEXT
         }
@@ -1045,10 +1088,10 @@ void InlineTextBox::paintDecoration(GraphicsContext* context, const FloatPoint&
             }
             default:
 #endif // CSS3_TEXT
-                context->drawLineForText(FloatPoint(localOrigin.x(), localOrigin.y() + 2 * baseline / 3), width, isPrinting);
+                drawLineForText(*context, FloatPoint(localOrigin.x(), localOrigin.y() + 2 * baseline / 3), width, isPrinting);
 #if ENABLE(CSS3_TEXT)
                 if (decorationStyle == TextDecorationStyleDouble)
-                    context->drawLineForText(FloatPoint(localOrigin.x(), localOrigin.y() + doubleOffset + 2 * baseline / 3), width, isPrinting);
+                    drawLineForText(*context, FloatPoint(localOrigin.x(), localOrigin.y() + doubleOffset + 2 * baseline / 3), width, isPrinting);
             }
 #endif // CSS3_TEXT
         }
@@ -1296,7 +1339,7 @@ void InlineTextBox::paintCompositionUnderline(GraphicsContext* ctx, const FloatP
 
     ctx->setStrokeColor(underline.color, renderer().style().colorSpace());
     ctx->setStrokeThickness(lineThickness);
-    ctx->drawLineForText(FloatPoint(boxOrigin.x() + start, boxOrigin.y() + logicalHeight() - lineThickness), width, renderer().document().printing());
+    drawLineForText(*ctx, FloatPoint(boxOrigin.x() + start, boxOrigin.y() + logicalHeight() - lineThickness), width, renderer().document().printing());
 }
 
 int InlineTextBox::caretMinOffset() const