Add initial implementation of cc::Surfaces backend for exo.

Add initial implementation of cc::Surfaces backend for exo.

This puts every buffer in a cc::Surface, and then uses a SurfaceLayer to
reference them from the compositor. This is disabled by default, but can
be enabled with the --use-exo-surface-layer flag.

At the moment it won't offer any performance improvements, as a new
SurfaceId is created for each frame, so the browser compositor has to
always commit and draw. This restriction can removed in future patches.

BUG=610086
Committed: https://crrev.com/bd9586a9e4a658743483a781faf7b9747cee4688
Cr-Commit-Position: refs/heads/master@{#396618}

[jbauman, 2016-05-27 00:56:18]

Description was changed from

==========
Exo Surfaces prototype

BUG=
==========

to

==========
Add initial implementation of using cc::Surfaces backend for exo.

This puts every buffer in a cc::Surface, and then uses a SurfaceLayer to
reference them from the compositor. This is disabled by default, but can be
enabled with the --use-exo-surface-layer flag.

At the moment it won't offer any performance improvements, as a new SurfaceId is
created for each frame, so the browser compositor has to always commit and draw.
This restriction can removed in future patches.

BUG=610086
==========

[jbauman, 2016-05-27 00:56:50]

Description was changed from

==========
Add initial implementation of using cc::Surfaces backend for exo.

This puts every buffer in a cc::Surface, and then uses a SurfaceLayer to
reference them from the compositor. This is disabled by default, but can be
enabled with the --use-exo-surface-layer flag.

At the moment it won't offer any performance improvements, as a new SurfaceId is
created for each frame, so the browser compositor has to always commit and draw.
This restriction can removed in future patches.

BUG=610086
==========

to

==========
Add initial implementation of using cc::Surfaces backend for exo.

This puts every buffer in a cc::Surface, and then uses a SurfaceLayer to
reference them from the compositor. This is disabled by default, but can
be enabled with the --use-exo-surface-layer flag.

At the moment it won't offer any performance improvements, as a new
SurfaceId is created for each frame, so the browser compositor has to
always commit and draw. This restriction can removed in future patches.


BUG=610086
==========

[jbauman, 2016-05-27 00:57:39]

Description was changed from

==========
Add initial implementation of using cc::Surfaces backend for exo.

This puts every buffer in a cc::Surface, and then uses a SurfaceLayer to
reference them from the compositor. This is disabled by default, but can
be enabled with the --use-exo-surface-layer flag.

At the moment it won't offer any performance improvements, as a new
SurfaceId is created for each frame, so the browser compositor has to
always commit and draw. This restriction can removed in future patches.


BUG=610086
==========

to

==========
Add initial implementation of cc::Surfaces backend for exo.

This puts every buffer in a cc::Surface, and then uses a SurfaceLayer to
reference them from the compositor. This is disabled by default, but can
be enabled with the --use-exo-surface-layer flag.

At the moment it won't offer any performance improvements, as a new
SurfaceId is created for each frame, so the browser compositor has to
always commit and draw. This restriction can removed in future patches.


BUG=610086
==========

[jbauman, 2016-05-27 00:57:47]

jbauman@chromium.org changed reviewers:
+ reveman@chromium.org

[reveman, 2016-05-27 06:18:40]

lg, any downsides with using this by default? I would prefer to just have one
implementation if possible. I can help test this with arc++ if that helps?

https://codereview.chromium.org/2008153002/diff/80001/components/exo/surface.h
File components/exo/surface.h (right):

https://codereview.chromium.org/2008153002/diff/80001/components/exo/surface....
components/exo/surface.h:55: // cc::SurfaceFactoryClient implementation.
nit: we've been using "// Overridden from cc::SurfaceFactoryClient:" comments
throughout the exo code so far

[reveman, 2016-05-27 18:25:42]

https://codereview.chromium.org/2008153002/diff/80001/components/exo/surface.cc
File components/exo/surface.cc (right):

https://codereview.chromium.org/2008153002/diff/80001/components/exo/surface....
components/exo/surface.cc:207:
CHECK(aura::Env::GetInstance()->context_factory());
do we need these CHECKs? can they be DCHECKs at least?

https://codereview.chromium.org/2008153002/diff/80001/components/exo/surface....
components/exo/surface.cc:548: float frame_device_scale_factor = 1.0;
is this correct when display DSF is 2? e.g. chromebook pixel.

[jbauman, 2016-05-27 23:02:57]

On 2016/05/27 06:18:40, reveman wrote:
> lg, any downsides with using this by default? I would prefer to just have one
> implementation if possible. I can help test this with arc++ if that helps?

I'd like if we'd have the flag temporarily to at least compare output with the
old version if something's broken.

It'd be great if you could help test with arc++. I'm not sure where to start
with that.

One issue with switching this on by default is the damage rects are much bigger
than they should be (still working on a fix for that). Though until
https://codereview.chromium.org/2012063002/ is landed, we have much larger
issues with damage rects.
> 
> https://codereview.chromium.org/2008153002/diff/80001/components/exo/surface.h
> File components/exo/surface.h (right):
> 
>
https://codereview.chromium.org/2008153002/diff/80001/components/exo/surface....
> components/exo/surface.h:55: // cc::SurfaceFactoryClient implementation.
> nit: we've been using "// Overridden from cc::SurfaceFactoryClient:" comments
> throughout the exo code so far

Done.

On 2016/05/27 18:25:42, reveman wrote:
>
https://codereview.chromium.org/2008153002/diff/80001/components/exo/surface.cc
> File components/exo/surface.cc (right):
> 
>
https://codereview.chromium.org/2008153002/diff/80001/components/exo/surface....
> components/exo/surface.cc:207:
> CHECK(aura::Env::GetInstance()->context_factory());
> do we need these CHECKs? can they be DCHECKs at least?
> 
Removed.

>
https://codereview.chromium.org/2008153002/diff/80001/components/exo/surface....
> components/exo/surface.cc:548: float frame_device_scale_factor = 1.0;
> is this correct when display DSF is 2? e.g. chromebook pixel.

I think this is correct. I've reorganized this code to try to make it a bit
clearer how it's supposed to work.

[reveman, 2016-05-27 23:30:45]

On 2016/05/27 at 23:02:57, jbauman wrote:
> On 2016/05/27 06:18:40, reveman wrote:
> > lg, any downsides with using this by default? I would prefer to just have
one
> > implementation if possible. I can help test this with arc++ if that helps?
> 
> I'd like if we'd have the flag temporarily to at least compare output with the
old version if something's broken.

Ok, let's do that. I'd like to see this become default asap though. Makes the
code easier to review and maintain.

> 
> It'd be great if you could help test with arc++. I'm not sure where to start
with that.

I'll try this current code out as soon as it lands.

> 
> One issue with switching this on by default is the damage rects are much
bigger than they should be (still working on a fix for that). Though until
https://codereview.chromium.org/2012063002/ is landed, we have much larger
issues with damage rects.
> > 

Right. The amount of extra work we do as a result of that is crazy.

Arc++ currently send damage rects for the full surface all the time. We'll
improve that in the future but maybe the current lack of damage tracking isn't
really going to cause any regression as a result.

lgtm with one possible nit

https://codereview.chromium.org/2008153002/diff/120001/components/exo/shell_s...
File components/exo/shell_surface.cc (right):

https://codereview.chromium.org/2008153002/diff/120001/components/exo/shell_s...
components/exo/shell_surface.cc:948: if (current_bounds_ != new_widget_bounds)
nit: would be nice to land this as a separate patch unless it's actually
specific the use of surface layers

[jbauman, 2016-05-28 00:17:09]

On 2016/05/27 23:30:45, reveman wrote:
> On 2016/05/27 at 23:02:57, jbauman wrote:
> > On 2016/05/27 06:18:40, reveman wrote:
> > > lg, any downsides with using this by default? I would prefer to just have
> one
> > > implementation if possible. I can help test this with arc++ if that helps?
> > 
> > I'd like if we'd have the flag temporarily to at least compare output with
the
> old version if something's broken.
> 
> Ok, let's do that. I'd like to see this become default asap though. Makes the
> code easier to review and maintain.

Yeah, definitely want to switch and remove the old code quickly.
> 
> > 
> > It'd be great if you could help test with arc++. I'm not sure where to start
> with that.
> 
> I'll try this current code out as soon as it lands.
> 
> > 
> > One issue with switching this on by default is the damage rects are much
> bigger than they should be (still working on a fix for that). Though until
> https://codereview.chromium.org/2012063002/ is landed, we have much larger
> issues with damage rects.
> > > 
> 
> Right. The amount of extra work we do as a result of that is crazy.
> 
> Arc++ currently send damage rects for the full surface all the time. We'll
> improve that in the future but maybe the current lack of damage tracking isn't
> really going to cause any regression as a result.

Ok, good to know. Also, looks like there's a bug in ui::Layer where we always
send full layer damage anyway, so I guess it wouldn't be a regression there.
> 
> lgtm with one possible nit
> 
>
https://codereview.chromium.org/2008153002/diff/120001/components/exo/shell_s...
> File components/exo/shell_surface.cc (right):
> 
>
https://codereview.chromium.org/2008153002/diff/120001/components/exo/shell_s...
> components/exo/shell_surface.cc:948: if (current_bounds_ != new_widget_bounds)
> nit: would be nice to land this as a separate patch unless it's actually
> specific the use of surface layers

Oops, I was playing around with this and accidentally included it.

[jbauman, 2016-05-28 00:17:29]

The CQ bit was checked by jbauman@chromium.org

[jbauman, 2016-05-28 00:17:29]

The patchset sent to the CQ was uploaded after l-g-t-m from reveman@chromium.org
Link to the patchset: https://codereview.chromium.org/2008153002/#ps140001
(title: " ")

[commit-bot: I haz the power, 2016-05-28 00:17:51]

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

[commit-bot: I haz the power, 2016-05-28 01:09:15]

Description was changed from

==========
Add initial implementation of cc::Surfaces backend for exo.

This puts every buffer in a cc::Surface, and then uses a SurfaceLayer to
reference them from the compositor. This is disabled by default, but can
be enabled with the --use-exo-surface-layer flag.

At the moment it won't offer any performance improvements, as a new
SurfaceId is created for each frame, so the browser compositor has to
always commit and draw. This restriction can removed in future patches.


BUG=610086
==========

to

==========
Add initial implementation of cc::Surfaces backend for exo.

This puts every buffer in a cc::Surface, and then uses a SurfaceLayer to
reference them from the compositor. This is disabled by default, but can
be enabled with the --use-exo-surface-layer flag.

At the moment it won't offer any performance improvements, as a new
SurfaceId is created for each frame, so the browser compositor has to
always commit and draw. This restriction can removed in future patches.


BUG=610086
==========

[commit-bot: I haz the power, 2016-05-28 01:09:16]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-05-28 01:10:29]

Description was changed from

==========
Add initial implementation of cc::Surfaces backend for exo.

This puts every buffer in a cc::Surface, and then uses a SurfaceLayer to
reference them from the compositor. This is disabled by default, but can
be enabled with the --use-exo-surface-layer flag.

At the moment it won't offer any performance improvements, as a new
SurfaceId is created for each frame, so the browser compositor has to
always commit and draw. This restriction can removed in future patches.


BUG=610086
==========

to

==========
Add initial implementation of cc::Surfaces backend for exo.

This puts every buffer in a cc::Surface, and then uses a SurfaceLayer to
reference them from the compositor. This is disabled by default, but can
be enabled with the --use-exo-surface-layer flag.

At the moment it won't offer any performance improvements, as a new
SurfaceId is created for each frame, so the browser compositor has to
always commit and draw. This restriction can removed in future patches.

BUG=610086

Committed: https://crrev.com/bd9586a9e4a658743483a781faf7b9747cee4688
Cr-Commit-Position: refs/heads/master@{#396618}
==========

[commit-bot: I haz the power, 2016-05-28 01:10:30]

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