Preliminary compositor disabling patch.

Preliminary compositor disabling patch.

This is going to get split up, tests added, etc

[achaulk, 2015-03-31 22:47:46]

achaulk@chromium.org changed reviewers:
+ danakj@chromium.org

[achaulk, 2015-03-31 22:47:47]

Here's a preliminary compositor disabling patch. I ended up having a single rect
in the render pass & damage tracker to represent all overlayable damage to make
it easy to access, while keeping the flag in the quad so it can be recalculated
after we take out quads. Since the only major change is splitting damage off
from damage_rect, I took a look at all the non-test uses of it. If we're just
reporting damage rects to elsewhere then I pass the combined rect, since that's
more accurate, if it's used on the rendering path then I use them separately.

A layer that wants to produce overlay-aware quads would call
SetOverlayUpdateRect instead and then produce quads with the flag set. After the
overlay implementation decides what it's doing, DirectRenderer collapses the
overlay damage into the regular damage rect and draws normally

I'm just looking for a quick review of the overall structure, I didn't want to
bother updating all the tests if I'm going to have to rewrite it

[danakj, 2015-04-03 22:10:17]

Hey,

some high level thoughts about where/how this is being computed.

https://codereview.chromium.org/1044093005/diff/20001/cc/layers/video_layer_i...
File cc/layers/video_layer_impl.cc (right):

https://codereview.chromium.org/1044093005/diff/20001/cc/layers/video_layer_i...
cc/layers/video_layer_impl.cc:130: video_can_overlay_ =
resource_provider->AllowOverlay(resource_id);
this is in a loop, so it would only take the value of the last resource?

https://codereview.chromium.org/1044093005/diff/20001/cc/quads/render_pass.cc
File cc/quads/render_pass.cc (right):

https://codereview.chromium.org/1044093005/diff/20001/cc/quads/render_pass.cc...
cc/quads/render_pass.cc:275: void RenderPass::RecalculateOverlayDamage() {
I don't like us computing damage inside RenderPass. I think all damage should be
computed during via DamageTracker. If it needs to output multiple damage rects
(surface_damage_rect, overlay_damage_rects[]) that's okay, but I want them to
all be computed there.

From looking at this code, it sounds like one overlay rect would be enough so
damage tracker would just need to output 2 rects?

This could also remove the overlay_damage_rect from LayerImpl? DamageTracker
would get some bools from layers about overlays as needed and put rects in the
right places. Duplicating the damage rect on LayerImpl seems like the wrong
place too.

https://codereview.chromium.org/1044093005/diff/20001/cc/quads/render_pass.cc...
cc/quads/render_pass.cc:288: damage_rect.Union(overlay_rect);
Not a fan of mutating RenderPass after construction like this. But I think this
falls into the above comment.

[achaulk, 2015-04-03 23:41:28]

https://codereview.chromium.org/1044093005/diff/20001/cc/quads/render_pass.cc
File cc/quads/render_pass.cc (right):

https://codereview.chromium.org/1044093005/diff/20001/cc/quads/render_pass.cc...
cc/quads/render_pass.cc:275: void RenderPass::RecalculateOverlayDamage() {
On 2015/04/03 22:10:17, danakj wrote:
> I don't like us computing damage inside RenderPass. I think all damage should
be
> computed during via DamageTracker. If it needs to output multiple damage rects
> (surface_damage_rect, overlay_damage_rects[]) that's okay, but I want them to
> all be computed there.
> 
> From looking at this code, it sounds like one overlay rect would be enough so
> damage tracker would just need to output 2 rects?
> 
> This could also remove the overlay_damage_rect from LayerImpl? DamageTracker
> would get some bools from layers about overlays as needed and put rects in the
> right places. Duplicating the damage rect on LayerImpl seems like the wrong
> place too.

We would at least need to be able to do it after modifying the quad list during
the overlay path. This plus the initial call to calculate it are the only places
its done.

If we have lists of damage rects then we need to associate quads with the
specific damage rect in the renderpass which feels awkward too

[achaulk, 2015-04-07 18:53:00]

https://codereview.chromium.org/1044093005/diff/20001/cc/layers/video_layer_i...
File cc/layers/video_layer_impl.cc (right):

https://codereview.chromium.org/1044093005/diff/20001/cc/layers/video_layer_i...
cc/layers/video_layer_impl.cc:130: video_can_overlay_ =
resource_provider->AllowOverlay(resource_id);
On 2015/04/03 22:10:17, danakj wrote:
> this is in a loop, so it would only take the value of the last resource?

Ah, yeah I'll probably just move it outside the loop and take the first one. If
only some of the resources are overlayable then you just won't get much of a
performance boost. And if something really isn't overlayable and we say it might
be, then it will just get handled specially along the way, it won't affect the
output

https://codereview.chromium.org/1044093005/diff/20001/cc/quads/render_pass.cc
File cc/quads/render_pass.cc (right):

https://codereview.chromium.org/1044093005/diff/20001/cc/quads/render_pass.cc...
cc/quads/render_pass.cc:288: damage_rect.Union(overlay_rect);
On 2015/04/03 22:10:17, danakj wrote:
> Not a fan of mutating RenderPass after construction like this. But I think
this
> falls into the above comment.

I'll see how much of it I can put into the damage tracker

[achaulk, 2015-04-14 20:18:36]

So, the closest I think I can get to what you want is to move all the
recalculation out of RenderPass itself and do it only in the overlay strategy,
though it's only shuffling around the existing code. The DamageTracker doesn't
leave LTHI, so we couldn't use that

[achaulk, 2015-04-15 18:30:54]

Okay, so I've moved the recalculation out into the overlay code and I'm pulling
the overlay damage out of DamageTracker now

[enne (OOO), 2015-04-20 17:37:14]

enne@chromium.org changed reviewers:
+ enne@chromium.org

[achaulk, 2015-06-17 18:23:00]

This seems to have fallen by the wayside, but it's still something we should try
to get in. I'm not sure if I can make it much cleaner than it is though