Summary: | [Compositor] Do not disable overlap testing for layers in front of 3D transformed layers | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alexandru Chiculita <achicu> | ||||||
Component: | Layout and Rendering | Assignee: | Alexandru Chiculita <achicu> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | buildbot, dglazkov, eric, leviw, ojan.autocc, ojan, rniwa, simon.fraser, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Alexandru Chiculita
2013-01-11 17:06:07 PST
Created attachment 182643 [details]
Patch V1
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. (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. 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 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 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.
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.
(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. Comment on attachment 182814 [details]
Patch V2
OK, thanks for looking at the tests.
Comment on attachment 182814 [details] Patch V2 Clearing flags on attachment: 182814 Committed r139794: <http://trac.webkit.org/changeset/139794> All reviewed patches have been landed. Closing bug. |