WebKit fails to render extreme border-radius #6632

vitorroriz · 2022-11-18T15:13:33Z

`8a3db1a`

WebKit fails to render extreme border-radius
https://bugs.webkit.org/show_bug.cgi?id=244638
rdar://99668793

Reviewed by Antti Koivisto.

* LayoutTests/imported/w3c/web-platform-tests/css/css-backgrounds/inner-border-non-renderable-expected.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-backgrounds/inner-border-non-renderable-ref.html: Added.
* LayoutTests/imported/w3c/web-platform-tests/css/css-backgrounds/inner-border-non-renderable.html: Added.
Testing that child-background doesn't bleed through its parent border for a inner-border-radius that is larger than the content rect.

* Source/WebCore/rendering/BorderPainter.cpp:
(WebCore::BorderPainter::paintOneBorderSide):
Improving readability.

* Source/WebCore/rendering/RenderBox.cpp:
(WebCore::RenderBox::clipContentForBorderRadius):
(WebCore::RenderBox::pushContentsClip):
The bug can be fixed by adjusting the inner-border radius, if not renderable, here. However, with we adjust it just here the bug can still be triggered in
other situations, for example, when applying a clip-path in an element with non-renderable border. Therefore, we are adjusting the non-renderable inner-border
directly in RenderStyle::getRoundedInnerBorderFor. Here, we are adding a helper function that still calls getRoundedInnerBorderFor for improving readability.

* Source/WebCore/rendering/RenderBox.h:
* Source/WebCore/rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::getRoundedInnerBorderFor):
Adjusting the inner-border-radii for non-renderable inner-border.

Canonical link: https://commits.webkit.org/256943@main

1a21d79

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	✅ 🛠 mac-AS-debug	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	~~🧪 api-mac~~	✅ 🧪 api-gtk
	✅ 🛠 tv	~~🧪 mac-wk1~~
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2
✅ 🛠 🧪 merge	✅ 🛠 watch	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-11-18T15:14:50Z

EWS run on previous version of this PR (hash 764d008)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	~~🛠 🧪 win~~
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	✅ 🛠 mac-AS-debug	✅ 🧪 gtk-wk2
	~~🧪 api-ios~~	✅ 🧪 api-mac	✅ 🧪 api-gtk
	✅ 🛠 tv	~~🧪 mac-wk1~~
	✅ 🛠 tv-sim	~~🧪 mac-wk2~~
	✅ 🛠 watch	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-11-18T15:15:28Z

EWS run on previous version of this PR (hash 764d008)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	⏳ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 🧪 win
✅ 🧪 bindings	⏳ 🛠 ios-sim	⏳ 🛠 mac-debug	⏳ 🛠 gtk	⏳ 🛠 wincairo
✅ 🧪 webkitperl	⏳ 🧪 ios-wk2	⏳ 🛠 mac-AS-debug	⏳ 🧪 gtk-wk2
	⏳ 🧪 api-ios	⏳ 🧪 api-mac	⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc	⏳ 🛠 tv	⏳ 🧪 mac-wk1	⏳ 🛠 jsc-armv7
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2	⏳ 🧪 jsc-armv7-tests
	⏳ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2	⏳ 🛠 jsc-mips
	⏳ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress	⏳ 🧪 jsc-mips-tests

Source/WebCore/rendering/BorderPainter.h

anttijk · 2022-11-18T15:51:39Z

Source/WebCore/rendering/RenderBox.cpp

+        auto innerBorder = style().getRoundedInnerBorderFor(LayoutRect(accumulatedOffset, size()));
+        if (innerBorder.isRenderable())
+            paintInfo.context().clipRoundedRect(innerBorder.pixelSnappedRoundedRectForPainting(deviceScaleFactor));
+        else {
+            paintInfo.context().clipRoundedRect(calculateAdjustedInnerBorder(innerBorder, BoxSide::Top).pixelSnappedRoundedRectForPainting(deviceScaleFactor));
+            paintInfo.context().clipRoundedRect(calculateAdjustedInnerBorder(innerBorder, BoxSide::Bottom).pixelSnappedRoundedRectForPainting(deviceScaleFactor));
+            paintInfo.context().clipRoundedRect(calculateAdjustedInnerBorder(innerBorder, BoxSide::Left).pixelSnappedRoundedRectForPainting(deviceScaleFactor));
+            paintInfo.context().clipRoundedRect(calculateAdjustedInnerBorder(innerBorder, BoxSide::Right).pixelSnappedRoundedRectForPainting(deviceScaleFactor));
+        }


Maybe you have a helper for this that takes GraphicsContext a parameter, clipContentForBorderRadius or something.

It seems wrong to compute a clip for each side (ending up with the intersection of 4 clips) rather than just having logic that fixes up the rounded rect.

Why not just do:

diff --git a/Source/WebCore/rendering/RenderBox.cpp b/Source/WebCore/rendering/RenderBox.cpp index 704c4286a85d4d304024cf7043d8f82637b39c34..6e6b63fc0229d372080fe08ae647ea8dfa778f89 100644 --- a/Source/WebCore/rendering/RenderBox.cpp +++ b/Source/WebCore/rendering/RenderBox.cpp @@ -2021,8 +2021,13 @@ bool RenderBox::pushContentsClip(PaintInfo& paintInfo, const LayoutPoint& accumu float deviceScaleFactor = document().deviceScaleFactor(); FloatRect clipRect = snapRectToDevicePixels((isControlClip ? controlClipRect(accumulatedOffset) : overflowClipRect(accumulatedOffset, nullptr, IgnoreOverlayScrollbarSize, paintInfo.phase)), deviceScaleFactor); paintInfo.context().save(); - if (style().hasBorderRadius()) - paintInfo.context().clipRoundedRect(style().getRoundedInnerBorderFor(LayoutRect(accumulatedOffset, size())).pixelSnappedRoundedRectForPainting(deviceScaleFactor)); + if (style().hasBorderRadius()) { + auto innerRoundedRect = style().getRoundedInnerBorderFor(LayoutRect(accumulatedOffset, size())).pixelSnappedRoundedRectForPainting(deviceScaleFactor); + if (!innerRoundedRect.isRenderable()) + innerRoundedRect.adjustRadii(); + paintInfo.context().clipRoundedRect(innerRoundedRect); + } + paintInfo.context().clip(clipRect); if (paintInfo.phase == PaintPhase::EventRegion)

webkit-early-warning-system · 2022-11-18T16:47:14Z

EWS run on previous version of this PR (hash 34dba54)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	❌ 🧪 ios-wk2	✅ 🛠 mac-AS-debug	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 api-mac	❌ 🧪 api-gtk
	✅ 🛠 tv	✅ 🧪 mac-wk1
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2
	✅ 🛠 watch	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-11-18T16:47:19Z

EWS run on current version of this PR (hash 34dba54)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	⏳ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 🧪 win
✅ 🧪 bindings	⏳ 🛠 ios-sim	⏳ 🛠 mac-debug	⏳ 🛠 gtk	⏳ 🛠 wincairo
✅ 🧪 webkitperl	⏳ 🧪 ios-wk2	⏳ 🛠 mac-AS-debug	⏳ 🧪 gtk-wk2
	⏳ 🧪 api-ios	⏳ 🧪 api-mac	⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc	⏳ 🛠 tv	⏳ 🧪 mac-wk1	⏳ 🛠 jsc-armv7
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2	⏳ 🧪 jsc-armv7-tests
	⏳ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2	⏳ 🛠 jsc-mips
	⏳ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress	⏳ 🧪 jsc-mips-tests

nt1m · 2022-11-18T19:06:14Z

...utTests/imported/w3c/web-platform-tests/css/css-backgrounds/inner-border-non-renderable.html

+<!DOCTYPE html>
+<html>
+<head>
+    <link rel="match" href="inner-border-ref.html">


The match link here seems wrong, should be inner-border-non-renderable-ref.html

nt1m · 2022-11-18T19:06:24Z

...sts/imported/w3c/web-platform-tests/css/css-backgrounds/inner-border-non-renderable-ref.html

+            border: 30px solid green;
+            border-top-color: gold;
+            border-top-right-radius: 150px 267px;
+            /* border-bottom-left-radius: 150px 267px; */


Is this commented out code useful? This whole file should probably match the -expected.html

nt1m · 2022-11-18T19:07:48Z

...utTests/imported/w3c/web-platform-tests/css/css-backgrounds/inner-border-non-renderable.html

+<html>
+<head>
+    <link rel="match" href="inner-border-ref.html">
+    <meta name=fuzzy content="0-144;0-484">


can you include maxDifference=/totalPixels= for clarity?

We don't need fuzziness anymore.

webkit-early-warning-system · 2022-11-21T19:16:12Z

EWS run on previous version of this PR (hash 0c3a00c)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	⏳ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 🧪 win
✅ 🧪 bindings	⏳ 🛠 ios-sim	⏳ 🛠 mac-debug	⏳ 🛠 gtk	⏳ 🛠 wincairo
✅ 🧪 webkitperl	⏳ 🧪 ios-wk2	⏳ 🛠 mac-AS-debug	⏳ 🧪 gtk-wk2
	⏳ 🧪 api-ios	⏳ 🧪 api-mac	⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc	⏳ 🛠 tv	⏳ 🧪 mac-wk1	⏳ 🛠 jsc-armv7
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2	⏳ 🧪 jsc-armv7-tests
	⏳ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2	⏳ 🛠 jsc-mips
	⏳ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress	⏳ 🧪 jsc-mips-tests

webkit-early-warning-system · 2022-11-21T19:17:32Z

EWS run on previous version of this PR (hash 0c3a00c)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🛠 mac-AS-debug	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	~~🧪 api-mac~~	~~🧪 api-gtk~~
	✅ 🛠 tv	~~🧪 mac-wk1~~
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2
	✅ 🛠 watch	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

webkit-early-warning-system · 2022-11-22T11:03:07Z

EWS run on current version of this PR (hash 1a21d79)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	⏳ 🛠 ios	⏳ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 🧪 win
✅ 🧪 bindings	⏳ 🛠 ios-sim	⏳ 🛠 mac-debug	⏳ 🛠 gtk	⏳ 🛠 wincairo
✅ 🧪 webkitperl	⏳ 🧪 ios-wk2	⏳ 🛠 mac-AS-debug	⏳ 🧪 gtk-wk2
	⏳ 🧪 api-ios	⏳ 🧪 api-mac	⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc	⏳ 🛠 tv	⏳ 🧪 mac-wk1	⏳ 🛠 jsc-armv7
	⏳ 🛠 tv-sim	⏳ 🧪 mac-wk2	⏳ 🧪 jsc-armv7-tests
	⏳ 🛠 watch	⏳ 🧪 mac-AS-debug-wk2	⏳ 🛠 jsc-mips
	⏳ 🛠 watch-sim	⏳ 🧪 mac-wk2-stress	⏳ 🧪 jsc-mips-tests

webkit-early-warning-system · 2022-11-22T11:05:32Z

EWS run on current version of this PR (hash 1a21d79)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 🧪 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-debug	✅ 🛠 gtk	✅ 🛠 wincairo
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	✅ 🛠 mac-AS-debug	✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	~~🧪 api-mac~~	✅ 🧪 api-gtk
	✅ 🛠 tv	~~🧪 mac-wk1~~
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2
✅ 🛠 🧪 merge	✅ 🛠 watch	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 watch-sim	✅ 🧪 mac-wk2-stress

https://bugs.webkit.org/show_bug.cgi?id=244638 rdar://99668793 Reviewed by Antti Koivisto. * LayoutTests/imported/w3c/web-platform-tests/css/css-backgrounds/inner-border-non-renderable-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-backgrounds/inner-border-non-renderable-ref.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-backgrounds/inner-border-non-renderable.html: Added. Testing that child-background doesn't bleed through its parent border for a inner-border-radius that is larger than the content rect. * Source/WebCore/rendering/BorderPainter.cpp: (WebCore::BorderPainter::paintOneBorderSide): Improving readability. * Source/WebCore/rendering/RenderBox.cpp: (WebCore::RenderBox::clipContentForBorderRadius): (WebCore::RenderBox::pushContentsClip): The bug can be fixed by adjusting the inner-border radius, if not renderable, here. However, with we adjust it just here the bug can still be triggered in other situations, for example, when applying a clip-path in an element with non-renderable border. Therefore, we are adjusting the non-renderable inner-border directly in RenderStyle::getRoundedInnerBorderFor. Here, we are adding a helper function that still calls getRoundedInnerBorderFor for improving readability. * Source/WebCore/rendering/RenderBox.h: * Source/WebCore/rendering/style/RenderStyle.cpp: (WebCore::RenderStyle::getRoundedInnerBorderFor): Adjusting the inner-border-radii for non-renderable inner-border. Canonical link: https://commits.webkit.org/256943@main

webkit-commit-queue · 2022-11-22T17:53:54Z

Committed 256943@main (8a3db1a): https://commits.webkit.org/256943@main

Reviewed commits have been landed. Closing PR #6632 and removing active labels.

vitorroriz self-assigned this Nov 18, 2022

vitorroriz added the Layout and Rendering For bugs with layout and rendering of Web pages. label Nov 18, 2022

vitorroriz requested a review from anttijk November 18, 2022 15:17

anttijk approved these changes Nov 18, 2022

View reviewed changes

vitorroriz force-pushed the eng/WebKit-fails-to-render-extreme-border-radius branch from 764d008 to 34dba54 Compare November 18, 2022 16:45

nt1m reviewed Nov 18, 2022

View reviewed changes

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Nov 19, 2022

vitorroriz removed the merging-blocked Applied to prevent a change from being merged label Nov 21, 2022

vitorroriz force-pushed the eng/WebKit-fails-to-render-extreme-border-radius branch from 34dba54 to 0c3a00c Compare November 21, 2022 19:15

vitorroriz force-pushed the eng/WebKit-fails-to-render-extreme-border-radius branch from 0c3a00c to 1a21d79 Compare November 22, 2022 11:02

vitorroriz changed the title ~~WebKit fails to clip non-renderable inner-border with border-radius~~ WebKit fails to render extreme border-radius Nov 22, 2022

nt1m added the merge-queue Applied to send a pull request to merge-queue label Nov 22, 2022

webkit-early-warning-system force-pushed the eng/WebKit-fails-to-render-extreme-border-radius branch from 1a21d79 to 8a3db1a Compare November 22, 2022 17:53

webkit-early-warning-system merged commit 8a3db1a into WebKit:main Nov 22, 2022

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Nov 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WebKit fails to render extreme border-radius #6632

WebKit fails to render extreme border-radius #6632

vitorroriz commented Nov 18, 2022 •

edited by webkit-early-warning-system

webkit-early-warning-system commented Nov 18, 2022 •

edited

webkit-early-warning-system commented Nov 18, 2022 •

edited

anttijk Nov 18, 2022

smfr Nov 18, 2022

smfr Nov 18, 2022

vitorroriz Nov 22, 2022

webkit-early-warning-system commented Nov 18, 2022 •

edited

webkit-early-warning-system commented Nov 18, 2022 •

edited

nt1m Nov 18, 2022

vitorroriz Nov 22, 2022

nt1m Nov 18, 2022

vitorroriz Nov 22, 2022

nt1m Nov 18, 2022

vitorroriz Nov 22, 2022

webkit-early-warning-system commented Nov 21, 2022 •

edited

webkit-early-warning-system commented Nov 21, 2022 •

edited

webkit-early-warning-system commented Nov 22, 2022 •

edited

webkit-early-warning-system commented Nov 22, 2022 •

edited

webkit-commit-queue commented Nov 22, 2022

WebKit fails to render extreme border-radius #6632

WebKit fails to render extreme border-radius #6632

Conversation

vitorroriz commented Nov 18, 2022 • edited by webkit-early-warning-system

webkit-early-warning-system commented Nov 18, 2022 • edited

webkit-early-warning-system commented Nov 18, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webkit-early-warning-system commented Nov 18, 2022 • edited

webkit-early-warning-system commented Nov 18, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webkit-early-warning-system commented Nov 21, 2022 • edited

webkit-early-warning-system commented Nov 21, 2022 • edited

webkit-early-warning-system commented Nov 22, 2022 • edited

webkit-early-warning-system commented Nov 22, 2022 • edited

webkit-commit-queue commented Nov 22, 2022

vitorroriz commented Nov 18, 2022 •

edited by webkit-early-warning-system

webkit-early-warning-system commented Nov 18, 2022 •

edited

webkit-early-warning-system commented Nov 18, 2022 •

edited

webkit-early-warning-system commented Nov 18, 2022 •

edited

webkit-early-warning-system commented Nov 18, 2022 •

edited

webkit-early-warning-system commented Nov 21, 2022 •

edited

webkit-early-warning-system commented Nov 21, 2022 •

edited

webkit-early-warning-system commented Nov 22, 2022 •

edited

webkit-early-warning-system commented Nov 22, 2022 •

edited