Parent stacking context of a backdrop filter should isolate itself

Parent stacking context of a backdrop filter should isolate itself

As spec'd by
https://drafts.fxtf.org/filters-2/#BackdropFilterProperty
https://drafts.fxtf.org/filters/#valdef-in-backgroundimage

BUG=497522
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

[trchen, 2016-09-29 00:29:06]

Description was changed from

==========
Parent stacking context of a backdrop filter should isolate itself

As spec'd by
https://drafts.fxtf.org/filters-2/#BackdropFilterProperty
https://drafts.fxtf.org/filters/#valdef-in-backgroundimage

BUG=497522
==========

to

==========
Parent stacking context of a backdrop filter should isolate itself

As spec'd by
https://drafts.fxtf.org/filters-2/#BackdropFilterProperty
https://drafts.fxtf.org/filters/#valdef-in-backgroundimage

BUG=497522
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[trchen, 2016-09-29 00:29:15]

The CQ bit was checked by trchen@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-29 00:29:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[trchen, 2016-09-29 00:33:51]

trchen@chromium.org changed reviewers:
+ senorblanco@chromium.org

[trchen, 2016-09-29 00:33:51]

BIG FAT WARNING: This CL can create compatibility issues against WebKit. 

WebKit behavior is erratic. Basically a stacking context creates isolation group
only if it happens to create a render pass. (e.g. opacity/clip with multiple
compositing descendants) IMO WebKit should either fix their behavior or add
quirks to the spec.

[commit-bot: I haz the power, 2016-09-29 01:59:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-29 01:59:15]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[pdr., 2016-09-29 21:54:48]

pdr@chromium.org changed reviewers:
+ pdr@chromium.org

[pdr., 2016-09-29 21:54:49]

Drive-by review

https://codereview.chromium.org/2375203003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/css3/filters/backdrop-filter-isolation.html
(right):

https://codereview.chromium.org/2375203003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/css3/filters/backdrop-filter-isolation.html:4:
<div style="width:100px; height:100px; backdrop-filter:invert(1);"></div>
Does this test fail without this patch?

https://codereview.chromium.org/2375203003/diff/1/third_party/WebKit/Source/c...
File
third_party/WebKit/Source/core/layout/compositing/CompositingRequirementsUpdater.cpp
(right):

https://codereview.chromium.org/2375203003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/compositing/CompositingRequirementsUpdater.cpp:431:
if (willBeCompositedOrSquashed &&
(layer->layoutObject()->style()->hasBlendMode() ||
layer->layoutObject()->style()->hasBackdropFilter()))
Since webkit has the only shipping implementation of backdrop filter, I think it
makes sense to fix them too. Can you file a bug in bugs.webkit.org?

[trchen, 2016-09-29 22:23:34]

https://codereview.chromium.org/2375203003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/css3/filters/backdrop-filter-isolation.html
(right):

https://codereview.chromium.org/2375203003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/css3/filters/backdrop-filter-isolation.html:4:
<div style="width:100px; height:100px; backdrop-filter:invert(1);"></div>
On 2016/09/29 21:54:49, pdr. wrote:
> Does this test fail without this patch?

Depends on another bug fix: https://codereview.chromium.org/2379803003/
Otherwise backdrop-filter here got automatically ignored. :(

https://codereview.chromium.org/2375203003/diff/1/third_party/WebKit/Source/c...
File
third_party/WebKit/Source/core/layout/compositing/CompositingRequirementsUpdater.cpp
(right):

https://codereview.chromium.org/2375203003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/compositing/CompositingRequirementsUpdater.cpp:431:
if (willBeCompositedOrSquashed &&
(layer->layoutObject()->style()->hasBlendMode() ||
layer->layoutObject()->style()->hasBackdropFilter()))
On 2016/09/29 21:54:49, pdr. wrote:
> Since webkit has the only shipping implementation of backdrop filter, I think
it
> makes sense to fix them too. Can you file a bug in bugs.webkit.org?

Sure! I'm doing now.

[trchen, 2016-09-30 00:38:44]

Description was changed from

==========
Parent stacking context of a backdrop filter should isolate itself

As spec'd by
https://drafts.fxtf.org/filters-2/#BackdropFilterProperty
https://drafts.fxtf.org/filters/#valdef-in-backgroundimage

BUG=497522
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

to

==========
Parent stacking context of a backdrop filter should isolate itself

As spec'd by
https://drafts.fxtf.org/filters-2/#BackdropFilterProperty
https://drafts.fxtf.org/filters/#valdef-in-backgroundimage

BUG=497522
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
==========

[trchen, 2016-09-30 00:43:04]

WebKit folks clarified that the spec needs to be revised to match actual
behavior.

Their intended behavior can't be easily represented in SPv2 data model. Probably
need a special object like BackdropDrawingDisplayItem. Let's punt it until other
things are finished.

[pdr., 2016-09-30 01:57:13]

On 2016/09/30 at 00:43:04, trchen wrote:
> WebKit folks clarified that the spec needs to be revised to match actual
behavior.
> 
> Their intended behavior can't be easily represented in SPv2 data model.
Probably need a special object like BackdropDrawingDisplayItem. Let's punt it
until other things are finished.

Can you explain why a little more why it doesn't work with spv2? Could spv2 use
a filter?

I didn't mean to step on your review. If this makes it better for spv1, lets go
ahead with the patch.

[trchen, 2016-09-30 17:21:23]

On 2016/09/30 01:57:13, pdr. wrote:
> On 2016/09/30 at 00:43:04, trchen wrote:
> > WebKit folks clarified that the spec needs to be revised to match actual
> behavior.
> > 
> > Their intended behavior can't be easily represented in SPv2 data model.
> Probably need a special object like BackdropDrawingDisplayItem. Let's punt it
> until other things are finished.
> 
> Can you explain why a little more why it doesn't work with spv2? Could spv2
use
> a filter?

Regular CSS filter property is fine. Advanced SVG filter (did we support it
anyway?) will be subject to the same problem.

It won't work because the backdrop filter needed some way to read back from
unfinished render surfaces. In SPv1 it is indicated using a layer property, then
the GL renderer would do special things when it sees quads from the layer. (And
this caused a know bug! Backdrop filter should be bounded by the border box, not
the visual rect of whatever composited into this layer.)

Actually now I think in SPv2 we can achieve similar thing using a foreign layer.
Perhaps we can make it a convention to use foreign layer for all irregular
drawing display items?

> I didn't mean to step on your review. If this makes it better for spv1, lets
go
> ahead with the patch.

Nah, the "corrected" behavior is not desired by WebKit/spec authors.

[Stephen White, 2016-09-30 18:05:28]

On 2016/09/30 17:21:23, trchen wrote:
> On 2016/09/30 01:57:13, pdr. wrote:
> > On 2016/09/30 at 00:43:04, trchen wrote:
> > > WebKit folks clarified that the spec needs to be revised to match actual
> > behavior.
> > > 
> > > Their intended behavior can't be easily represented in SPv2 data model.
> > Probably need a special object like BackdropDrawingDisplayItem. Let's punt
it
> > until other things are finished.
> > 
> > Can you explain why a little more why it doesn't work with spv2? Could spv2
> use
> > a filter?
> 
> Regular CSS filter property is fine. Advanced SVG filter (did we support it
> anyway?) will be subject to the same problem.
> 
> It won't work because the backdrop filter needed some way to read back from
> unfinished render surfaces. In SPv1 it is indicated using a layer property,
then
> the GL renderer would do special things when it sees quads from the layer.
(And
> this caused a know bug! Backdrop filter should be bounded by the border box,
not
> the visual rect of whatever composited into this layer.)
> 
> Actually now I think in SPv2 we can achieve similar thing using a foreign
layer.
> Perhaps we can make it a convention to use foreign layer for all irregular
> drawing display items?
> 
> > I didn't mean to step on your review. If this makes it better for spv1, lets
> go
> > ahead with the patch.
> 
> Nah, the "corrected" behavior is not desired by WebKit/spec authors.

Thanks for reaching out to WebKit to clarify.

Tangentially,, I'd like to reserve the possibility of implementing a "software"
(non-cc) path for backdrop-filter 
at some point. This would bring it in line with the (foreground) filters
implementation, and would mean we'd
lean less on cc for this feature.