Bug 106549 – [CSS Filters] CSS opacity property clips filter outsets

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Mike Sierra]

The first box in the example transitions using a blur() filter, and the second uses an offset & blurred drop-shadow().  On Chrome, both blur effects render abruptly outside of the rectangle only after the transition is complete.

[Mike Sierra]

For drop-shadow(), it's not just the blur, but also the shadow offsets that don't render outside the box while transitioning.  You can see this if you set the blur distance to 0:
    -webkit-filter: drop-shadow(20px 20px 0px red);

[Mike Sierra]

See also this test:
https://github.com/mike-sierra/webplatform/blob/master/bugs/opacity.html
Choose either the blur() or drop-shadow() filter functions from the popup list.
You see different results depending on whether the opacity() function of opacity property is applied along with it.

[Dirk Schulze]

(In reply to comment #2)
> See also this test:
> https://github.com/mike-sierra/webplatform/blob/master/bugs/opacity.html
> Choose either the blur() or drop-shadow() filter functions from the popup list.
> You see different results depending on whether the opacity() function of opacity property is applied along with it.

I would be great if you can upload an example on this bug report. Thanks!

[Mike Sierra]

Created attachment 182470 [details]
drop-shadow()/opacity() [left] && drop-shdaow() with "opacity" property

[Mike Sierra]

Created attachment 182471 [details]
blur()/opacity() [left] && blur() with "opacity" property

[Mike Sierra]

To be clear, in both of these examples a drop-shadow() or blur() is first applied (by choosing from the page's popup options).  In each LEFT image, a subsequent opacity() filter is applied.  Each RIGHT example,  the corresponding "opacity" property is applied instead.

[Dirk Schulze]

Sorry, I didn't get the problem initially. The problem is that the 'opacity' property clips content from an applied filter.

    filter: drop-shadow: 10px 10px 10px green;
    opacity: 0.5;

clips the shadow away. Similar to all other filter functions extending the boundaries.

[Alexandru Chiculita]

(In reply to comment #7)
> Sorry, I didn't get the problem initially. The problem is that the 'opacity' property clips content from an applied filter.
> 
>     filter: drop-shadow: 10px 10px 10px green;
>     opacity: 0.5;
> 
> clips the shadow away. Similar to all other filter functions extending the boundaries.

Looks like an issue in RenderLayer::beginTransparencyLayers that clips the transparency layer to the minimum amount needed. 

The fix would be to make "static LayoutRect transparencyClipBox"  expand the clip-rect by the filter bounds, like we do for RenderLayer::setFilterBackendNeedsRepaintingInRect.

[Max Vujovic]

(In reply to comment #8)
> The fix would be to make "static LayoutRect transparencyClipBox"  expand the clip-rect by the filter bounds, like we do for RenderLayer::setFilterBackendNeedsRepaintingInRect.

That sounds right. I'll assign this bug to myself unless one of you wants to fix it. I'd like to factor out the filter outset expansion into a function.

[Dirk Schulze]

(In reply to comment #8)
> (In reply to comment #7)
> > Sorry, I didn't get the problem initially. The problem is that the 'opacity' property clips content from an applied filter.
> > 
> >     filter: drop-shadow: 10px 10px 10px green;
> >     opacity: 0.5;
> > 
> > clips the shadow away. Similar to all other filter functions extending the boundaries.
> 
> Looks like an issue in RenderLayer::beginTransparencyLayers that clips the transparency layer to the minimum amount needed. 
> 
> The fix would be to make "static LayoutRect transparencyClipBox"  expand the clip-rect by the filter bounds, like we do for RenderLayer::setFilterBackendNeedsRepaintingInRect.

Isn't it fixed already? Why so slow today ;)

[Max Vujovic]

(In reply to comment #10)
> Isn't it fixed already? Why so slow today ;)

Moving the web forward in other patches :). I'll start working on this soon.

[Max Vujovic]

Created attachment 184038 [details]
Patch

[Dirk Schulze]

Comment on attachment 184038 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=184038&action=review

Patch looks great. Just a snippet. r=me with a change request.

> Source/WebCore/rendering/RenderLayer.cpp:1344
> +void RenderLayer::expandRectForFilterOutsets(LayoutRect& rect) const

Can you make this function inline and pass the renderer as well? Add an assert for the passed renderer, just in case :).

[Mike Sierra]

Will this patch fix both scenarios: both the animated blur & when paired with the opacity property?  I added a note of caution about the latter in the CSS filter doc, but based on these comments think it's probably unnecessary. Are there any other scenarios where outside-the-box pixels might get clipped that are not covered here?

[Max Vujovic]

(In reply to comment #14)
> Will this patch fix both scenarios: both the animated blur & when paired with the opacity property?  

I think there might two separate bugs here:

1) Filters are clipped during transitions in Chromium, regardless of opacity.
2) Filters are being clipped by the CSS opacity property in both Chromium and Safari, regardless of transition.

I've fixed #2 in this patch.

I came to this conclusion by creating two filter transition tests. I've documented their behavior below:

1) blur filter + css transition + css opacity property : http://jsfiddle.net/2N6s5/

a. Chromium, before this patch: blur is clipped during the transition and after the transition
b. Chromium, after this patch: blur is clipped during the transition, but looks good after the transition

c. Safari, before this patch: blur looks good during the transition, but is clipped after the transition
d. Safari, after this patch: blur looks good during the transition and after the transition

2) blur filter + css transition (no css opacity property) : http://jsfiddle.net/UF8dq/1/

a. Chromium, before this patch: blur is clipped during the transition, but looks good after the transition
b. Chromium, after this patch: same as above

c. Safari, before this patch: blur looks good during the transition and after the transition
d. Safari, after this patch: same as above

Looks like the blur filter being clipped during the transition in Chromium in #1a and #2a is a separate bug, since it is clipped in Chromium regardless of opacity.

I will file that as a separate bug, and update this bug's title.

> I added a note of caution about the latter in the CSS filter doc, but based on these comments think it's probably unnecessary.

I'm not sure what you mean. Could you elaborate?

> Are there any other scenarios where outside-the-box pixels might get clipped that are not covered here?

I'm not sure, but please continue to file bugs when you do see issues :)

[Max Vujovic]

I've updated the title of this bug, and I've created bug 107708 for the Chromium filter animation issue.

[Mike Sierra]

> I added a note of caution about the latter in the CSS filter doc, but based on these comments think it's > probably unnecessary.
> I'm not sure what you mean. Could you elaborate?

Sorry, Dirk knows about this doc: http://docs.webplatform.org/wiki/css/properties/filter

[Max Vujovic]

(In reply to comment #17)
> > I added a note of caution about the latter in the CSS filter doc, but based on these comments think it's > probably unnecessary.
> > I'm not sure what you mean. Could you elaborate?
> 
> Sorry, Dirk knows about this doc: http://docs.webplatform.org/wiki/css/properties/filter

Ah no worries. Thanks.

[Max Vujovic]

Created attachment 184293 [details]
Patch

[Max Vujovic]

(In reply to comment #13)
> (From update of attachment 184038 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=184038&action=review
> 
> Patch looks great. Just a snippet. r=me with a change request.
> 
> > Source/WebCore/rendering/RenderLayer.cpp:1344
> > +void RenderLayer::expandRectForFilterOutsets(LayoutRect& rect) const
> 
> Can you make this function inline and pass the renderer as well? 

Done. Its signature is now:

static inline void expandRectForFilterOutsets(LayoutRect& rect, const RenderLayerModelObject* renderer)

> Add an assert for the passed renderer, just in case :).

Done.

Alex also suggested to refactor the "#if ENABLE(CSS_FILTERS)" guards out of the function and into the call sites, so I did that and moved the entire function behind enable guards. It makes sense since this function is all about filters.

[Max Vujovic]

Comment on attachment 184293 [details]
Patch

Bots look good. Setting cq+.

I won't be available to watch the build bots today, so achicu will watch them for me.

[WebKit Review Bot]

Comment on attachment 184293 [details]
Patch

Rejecting attachment 184293 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=gce-cq-03', 'apply-attachment', '--no-update', '--non-interactive', 184293, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Last 500 characters of output:
p7dzCZW : No space left on device
patch: **** Can't create file /tmp/pp0ColXs : No space left on device
fatal: pathspec 'LayoutTests/css3/filters/css-opacity-with-drop-shadow-expected.html' did not match any files
Failed to git add LayoutTests/css3/filters/css-opacity-with-drop-shadow-expected.html. at /mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply line 448.

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/16077809

[Alexandru Chiculita]

Comment on attachment 184293 [details]
Patch

Setting the CQ+ again as the queue was out of space.

[WebKit Review Bot]

Comment on attachment 184293 [details]
Patch

Clearing flags on attachment: 184293

Committed r140702: <http://trac.webkit.org/changeset/140702>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.