O2D:...

O2D:
- New APIs:
  - Add a "visible" property to the Layer class to mimick the visible property of the Transform class.
  - Add a method to set an affine transform on Pattern objects so that texture patterns can be transformed independently from the Layers that use them.
  - Add a "paint operators" API for Layers and define four operators (currently only three of them implemented). The previous hard-coded behaviour was equivalent to the BLEND_WITH_TRANSPARENCY operator.
- Fixes:
  - Replace the "mask" nomenclature with "clip", which is a more standard name.
  - Don't clip alpha-blended layers.
  - Restrict Layer painting to the rectangle defined by the attributes.
  - Set default alpha value to 0.0 instead of 1.0 to match what JavaScript expects.

TEST=loaded O2D and verified all new functionality is working
BUG=none


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

[Tristan Schmelcher 2, 2011-01-19 18:40:25]

Ping?

[fbarchard, 2011-01-19 18:51:20]

http://codereview.chromium.org/6255003/diff/1/core/cross/cairo/renderer_cairo.cc
File core/cross/cairo/renderer_cairo.cc (right):

http://codereview.chromium.org/6255003/diff/1/core/cross/cairo/renderer_cairo...
core/cross/cairo/renderer_cairo.cc:2: * Copyright 2010, Google Inc.
update to 2011

http://codereview.chromium.org/6255003/diff/1/core/cross/cairo/renderer_cairo...
core/cross/cairo/renderer_cairo.cc:136: cur->alpha());
Can transparency be done with flat shading?
provide an ARGB to shade the texture with, supplying the alpha, and render with
transparency?

http://codereview.chromium.org/6255003/diff/1/core/cross/cairo/renderer_cairo...
core/cross/cairo/renderer_cairo.cc:143: case Layer::COPY_WITH_FADING:  // Not
implemented yet; treat as COPY
mark as todo?
Can also be done with shading?
or attenuate the source.  But changing the source will hurt upload speed

http://codereview.chromium.org/6255003/diff/1/plugin/idl/layer.idl
File plugin/idl/layer.idl (right):

http://codereview.chromium.org/6255003/diff/1/plugin/idl/layer.idl#newcode46
plugin/idl/layer.idl:46: alpha channel. (Default)
preattenuated or unattenuated alpha?

http://codereview.chromium.org/6255003/diff/1/plugin/idl/layer.idl#newcode49
plugin/idl/layer.idl:49: additional fractional transparency.
is this destination alpha blending?  (aka Disney method)

http://codereview.chromium.org/6255003/diff/1/plugin/idl/layer.idl#newcode54
plugin/idl/layer.idl:54: fractional brightness.
just alpha, or full ARGB shading?

http://codereview.chromium.org/6255003/diff/1/plugin/idl/pattern.idl
File plugin/idl/pattern.idl (right):

http://codereview.chromium.org/6255003/diff/1/plugin/idl/pattern.idl#newcode89
plugin/idl/pattern.idl:89: %]
this is a 2x2 matrix rotation?  So you can rotate by an arbitrary angle?

[Tristan Schmelcher 2, 2011-01-20 00:26:16]

http://codereview.chromium.org/6255003/diff/1/core/cross/cairo/renderer_cairo.cc
File core/cross/cairo/renderer_cairo.cc (right):

http://codereview.chromium.org/6255003/diff/1/core/cross/cairo/renderer_cairo...
core/cross/cairo/renderer_cairo.cc:2: * Copyright 2010, Google Inc.
On 2011/01/19 18:51:20, fbarchard wrote:
> update to 2011

Done.

http://codereview.chromium.org/6255003/diff/1/core/cross/cairo/renderer_cairo...
core/cross/cairo/renderer_cairo.cc:136: cur->alpha());
On 2011/01/19 18:51:20, fbarchard wrote:
> Can transparency be done with flat shading?
> provide an ARGB to shade the texture with, supplying the alpha, and render
with
> transparency?

That's basically what this is. The "mask" is an ARGB value whose colour content
is ignored and cairo_mask renders the source using the alpha component of the
mask as the transparency. I don't know whether it uses a shader internally or
not, but that doesn't matter--Cairo doesn't support user-defined shaders, so
this is the best option AFAICT.

http://codereview.chromium.org/6255003/diff/1/core/cross/cairo/renderer_cairo...
core/cross/cairo/renderer_cairo.cc:143: case Layer::COPY_WITH_FADING:  // Not
implemented yet; treat as COPY
On 2011/01/19 18:51:20, fbarchard wrote:
> mark as todo?

Done.

> Can also be done with shading?

No user-defined shader support.

> or attenuate the source.  But changing the source will hurt upload speed

The best option I've found so far is to first paint opaque black and then do
BLEND_WITH_TRANSPARENCY on top of that. I'm hoping to find something better in
the Cairo API though. But this paint operator is only used when fading in feeds,
which isn't the "steady-state" behaviour, so the speed of it isn't critical.

http://codereview.chromium.org/6255003/diff/1/plugin/idl/layer.idl
File plugin/idl/layer.idl (right):

http://codereview.chromium.org/6255003/diff/1/plugin/idl/layer.idl#newcode46
plugin/idl/layer.idl:46: alpha channel. (Default)
On 2011/01/19 18:51:20, fbarchard wrote:
> preattenuated or unattenuated alpha?

I've never heard of preattenuated/unattenuated alpha ... do you mean
premultiplied/unpremultiplied alpha? Cairo uses premultiplied alpha for
everything, but that's just an implementation detail as far as the JS API is
concerned.

http://codereview.chromium.org/6255003/diff/1/plugin/idl/layer.idl#newcode49
plugin/idl/layer.idl:49: additional fractional transparency.
On 2011/01/19 18:51:20, fbarchard wrote:
> is this destination alpha blending?  (aka Disney method)

No, just plain old source alpha blending. The _WITH_TRANSPARENCY version just
means that in addition to using the alpha channel of the Pattern we also use the
alpha property of the Layer. So the blend function is basically (1 - (pattern
alpha * layer alpha)).

http://codereview.chromium.org/6255003/diff/1/plugin/idl/layer.idl#newcode54
plugin/idl/layer.idl:54: fractional brightness.
On 2011/01/19 18:51:20, fbarchard wrote:
> just alpha, or full ARGB shading?

I'm not sure what you're referring to here.

http://codereview.chromium.org/6255003/diff/1/plugin/idl/pattern.idl
File plugin/idl/pattern.idl (right):

http://codereview.chromium.org/6255003/diff/1/plugin/idl/pattern.idl#newcode89
plugin/idl/pattern.idl:89: %]
On 2011/01/19 18:51:20, fbarchard wrote:
> this is a 2x2 matrix rotation?  So you can rotate by an arbitrary angle?

No, it is an affine transform, i.e. a 3x3 matrix. It is more powerful than a
linear transform. Rotation is possible, but we are only going to use it for
reflection.

[zhurunz, 2011-01-20 00:46:51]

http://codereview.chromium.org/6255003/diff/13001/core/cross/cairo/pattern.cc
File core/cross/cairo/pattern.cc (right):

http://codereview.chromium.org/6255003/diff/13001/core/cross/cairo/pattern.cc...
core/cross/cairo/pattern.cc:74: }
put constructor and destructor to the beginning.

http://codereview.chromium.org/6255003/diff/13001/core/cross/cairo/renderer_c...
File core/cross/cairo/renderer_cairo.cc (right):

http://codereview.chromium.org/6255003/diff/13001/core/cross/cairo/renderer_c...
core/cross/cairo/renderer_cairo.cc:130: 
Should we remove the blank line?

http://codereview.chromium.org/6255003/diff/13001/core/cross/cairo/renderer_c...
core/cross/cairo/renderer_cairo.cc:143: case Layer::COPY_WITH_FADING:  //
TODO(tschmelcher): Not implemented yet.
The comment say it is Not implemented yet.
But there is some "implementation" below. 
Which is true?

http://codereview.chromium.org/6255003/diff/13001/core/cross/cairo/renderer_c...
core/cross/cairo/renderer_cairo.cc:148: }
Better adding a default. 
Even if just DCheck().

[Tristan Schmelcher 2, 2011-01-20 01:12:38]

http://codereview.chromium.org/6255003/diff/13001/core/cross/cairo/renderer_c...
File core/cross/cairo/renderer_cairo.cc (right):

http://codereview.chromium.org/6255003/diff/13001/core/cross/cairo/renderer_c...
core/cross/cairo/renderer_cairo.cc:130: 
On 2011/01/20 00:46:51, zhurunz wrote:
> Should we remove the blank line?

I think putting a blank line here is typical.

http://codereview.chromium.org/6255003/diff/13001/core/cross/cairo/renderer_c...
core/cross/cairo/renderer_cairo.cc:143: case Layer::COPY_WITH_FADING:  //
TODO(tschmelcher): Not implemented yet.
On 2011/01/20 00:46:51, zhurunz wrote:
> The comment say it is Not implemented yet.
> But there is some "implementation" below. 
> Which is true?

The below stuff is the code for COPY. I'm treating the _WITH_FADING version as
COPY for now until it's implemented. I had a comment about that before but I
took it out to make space for the TODO ... I guess I'll add it back.

http://codereview.chromium.org/6255003/diff/13001/core/cross/cairo/renderer_c...
core/cross/cairo/renderer_cairo.cc:148: }
On 2011/01/20 00:46:51, zhurunz wrote:
> Better adding a default. 
> Even if just DCheck().

Done.

[zhurunz, 2011-01-20 17:36:49]

LGTM

[fbarchard, 2011-01-20 18:06:05]

clarifying blend modes

http://codereview.chromium.org/6255003/diff/1/core/cross/cairo/renderer_cairo.cc
File core/cross/cairo/renderer_cairo.cc (right):

http://codereview.chromium.org/6255003/diff/1/core/cross/cairo/renderer_cairo...
core/cross/cairo/renderer_cairo.cc:136: cur->alpha());
shading, not shaders.
flat shading is like Gouraud, but uses the same color value for all pixels.
Its unclear if you've implemented this correctly.

http://codereview.chromium.org/6255003/diff/1/plugin/idl/layer.idl
File plugin/idl/layer.idl (right):

http://codereview.chromium.org/6255003/diff/1/plugin/idl/layer.idl#newcode46
plugin/idl/layer.idl:46: alpha channel. (Default)
same thing.
preattenuated = premultiplied
unattenuated = unpremultiplied
So Cairo is premultiplied.  But hardware (o3d) tends to not be.  Maybe worth a
note.

http://codereview.chromium.org/6255003/diff/1/plugin/idl/layer.idl#newcode49
plugin/idl/layer.idl:49: additional fractional transparency.
Thats basically what I mean by destination alpha.  The alpha in the frame buffer
is used.  But there are different ways to use that alpha... the most common
being disney's method.
Just make sure you get the same results as O3d.

http://codereview.chromium.org/6255003/diff/1/plugin/idl/layer.idl#newcode54
plugin/idl/layer.idl:54: fractional brightness.
So the alpha in the destination layer is used to multiply by the ARGB of the
source before doing a 'COPY'.  The destinations ARGB is replaced by the 'faded'
image.

[Tristan Schmelcher 2, 2011-01-20 20:43:32]

http://codereview.chromium.org/6255003/diff/1/core/cross/cairo/renderer_cairo.cc
File core/cross/cairo/renderer_cairo.cc (right):

http://codereview.chromium.org/6255003/diff/1/core/cross/cairo/renderer_cairo...
core/cross/cairo/renderer_cairo.cc:136: cur->alpha());
On 2011/01/20 18:06:05, fbarchard wrote:
> shading, not shaders.
> flat shading is like Gouraud, but uses the same color value for all pixels.
> Its unclear if you've implemented this correctly.

I'm pretty sure it's right. You can look up the documentation on
cairo_mask()--it sounds like exactly what we want. And it looks correct on
screen.

http://codereview.chromium.org/6255003/diff/1/plugin/idl/layer.idl
File plugin/idl/layer.idl (right):

http://codereview.chromium.org/6255003/diff/1/plugin/idl/layer.idl#newcode46
plugin/idl/layer.idl:46: alpha channel. (Default)
On 2011/01/20 18:06:05, fbarchard wrote:
> same thing.
> preattenuated = premultiplied
> unattenuated = unpremultiplied
> So Cairo is premultiplied.  But hardware (o3d) tends to not be.  Maybe worth a
> note.

It's noted in my new CL to convert to premultiplied alpha in texture_cairo.cc.

http://codereview.chromium.org/6255003/diff/1/plugin/idl/layer.idl#newcode49
plugin/idl/layer.idl:49: additional fractional transparency.
On 2011/01/20 18:06:05, fbarchard wrote:
> Thats basically what I mean by destination alpha.  The alpha in the frame
buffer
> is used.

No, "layer alpha" above means the alpha property of _this_ Layer object, not the
per-pixel alpha channel of the destination of the paint operation. The
destination's alpha is not used for anything. In fact the destination does not
even have an alpha channel at all.

> But there are different ways to use that alpha... the most common
> being disney's method.
> Just make sure you get the same results as O3d.
> 

It does appear to look the same as O3D.

http://codereview.chromium.org/6255003/diff/1/plugin/idl/layer.idl#newcode54
plugin/idl/layer.idl:54: fractional brightness.
On 2011/01/20 18:06:05, fbarchard wrote:
> So the alpha in the destination layer is used to multiply by the ARGB of the
> source before doing a 'COPY'.  The destinations ARGB is replaced by the
'faded'
> image.

No, "alpha property of the Layer" means the alpha property of this Layer object,
i.e. the value returned by this->alpha(). That is used to multiply the ARGB of
the source to fade the image, and then the destination's ARGB is replaced with
that faded image.

[fbarchard, 2011-01-20 20:53:41]

LGTM as long as its tested to be the same as O3d.

Here's a reference for destination alpha blending

http://my.opera.com/Vorlath/blog/2006/12/20/project-v-advanced-alpha-blending