Bug 167456 - Element with a backdrop-filter and a mask may not correctly mask the backdrop
Summary: Element with a backdrop-filter and a mask may not correctly mask the backdrop
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-01-26 08:03 PST by Antoine Quint
Modified: 2017-01-27 15:46 PST (History)
7 users (show)

See Also:


Attachments
Patch (2.89 KB, patch)
2017-01-26 08:14 PST, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (10.72 KB, patch)
2017-01-27 08:40 PST, Antoine Quint
no flags Details | Formatted Diff | Diff
Patch (7.47 KB, patch)
2017-01-27 14:34 PST, Simon Fraser (smfr)
graouts: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-elcapitan (878.17 KB, application/zip)
2017-01-27 15:36 PST, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Quint 2017-01-26 08:03:01 PST
We did some work in this area already as part of https://bugs.webkit.org/show_bug.cgi?id=142662, but there is a case where an element with both a "backdrop-filter" and a "mask" may not correctly clip the backdrop layer. This happens mostly on WK1 but can happen infrequently on WK2 as well.
Comment 1 Antoine Quint 2017-01-26 08:03:28 PST
<rdar://problem/29320059>
Comment 2 Antoine Quint 2017-01-26 08:14:06 PST
Created attachment 299811 [details]
Patch
Comment 3 Antoine Quint 2017-01-26 08:15:20 PST
Comment on attachment 299811 [details]
Patch

I'll have a test up later, but I wanted to let the bots have a try with this patch to see if it breaks anything.
Comment 4 Simon Fraser (smfr) 2017-01-26 08:24:27 PST
Comment on attachment 299811 [details]
Patch

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

Why no test?

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2018
> -    updateClippingStrategy(*m_backdropLayer, m_backdropClippingLayer, m_backdropFiltersRect);
> +    if (!m_maskLayer)
> +        updateClippingStrategy(*m_backdropLayer, m_backdropClippingLayer, m_backdropFiltersRect);

Is it ever the case that you have both a m_maskLayer and m_backdropClippingLayer here? In that case, I think this breaks things. I think the layer might have a mask for normal masking, but also a backdrop (possibly with m_backdropClippingLayer because of rounded corners).
Comment 5 Antoine Quint 2017-01-26 09:34:46 PST
(In reply to comment #4)
> Comment on attachment 299811 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=299811&action=review
> 
> Why no test?

See my previous comment, this is only temporary.

> > Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:2018
> > -    updateClippingStrategy(*m_backdropLayer, m_backdropClippingLayer, m_backdropFiltersRect);
> > +    if (!m_maskLayer)
> > +        updateClippingStrategy(*m_backdropLayer, m_backdropClippingLayer, m_backdropFiltersRect);
> 
> Is it ever the case that you have both a m_maskLayer and
> m_backdropClippingLayer here? In that case, I think this breaks things. I
> think the layer might have a mask for normal masking, but also a backdrop
> (possibly with m_backdropClippingLayer because of rounded corners).

So… m_backdropClippingLayer is set only as a result of calls made to updateClippingStrategy() in updateBackdropFiltersRect(), which then sets the m_backdropClippingLayer as the m_backdropLayer. It seems to me we'd get in a situation where we may have both a m_backdropClippingLayer and a m_maskLayer if the element has backdrop-filter, border-radius and mask CSS properties set. The current code doesn't know how to deal with that as far as I can tell and our current test coverage doesn't seem to test this, or at least my change doesn't break that. I think with my change `mask` would take precedence over border-radius if both are specified.
Comment 6 Antoine Quint 2017-01-26 14:28:17 PST
Actually, in the particular case I've been dealing with it's a "clip-path" property, but I expect both "mask" and "clip-path" both end up setting the m_maskLayer on a GraphicsLayerCA.
Comment 7 Antoine Quint 2017-01-27 08:40:30 PST
Created attachment 299934 [details]
Patch
Comment 8 Simon Fraser (smfr) 2017-01-27 14:34:46 PST
Created attachment 299963 [details]
Patch
Comment 9 Tim Horton 2017-01-27 14:36:22 PST
Comment on attachment 299963 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        If a layer had a backgrop filter, but also corner radii that triggered using a mask layer,

BACKGROP!
Comment 10 Antoine Quint 2017-01-27 15:07:48 PST
Comment on attachment 299963 [details]
Patch

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

> LayoutTests/css3/filters/backdrop/backdrop-filter-uneven-corner-radii-expected.html:34
> +    <div class="clipped container">

"clipped" class doesn't do anything

> LayoutTests/css3/filters/backdrop/backdrop-filter-uneven-corner-radii.html:35
> +    <div class="clipped container">

"clipped" class doesn't do anything
Comment 11 Build Bot 2017-01-27 15:36:44 PST
Comment on attachment 299963 [details]
Patch

Attachment 299963 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/2960332

New failing tests:
css3/filters/backdrop/backdrop-filter-uneven-corner-radii.html
Comment 12 Build Bot 2017-01-27 15:36:48 PST
Created attachment 299970 [details]
Archive of layout-test-results from ews101 for mac-elcapitan

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-elcapitan  Platform: Mac OS X 10.11.6
Comment 13 Simon Fraser (smfr) 2017-01-27 15:46:10 PST
https://trac.webkit.org/r211305
Test fix in https://trac.webkit.org/r211307