aura: Add Layer::LAYER_SOLID_COLOR to compositor.

aura: Add Layer::LAYER_SOLID_COLOR to compositor.

This adds a new layer type backed by
WebKit::WebSolidColorLayer.  I'm using it for the power
button animation's background (which previously had a
fullscreen texture that it just painted black).

I'm also renaming LAYER_HAS_TEXTURE to LAYER_TEXTURED and
LAYER_HAS_NO_TEXTURE to LAYER_NOT_DRAWN.

BUG=109083
TEST=manual: checked that the power button animation looks the same as before
TBR=sky

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=119580

[Daniel Erat, 2012-01-26 01:59:47]

I'm not particularly familiar with Layer and could very easily believe that I'm
getting something wrong here (although this seems to work).  More specifically,
I'm not sure of the distribution of e.g. clipping optimizations between WebKit
and Chrome.

[piman, 2012-01-26 02:16:06]

https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/la...
File ui/gfx/compositor/layer.cc (right):

https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/la...
ui/gfx/compositor/layer.cc:249: return;
Mmh, I feel like we should setOpaque somewhere. I'm not sure what happens if the
opaque() value on the layer doesn't match the opaque() value computed on the
quad (coming from the color). It doesn't seem like it's propagated in the webkit
side. James?

[wjmaclean, 2012-01-26 14:07:48]

On 2012/01/26 02:16:06, piman wrote:
>
https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/la...
> File ui/gfx/compositor/layer.cc (right):
> 
>
https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/la...
> ui/gfx/compositor/layer.cc:249: return;
> Mmh, I feel like we should setOpaque somewhere. I'm not sure what happens if
the
> opaque() value on the layer doesn't match the opaque() value computed on the
> quad (coming from the color). It doesn't seem like it's propagated in the
webkit
> side. James?

WebSolidColorLayer is derived from WebLayer, and as best I can see WebLayer does
pass calls to setOpaque(), setOpacity(), etc. through to the underlying Impl
layer, which is in our case SolidColorLayerChromium, derived from LayerChromium.

In short I think it should work, although I haven't tried it explicitly.

The default values for LayerChromium are:

opaque = false
opacity = 1.0

So for the sake of culling (in case anything ends up behind this layer), we
should set opaque = true if we're not using any transparency.

[piman, 2012-01-26 16:32:47]

On Thu, Jan 26, 2012 at 6:07 AM, <wjmaclean@chromium.org> wrote:

> On 2012/01/26 02:16:06, piman wrote:
>
> https://chromiumcodereview.**appspot.com/9289036/diff/2001/**
>
ui/gfx/compositor/layer.cc<https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/layer.cc>
>
>> File ui/gfx/compositor/layer.cc (right):
>>
>
>
> https://chromiumcodereview.**appspot.com/9289036/diff/2001/**
>
ui/gfx/compositor/layer.cc#**newcode249<https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/layer.cc#newcode249>
>
>> ui/gfx/compositor/layer.cc:**249: return;
>> Mmh, I feel like we should setOpaque somewhere. I'm not sure what happens
>> if
>>
> the
>
>> opaque() value on the layer doesn't match the opaque() value computed on
>> the
>> quad (coming from the color). It doesn't seem like it's propagated in the
>>
> webkit
>
>> side. James?
>>
>
> WebSolidColorLayer is derived from WebLayer, and as best I can see
> WebLayer does
> pass calls to setOpaque(), setOpacity(), etc. through to the underlying
> Impl
> layer, which is in our case SolidColorLayerChromium, derived from
> LayerChromium.
>
> In short I think it should work, although I haven't tried it explicitly.
>
> The default values for LayerChromium are:
>
> opaque = false
> opacity = 1.0
>
> So for the sake of culling (in case anything ends up behind this layer), we
> should set opaque = true if we're not using any transparency.
>

So, it sound like we should call web_layer_.setOpaque(color.alpha == 1) or
whatever in Layer::SetColor.

Antoine


>
https://chromiumcodereview.**appspot.com/9289036/<https://chromiumcodereview....
>

[Daniel Erat, 2012-01-26 18:10:26]

https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/la...
File ui/gfx/compositor/layer.cc (right):

https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/la...
ui/gfx/compositor/layer.cc:249: return;
On 2012/01/26 02:16:07, piman wrote:
> Mmh, I feel like we should setOpaque somewhere. I'm not sure what happens if
the
> opaque() value on the layer doesn't match the opaque() value computed on the
> quad (coming from the color). It doesn't seem like it's propagated in the
webkit
> side. James?

Thanks, makes sense.  I've added a call to setOpaque() here and will reupload
after testing with a change that creates a layer with a non-opaque color.

[Daniel Erat, 2012-01-26 21:11:31]

On 2012/01/26 18:10:26, Daniel Erat wrote:
>
https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/la...
> File ui/gfx/compositor/layer.cc (right):
> 
>
https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/la...
> ui/gfx/compositor/layer.cc:249: return;
> On 2012/01/26 02:16:07, piman wrote:
> > Mmh, I feel like we should setOpaque somewhere. I'm not sure what happens if
> the
> > opaque() value on the layer doesn't match the opaque() value computed on the
> > quad (coming from the color). It doesn't seem like it's propagated in the
> webkit
> > side. James?
> 
> Thanks, makes sense.  I've added a call to setOpaque() here and will reupload
> after testing with a change that creates a layer with a non-opaque color.

I've added a call to SetFillsBoundsOpaquely() instead of setOpaque() since it
seems a bit safer (it should result in fills_bounds_opaquely_ being set
correctly).

When I actually give a layer a color like SkColorSetARGB(0x2f, 0xff, 0x0, 0xff),
it's opaque, though.  Any chance that the WebKit side has issues with non-opaque
colors?

[wjmaclean, 2012-01-26 21:16:07]

On 2012/01/26 21:11:31, Daniel Erat wrote:
> On 2012/01/26 18:10:26, Daniel Erat wrote:
> >
>
https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/la...
> > File ui/gfx/compositor/layer.cc (right):
> > 
> >
>
https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/la...
> > ui/gfx/compositor/layer.cc:249: return;
> > On 2012/01/26 02:16:07, piman wrote:
> > > Mmh, I feel like we should setOpaque somewhere. I'm not sure what happens
if
> > the
> > > opaque() value on the layer doesn't match the opaque() value computed on
the
> > > quad (coming from the color). It doesn't seem like it's propagated in the
> > webkit
> > > side. James?
> > 
> > Thanks, makes sense.  I've added a call to setOpaque() here and will
reupload
> > after testing with a change that creates a layer with a non-opaque color.
> 
> I've added a call to SetFillsBoundsOpaquely() instead of setOpaque() since it
> seems a bit safer (it should result in fills_bounds_opaquely_ being set
> correctly).
> 
> When I actually give a layer a color like SkColorSetARGB(0x2f, 0xff, 0x0,
0xff),
> it's opaque, though.  Any chance that the WebKit side has issues with
non-opaque
> colors?

I think as long as the layer knows it's non-opaque it should work ok. If
setOpaque(true) has been called, it may ignore the alpha value I think, even if
they're not 0xFF.

[Daniel Erat, 2012-01-26 21:28:45]

On 2012/01/26 21:16:07, wjmaclean wrote:
> On 2012/01/26 21:11:31, Daniel Erat wrote:
> > On 2012/01/26 18:10:26, Daniel Erat wrote:
> > >
> >
>
https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/la...
> > > File ui/gfx/compositor/layer.cc (right):
> > > 
> > >
> >
>
https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/la...
> > > ui/gfx/compositor/layer.cc:249: return;
> > > On 2012/01/26 02:16:07, piman wrote:
> > > > Mmh, I feel like we should setOpaque somewhere. I'm not sure what
happens
> if
> > > the
> > > > opaque() value on the layer doesn't match the opaque() value computed on
> the
> > > > quad (coming from the color). It doesn't seem like it's propagated in
the
> > > webkit
> > > > side. James?
> > > 
> > > Thanks, makes sense.  I've added a call to setOpaque() here and will
> reupload
> > > after testing with a change that creates a layer with a non-opaque color.
> > 
> > I've added a call to SetFillsBoundsOpaquely() instead of setOpaque() since
it
> > seems a bit safer (it should result in fills_bounds_opaquely_ being set
> > correctly).
> > 
> > When I actually give a layer a color like SkColorSetARGB(0x2f, 0xff, 0x0,
> 0xff),
> > it's opaque, though.  Any chance that the WebKit side has issues with
> non-opaque
> > colors?
> 
> I think as long as the layer knows it's non-opaque it should work ok. If
> setOpaque(true) has been called, it may ignore the alpha value I think, even
if
> they're not 0xFF.

Hmm, this doesn't seem to be the case.  (Note that SetFillsBoundsOpaquely()
calls setOpaque().)

[wjmaclean, 2012-01-26 21:46:33]

On 2012/01/26 21:28:45, Daniel Erat wrote:
> On 2012/01/26 21:16:07, wjmaclean wrote:
> > On 2012/01/26 21:11:31, Daniel Erat wrote:
> > > On 2012/01/26 18:10:26, Daniel Erat wrote:
> > > >
> > >
> >
>
https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/la...
> > > > File ui/gfx/compositor/layer.cc (right):
> > > > 
> > > >
> > >
> >
>
https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/la...
> > > > ui/gfx/compositor/layer.cc:249: return;
> > > > On 2012/01/26 02:16:07, piman wrote:
> > > > > Mmh, I feel like we should setOpaque somewhere. I'm not sure what
> happens
> > if
> > > > the
> > > > > opaque() value on the layer doesn't match the opaque() value computed
on
> > the
> > > > > quad (coming from the color). It doesn't seem like it's propagated in
> the
> > > > webkit
> > > > > side. James?
> > > > 
> > > > Thanks, makes sense.  I've added a call to setOpaque() here and will
> > reupload
> > > > after testing with a change that creates a layer with a non-opaque
color.
> > > 
> > > I've added a call to SetFillsBoundsOpaquely() instead of setOpaque() since
> it
> > > seems a bit safer (it should result in fills_bounds_opaquely_ being set
> > > correctly).
> > > 
> > > When I actually give a layer a color like SkColorSetARGB(0x2f, 0xff, 0x0,
> > 0xff),
> > > it's opaque, though.  Any chance that the WebKit side has issues with
> > non-opaque
> > > colors?
> > 
> > I think as long as the layer knows it's non-opaque it should work ok. If
> > setOpaque(true) has been called, it may ignore the alpha value I think, even
> if
> > they're not 0xFF.
> 
> Hmm, this doesn't seem to be the case.  (Note that SetFillsBoundsOpaquely()
> calls setOpaque().)

I would expect SetFillsBoundsOpaquely() to call setOpaque().

I know there were efforts to turn off blending for opaque layers, which (I
think) is equivalent to ignoring the alpha.

Perhaps, since we're storing the color in LayerChromium's background color
variable, maybe it thinks background colours shouldn't be transparent?

CCDrawQuads will request blending if (1) they are marked non-opaque, (2) they
have opacity != 1, or (3)they are specifically marked as needsBlending. I would
guess that blending is off, alpha values are ignored?

[Daniel Erat, 2012-01-26 21:54:17]

On 2012/01/26 21:46:33, wjmaclean wrote:
> On 2012/01/26 21:28:45, Daniel Erat wrote:
> > On 2012/01/26 21:16:07, wjmaclean wrote:
> > > On 2012/01/26 21:11:31, Daniel Erat wrote:
> > > > On 2012/01/26 18:10:26, Daniel Erat wrote:
> > > > >
> > > >
> > >
> >
>
https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/la...
> > > > > File ui/gfx/compositor/layer.cc (right):
> > > > > 
> > > > >
> > > >
> > >
> >
>
https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/la...
> > > > > ui/gfx/compositor/layer.cc:249: return;
> > > > > On 2012/01/26 02:16:07, piman wrote:
> > > > > > Mmh, I feel like we should setOpaque somewhere. I'm not sure what
> > happens
> > > if
> > > > > the
> > > > > > opaque() value on the layer doesn't match the opaque() value
computed
> on
> > > the
> > > > > > quad (coming from the color). It doesn't seem like it's propagated
in
> > the
> > > > > webkit
> > > > > > side. James?
> > > > > 
> > > > > Thanks, makes sense.  I've added a call to setOpaque() here and will
> > > reupload
> > > > > after testing with a change that creates a layer with a non-opaque
> color.
> > > > 
> > > > I've added a call to SetFillsBoundsOpaquely() instead of setOpaque()
since
> > it
> > > > seems a bit safer (it should result in fills_bounds_opaquely_ being set
> > > > correctly).
> > > > 
> > > > When I actually give a layer a color like SkColorSetARGB(0x2f, 0xff,
0x0,
> > > 0xff),
> > > > it's opaque, though.  Any chance that the WebKit side has issues with
> > > non-opaque
> > > > colors?
> > > 
> > > I think as long as the layer knows it's non-opaque it should work ok. If
> > > setOpaque(true) has been called, it may ignore the alpha value I think,
even
> > if
> > > they're not 0xFF.
> > 
> > Hmm, this doesn't seem to be the case.  (Note that SetFillsBoundsOpaquely()
> > calls setOpaque().)
> 
> I would expect SetFillsBoundsOpaquely() to call setOpaque().
> 
> I know there were efforts to turn off blending for opaque layers, which (I
> think) is equivalent to ignoring the alpha.
> 
> Perhaps, since we're storing the color in LayerChromium's background color
> variable, maybe it thinks background colours shouldn't be transparent?
> 
> CCDrawQuads will request blending if (1) they are marked non-opaque, (2) they
> have opacity != 1, or (3)they are specifically marked as needsBlending. I
would
> guess that blending is off, alpha values are ignored?

If it's simpler, I'm fine with just adding a comment (and DCHECK()) saying that
non-opaque colors are unsupported.  I don't see a particular need for them; it's
at least as easy to just make the layer itself non-opaque.

[piman, 2012-01-26 23:02:08]

On 2012/01/26 21:54:17, Daniel Erat wrote:
> On 2012/01/26 21:46:33, wjmaclean wrote:
> > On 2012/01/26 21:28:45, Daniel Erat wrote:
> > > On 2012/01/26 21:16:07, wjmaclean wrote:
> > > > On 2012/01/26 21:11:31, Daniel Erat wrote:
> > > > > On 2012/01/26 18:10:26, Daniel Erat wrote:
> > > > > >
> > > > >
> > > >
> > >
> >
>
https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/la...
> > > > > > File ui/gfx/compositor/layer.cc (right):
> > > > > > 
> > > > > >
> > > > >
> > > >
> > >
> >
>
https://chromiumcodereview.appspot.com/9289036/diff/2001/ui/gfx/compositor/la...
> > > > > > ui/gfx/compositor/layer.cc:249: return;
> > > > > > On 2012/01/26 02:16:07, piman wrote:
> > > > > > > Mmh, I feel like we should setOpaque somewhere. I'm not sure what
> > > happens
> > > > if
> > > > > > the
> > > > > > > opaque() value on the layer doesn't match the opaque() value
> computed
> > on
> > > > the
> > > > > > > quad (coming from the color). It doesn't seem like it's propagated
> in
> > > the
> > > > > > webkit
> > > > > > > side. James?
> > > > > > 
> > > > > > Thanks, makes sense.  I've added a call to setOpaque() here and will
> > > > reupload
> > > > > > after testing with a change that creates a layer with a non-opaque
> > color.
> > > > > 
> > > > > I've added a call to SetFillsBoundsOpaquely() instead of setOpaque()
> since
> > > it
> > > > > seems a bit safer (it should result in fills_bounds_opaquely_ being
set
> > > > > correctly).
> > > > > 
> > > > > When I actually give a layer a color like SkColorSetARGB(0x2f, 0xff,
> 0x0,
> > > > 0xff),
> > > > > it's opaque, though.  Any chance that the WebKit side has issues with
> > > > non-opaque
> > > > > colors?
> > > > 
> > > > I think as long as the layer knows it's non-opaque it should work ok. If
> > > > setOpaque(true) has been called, it may ignore the alpha value I think,
> even
> > > if
> > > > they're not 0xFF.
> > > 
> > > Hmm, this doesn't seem to be the case.  (Note that
SetFillsBoundsOpaquely()
> > > calls setOpaque().)
> > 
> > I would expect SetFillsBoundsOpaquely() to call setOpaque().
> > 
> > I know there were efforts to turn off blending for opaque layers, which (I
> > think) is equivalent to ignoring the alpha.
> > 
> > Perhaps, since we're storing the color in LayerChromium's background color
> > variable, maybe it thinks background colours shouldn't be transparent?
> > 
> > CCDrawQuads will request blending if (1) they are marked non-opaque, (2)
they
> > have opacity != 1, or (3)they are specifically marked as needsBlending. I
> would
> > guess that blending is off, alpha values are ignored?
> 
> If it's simpler, I'm fine with just adding a comment (and DCHECK()) saying
that
> non-opaque colors are unsupported.  I don't see a particular need for them;
it's
> at least as easy to just make the layer itself non-opaque.

I don't see the need to not support them. This patch LGTM. If you have a
transparent color, it's marked non-opaque. If you have an opaque color, it's
marked opaque.
I don't see the need to support the "I gave you a color with alpha, but I want
you to ignore it" case.

[Daniel Erat, 2012-01-27 18:47:49]

Mind approving, Ben?  The only marginally non-mechanical bits are in Layer and
PowerButtonController.

[wjmaclean, 2012-01-27 19:00:15]

btw, LGTM

[Ben Goodger (Google), 2012-01-27 21:18:30]

lgtm

[commit-bot: I haz the power, 2012-01-28 00:29:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/derat@chromium.org/9289036/13001

[commit-bot: I haz the power, 2012-01-28 00:29:40]

Presubmit check for 9289036-13001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for: chrome/browser/ui/tabs/dock_info_aura.cc

Presubmit checks took 2.9s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[commit-bot: I haz the power, 2012-01-28 00:32:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/derat@chromium.org/9289036/13001

[commit-bot: I haz the power, 2012-01-28 03:19:14]

Change committed as 119580