Bug 62487 - Keep overlap testing for compositing on pages with 3d transforms when possible
Summary: Keep overlap testing for compositing on pages with 3d transforms when possible
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-06-10 15:40 PDT by Simon Fraser (smfr)
Modified: 2012-07-27 05:03 PDT (History)
10 users (show)

See Also:


Attachments
Patch (22.29 KB, patch)
2012-05-02 21:43 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-04 (6.13 MB, application/zip)
2012-05-02 22:14 PDT, WebKit Review Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2011-06-10 15:40:52 PDT
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.
Comment 1 Radar WebKit Bug Importer 2012-04-26 20:57:11 PDT
<rdar://problem/11333154>
Comment 2 Simon Fraser (smfr) 2012-04-26 20:59:02 PDT
<rdar://problem/7749080>
Comment 3 Simon Fraser (smfr) 2012-05-02 21:43:13 PDT
Created attachment 139947 [details]
Patch
Comment 4 Simon Fraser (smfr) 2012-05-02 21:46:18 PDT
This patch only handles the case where overflow elements already are stacking contexts for other reasons
Comment 5 WebKit Review Bot 2012-05-02 22:13:54 PDT
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
Comment 6 WebKit Review Bot 2012-05-02 22:14:01 PDT
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
Comment 7 Simon Fraser (smfr) 2012-05-02 22:20:45 PDT
Why does chromium-ews reliably fail *every* new test added by a patch?
Comment 8 Eric Seidel (no email) 2012-05-02 22:23:39 PDT
It looks like these are real failures?  I assume layers just work different on chromium?  James?
Comment 9 Simon Fraser (smfr) 2012-05-02 22:29:40 PDT
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.
Comment 10 Antti Koivisto 2012-05-03 10:03:49 PDT
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?
Comment 11 Simon Fraser (smfr) 2012-05-03 10:49:47 PDT
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.
Comment 12 Simon Fraser (smfr) 2012-05-03 10:50:09 PDT
(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.
Comment 13 WebKit Review Bot 2012-07-27 05:03:17 PDT
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.