Changeset 284380 in webkit

Timestamp:

Oct 18, 2021 10:17:56 AM (9 months ago)

Author:

Antti Koivisto

Message:

background-clip:text doesn't paint correctly for inline box spanning multiple lines
​https://bugs.webkit.org/show_bug.cgi?id=231891

Reviewed by Alan Bujtas.

Source/WebCore:

'background-clip: text' paints to a wrong position if the inline box spans multiple lines.

Test: fast/inline/inline-background-clip-text-multiline.html

• rendering/InlineBoxPainter.cpp:

(WebCore::InlineBoxPainter::paintFillLayer):

• rendering/RenderBox.cpp:

(WebCore::RenderBox::paintFillLayer):

• rendering/RenderBoxModelObject.cpp:

(WebCore::RenderBoxModelObject::getBackgroundRoundedRect const):
(WebCore::RenderBoxModelObject::backgroundRoundedRectAdjustedForBleedAvoidance const):

No need to pass box size separately as it now always matches the passed in border rect.

(WebCore::RenderBoxModelObject::paintFillLayerExtended):

The 'rect' parameter used to be either the border box rect of the box being painted or,
in the case of multiline inline box background image, the image strip being painted.

Make the 'rect' parameter to always be the border box and pass the background image strip
separately.

Use the background image strip only during background image painting.

• rendering/RenderBoxModelObject.h:

(WebCore::RenderBoxModelObject::paintFillLayerExtended):

LayoutTests:

• fast/inline/inline-background-clip-text-multiline-expected.html: Added.
• fast/inline/inline-background-clip-text-multiline.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/inline/inline-background-clip-text-multiline-expected.html (added)
• LayoutTests/fast/inline/inline-background-clip-text-multiline.html (added)
• LayoutTests/platform/win/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/InlineBoxPainter.cpp (modified) (3 diffs)
• Source/WebCore/rendering/RenderBox.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderBoxModelObject.cpp (modified) (7 diffs)
• Source/WebCore/rendering/RenderBoxModelObject.h (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-10-18  Antti Koivisto  <antti@apple.com>
+
+        background-clip:text doesn't paint correctly for inline box spanning multiple lines
+        https://bugs.webkit.org/show_bug.cgi?id=231891
+
+        Reviewed by Alan Bujtas.
+
+        * fast/inline/inline-background-clip-text-multiline-expected.html: Added.
+        * fast/inline/inline-background-clip-text-multiline.html: Added.
+
 2021-10-18  Ayumi Kojima  <ayumi_kojima@apple.com>
 

trunk/LayoutTests/platform/win/TestExpectations

@@ -2913 +2913 @@
 fast/inline/padding-ellipsis-right.html [ ImageOnlyFailure ]
 fast/inline/inline-background-clip-text.html [ ImageOnlyFailure ]
+fast/inline/inline-background-clip-text-multiline.html [ ImageOnlyFailure ]
 
 # This test hardcodes the result of a platform-dependent font lookup algorithm.

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-10-18  Antti Koivisto  <antti@apple.com>
+
+        background-clip:text doesn't paint correctly for inline box spanning multiple lines
+        https://bugs.webkit.org/show_bug.cgi?id=231891
+
+        Reviewed by Alan Bujtas.
+
+        'background-clip: text' paints to a wrong position if the inline box spans multiple lines.
+
+        Test: fast/inline/inline-background-clip-text-multiline.html
+
+        * rendering/InlineBoxPainter.cpp:
+        (WebCore::InlineBoxPainter::paintFillLayer):
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::paintFillLayer):
+        * rendering/RenderBoxModelObject.cpp:
+        (WebCore::RenderBoxModelObject::getBackgroundRoundedRect const):
+        (WebCore::RenderBoxModelObject::backgroundRoundedRectAdjustedForBleedAvoidance const):
+
+        No need to pass box size separately as it now always matches the passed in border rect.
+
+        (WebCore::RenderBoxModelObject::paintFillLayerExtended):
+
+        The 'rect' parameter used to be either the border box rect of the box being painted or,
+        in the case of multiline inline box background image, the image strip being painted.
+
+        Make the 'rect' parameter to always be the border box and pass the background image strip
+        separately.
+
+        Use the background image strip only during background image painting.
+
+        * rendering/RenderBoxModelObject.h:
+        (WebCore::RenderBoxModelObject::paintFillLayerExtended):
+
 2021-10-18  Kimmo Kinnunen  <kkinnunen@apple.com>
 

trunk/Source/WebCore/rendering/InlineBoxPainter.cpp

@@ -299 +299 @@
 
     if (!hasFillImageOrBorderRadious || hasSingleLine || m_isRootInlineBox) {
-        renderer().paintFillLayerExtended(m_paintInfo, color, fillLayer, rect, BackgroundBleedNone, m_inlineBox, rect.size(), op);
+        renderer().paintFillLayerExtended(m_paintInfo, color, fillLayer, rect, BackgroundBleedNone, m_inlineBox, { }, op);
         return;
     }
@@ -307 +307 @@
         GraphicsContextStateSaver stateSaver(m_paintInfo.context());
         m_paintInfo.context().clip({ rect.x(), rect.y(), LayoutUnit(m_inlineBox.rect().width()), LayoutUnit(m_inlineBox.rect().height()) });
-        renderer().paintFillLayerExtended(m_paintInfo, color, fillLayer, rect, BackgroundBleedNone, m_inlineBox, rect.size(), op);
+        renderer().paintFillLayerExtended(m_paintInfo, color, fillLayer, rect, BackgroundBleedNone, m_inlineBox, { }, op);
         return;
     }
@@ -333 +333 @@
             totalLogicalWidth += box->logicalWidth();
     }
-    LayoutUnit stripX = rect.x() - (isHorizontal() ? logicalOffsetOnLine : 0_lu);
-    LayoutUnit stripY = rect.y() - (isHorizontal() ? 0_lu : logicalOffsetOnLine);
-    LayoutUnit stripWidth = isHorizontal() ? totalLogicalWidth : LayoutUnit(m_inlineBox.rect().width());
-    LayoutUnit stripHeight = isHorizontal() ? LayoutUnit(m_inlineBox.rect().height()) : totalLogicalWidth;
+    LayoutRect backgroundImageStrip {
+        rect.x() - (isHorizontal() ? logicalOffsetOnLine : 0_lu),
+        rect.y() - (isHorizontal() ? 0_lu : logicalOffsetOnLine),
+        isHorizontal() ? totalLogicalWidth : LayoutUnit(m_inlineBox.rect().width()),
+        isHorizontal() ? LayoutUnit(m_inlineBox.rect().height()) : totalLogicalWidth
+    };
 
     GraphicsContextStateSaver stateSaver(m_paintInfo.context());
-    m_paintInfo.context().clip({ rect.x(), rect.y(), LayoutUnit(m_inlineBox.rect().width()), LayoutUnit(m_inlineBox.rect().height()) });
-    renderer().paintFillLayerExtended(m_paintInfo, color, fillLayer, LayoutRect(stripX, stripY, stripWidth, stripHeight), BackgroundBleedNone, m_inlineBox, rect.size(), op);
+    m_paintInfo.context().clip(FloatRect { rect });
+    renderer().paintFillLayerExtended(m_paintInfo, color, fillLayer, rect, BackgroundBleedNone, m_inlineBox, backgroundImageStrip, op);
 }
 

trunk/Source/WebCore/rendering/RenderBox.cpp

@@ -1884 +1884 @@
     BackgroundBleedAvoidance bleedAvoidance, CompositeOperator op, RenderElement* backgroundObject, BaseBackgroundColorUsage baseBgColorUsage)
 {
-    paintFillLayerExtended(paintInfo, c, fillLayer, rect, bleedAvoidance, { }, LayoutSize(), op, backgroundObject, baseBgColorUsage);
+    paintFillLayerExtended(paintInfo, c, fillLayer, rect, bleedAvoidance, { }, { }, op, backgroundObject, baseBgColorUsage);
 }
 

trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp

@@ -624 +624 @@
 }
 
-RoundedRect RenderBoxModelObject::getBackgroundRoundedRect(const LayoutRect& borderRect, const InlineIterator::InlineBoxIterator& box, LayoutUnit inlineBoxWidth, LayoutUnit inlineBoxHeight,
+RoundedRect RenderBoxModelObject::getBackgroundRoundedRect(const LayoutRect& borderRect, const InlineIterator::InlineBoxIterator& box,
     bool includeLogicalLeftEdge, bool includeLogicalRightEdge) const
 {
     RoundedRect border = style().getRoundedBorderFor(borderRect, includeLogicalLeftEdge, includeLogicalRightEdge);
     if (box && (box->nextInlineBox() || box->previousInlineBox())) {
-        RoundedRect segmentBorder = style().getRoundedBorderFor(LayoutRect(0_lu, 0_lu, inlineBoxWidth, inlineBoxHeight), includeLogicalLeftEdge, includeLogicalRightEdge);
+        RoundedRect segmentBorder = style().getRoundedBorderFor(LayoutRect(0_lu, 0_lu, borderRect.width(), borderRect.height()), includeLogicalLeftEdge, includeLogicalRightEdge);
         border.setRadii(segmentBorder.radii());
     }
@@ -685 +685 @@
 }
 
-RoundedRect RenderBoxModelObject::backgroundRoundedRectAdjustedForBleedAvoidance(const GraphicsContext& context, const LayoutRect& borderRect, BackgroundBleedAvoidance bleedAvoidance, const InlineIterator::InlineBoxIterator& box, const LayoutSize& boxSize, bool includeLogicalLeftEdge, bool includeLogicalRightEdge) const
+RoundedRect RenderBoxModelObject::backgroundRoundedRectAdjustedForBleedAvoidance(const GraphicsContext& context, const LayoutRect& borderRect, BackgroundBleedAvoidance bleedAvoidance, const InlineIterator::InlineBoxIterator& box, bool includeLogicalLeftEdge, bool includeLogicalRightEdge) const
 {
     if (bleedAvoidance == BackgroundBleedShrinkBackground) {
         // We shrink the rectangle by one device pixel on each side because the bleed is one pixel maximum.
-        return getBackgroundRoundedRect(shrinkRectByOneDevicePixel(context, borderRect, document().deviceScaleFactor()), box, boxSize.width(), boxSize.height(),
+        return getBackgroundRoundedRect(shrinkRectByOneDevicePixel(context, borderRect, document().deviceScaleFactor()), box,
             includeLogicalLeftEdge, includeLogicalRightEdge);
     }
@@ -695 +695 @@
         return style().getRoundedInnerBorderFor(borderRect, includeLogicalLeftEdge, includeLogicalRightEdge);
 
-    return getBackgroundRoundedRect(borderRect, box, boxSize.width(), boxSize.height(), includeLogicalLeftEdge, includeLogicalRightEdge);
+    return getBackgroundRoundedRect(borderRect, box, includeLogicalLeftEdge, includeLogicalRightEdge);
 }
 
@@ -738 +738 @@
 
 void RenderBoxModelObject::paintFillLayerExtended(const PaintInfo& paintInfo, const Color& color, const FillLayer& bgLayer, const LayoutRect& rect,
-    BackgroundBleedAvoidance bleedAvoidance, const InlineIterator::InlineBoxIterator& box, const LayoutSize& boxSize, CompositeOperator op, RenderElement* backgroundObject, BaseBackgroundColorUsage baseBgColorUsage)
+    BackgroundBleedAvoidance bleedAvoidance, const InlineIterator::InlineBoxIterator& box, const LayoutRect& backgroundImageStrip, CompositeOperator op, RenderElement* backgroundObject, BaseBackgroundColorUsage baseBgColorUsage)
 {
     GraphicsContext& context = paintInfo.context();
@@ -813 +813 @@
 
         if (hasRoundedBorder && bleedAvoidance != BackgroundBleedUseTransparencyLayer) {
-            FloatRoundedRect pixelSnappedBorder = backgroundRoundedRectAdjustedForBleedAvoidance(context, rect, bleedAvoidance, box, boxSize,
+            FloatRoundedRect pixelSnappedBorder = backgroundRoundedRectAdjustedForBleedAvoidance(context, rect, bleedAvoidance, box,
                 includeLeftEdge, includeRightEdge).pixelSnappedRoundedRectForPainting(deviceScaleFactor);
             if (pixelSnappedBorder.isRenderable()) {
@@ -841 +841 @@
     GraphicsContextStateSaver clipToBorderStateSaver(context, clipToBorderRadius);
     if (clipToBorderRadius) {
-        RoundedRect border = isBorderFill ? backgroundRoundedRectAdjustedForBleedAvoidance(context, rect, bleedAvoidance, box, boxSize, includeLeftEdge, includeRightEdge) : getBackgroundRoundedRect(rect, box, boxSize.width(), boxSize.height(), includeLeftEdge, includeRightEdge);
+        RoundedRect border = isBorderFill ? backgroundRoundedRectAdjustedForBleedAvoidance(context, rect, bleedAvoidance, box, includeLeftEdge, includeRightEdge) : getBackgroundRoundedRect(rect, box, includeLeftEdge, includeRightEdge);
 
         // Clip to the padding or content boxes as necessary.
@@ -955 +955 @@
     // no progressive loading of the background image
     if (!baseBgColorOnly && shouldPaintBackgroundImage) {
-        auto geometry = calculateBackgroundImageGeometry(paintInfo.paintContainer, bgLayer, rect.location(), scrolledPaintRect, backgroundObject);
+        // Multiline inline boxes paint like the image was one long strip spanning lines. The backgroundImageStrip is this fictional rectangle.
+        auto imageRect = backgroundImageStrip.isEmpty() ? scrolledPaintRect : backgroundImageStrip;
+        auto paintOffset = backgroundImageStrip.isEmpty() ? rect.location() : backgroundImageStrip.location();
+        auto geometry = calculateBackgroundImageGeometry(paintInfo.paintContainer, bgLayer, paintOffset, imageRect, backgroundObject);
         geometry.clip(LayoutRect(pixelSnappedRect));
         RefPtr<Image> image;

trunk/Source/WebCore/rendering/RenderBoxModelObject.h

@@ -214 +214 @@
     bool paintNinePieceImage(GraphicsContext&, const LayoutRect&, const RenderStyle&, const NinePieceImage&, CompositeOperator = CompositeOperator::SourceOver);
     void paintBoxShadow(const PaintInfo&, const LayoutRect&, const RenderStyle&, ShadowStyle, bool includeLogicalLeftEdge = true, bool includeLogicalRightEdge = true);
-    void paintFillLayerExtended(const PaintInfo&, const Color&, const FillLayer&, const LayoutRect&, BackgroundBleedAvoidance, const InlineIterator::InlineBoxIterator&, const LayoutSize& = LayoutSize(), CompositeOperator = CompositeOperator::SourceOver, RenderElement* backgroundObject = nullptr, BaseBackgroundColorUsage = BaseBackgroundColorUse);
+    void paintFillLayerExtended(const PaintInfo&, const Color&, const FillLayer&, const LayoutRect&, BackgroundBleedAvoidance, const InlineIterator::InlineBoxIterator&, const LayoutRect& backgroundImageStrip = { }, CompositeOperator = CompositeOperator::SourceOver, RenderElement* backgroundObject = nullptr, BaseBackgroundColorUsage = BaseBackgroundColorUse);
 
     virtual bool boxShadowShouldBeAppliedToBackground(const LayoutPoint& absolutePaintPostion, BackgroundBleedAvoidance, const InlineIterator::InlineBoxIterator&) const;
@@ -260 +260 @@
     bool borderObscuresBackgroundEdge(const FloatSize& contextScale) const;
     bool borderObscuresBackground() const;
-    RoundedRect backgroundRoundedRectAdjustedForBleedAvoidance(const GraphicsContext&, const LayoutRect&, BackgroundBleedAvoidance, const InlineIterator::InlineBoxIterator&, const LayoutSize&, bool includeLogicalLeftEdge, bool includeLogicalRightEdge) const;
+    RoundedRect backgroundRoundedRectAdjustedForBleedAvoidance(const GraphicsContext&, const LayoutRect&, BackgroundBleedAvoidance, const InlineIterator::InlineBoxIterator&, bool includeLogicalLeftEdge, bool includeLogicalRightEdge) const;
     LayoutRect borderInnerRectAdjustedForBleedAvoidance(const GraphicsContext&, const LayoutRect&, BackgroundBleedAvoidance) const;
 
@@ -311 +311 @@
     LayoutSize calculateFillTileSize(const FillLayer&, const LayoutSize& scaledPositioningAreaSize) const;
 
-    RoundedRect getBackgroundRoundedRect(const LayoutRect&, const InlineIterator::InlineBoxIterator&, LayoutUnit inlineBoxWidth, LayoutUnit inlineBoxHeight,
-        bool includeLogicalLeftEdge, bool includeLogicalRightEdge) const;
+    RoundedRect getBackgroundRoundedRect(const LayoutRect&, const InlineIterator::InlineBoxIterator&, bool includeLogicalLeftEdge, bool includeLogicalRightEdge) const;
     
     bool fixedBackgroundPaintsInLocalCoordinates() const;