Bug 21789 - -webkit-border-radius clipping
Summary: -webkit-border-radius clipping
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.5
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-10-22 04:38 PDT by Michal
Modified: 2009-04-29 14:54 PDT (History)
0 users

See Also:


Attachments
Test case showing text shown beyond the curved border of an element (no clipping). (440 bytes, text/html)
2008-10-22 04:42 PDT, Michal
no flags Details
Patch to make replaced elements work properly with border-radius clipping (82.83 KB, patch)
2009-03-24 12:18 PDT, Dave Hyatt
simon.fraser: review-
Details | Formatted Diff | Diff
Patch to address Simon's comments (82.85 KB, patch)
2009-03-24 12:32 PDT, Dave Hyatt
simon.fraser: review+
Details | Formatted Diff | Diff
Make non-self-painting overflow layers clip their foreground content to border-radius. (28.08 KB, patch)
2009-03-24 13:12 PDT, Dave Hyatt
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michal 2008-10-22 04:38:52 PDT
The CSS property -webkit-border-radius does not clip any elements at the corners (even with overflow:hidden also enabled). I have tested this both with images and text nodes: they each escape out of the curved border area.
Comment 1 Michal 2008-10-22 04:42:41 PDT
Created attachment 24556 [details]
Test case showing text shown beyond the curved border of an element (no clipping).
Comment 2 Dave Hyatt 2009-03-24 12:18:34 PDT
Created attachment 28896 [details]
Patch to make replaced elements work properly with border-radius clipping

This is just step one.  Next step will be to make normal flow overflow blocks work.  Third step will be to make RenderLayers work.
Comment 3 Dave Hyatt 2009-03-24 12:23:29 PDT
Comment on attachment 28896 [details]
Patch to make replaced elements work properly with border-radius clipping

Need to handle hit testing also.
Comment 4 Dave Hyatt 2009-03-24 12:25:57 PDT
Comment on attachment 28896 [details]
Patch to make replaced elements work properly with border-radius clipping

I changed my mind.  I think hit testing is going to be nontrivial enough that it should be done in its own patch.  The inconsistency between hit testing and painting can persist for a little while (after all we were already hit testing incorrectly anyway).
Comment 5 Simon Fraser (smfr) 2009-03-24 12:26:56 PDT
Comment on attachment 28896 [details]
Patch to make replaced elements work properly with border-radius clipping

> Index: WebCore/rendering/RenderReplaced.cpp
> ===================================================================

> +    if (style()->overflowX() != OVISIBLE && style()->hasBorderRadius()) {
> +        // Push a clip if we have a border radius, since we want to round the foreground content that gets painted.
> +        paintInfo.context->save();
> +        paintInfo.context->addRoundedRectClip(IntRect(tx, ty, width(), height()),
> +                                              style()->borderTopLeftRadius(),
> +                                              style()->borderTopRightRadius(), 
> +                                              style()->borderBottomLeftRadius(),
> +                                              style()->borderBottomRightRadius());
> +    }
> +
>      paintReplaced(paintInfo, tx, ty);
> -    
> +
> +    if (style()->overflowX() != OVISIBLE && style()->hasBorderRadius())
> +        paintInfo.context->restore();

I know you minused already, but a brief comment. In general code like the above seems
prone to mismatched push/pop, and you're doing an complex test twice. Maybe either
stash the result in a boolean:

bool haveRoundedClip = style()->overflowX() != OVISIBLE && style()->hasBorderRadius();
if (haveRoundedClip) {
  paintInfo.context->save();

...
if (haveRoundedClip) {
  paintInfo.context->restore();

or we should make a little stack-based state maintainer.
Comment 6 Dave Hyatt 2009-03-24 12:32:39 PDT
Created attachment 28898 [details]
Patch to address Simon's comments
Comment 7 Simon Fraser (smfr) 2009-03-24 12:33:52 PDT
Comment on attachment 28898 [details]
Patch to address Simon's comments

r=me
Comment 8 Dave Hyatt 2009-03-24 12:35:52 PDT
First patch landed in r41945.
Comment 9 Dave Hyatt 2009-03-24 13:12:47 PDT
Created attachment 28907 [details]
Make non-self-painting overflow layers clip their foreground content to border-radius.
Comment 10 Simon Fraser (smfr) 2009-03-24 13:14:21 PDT
Comment on attachment 28907 [details]
Make non-self-painting overflow layers clip their foreground content to border-radius.

r=me
Comment 11 Dave Hyatt 2009-03-24 13:24:00 PDT
Second patch landed in r41948.  Now for the hard part.

Comment 12 Eric Seidel (no email) 2009-04-29 14:54:35 PDT
This was landed as http://trac.webkit.org/changeset/41948.  Closing.