Mac Overlays: Add sandwich strategy

Mac Overlays: Add sandwich strategy

On Mac, there can be arbitrary layering of overlays, which allows
for more flexiable strategies.

In this strategy, we create a sandwich of 3 layers
- The bottom bread is the output surface's main image.
- The meat is the video overlay. In principle, there could be an
  arbitrary number of overlays in the middle. They must be opaque, and
  are placed on top of the main image.
- The top bread is again the output surface's main image, this time
  cropped to only include the damaged regions on top of the video
  overlays. In principle, we should have as many small quads as possible
  for this top layer (so that we do as little blending as possible). In
  practice, the inflexibility of the QuadList structure makes having
  more than one somewhat complicated.

On Mac, all overlays must be DIP aligned, because CALayer frames are
specified in DIPs, so plumb this down to the OverlayStrategy. Note that
it is not sufficient to put it in the OverlayValidator, because the
alignment of the overlays created by the OverlayStrategy needs to take
this into account.

Using the output surface's main image as the content for an overlay also
doesn't quite fit in the OverlayCandidate structure because the main
image doesn't have a ResourceId associated with it (only, rather, a
TextureId). Create a tag for "main image" OverlayCandidates, and pull
out the TextureId from the output surface in the GLRenderer at the last
moment (note that we haven't decided the TextureId when the strategy is
determined.

Break OverlayStrategyCommon's one virtual method into a
OverlayStrategyCommonDelegate, since the derived classes share no state
with their superclass.

Enumerate the strategies in the overlay candidate validator so that we
have a place to make Mac only use the sandwich validator (and ensure no
other platforms attempt it).

BUG=522627
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/cf57c8d56ce3562cd6d4707394197dfb007af040
Cr-Commit-Position: refs/heads/master@{#345745}

[ccameron, 2015-08-23 21:18:25]

ccameron@chromium.org changed reviewers:
+ alexst@chromium.org

[ccameron, 2015-08-23 21:29:05]

This isn't ready for a full review -- I just want something to the effect of
"yes, the general strategies are good" or "no they aren't, change it to this".

In particular, I'd like to turn this into 2 patches
1. Add the RendererState to OverlayStrategy and actually handle the primary
plane in GLRenderer (for all strategies).
2. Add the OverlayStrategySandwich for Mac.

https://codereview.chromium.org/1304303002/diff/20001/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/1304303002/diff/20001/cc/output/gl_renderer.c...
cc/output/gl_renderer.cc:3525: unsigned texture_id = 0;
This is the bit where we pull the texture out from the OutputSurface. If we
handled the primary plane here, this would be simpler (is there a reason we
don't -- could we pack more stuff in the RendererState so that we could create a
complete description of the primary plane and unify all of this? or is there
something more going on?).

https://codereview.chromium.org/1304303002/diff/20001/cc/output/overlay_proce...
File cc/output/overlay_processor.cc (right):

https://codereview.chromium.org/1304303002/diff/20001/cc/output/overlay_proce...
cc/output/overlay_processor.cc:28: #if defined(OS_MACOSX)
Is this (the #ifdef) a reasonable way to go about this? Or would you prefer
something more dynamic.

I will not want to use the other strategies on Mac (Sandwich handles both).

https://codereview.chromium.org/1304303002/diff/20001/cc/output/overlay_strat...
File cc/output/overlay_strategy_sandwich.cc (right):

https://codereview.chromium.org/1304303002/diff/20001/cc/output/overlay_strat...
cc/output/overlay_strategy_sandwich.cc:99: // TODO(ccameron): Create an
OverlayCandidate for each rect in the region.
This will require cleaning up ListContainerBase a bit ... that's a project for a
separate CL.

[alexst (slow to review), 2015-08-24 15:07:02]

https://codereview.chromium.org/1304303002/diff/20001/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/1304303002/diff/20001/cc/output/gl_renderer.c...
cc/output/gl_renderer.cc:3525: unsigned texture_id = 0;
On 2015/08/23 21:29:05, ccameron wrote:
> This is the bit where we pull the texture out from the OutputSurface. If we
> handled the primary plane here, this would be simpler (is there a reason we
> don't -- could we pack more stuff in the RendererState so that we could create
a
> complete description of the primary plane and unify all of this? or is there
> something more going on?).

Some background. In theory, we could have overlays with a regular old egl
surface. In that case, we don't really have a resource or texture for the
primary plane, so we assume the gpu process knows how to get it lower in the
platform and we only schedule overlays.

With surfaceless, which is what osx and cros are now using, we do have a
resource for the primary plane, and I think it probably makes sense to refactor
things a bit so that it shows up here along with other overlay planes instead of
being scheduled separately. 

There is nothing fancy going on, just that this code existed before surfaceless.

https://codereview.chromium.org/1304303002/diff/20001/cc/output/overlay_proce...
File cc/output/overlay_processor.cc (right):

https://codereview.chromium.org/1304303002/diff/20001/cc/output/overlay_proce...
cc/output/overlay_processor.cc:28: #if defined(OS_MACOSX)
On 2015/08/23 21:29:05, ccameron wrote:
> Is this (the #ifdef) a reasonable way to go about this? Or would you prefer
> something more dynamic.
> 
> I will not want to use the other strategies on Mac (Sandwich handles both).

I hope to keep this platform agnostic. Maybe we could ask the output surface for
it's strategy list if possible? That way things will be hidden in the code
that's already OSX specific.

[ccameron, 2015-08-25 01:52:40]

Okay, this is ready for review now.

I didn't bother forcing the main image to be scheduled, cause it further-bloats
an already-bloated CL (that already needs to be merged...). Did the rest of the
refactoring (and then some).

[alexst (slow to review), 2015-08-25 18:49:38]

looks good, just some nits.

https://codereview.chromium.org/1304303002/diff/120001/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/1304303002/diff/120001/cc/output/gl_renderer....
cc/output/gl_renderer.cc:3523: texture_id =
output_surface_->GetOverlayTextureId();
DCHECK that this is not 0 because the surface has to support it.

https://codereview.chromium.org/1304303002/diff/120001/cc/output/output_surfa...
File cc/output/output_surface.h (right):

https://codereview.chromium.org/1304303002/diff/120001/cc/output/output_surfa...
cc/output/output_surface.h:123: gfx::Size SurfaceSize() const { return
surface_size_; }
surface_size(), hacker style since it's simple, although I imagine it will
trickle to other places.

https://codereview.chromium.org/1304303002/diff/120001/cc/output/overlay_stra...
File cc/output/overlay_strategy_sandwich.cc (right):

https://codereview.chromium.org/1304303002/diff/120001/cc/output/overlay_stra...
cc/output/overlay_strategy_sandwich.cc:55: {
Why the extra scope, there is nothing in there that's scope specific that I can
see?

https://codereview.chromium.org/1304303002/diff/120001/cc/output/overlay_stra...
cc/output/overlay_strategy_sandwich.cc:59: if (candidate_pixel_rect !=
candidate_pixel_rect_float)
Is rect and recf directly comparable like that?

[danakj, 2015-08-25 18:50:42]

https://codereview.chromium.org/1304303002/diff/120001/cc/output/overlay_stra...
File cc/output/overlay_strategy_sandwich.cc (right):

https://codereview.chromium.org/1304303002/diff/120001/cc/output/overlay_stra...
cc/output/overlay_strategy_sandwich.cc:59: if (candidate_pixel_rect !=
candidate_pixel_rect_float)
On 2015/08/25 18:49:38, alexst wrote:
> Is rect and recf directly comparable like that?

The ints will become floats. That's lossy tho fwiw. Sad implicit conversion
panda.

[ccameron, 2015-08-26 21:36:13]

Thanks! Updated.

https://codereview.chromium.org/1304303002/diff/120001/cc/output/gl_renderer.cc
File cc/output/gl_renderer.cc (right):

https://codereview.chromium.org/1304303002/diff/120001/cc/output/gl_renderer....
cc/output/gl_renderer.cc:3523: texture_id =
output_surface_->GetOverlayTextureId();
On 2015/08/25 18:49:38, alexst wrote:
> DCHECK that this is not 0 because the surface has to support it.

Done.

https://codereview.chromium.org/1304303002/diff/120001/cc/output/output_surfa...
File cc/output/output_surface.h (right):

https://codereview.chromium.org/1304303002/diff/120001/cc/output/output_surfa...
cc/output/output_surface.h:123: gfx::Size SurfaceSize() const { return
surface_size_; }
On 2015/08/25 18:49:38, alexst wrote:
> surface_size(), hacker style since it's simple, although I imagine it will
> trickle to other places.

I agree -- mind if I do this in a separate CL (cause this guy is going to need a
merge to M46)?

https://codereview.chromium.org/1304303002/diff/120001/cc/output/overlay_stra...
File cc/output/overlay_strategy_sandwich.cc (right):

https://codereview.chromium.org/1304303002/diff/120001/cc/output/overlay_stra...
cc/output/overlay_strategy_sandwich.cc:55: {
On 2015/08/25 18:49:38, alexst wrote:
> Why the extra scope, there is nothing in there that's scope specific that I
can
> see?

Just a style choice -- removed.

https://codereview.chromium.org/1304303002/diff/120001/cc/output/overlay_stra...
cc/output/overlay_strategy_sandwich.cc:59: if (candidate_pixel_rect !=
candidate_pixel_rect_float)
On 2015/08/25 18:50:42, danakj wrote:
> On 2015/08/25 18:49:38, alexst wrote:
> > Is rect and recf directly comparable like that?
> 
> The ints will become floats. That's lossy tho fwiw. Sad implicit conversion
> panda.

True -- the thing I'm checking here is that candidate_pixel_rect_float is
actually representable as integers (might be a nice utility function to have
somewhere).

[alexst (slow to review), 2015-08-26 21:54:18]

lgtm

https://codereview.chromium.org/1304303002/diff/120001/cc/output/output_surfa...
File cc/output/output_surface.h (right):

https://codereview.chromium.org/1304303002/diff/120001/cc/output/output_surfa...
cc/output/output_surface.h:123: gfx::Size SurfaceSize() const { return
surface_size_; }
On 2015/08/26 21:36:13, ccameron wrote:
> On 2015/08/25 18:49:38, alexst wrote:
> > surface_size(), hacker style since it's simple, although I imagine it will
> > trickle to other places.
> 
> I agree -- mind if I do this in a separate CL (cause this guy is going to need
a
> merge to M46)?

No problem, probably better in a separate CL anyway to reduce noise.

https://codereview.chromium.org/1304303002/diff/120001/cc/output/overlay_stra...
File cc/output/overlay_strategy_sandwich.cc (right):

https://codereview.chromium.org/1304303002/diff/120001/cc/output/overlay_stra...
cc/output/overlay_strategy_sandwich.cc:59: if (candidate_pixel_rect !=
candidate_pixel_rect_float)
On 2015/08/26 21:36:13, ccameron wrote:
> On 2015/08/25 18:50:42, danakj wrote:
> > On 2015/08/25 18:49:38, alexst wrote:
> > > Is rect and recf directly comparable like that?
> > 
> > The ints will become floats. That's lossy tho fwiw. Sad implicit conversion
> > panda.
> 
> True -- the thing I'm checking here is that candidate_pixel_rect_float is
> actually representable as integers (might be a nice utility function to have
> somewhere).

Maybe promote your comment here to the code. When I see type mismatches like
this I always ponder, and what you said clarifies it.

// Check that |candidate_pixel_rect_float| is actually representable as
integers.

[ccameron, 2015-08-26 22:52:58]

Thanks!

https://codereview.chromium.org/1304303002/diff/120001/cc/output/overlay_stra...
File cc/output/overlay_strategy_sandwich.cc (right):

https://codereview.chromium.org/1304303002/diff/120001/cc/output/overlay_stra...
cc/output/overlay_strategy_sandwich.cc:59: if (candidate_pixel_rect !=
candidate_pixel_rect_float)
On 2015/08/26 21:54:18, alexst wrote:
> On 2015/08/26 21:36:13, ccameron wrote:
> > On 2015/08/25 18:50:42, danakj wrote:
> > > On 2015/08/25 18:49:38, alexst wrote:
> > > > Is rect and recf directly comparable like that?
> > > 
> > > The ints will become floats. That's lossy tho fwiw. Sad implicit
conversion
> > > panda.
> > 
> > True -- the thing I'm checking here is that candidate_pixel_rect_float is
> > actually representable as integers (might be a nice utility function to have
> > somewhere).
> 
> Maybe promote your comment here to the code. When I see type mismatches like
> this I always ponder, and what you said clarifies it.
> 
> // Check that |candidate_pixel_rect_float| is actually representable as
> integers.

Oh -- non-integers may be perfectly valid (for all I know) -- it's just that we
can't handle them as overlays.

[ccameron, 2015-08-26 22:57:58]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2015-08-26 22:59:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1304303002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1304303002/140001

[commit-bot: I haz the power, 2015-08-27 00:33:03]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-08-27 00:33:36]

Patchset 8 (id:??) landed as
https://crrev.com/cf57c8d56ce3562cd6d4707394197dfb007af040
Cr-Commit-Position: refs/heads/master@{#345745}

[danakj, 2015-08-27 18:20:03]

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

[danakj, 2015-08-27 18:20:04]

https://codereview.chromium.org/1304303002/diff/120001/cc/output/overlay_stra...
File cc/output/overlay_strategy_sandwich.cc (right):

https://codereview.chromium.org/1304303002/diff/120001/cc/output/overlay_stra...
cc/output/overlay_strategy_sandwich.cc:59: if (candidate_pixel_rect !=
candidate_pixel_rect_float)
Want to add a RectF::IsRepresentableAsRect?

[danakj, 2015-08-27 18:20:09]

danakj@chromium.org changed reviewers:
- danakj@chromium.org

[ccameron, 2015-08-27 19:34:30]

On 2015/08/27 18:20:04, danakj wrote:
>
https://codereview.chromium.org/1304303002/diff/120001/cc/output/overlay_stra...
> File cc/output/overlay_strategy_sandwich.cc (right):
> 
>
https://codereview.chromium.org/1304303002/diff/120001/cc/output/overlay_stra...
> cc/output/overlay_strategy_sandwich.cc:59: if (candidate_pixel_rect !=
> candidate_pixel_rect_float)
> Want to add a RectF::IsRepresentableAsRect?

Sure -- I'll grab that.