Bug 88435 – [chromium] Skip willDraw() and didDraw() on fully occluded layers

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dana Jansens]

[chromium] Skip willDraw() on occluded layers, and only call didDraw() when willDraw() was called

[Dana Jansens]

Created attachment 146069 [details]
Patch

[Dana Jansens]

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.

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Dana Jansens]

Created attachment 146310 [details]
Patch

[Adrienne Walker]

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.

[Dana Jansens]

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?

[Adrienne Walker]

(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.

[Dana Jansens]

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.

[WebKit Review Bot]

Comment on attachment 146310 [details]
Patch

Clearing flags on attachment: 146310

Committed r119867: <http://trac.webkit.org/changeset/119867>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.