Bug 21789 – -webkit-border-radius clipping

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Michal]

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.

[Michal]

Created attachment 24556 [details]
Test case showing text shown beyond the curved border of an element (no clipping).

[Dave Hyatt]

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.

[Dave Hyatt]

Comment on attachment 28896 [details]
Patch to make replaced elements work properly with border-radius clipping

Need to handle hit testing also.

[Dave Hyatt]

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).

[Simon Fraser (smfr)]

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.

[Dave Hyatt]

Created attachment 28898 [details]
Patch to address Simon's comments

[Simon Fraser (smfr)]

Comment on attachment 28898 [details]
Patch to address Simon's comments

r=me

[Dave Hyatt]

First patch landed in r41945.

[Dave Hyatt]

Created attachment 28907 [details]
Make non-self-painting overflow layers clip their foreground content to border-radius.

[Simon Fraser (smfr)]

Comment on attachment 28907 [details]
Make non-self-painting overflow layers clip their foreground content to border-radius.

r=me

[Dave Hyatt]

Second patch landed in r41948.  Now for the hard part.

[Eric Seidel (no email)]

This was landed as http://trac.webkit.org/changeset/41948.  Closing.