Bug 62487 – Keep overlap testing for compositing on pages with 3d transforms when possible

• Status: ASSIGNED
• Component: Layout and Rendering

[Simon Fraser (smfr)]

We should try to keep overlap testing on pages with actual 3d transforms. One way to do this would be to consider overflow:hidden elements are boundaries between which you keep overlap testing.

Sadly overflow:hidden doesn't create stacking context, so you might have to only do it if the element with overflow is also a stacking context.

[Radar WebKit Bug Importer]

<rdar://problem/11333154>

[Simon Fraser (smfr)]

<rdar://problem/7749080>

[Simon Fraser (smfr)]

Created attachment 139947 [details]
Patch

[Simon Fraser (smfr)]

This patch only handles the case where overflow elements already are stacking contexts for other reasons

[WebKit Review Bot]

Comment on attachment 139947 [details]
Patch

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

New failing tests:
compositing/geometry/foreground-layer.html
compositing/overflow/clip-descendents.html
compositing/geometry/ancestor-overflow-change.html
compositing/iframes/invisible-nested-iframe-show.html

[WebKit Review Bot]

Created attachment 139951 [details]
Archive of layout-test-results from ec2-cr-linux-04

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-04  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Simon Fraser (smfr)]

Why does chromium-ews reliably fail *every* new test added by a patch?

[Eric Seidel (no email)]

It looks like these are real failures?  I assume layers just work different on chromium?  James?

[Simon Fraser (smfr)]

cr-linux will need new baselines. Looks like cr-linux has its own expected results because of a few pixel diffs:

 22$ $ diff geometry/ancestor-overflow-change-expected.txt /Volumes/DataSSD/Development/apple/webkit/WebKit.git/LayoutTests/compositing/geometry/ancestor-overflow-change-expected.txt 
16c16
<           (bounds 788.00 20.00)
---
>           (bounds 788.00 19.00)

We should strive to avoid those.

[Antti Koivisto]

Comment on attachment 139947 [details]
Patch

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

r=me

> Source/WebCore/ChangeLog:43
> +        (WebCore::RenderLayerCompositor::hasNonIdentity3DTransform): No longer check
> +        perspective here, since that doesn't affect whether _this_ layer should disable
> +        overlap testing. Checking for a non-affine transform is sufficient.

What does overlap testing have to do with whether a transform is non-identity? Is this method misnamed?

> Source/WebCore/rendering/RenderLayerCompositor.cpp:1335
> +    // FIXME: remove this method.
>  }

Why not just remove it?

[Simon Fraser (smfr)]

Comment on attachment 139947 [details]
Patch

http://trac.webkit.org/changeset/115989

I'll keep the bug open to deal with non-stacking context overflow:hidden.

[Simon Fraser (smfr)]

(In reply to comment #10)
> (From update of attachment 139947 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=139947&action=review
> 
> r=me
> 
> > Source/WebCore/ChangeLog:43
> > +        (WebCore::RenderLayerCompositor::hasNonIdentity3DTransform): No longer check
> > +        perspective here, since that doesn't affect whether _this_ layer should disable
> > +        overlap testing. Checking for a non-affine transform is sufficient.
> 
> What does overlap testing have to do with whether a transform is non-identity? Is this method misnamed?

Yes, I renamed it.
 
> > Source/WebCore/rendering/RenderLayerCompositor.cpp:1335
> > +    // FIXME: remove this method.
> >  }
> 
> Why not just remove it?

I will in a separate commit.

[WebKit Review Bot]

Comment on attachment 139947 [details]
Patch

Cleared Antti Koivisto's review+ from obsolete attachment 139947 [details] so that this bug does not appear in http://webkit.org/pending-commit.