Attempt to write a RenderShadowedBox

Attempt to write a RenderShadowedBox

The goal of this is to have a Renderer-layer abstraction
around the Sky Engine c++ apis for drawing a shadow.

R=mpcomplete@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/f8f5f34ff27447e70fa30b0bf79171c0cad40558

[Matt Perry, 2015-06-02 20:57:37]

https://codereview.chromium.org/1146893004/diff/40001/sky/sdk/lib/framework/r...
File sky/sdk/lib/framework/rendering/render_box.dart (right):

https://codereview.chromium.org/1146893004/diff/40001/sky/sdk/lib/framework/r...
sky/sdk/lib/framework/rendering/render_box.dart:283: BoxShadow get shadow =>
_decoration;
Did you mean to return _shadow here?

https://codereview.chromium.org/1146893004/diff/40001/sky/sdk/lib/framework/r...
sky/sdk/lib/framework/rendering/render_box.dart:292: sky.Paint paint = new
sky.Paint()..color = shadow.color;
This should be the color of the Main layer, not the shadow.

https://codereview.chromium.org/1146893004/diff/40001/sky/sdk/lib/framework/r...
sky/sdk/lib/framework/rendering/render_box.dart:301: layerPaint.setARGB(128, 55,
55, 55);
This is the shadow color. Since we're using TransferMode.srcInMode, the
layerPaint.color here overrides paint.color.

[Matt Perry, 2015-06-02 21:19:01]

https://codereview.chromium.org/1146893004/diff/40001/sky/sdk/lib/framework/r...
File sky/sdk/lib/framework/rendering/render_box.dart (right):

https://codereview.chromium.org/1146893004/diff/40001/sky/sdk/lib/framework/r...
sky/sdk/lib/framework/rendering/render_box.dart:299:
..setColorMode(sky.TransferMode.srcInMode),
BTW, the existing code[1] uses srcMode. We might want to use that here.

[1]
https://github.com/domokit/mojo/blob/2aad0d73122aca64491b96c69530578e6f0d81fd...

[eseidel, 2015-06-03 19:05:34]

Actually seems to work now.  ptal.

[Matt Perry, 2015-06-03 19:13:59]

The render_box.dart rename ruins the diff so I can't see what changed from
previous versions. Do you know why it works now and didn't before?

https://codereview.chromium.org/1146893004/diff/80001/sky/sdk/lib/framework/r...
File sky/sdk/lib/framework/rendering/box.dart (right):

https://codereview.chromium.org/1146893004/diff/80001/sky/sdk/lib/framework/r...
sky/sdk/lib/framework/rendering/box.dart:399:
..setColorMode(sky.TransferMode.srcInMode),
Did you mean to revert to srcInMode here?

https://codereview.chromium.org/1146893004/diff/80001/sky/sdk/lib/framework/r...
sky/sdk/lib/framework/rendering/box.dart:402: // TODO(mpcomplete): blur should
respect the offset?
Are you saying it doesn't, or that it does but you're not sure if it should?
(FWIW I don't even know what it means for blur to respect the offset :)

[eseidel, 2015-06-04 21:24:10]

On 2015/06/03 at 19:13:59, mpcomplete wrote:
> The render_box.dart rename ruins the diff so I can't see what changed from
previous versions. Do you know why it works now and didn't before?
> 
>
https://codereview.chromium.org/1146893004/diff/80001/sky/sdk/lib/framework/r...
> File sky/sdk/lib/framework/rendering/box.dart (right):
> 
>
https://codereview.chromium.org/1146893004/diff/80001/sky/sdk/lib/framework/r...
> sky/sdk/lib/framework/rendering/box.dart:399:
..setColorMode(sky.TransferMode.srcInMode),
> Did you mean to revert to srcInMode here?

I just matched what shadow.dart did:
https://github.com/domokit/mojo/blob/master/sky/examples/raw/shadow.dart#L20

>
https://codereview.chromium.org/1146893004/diff/80001/sky/sdk/lib/framework/r...
> sky/sdk/lib/framework/rendering/box.dart:402: // TODO(mpcomplete): blur should
respect the offset?
> Are you saying it doesn't, or that it does but you're not sure if it should?
(FWIW I don't even know what it means for blur to respect the offset :)

Should blur get bigger as the offset gets bigger?  We should make blur
configurable is really what I mean.

[Matt Perry, 2015-06-04 21:28:45]

On 2015/06/04 21:24:10, eseidel wrote:
> On 2015/06/03 at 19:13:59, mpcomplete wrote:
> > The render_box.dart rename ruins the diff so I can't see what changed from
> previous versions. Do you know why it works now and didn't before?
> > 
> >
>
https://codereview.chromium.org/1146893004/diff/80001/sky/sdk/lib/framework/r...
> > File sky/sdk/lib/framework/rendering/box.dart (right):
> > 
> >
>
https://codereview.chromium.org/1146893004/diff/80001/sky/sdk/lib/framework/r...
> > sky/sdk/lib/framework/rendering/box.dart:399:
> ..setColorMode(sky.TransferMode.srcInMode),
> > Did you mean to revert to srcInMode here?
> 
> I just matched what shadow.dart did:
> https://github.com/domokit/mojo/blob/master/sky/examples/raw/shadow.dart#L20

The blink shadow code uses srcIn:
https://github.com/domokit/mojo/blob/2aad0d73122aca64491b96c69530578e6f0d81fd...

> 
> >
>
https://codereview.chromium.org/1146893004/diff/80001/sky/sdk/lib/framework/r...
> > sky/sdk/lib/framework/rendering/box.dart:402: // TODO(mpcomplete): blur
should
> respect the offset?
> > Are you saying it doesn't, or that it does but you're not sure if it should?
> (FWIW I don't even know what it means for blur to respect the offset :)
> 
> Should blur get bigger as the offset gets bigger?  We should make blur
> configurable is really what I mean.

Ah OK. That's the second param (sigma) to MaskFilter.Blur.

[eseidel, 2015-06-04 21:39:11]

Updated.  ptal.

[Matt Perry, 2015-06-04 21:42:27]

lgtm

[eseidel, 2015-06-04 21:49:28]

Committed patchset #7 (id:120001) manually as
f8f5f34ff27447e70fa30b0bf79171c0cad40558 (presubmit successful).