Skip to content

Commit

Permalink
Mix-blend-mode incorrectly blends with view base color.
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=261708
<rdar://problem/115688282>

Reviewed by Simon Fraser.

Mix-blend-mode on an element that blends into the 'Root Element Group' should have only the root element's background color, or transparent if none. We're including the opaque white base color of the RenderView too, which is incorrect.

This change changes BackgroundPainter so that we only draw the base color if it's ok to draw it as part of the root element background (if the root background isn't an isolated group).

It also changes RenderView to make sure we do paint the base color (for iframes, particularly) if the root element won't be painting it.

Removes some tests (including one WPT) that were explicitly trying to test the wrong thing (and failed on everything except WebKit).

* LayoutTests/css3/blending/blend-mode-body-child-expected.html: Removed.
* LayoutTests/css3/blending/blend-mode-body-child.html: Removed.
* LayoutTests/css3/blending/blend-mode-body-composited-child-expected.html: Removed.
* LayoutTests/css3/blending/blend-mode-body-composited-child.html: Removed.
* LayoutTests/imported/w3c/web-platform-tests/css/compositing/compositing_simple_div-expected.html: Removed.
* LayoutTests/imported/w3c/web-platform-tests/css/compositing/compositing_simple_div-ref.html: Removed.
* LayoutTests/imported/w3c/web-platform-tests/css/compositing/compositing_simple_div.html: Removed.
* LayoutTests/imported/w3c/web-platform-tests/css/compositing/mix-blend-mode/mix-blend-mode-root-element-group-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/compositing/mix-blend-mode/mix-blend-mode-root-element-group-ref.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/compositing/mix-blend-mode/mix-blend-mode-root-element-group.html: Added.
* LayoutTests/svg/css/mix-blend-mode-with-inline-svg.html:
* Source/WebCore/rendering/BackgroundPainter.cpp:
(WebCore::BackgroundPainter::paintFillLayer):
* Source/WebCore/rendering/RenderLayerInlines.h:
(WebCore::RenderLayer::hasNonOpacityTransparency const):
* Source/WebCore/rendering/RenderView.cpp:
(WebCore::RenderView::paintBoxDecorations):
(WebCore::RenderView::rootElementShouldPaintBaseBackground const):
* Source/WebCore/rendering/RenderView.h:

Canonical link: https://commits.webkit.org/268173@main
  • Loading branch information
mattwoodrow committed Sep 20, 2023
1 parent fb59fe5 commit 4f26343
Show file tree
Hide file tree
Showing 17 changed files with 126 additions and 102 deletions.
5 changes: 0 additions & 5 deletions LayoutTests/css3/blending/blend-mode-body-child-expected.html

This file was deleted.

10 changes: 0 additions & 10 deletions LayoutTests/css3/blending/blend-mode-body-child.html

This file was deleted.

This file was deleted.

12 changes: 0 additions & 12 deletions LayoutTests/css3/blending/blend-mode-body-composited-child.html

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<!DOCTYPE html>
<html>
<head>
<title>CSS Reftest Reference</title>
<link rel="author" title="Matt Woodrow" href="mailto:mattwoodrow@apple.com">
<link rel="help" href="https://drafts.fxtf.org/compositing-1/#mix-blend-mode">
<style type="text/css">
div {
width: 200px;
height: 200px;
}
.green {
background: rgba(0,160,0,0.5);
mix-blend-mode: soft-light;
}
.blue {
position: absolute;
top: 75px;
left: 75px;
background: rgba(0,0,160,0.5)
}
.wrapper {
isolation: isolate;
}
</style>
</head>
<body>
<div class="wrapper">
<div class="blue"></div>
<div class="green"></div>
</div>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<!DOCTYPE html>
<html>
<head>
<title>CSS Reftest Reference</title>
<link rel="author" title="Matt Woodrow" href="mailto:mattwoodrow@apple.com">
<link rel="help" href="https://drafts.fxtf.org/compositing-1/#mix-blend-mode">
<style type="text/css">
div {
width: 200px;
height: 200px;
}
.green {
background: rgba(0,160,0,0.5);
mix-blend-mode: soft-light;
}
.blue {
position: absolute;
top: 75px;
left: 75px;
background: rgba(0,0,160,0.5)
}
.wrapper {
isolation: isolate;
}
</style>
</head>
<body>
<div class="wrapper">
<div class="blue"></div>
<div class="green"></div>
</div>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<!DOCTYPE html>
<html>
<head>
<title>CSS Test: mix-blend-mode in the Root Element Group should have a transparent backdrop</title>
<link rel="author" title="Matt Woodrow" href="mailto:mattwoodrow@apple.com">
<link rel="help" href="https://drafts.fxtf.org/compositing-1/#mix-blend-mode">
<meta name="assert" content="Test checks that the Root Element Group has a transparent backdrop">
<link rel="match" href="mix-blend-mode-root-element-group-ref.html">
<style type="text/css">
div {
width: 200px;
height: 200px;
}
.green {
background: rgba(0,160,0,0.5);
mix-blend-mode: soft-light;
}
.blue {
position: absolute;
top: 75px;
left: 75px;
background: rgba(0,0,160,0.5)
}
</style>
</head>
<body>
<div class="blue"></div>
<div class="green"></div>
</body>
</html>
1 change: 0 additions & 1 deletion LayoutTests/platform/glib/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,6 @@ webkit.org/b/214417 [ Debug ] css3/color/composited-solid-backgrounds.html [ Cra
# Failures related with ENABLE_CSS_COMPOSITING=ON
webkit.org/b/169916 css3/blending/blend-mode-accelerated-parent-overflow-hidden.html [ ImageOnlyFailure ]
webkit.org/b/169916 css3/blending/blend-mode-body-composited-child-background-color.html [ ImageOnlyFailure ]
webkit.org/b/169916 css3/blending/blend-mode-body-composited-child.html [ ImageOnlyFailure ]
webkit.org/b/169916 css3/blending/blend-mode-clip-accelerated-blending-canvas.html [ Failure ]
webkit.org/b/169916 css3/blending/blend-mode-clip-accelerated-blending-child.html [ ImageOnlyFailure ]
webkit.org/b/169916 css3/blending/blend-mode-clip-accelerated-blending-double.html [ ImageOnlyFailure ]
Expand Down
8 changes: 8 additions & 0 deletions LayoutTests/platform/mac-wk1/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -2664,6 +2664,14 @@ fast/mediastream/device-change-event-2.html [ Failure ]
imported/w3c/web-platform-tests/css/compositing/mix-blend-mode/mix-blend-mode-parent-with-text.html [ Pass ]
imported/w3c/web-platform-tests/css/compositing/mix-blend-mode/mix-blend-mode-plus-lighter-svg.html [ Pass ]

# WK1-specific subtle blending differences
imported/w3c/web-platform-tests/css/compositing/mix-blend-mode/mix-blend-mode-canvas-parent.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/compositing/mix-blend-mode/mix-blend-mode-canvas-sibling.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/compositing/mix-blend-mode/mix-blend-mode-iframe-parent.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/compositing/mix-blend-mode/mix-blend-mode-iframe-sibling.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/compositing/mix-blend-mode/mix-blend-mode-paragraph-background-image.html [ ImageOnlyFailure ]
imported/w3c/web-platform-tests/css/compositing/mix-blend-mode/mix-blend-mode-paragraph.html [ ImageOnlyFailure ]

webkit.org/b/261305 [ Monterey+ Debug ] editing/inserting/break-out-of-nested-lists.html [ Crash ]

# webkit.org/b/261304 Batch mark expectations for flaky media tests
Expand Down
2 changes: 1 addition & 1 deletion LayoutTests/svg/css/mix-blend-mode-with-inline-svg.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<!DOCTYPE html>
<body>
<body style="background: white">
<svg width="300" height="300" style="mix-blend-mode: difference;">
<rect x="20" y="20" width="100" height="100" fill="rgb(255,0,0)"/>
<rect x="140" y="20" width="100" height="100" fill="rgb(0,255,0)"/>
Expand Down
6 changes: 5 additions & 1 deletion Source/WebCore/rendering/BackgroundPainter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,11 @@ void BackgroundPainter::paintFillLayer(const Color& color, const FillLayer& bgLa

auto isOpaqueRoot = false;
if (isRoot) {
isOpaqueRoot = bgLayer.next() || bgColor.isOpaque() || view().shouldPaintBaseBackground();
bool shouldPaintBaseBackground = view().rootElementShouldPaintBaseBackground();
isOpaqueRoot = bgLayer.next() || bgColor.isOpaque() || shouldPaintBaseBackground;
if (!shouldPaintBaseBackground)
baseBgColorUsage = BaseBackgroundColorSkip;

view().frameView().setContentIsOpaque(isOpaqueRoot);
}

Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/rendering/RenderLayerInlines.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ namespace WebCore {
inline bool RenderLayer::canPaintTransparencyWithSetOpacity() const { return isBitmapOnly() && !hasNonOpacityTransparency(); }
inline bool RenderLayer::hasBackdropFilter() const { return renderer().hasBackdropFilter(); }
inline bool RenderLayer::hasFilter() const { return renderer().hasFilter(); }
inline bool RenderLayer::hasNonOpacityTransparency() const { return renderer().hasMask() || hasBlendMode() || (isolatesBlending() && !renderer().isDocumentElementRenderer()); }
inline bool RenderLayer::hasNonOpacityTransparency() const { return renderer().hasMask() || hasBlendMode() || isolatesBlending(); }
inline bool RenderLayer::hasPerspective() const { return renderer().style().hasPerspective(); }
inline bool RenderLayer::isTransformed() const { return renderer().isTransformed(); }
inline bool RenderLayer::isTransparent() const { return renderer().isTransparent() || renderer().hasMask(); }
Expand Down
16 changes: 14 additions & 2 deletions Source/WebCore/rendering/RenderView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ void RenderView::paintBoxDecorations(PaintInfo& paintInfo, const LayoutPoint&)
}
}

if (document().ownerElement())
if (!shouldPaintBaseBackground())
return;

if (paintInfo.skipRootBackground())
Expand All @@ -464,7 +464,7 @@ void RenderView::paintBoxDecorations(PaintInfo& paintInfo, const LayoutPoint&)
float pageScaleFactor = page ? page->pageScaleFactor() : 1;

// If painting will entirely fill the view, no need to fill the background.
if (rootFillsViewport && rootObscuresBackground && pageScaleFactor >= 1)
if (rootFillsViewport && rootObscuresBackground && pageScaleFactor >= 1 && rootElementShouldPaintBaseBackground())
return;

// This code typically only executes if the root element's visibility has been set to hidden,
Expand Down Expand Up @@ -693,6 +693,18 @@ bool RenderView::shouldPaintBaseBackground() const

return false;
}

bool RenderView::rootElementShouldPaintBaseBackground() const
{
auto* documentElement = document().documentElement();
if (RenderElement* rootRenderer = documentElement ? documentElement->renderer() : nullptr) {
// The document element's renderer is currently forced to be a block, but may not always be.
auto* rootBox = dynamicDowncast<RenderBox>(*rootRenderer);
if (rootBox && rootBox->hasLayer() && rootBox->layer()->isolatesBlending())
return false;
}
return shouldPaintBaseBackground();
}

LayoutRect RenderView::unextendedBackgroundRect() const
{
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/rendering/RenderView.h
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ class RenderView final : public RenderBlockFlow {
// Renderer that paints the root background has background-images which all have background-attachment: fixed.
bool rootBackgroundIsEntirelyFixed() const;

bool rootElementShouldPaintBaseBackground() const;
bool shouldPaintBaseBackground() const;

FloatSize sizeForCSSSmallViewportUnits() const;
Expand Down

0 comments on commit 4f26343

Please sign in to comment.