Bug 88435 - [chromium] Skip willDraw() and didDraw() on fully occluded layers
Summary: [chromium] Skip willDraw() and didDraw() on fully occluded layers
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dana Jansens
URL:
Keywords:
Depends on:
Blocks: 88363
  Show dependency treegraph
 
Reported: 2012-06-06 11:10 PDT by Dana Jansens
Modified: 2012-06-08 14:38 PDT (History)
9 users (show)

See Also:


Attachments
Patch (9.32 KB, patch)
2012-06-06 11:24 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-04 (806.45 KB, application/zip)
2012-06-06 22:26 PDT, WebKit Review Bot
no flags Details
Patch (9.11 KB, patch)
2012-06-07 09:37 PDT, Dana Jansens
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dana Jansens 2012-06-06 11:10:25 PDT
[chromium] Skip willDraw() on occluded layers, and only call didDraw() when willDraw() was called
Comment 1 Dana Jansens 2012-06-06 11:24:13 PDT
Created attachment 146069 [details]
Patch
Comment 2 Dana Jansens 2012-06-06 11:29:07 PDT
FYI This will allow us to do the texture upload in CCVideoLayerImpl::willDraw().

I will do that in a way that the layer doesn't need to access the 3d context though, using the CCRenderer's interfaces instead.
Comment 3 WebKit Review Bot 2012-06-06 22:26:25 PDT
Comment on attachment 146069 [details]
Patch

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

New failing tests:
css3/filters/effect-brightness-hw.html
compositing/reflections/load-video-in-reflection.html
compositing/reflections/reflection-on-composited.html
platform/chromium/compositing/child-layer-3d-sorting.html
css3/filters/effect-invert-hw.html
css3/filters/effect-contrast-hw.html
compositing/reflections/reflection-ordering.html
Comment 4 WebKit Review Bot 2012-06-06 22:26:29 PDT
Created attachment 146198 [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 5 Dana Jansens 2012-06-07 09:37:27 PDT
Created attachment 146310 [details]
Patch
Comment 6 Adrienne Walker 2012-06-08 13:43:24 PDT
Comment on attachment 146310 [details]
Patch

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

R=me.  I'm curious how video frame uploads will work in an ubercomp world, but this looks good for now.

> Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp:531
> +    // Make the viewport large so that we can have large layers that get considered for occlusion (small layers do not).

Sad to have to bake these heuristic assumptions into tests.
Comment 7 Dana Jansens 2012-06-08 14:00:35 PDT
Comment on attachment 146310 [details]
Patch

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

>> Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp:531
>> +    // Make the viewport large so that we can have large layers that get considered for occlusion (small layers do not).
> 
> Sad to have to bake these heuristic assumptions into tests.

Yeh, me too.. We'd have to expose something on CCLTHImpl though otherwise which seemed too much to me. Wdyt is the right thing to do?
Comment 8 Adrienne Walker 2012-06-08 14:15:34 PDT
(In reply to comment #7)
> (From update of attachment 146310 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=146310&action=review
> 
> >> Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp:531
> >> +    // Make the viewport large so that we can have large layers that get considered for occlusion (small layers do not).
> > 
> > Sad to have to bake these heuristic assumptions into tests.
> 
> Yeh, me too.. We'd have to expose something on CCLTHImpl though otherwise which seemed too much to me. Wdyt is the right thing to do?

Talking out loud, maybe this sort of thing should be a setting, where you could have one test for the setting itself, but other tests could assume a less hairy default behavior.

I don't know that it's that important to fix; I was just remarking that it's unfortunate.
Comment 9 Dana Jansens 2012-06-08 14:24:59 PDT
Comment on attachment 146310 [details]
Patch

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

>>>> Source/WebKit/chromium/tests/CCLayerTreeHostImplTest.cpp:531
>>>> +    // Make the viewport large so that we can have large layers that get considered for occlusion (small layers do not).
>>> 
>>> Sad to have to bake these heuristic assumptions into tests.
>> 
>> Yeh, me too.. We'd have to expose something on CCLTHImpl though otherwise which seemed too much to me. Wdyt is the right thing to do?
> 
> Talking out loud, maybe this sort of thing should be a setting, where you could have one test for the setting itself, but other tests could assume a less hairy default behavior.
> 
> I don't know that it's that important to fix; I was just remarking that it's unfortunate.

Oh! A CCLayerTreeHostSetting sounds nice.. thanks for that. I'll make a bug.
Comment 10 WebKit Review Bot 2012-06-08 14:38:08 PDT
Comment on attachment 146310 [details]
Patch

Clearing flags on attachment: 146310

Committed r119867: <http://trac.webkit.org/changeset/119867>
Comment 11 WebKit Review Bot 2012-06-08 14:38:12 PDT
All reviewed patches have been landed.  Closing bug.