For this page on IE9 Test Drive we don't clip the background gradient to the rounded rectangle: http://ie.microsoft.com/testdrive/HTML5/DayNight/Default.html A cursory exam shows that RenderBox::pushContentsClip() returns early because (!isControlClip && !isOverflowClip).
Created attachment 78399 [details] Patch
Comment on attachment 78399 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=78399&action=review > ChangeLog:9 > + CSS3 standard requires the clipping of box's background to the appropriate > + curve of the border: http://www.w3.org/TR/css3-background/#corner-clipping No, that text only refers to backgrounds. It does not talk about clipping children (which happens correctly if you set overflow:hidden). It's not clear to me that there's a bug here (though the issue has been discussed a few times on www-style).
(In reply to comment #2) > No, that text only refers to backgrounds. It does not talk about clipping children (which happens correctly if you set overflow:hidden). > > It's not clear to me that there's a bug here (though the issue has been discussed a few times on www-style). Right, so the bug here is that we're not clipping the background (rather than the children) to the curved border. Or am I missing something?
Comment on attachment 78399 [details] Patch Although this isn't technically wrong, the reason self-painting layers were excluded is that they need to be patched to account for border-radius clipping. I kind of always assumed we'd just fix that over in the RenderLayer code. I don't like this change because you cause the overflow clip for self-painting layers to get pushed twice.
I assume that "just fix that over in the RenderLayer code" you mean teach RenderLayer about non-rectangular clip regions?
Created attachment 78685 [details] Teaching RenderLayer about non-rectangular clip regions :)
Comment on attachment 78685 [details] Teaching RenderLayer about non-rectangular clip regions :) View in context: https://bugs.webkit.org/attachment.cgi?id=78685&action=review > WebCore/rendering/RenderLayer.cpp:2513 > + if (renderer()->style()->hasBorderRadius()) { > + IntSize topLeft, topRight, bottomLeft, bottomRight; > + renderer()->style()->getBorderRadiiForRect(clipRectToApply, topLeft, topRight, bottomLeft, bottomRight); > + paintInfo.context->addRoundedRectClip(clipRectToApply, topLeft, topRight, bottomLeft, bottomRight); > + } Sorry, this isn't enough. You need to change ClipRects to handle non-rectangular regions. To see this in your testcase, give .inner opacity for example.
(In reply to comment #7) > (From update of attachment 78685 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=78685&action=review > > > WebCore/rendering/RenderLayer.cpp:2513 > > + if (renderer()->style()->hasBorderRadius()) { > > + IntSize topLeft, topRight, bottomLeft, bottomRight; > > + renderer()->style()->getBorderRadiiForRect(clipRectToApply, topLeft, topRight, bottomLeft, bottomRight); > > + paintInfo.context->addRoundedRectClip(clipRectToApply, topLeft, topRight, bottomLeft, bottomRight); > > + } > > Sorry, this isn't enough. You need to change ClipRects to handle non-rectangular regions. Could you give me some advice how to do it?
Created attachment 81040 [details] Patch
You need a testcase with overflow:hidden on the outer, and something like position:relative on the inner. When I say that RenderLayer clipRects need to be radius-aware, I mean the code in RenderLayer::calculateClipRects() and the entire ClipRects class.
here's another test case that shows the inconsistent clipping inside/outside the border of the outer div: http://jsfiddle.net/N2cXJ/1/
I'm not sure if this is the same bug or something different, but it seems to have the same symptoms, except with just a single div. Here's my test case: http://jsfiddle.net/5dGAt/2/ Description: if position is static, then the contents of a container element clip to its border. If position is anything else, then the contents clip to its outline. Note that in either case, the contents wrap at the outline.
*** Bug 41356 has been marked as a duplicate of this bug. ***
Is this the same underlying problem, or is there another bug for border-radius not clipping the video element?: http://jsfiddle.net/vDPW2/68/
Here's another comprehensive test case exhibiting the buggy behavior (forked from piersh@hotmail.com's example): http://jsfiddle.net/timbowhite/wnHaU/7/ The 1st example (static > static) correctly clips the corners of the inner div to the inner border of the outer div. The 2nd example (static > absolute) correctly busts out the inner div over the outer div. Apparently, since the rest of the examples do not have both divs positioned "static", they fail to render correctly. Poop. The expected behavior for the remaining examples is that the inner divs' corners should be clipped to the inner border of the outer div. See http://www.w3.org/TR/css3-background/#corner-clipping. The spec doesn't mention anything about relative position changing this behavior. This bug is kinda a bummer for doing fancy stuff like images with round corners. Apologies for name dropping, but Firefox 4+ and IE9 got this one licked. Thanks!
Created attachment 107529 [details] Patch that fixes the bug.
Attachment 107529 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/rendering/RenderLayer.cpp:2585: This { should be at the end of the previous line [whitespace/braces] [4] LayoutTests/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Source/WebCore/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Source/WebCore/rendering/RenderLayer.h:77: Missing space inside { }. [whitespace/braces] [5] Total errors found: 4 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 107529 [details] Patch that fixes the bug. Didn't mean to flag. Just showing the approach.
Created attachment 107541 [details] Patch
Attachment 107541 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/fast..." exit_code: 1 Source/WebCore/rendering/RenderLayer.cpp:2585: This { should be at the end of the previous line [whitespace/braces] [4] Total errors found: 1 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Fixed in r95239.
This appears to have caused the web process to get in to an infinite loop in some cases. See bug 68314.
(In reply to comment #22) > This appears to have caused the web process to get in to an infinite loop in some cases. See bug 68314. is not it what bug 68040 fixed?
This changeset fixed rounded rectangle clipping, but utterly destroyed the performance of Chromium on twitter.com (http://code.google.com/p/chromium/issues/detail?id=97716). I'm posting this as a heads-up that we'll be working in the area, and would welcome any insight from the people who have been here recently.
(In reply to comment #24) > This changeset fixed rounded rectangle clipping, but utterly destroyed the performance of Chromium on twitter.com (http://code.google.com/p/chromium/issues/detail?id=97716). I'm posting this as a heads-up that we'll be working in the area, and would welcome any insight from the people who have been here recently. What makes clipping hard is that it doesn't necessarily follow the RenderLayer painting hierarchy. I think what's needed to improve clipping performance on pages with large numbers of layers (that themselves all cllp) like Twitter is to try to come up with a scheme where the parent layer pushes the clip, and the children don't have to pop or re-push if they end up wanting to respect that clip. In other words, right now the whole system is designed around the exceptions rather than the common case. This will probably be extremely difficult for anyone not very familiar with this code to fix. (Also note Twitter performance is bad already before this patch, and it's basically the same issue. The clips just got more expensive, so they're showing off the already-existing problem even more.)
The most promising improvement would probably be to find a way to treat positioned layers more like transform layers and go ahead and translate/push their clips/transparency layers aggressively. You can't do this is you're tainted with any fixed positioned children, however (well, not without popping all of the clips and repushing them again somehow). I tried a simple experiment doing this and Twitter performance seemed good with that change.
Filed https://bugs.webkit.org/show_bug.cgi?id=68733 to cover the regression.
Is this not a duplicate of 9543?
*** Bug 9543 has been marked as a duplicate of this bug. ***