Compute damage rect for Cast video compositing

Compute damage rect for Cast video compositing

During Cast video playback (using underlay), we can save substantial
power by providing a damage rect that indicates which areas of the
GL buffer actually need drawing.  Intuitively, this is just the area
of subtitles or other graphics occluding the video.  The system will
just read from the video underlay outside this area (and default to
black outside the video's rect).

BUG=internal b/31264129
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

[halliwell, 2016-12-14 03:27:51]

Description was changed from

==========
Compute damage rect for Cast video compositing

During Cast video playback (using underlay), we can save substantial
power by providing a damage rect that indicates which areas of the
GL buffer actually need drawing.  Intuitively, this is just the area
of subtitles or other graphics occluding the video.  The system will
just read from the video underlay outside this area (and default to
black outside the video's rect).

BUG=internal b/31264129
==========

to

==========
Compute damage rect for Cast video compositing

During Cast video playback (using underlay), we can save substantial
power by providing a damage rect that indicates which areas of the
GL buffer actually need drawing.  Intuitively, this is just the area
of subtitles or other graphics occluding the video.  The system will
just read from the video underlay outside this area (and default to
black outside the video's rect).

BUG=internal b/31264129
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[halliwell, 2016-12-14 03:30:53]

Description was changed from

==========
Compute damage rect for Cast video compositing

During Cast video playback (using underlay), we can save substantial
power by providing a damage rect that indicates which areas of the
GL buffer actually need drawing.  Intuitively, this is just the area
of subtitles or other graphics occluding the video.  The system will
just read from the video underlay outside this area (and default to
black outside the video's rect).

BUG=internal b/31264129
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Compute damage rect for Cast video compositing

During Cast video playback (using underlay), we can save substantial
power by providing a damage rect that indicates which areas of the
GL buffer actually need drawing.  Intuitively, this is just the area
of subtitles or other graphics occluding the video.  The system will
just read from the video underlay outside this area (and default to
black outside the video's rect).

BUG=internal b/31264129
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[halliwell, 2016-12-14 03:30:53]

halliwell@chromium.org changed reviewers:
+ ccameron@chromium.org, danakj@chromium.org

[ccameron, 2016-12-20 20:56:00]

[back from OOO, sorry for the delay]

We have a lot of infrastructure for supporting partial swap with overlays
already, and I'd like to reuse it. But ... this isn't exactly PostSubBuffer ...
so my in inclination is the opposite of what piman asked in
https://codereview.chromium.org/2302393002.

That said, what I'd like is as follows:

1. Have a cc::RendererSettings::partial_swap_needs_full_draw bit

2. In DirectRenderer::DrawFrame, at
https://cs.chromium.org/chromium/src/cc/output/direct_renderer.cc?rcl=0&l=302,
have: 

  gfx::Rect swap_with_damage_rect = frame.root_damage_rect;
  if (settings_->partial_swap_needs_full_draw) {
    frame.root_damage_rect = root_render_pass->output_rect;

3. And an argument to cc::Renderer::FinishDrawingFrame, "gfx::Rect
swap_with_damage_rect"

4. In cc::GLRenderer::FinishDrawingFrame, have
"swap_buffer_rect_.Union(swap_with_damage_rect)" instead of using
frame.root_damage_rect.

5. In GpuBrowserCompositorOutputSurface::SwapBuffers
- also add the "gfx::Rect swap_with_damage_rect" argument
- DCHECK that it contains frame.sub_buffer_rect
- use that instead of frame.swap_with_damage_rect for the diff ~line 108

Then you don't need any of the changes to cc/output/overlay_* or
components/display_compostor/* changes, or the other parts in GLRenderer.

WDYT? I may have understood the problem wrong -- LMK if you want to quickly VC
if what I said amounts to nonsense.

[halliwell, 2016-12-20 21:38:43]

On 2016/12/20 20:56:00, ccameron wrote:
> [back from OOO, sorry for the delay]
> 
> We have a lot of infrastructure for supporting partial swap with overlays
> already, and I'd like to reuse it. But ... this isn't exactly PostSubBuffer
...
> so my in inclination is the opposite of what piman asked in
> https://codereview.chromium.org/2302393002.
> 
> That said, what I'd like is as follows:
> 
> 1. Have a cc::RendererSettings::partial_swap_needs_full_draw bit
> 
> 2. In DirectRenderer::DrawFrame, at
> https://cs.chromium.org/chromium/src/cc/output/direct_renderer.cc?rcl=0&l=302,
> have: 
> 
>   gfx::Rect swap_with_damage_rect = frame.root_damage_rect;
>   if (settings_->partial_swap_needs_full_draw) {
>     frame.root_damage_rect = root_render_pass->output_rect;
> 
> 3. And an argument to cc::Renderer::FinishDrawingFrame, "gfx::Rect
> swap_with_damage_rect"
> 
> 4. In cc::GLRenderer::FinishDrawingFrame, have
> "swap_buffer_rect_.Union(swap_with_damage_rect)" instead of using
> frame.root_damage_rect.
> 
> 5. In GpuBrowserCompositorOutputSurface::SwapBuffers
> - also add the "gfx::Rect swap_with_damage_rect" argument
> - DCHECK that it contains frame.sub_buffer_rect
> - use that instead of frame.swap_with_damage_rect for the diff ~line 108
> 
> Then you don't need any of the changes to cc/output/overlay_* or
> components/display_compostor/* changes, or the other parts in GLRenderer.
> 
> WDYT? I may have understood the problem wrong -- LMK if you want to quickly VC
> if what I said amounts to nonsense.

Thanks, that sounds promising, VC sounds useful though to go through the details
:)

[danakj, 2017-01-19 19:20:52]

Did this move forward?

[halliwell, 2017-01-19 19:38:21]

On 2017/01/19 19:20:52, danakj (slow) wrote:
> Did this move forward?

No ... it got lost in holidays :)

Rather than reviewing the code, I'd appreciate your thoughts on a doc that I
threw together, just adding you now.  It seems like what we're doing isn't
actually a damage rect, and I'm not clear on the best way to integrate that with
Chromium.