Bug 106721 – [Compositor] Do not disable overlap testing for layers in front of 3D transformed layers

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Alexandru Chiculita]

RenderLayerCompositor::computeCompositingRequirements disables the overlap testing for layers that display in front of 3D transformed layers. That is supposed to be used by compositor driven animations, as their bounds are hard to compute in WebKit (although we could do it for simple transforms). However, 3D layer bounds can be calculated in software, so there's no need to always force the GPU layers.

[Alexandru Chiculita]

Created attachment 182643 [details]
Patch V1

[Alexandru Chiculita]

Comment on attachment 182643 [details]
Patch V1

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

> LayoutTests/compositing/layer-creation/overlap-animation-clipping.html:26
> +      -webkit-transform: translateZ(-1px);

I will revert this change. It looks like it is a bug in Safari's Compositor (as Chromium gots it right). There is no 3D rendering context, but the layers were still sorted out based on the Z component. I will log a different bug, as the test description was right about it.

[Alexandru Chiculita]

(In reply to comment #2)
> (From update of attachment 182643 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=182643&action=review
> 
> > LayoutTests/compositing/layer-creation/overlap-animation-clipping.html:26
> > +      -webkit-transform: translateZ(-1px);
> 
> I will revert this change. It looks like it is a bug in Safari's Compositor (as Chromium gots it right). There is no 3D rendering context, but the layers were still sorted out based on the Z component. I will log a different bug, as the test description was right about it.

I've added https://bugs.webkit.org/show_bug.cgi?id=106841 to track that issue.

[WebKit Review Bot]

Comment on attachment 182643 [details]
Patch V1

Attachment 182643 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15889008

New failing tests:
css3/filters/filtered-compositing-descendant.html

[Build Bot]

Comment on attachment 182643 [details]
Patch V1

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

New failing tests:
compositing/layer-creation/overlap-transformed-3d.html

[Alexandru Chiculita]

Created attachment 182814 [details]
Patch V2

Fixed the Chrome and Mac test results and removed the change in the old test that is not related to this patch.

[Simon Fraser (smfr)]

Comment on attachment 182814 [details]
Patch V2

I think this is a good change but I worry that some of the tests are no longer testing their original issue. Unfortunately fixing that would require study of each test with changed results.

[Alexandru Chiculita]

(In reply to comment #7)
> (From update of attachment 182814 [details])
> I think this is a good change but I worry that some of the tests are no longer testing their original issue. Unfortunately fixing that would require study of each test with changed results.

I've used the RenderLayer tree in the Debug menu of Safari and compared before & after. Looks like the tests are still asserting the right functionality. All the layers that were required to be composited still get a composited layer as they were forced to use a 3D transforms.

compositing/geometry/ancestor-overflow-change.html
-> #layers element has no composited layer anymore, which seems ok (no 3D or overlapping).

* compositing/geometry/clip-expected.txt:
-> second ".box" is not composited anymore, which seems ok (no 3D or overlapping). 
-> This test seems to fail with stable/nightly versions of both Safari and Chromium. The comment says both boxes should look the same, but the right one has a light gray border. Judging by the CSS properties, the right box renders correctly. It looks like when "clip" is applied on composited layers, it cannot have negative offsets (as if the bounds of the clip are intersected with the bounds of the border box). That is a separate bug.

* compositing/geometry/clip-inside-expected.txt:
-> second ".box" is not composited anymore, which seems Ok.
-> This is similar to geometry/clip, just that the clip rectangle is contained inside the border box, so the images match.

* compositing/geometry/foreground-layer-expected.txt:
-> the change in expected results was caused by the previous patch that fixed the clipping areas. Before that change, the overlap testing was re-enabled after a clipping-rectangle. With that change, the clipping-rectangle was not re-enabling the overlap testing because there was a 3D layer behind it. With the current change we get back to normal, where 3D layers will not disable the optimization in the first place.

* compositing/overflow/clip-descendents-expected.txt:
-> the first .container > .intermediate > .box is composited, so it forced the next set of ".container > .intermediate" to both become composited, as .container is not a stacking context, so they were siblings in the render layer tree.

* css3/filters/filtered-compositing-descendant-expected.txt:
-> it is the "#layer-tree" that is not composited anymore, which seems ok (no 3D or overlapping).

* compositing/layer-creation/overlap-animation-clipping-expected.txt:
-> the second gray box inside the clipped area does not need a composited layer. The first layer has a 3D transforms and that one needs a composited layer and it forced the next one too.

* compositing/layer-creation/overlap-animation-container-expected.txt:
-> in the first state, the green boxes don't need composited layers. In the second state, the first green box rotates, so it will overlap the first gray box, which is composited, so it will be forced to become composited too. Because all the green boxes overlap each other, they all become composited.

However, the following one change behavior:

* compositing/layer-creation/overlap-transforms-expected.txt:
-> It was checking that when we disable the overlap testing, the flag is not going to be propagated outside the clip-rectangle. In this case, the overlap testing is not disabled in the first place, as 3D transforms don't do that anymore.
-> The change in expected result comes from the fact that the box in the middle doesn't need a composited layer anymore.
-> I would still keep the test, as it makes sure that the overlap testing is not broken in the future.

[Simon Fraser (smfr)]

Comment on attachment 182814 [details]
Patch V2

OK, thanks for looking at the tests.

[WebKit Review Bot]

Comment on attachment 182814 [details]
Patch V2

Clearing flags on attachment: 182814

Committed r139794: <http://trac.webkit.org/changeset/139794>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.