Changeset 286955 in webkit

Timestamp:

Dec 13, 2021 10:23:00 AM (7 months ago)

Author:

ntim@apple.com

Message:

Fix paint order of CSS text decorations
​https://bugs.webkit.org/show_bug.cgi?id=227445

Reviewed by Simon Fraser.

Source/WebCore:

• rendering/TextBoxPainter.cpp:

(WebCore::TextBoxPainter::paintForegroundAndDecorations):
(WebCore::TextBoxPainter::createDecorationPainter):
(WebCore::TextBoxPainter::paintBackgroundDecorations):
(WebCore::TextBoxPainter::paintForegroundDecorations):
(WebCore::TextBoxPainter::paintDecoration): Deleted.

• rendering/TextBoxPainter.h:
• rendering/TextDecorationPainter.cpp:

(WebCore::TextDecorationPainter::paintBackgroundDecorations):
(WebCore::TextDecorationPainter::paintForegroundDecorations):
(WebCore::TextDecorationPainter::paintLineThrough):
(WebCore::TextDecorationPainter::paintTextDecoration): Deleted.

• rendering/TextDecorationPainter.h:

LayoutTests:

• TestExpectations:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (2 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/TextBoxPainter.cpp (modified) (5 diffs)
• Source/WebCore/rendering/TextBoxPainter.h (modified) (2 diffs)
• Source/WebCore/rendering/TextDecorationPainter.cpp (modified) (4 diffs)
• Source/WebCore/rendering/TextDecorationPainter.h (modified) (1 diff)
• Source/WebCore/style/InlineTextBoxStyle.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-12-13  Tim Nguyen  <ntim@apple.com>
+
+        Fix paint order of CSS text decorations
+        https://bugs.webkit.org/show_bug.cgi?id=227445
+
+        Reviewed by Simon Fraser.
+
+        * TestExpectations:
+
 2021-12-13  Sergio Villar Senin  <svillar@igalia.com>
 

trunk/LayoutTests/TestExpectations

@@ -3432 +3432 @@
 webkit.org/b/230041 imported/w3c/web-platform-tests/css/css-text-decor/text-decoration-color-selection-pseudo-01.html [ ImageOnlyFailure ]
 webkit.org/b/203530 imported/w3c/web-platform-tests/css/css-text-decor/text-decoration-line-recalc.html [ ImageOnlyFailure ]
-webkit.org/b/203531 imported/w3c/web-platform-tests/css/css-text-decor/text-decoration-propagation-shadow.html [ ImageOnlyFailure ]
 webkit.org/b/203528 imported/w3c/web-platform-tests/css/css-text-decor/text-decoration-thickness-linethrough-001.html [ ImageOnlyFailure ]
 webkit.org/b/203528 imported/w3c/web-platform-tests/css/css-text-decor/text-decoration-thickness-overline-001.html [ ImageOnlyFailure ]
@@ -3472 +3471 @@
 imported/w3c/web-platform-tests/css/css-text-decor/text-emphasis-style-string-001.xht [ ImageOnlyFailure ]
 webkit.org/b/230083 imported/w3c/web-platform-tests/css/css-text-decor/text-underline-offset-001.html [ ImageOnlyFailure ]
-imported/w3c/web-platform-tests/css/css-text-decor/text-underline-offset-zero-position.html [ ImageOnlyFailure ]
 
 # wpt css-position failures

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-12-13  Tim Nguyen  <ntim@apple.com>
+
+        Fix paint order of CSS text decorations
+        https://bugs.webkit.org/show_bug.cgi?id=227445
+
+        Reviewed by Simon Fraser.
+
+        * rendering/TextBoxPainter.cpp:
+        (WebCore::TextBoxPainter::paintForegroundAndDecorations):
+        (WebCore::TextBoxPainter::createDecorationPainter):
+        (WebCore::TextBoxPainter::paintBackgroundDecorations):
+        (WebCore::TextBoxPainter::paintForegroundDecorations):
+        (WebCore::TextBoxPainter::paintDecoration): Deleted.
+        * rendering/TextBoxPainter.h:
+        * rendering/TextDecorationPainter.cpp:
+        (WebCore::TextDecorationPainter::paintBackgroundDecorations):
+        (WebCore::TextDecorationPainter::paintForegroundDecorations):
+        (WebCore::TextDecorationPainter::paintLineThrough):
+        (WebCore::TextDecorationPainter::paintTextDecoration): Deleted.
+        * rendering/TextDecorationPainter.h:
+
 2021-12-13  Jer Noble  <jer.noble@apple.com>
 

trunk/Source/WebCore/rendering/TextBoxPainter.cpp

@@ -189 +189 @@
     auto coalescedStyledMarkedTexts = StyledMarkedText::coalesceAdjacentWithEqualForeground(styledMarkedTexts);
 
-    for (auto& markedText : coalescedStyledMarkedTexts)
-        paintForeground(markedText);
-
     auto textDecorations = m_style.textDecorationsInEffect();
     bool highlightDecorations = !MarkedText::collectForHighlights(m_renderer, m_selectableRange, MarkedText::PaintPhase::Decoration).isEmpty();
@@ -225 +222 @@
         auto coalescedStyledMarkedTexts = StyledMarkedText::coalesceAdjacentWithEqualDecorations(styledMarkedTexts);
 
+        for (auto& markedText : coalescedStyledMarkedTexts) {
+            unsigned startOffset = markedText.startOffset;
+            unsigned endOffset = markedText.endOffset;
+            if (startOffset < endOffset) {
+                // Avoid measuring the text when the entire line box is selected as an optimization.
+                FloatRect snappedSelectionRect = m_paintRect;
+                if (startOffset || endOffset != m_paintTextRun.length()) {
+                    LayoutRect selectionRect = { m_paintRect.x(), m_paintRect.y(), m_paintRect.width(), m_paintRect.height() };
+                    fontCascade().adjustSelectionRectForText(m_paintTextRun, selectionRect, startOffset, endOffset);
+                    snappedSelectionRect = snapRectToDevicePixelsWithWritingDirection(selectionRect, m_document.deviceScaleFactor(), m_paintTextRun.ltr());
+                }
+
+                TextDecorationPainter decorationPainter = createDecorationPainter(markedText, textDecorationSelectionClipOutRect, snappedSelectionRect);
+                paintBackgroundDecorations(decorationPainter, markedText, snappedSelectionRect);
+                paintForeground(markedText);
+                paintForegroundDecorations(decorationPainter, snappedSelectionRect);
+            }
+        }
+    } else {
         for (auto& markedText : coalescedStyledMarkedTexts)
-            paintDecoration(markedText, textDecorationSelectionClipOutRect);
+            paintForeground(markedText);
     }
 }
@@ -340 +356 @@
 }
 
-void TextBoxPainter::paintDecoration(const StyledMarkedText& markedText, const FloatRect& clipOutRect)
-{
-    // 1. Compute text selection
-    unsigned startOffset = markedText.startOffset;
-    unsigned endOffset = markedText.endOffset;
-    if (startOffset >= endOffset)
-        return;
-
+TextDecorationPainter TextBoxPainter::createDecorationPainter(const StyledMarkedText& markedText, const FloatRect& clipOutRect, const FloatRect& snappedSelectionRect)
+{
     GraphicsContext& context = m_paintInfo.context();
-    const FontCascade& font = fontCascade();
 
     updateGraphicsContext(context, markedText.style.textStyles);
@@ -359 +368 @@
     // Note that if the text is truncated, we let the thing being painted in the truncation
     // draw its own decoration.
-
-    // Avoid measuring the text when the entire line box is selected as an optimization.
-    FloatRect snappedSelectionRect = m_paintRect;
-    if (startOffset || endOffset != m_paintTextRun.length()) {
-        LayoutRect selectionRect = { m_paintRect.x(), m_paintRect.y(), m_paintRect.width(), m_paintRect.height() };
-        font.adjustSelectionRectForText(m_paintTextRun, selectionRect, startOffset, endOffset);
-        snappedSelectionRect = snapRectToDevicePixelsWithWritingDirection(selectionRect, m_document.deviceScaleFactor(), m_paintTextRun.ltr());
-    }
-
-    // 2. Paint
+    GraphicsContextStateSaver stateSaver { context, false };
+    bool isDraggedContent = markedText.type == MarkedText::DraggedContent;
+    if (isDraggedContent || !clipOutRect.isEmpty()) {
+        stateSaver.save();
+        if (isDraggedContent)
+            context.setAlpha(markedText.style.alpha);
+        if (!clipOutRect.isEmpty())
+            context.clipOut(clipOutRect);
+    }
+
+    // Create painter
+    const FontCascade& font = fontCascade();
     auto textDecorations = m_style.textDecorationsInEffect();
     textDecorations.add(TextDecorationPainter::textDecorationsInEffectForStyle(markedText.style.textDecorationStyles));
@@ -381 +392 @@
     }
 
-    {
-        GraphicsContextStateSaver stateSaver { context, false };
-        bool isDraggedContent = markedText.type == MarkedText::DraggedContent;
-        if (isDraggedContent || !clipOutRect.isEmpty()) {
-            stateSaver.save();
-            if (isDraggedContent)
-                context.setAlpha(markedText.style.alpha);
-            if (!clipOutRect.isEmpty())
-                context.clipOut(clipOutRect);
-        }
-        decorationPainter.paintTextDecoration(m_paintTextRun.subRun(startOffset, endOffset - startOffset), textOriginFromPaintRect(snappedSelectionRect), snappedSelectionRect.location());
-    }
-
-    if (isCombinedText)
+    return decorationPainter;
+}
+
+void TextBoxPainter::paintBackgroundDecorations(TextDecorationPainter& decorationPainter, const StyledMarkedText& markedText, const FloatRect& snappedSelectionRect)
+{
+    decorationPainter.paintBackgroundDecorations(m_paintTextRun.subRun(markedText.startOffset, markedText.endOffset - markedText.startOffset), textOriginFromPaintRect(snappedSelectionRect), snappedSelectionRect.location());
+
+    if (textBox().isCombinedText()) {
+        GraphicsContext& context = m_paintInfo.context();
         context.concatCTM(rotation(m_paintRect, Counterclockwise));
+    }
+}
+
+void TextBoxPainter::paintForegroundDecorations(TextDecorationPainter& decorationPainter, const FloatRect& snappedSelectionRect)
+{
+    decorationPainter.paintForegroundDecorations(snappedSelectionRect.location());
+
+    if (textBox().isCombinedText()) {
+        GraphicsContext& context = m_paintInfo.context();
+        context.concatCTM(rotation(m_paintRect, Counterclockwise));
+    }
 }
 

trunk/Source/WebCore/rendering/TextBoxPainter.h

@@ -40 +40 @@
 class RenderText;
 class ShadowData;
+class TextDecorationPainter;
 struct CompositionUnderline;
 struct MarkedText;
@@ -72 +73 @@
     void paintBackground(const StyledMarkedText&);
     void paintForeground(const StyledMarkedText&);
-    void paintDecoration(const StyledMarkedText&, const FloatRect& clipOutRect);
+    TextDecorationPainter createDecorationPainter(const StyledMarkedText&, const FloatRect&, const FloatRect&);
+    void paintBackgroundDecorations(TextDecorationPainter&, const StyledMarkedText&, const FloatRect&);
+    void paintForegroundDecorations(TextDecorationPainter&, const FloatRect&);
     void paintCompositionUnderline(const CompositionUnderline&);
     void paintPlatformDocumentMarker(const MarkedText&);

trunk/Source/WebCore/rendering/TextDecorationPainter.cpp

@@ -202 +202 @@
 }
 
-void TextDecorationPainter::paintTextDecoration(const TextRun& textRun, const FloatPoint& textOrigin, const FloatPoint& boxOrigin)
+// Paint text-shadow, underline, overline
+void TextDecorationPainter::paintBackgroundDecorations(const TextRun& textRun, const FloatPoint& textOrigin, const FloatPoint& boxOrigin)
 {
     const auto& fontMetrics = m_lineStyle.fontMetrics();
@@ -229 +230 @@
                 m_context.drawLineForText(rect, m_isPrinting, style == TextDecorationStyle::Double, strokeStyle);
             }
-        } else {
-            ASSERT(decoration == TextDecorationLine::LineThrough);
-            m_context.drawLineForText(rect, m_isPrinting, style == TextDecorationStyle::Double, strokeStyle);
-        }
+        } else
+            ASSERT_NOT_REACHED();
     };
 
@@ -294 +293 @@
             paintDecoration(TextDecorationLine::Overline, m_styles.overlineStyle, m_styles.overlineColor, rect);
         }
-        if (m_decorations.contains(TextDecorationLine::LineThrough)) {
-            FloatRect rect(localOrigin, FloatSize(m_width, textDecorationThickness));
-            float autoTextDecorationThickness = TextDecorationThickness::createWithAuto().resolve(m_lineStyle.computedFontSize(), fontMetrics);
-            auto center = 2 * fontMetrics.floatAscent() / 3 + autoTextDecorationThickness / 2;
-            rect.move(0, center - textDecorationThickness / 2);
-            paintDecoration(TextDecorationLine::LineThrough, m_styles.linethroughStyle, m_styles.linethroughColor, rect);
-        }
+
+        // We only want to paint the shadow, hence the transparent color, not the actual line-through,
+        // which will be painted in paintForegroundDecorations().
+        if (shadow && m_decorations.contains(TextDecorationLine::LineThrough))
+            paintLineThrough(Color::transparentBlack, textDecorationThickness, localOrigin);
     } while (shadow);
 
@@ -307 +304 @@
     else if (m_shadow)
         m_context.clearShadow();
+}
+
+void TextDecorationPainter::paintForegroundDecorations(const FloatPoint& boxOrigin)
+{
+    if (!m_decorations.contains(TextDecorationLine::LineThrough))
+        return;
+
+    float textDecorationThickness = m_lineStyle.textDecorationThickness().resolve(m_lineStyle.computedFontSize(), m_lineStyle.fontMetrics());
+    paintLineThrough(m_styles.linethroughColor, textDecorationThickness, boxOrigin);
+}
+
+void TextDecorationPainter::paintLineThrough(const Color& color, float thickness, const FloatPoint& localOrigin)
+{
+    const auto& fontMetrics = m_lineStyle.fontMetrics();
+    FloatRect rect(localOrigin, FloatSize(m_width, thickness));
+    float autoTextDecorationThickness = TextDecorationThickness::createWithAuto().resolve(m_lineStyle.computedFontSize(), fontMetrics);
+    auto center = 2 * fontMetrics.floatAscent() / 3 + autoTextDecorationThickness / 2;
+    rect.move(0, center - thickness / 2);
+
+    m_context.setStrokeColor(color);
+
+    TextDecorationStyle style = m_styles.linethroughStyle;
+    auto strokeStyle = textDecorationStyleToStrokeStyle(style);
+
+    if (style == TextDecorationStyle::Wavy)
+        strokeWavyTextDecoration(m_context, rect, m_lineStyle.computedFontPixelSize());
+    else
+        m_context.drawLineForText(rect, m_isPrinting, style == TextDecorationStyle::Double, strokeStyle);
 }
 

trunk/Source/WebCore/rendering/TextDecorationPainter.h

@@ -53 +53 @@
     void setShadowColorFilter(const FilterOperations* colorFilter) { m_shadowColorFilter = colorFilter; }
 
-    void paintTextDecoration(const TextRun&, const FloatPoint& textOrigin, const FloatPoint& boxOrigin);
+    void paintBackgroundDecorations(const TextRun&, const FloatPoint& textOrigin, const FloatPoint& boxOrigin);
+    void paintForegroundDecorations(const FloatPoint& boxOrigin);
+    void paintLineThrough(const Color&, float thickness, const FloatPoint& localOrigin);
 
     struct Styles {

trunk/Source/WebCore/style/InlineTextBoxStyle.cpp

@@ -190 +190 @@
 
     // These metrics must match where underlines get drawn.
-    // FIXME: Share the code in TextDecorationPainter::paintTextDecoration() so we can just query it for the painted geometry.
+    // FIXME: Share the code in TextDecorationPainter::paintBackgroundDecorations() so we can just query it for the painted geometry.
     if (decoration & TextDecorationLine::Underline) {
         // Compensate for the integral ceiling in GraphicsContext::computeLineBoundsAndAntialiasingModeForText()