Support for background images on cards, style demo home

Support for background images on cards, style demo home

R=ianh@google.com, abarth, hixie

Committed: https://chromium.googlesource.com/external/mojo/+/2004a15b5e54791a07cca9baac9573049a8fb41b

[jackson, 2015-06-26 23:07:42]

(I'll move the thumbnails into the proper directories)

[Hixie, 2015-06-26 23:13:26]

ianh@google.com changed reviewers:
+ ianh@google.com

[Hixie, 2015-06-26 23:13:27]

lgtm

https://codereview.chromium.org/1217623002/diff/40001/sky/sdk/home.dart
File sky/sdk/home.dart (right):

https://codereview.chromium.org/1217623002/diff/40001/sky/sdk/home.dart#newco...
sky/sdk/home.dart:106: // new SkyDemo('Licences (Old)', 'LICENSES.sky'),
we should get this working asap...

https://codereview.chromium.org/1217623002/diff/40001/sky/sdk/lib/painting/bo...
File sky/sdk/lib/painting/box_painter.dart (right):

https://codereview.chromium.org/1217623002/diff/40001/sky/sdk/lib/painting/bo...
sky/sdk/lib/painting/box_painter.dart:130: final String src;
I don't think we should have the painter code working with URLs.

I think we should abstract this out into a separate class that handles the image
caching and so forth, which has callbacks for "size changed" and "image
changed". This would also enable us to do animated images.

It's fine if you just file a bug for this or put a TODO or whatever.

https://codereview.chromium.org/1217623002/diff/40001/sky/sdk/lib/painting/bo...
sky/sdk/lib/painting/box_painter.dart:162: String toString() =>
'BackgroundImage($image, $fit, $repeat)';
what does Image.toString() do?

https://codereview.chromium.org/1217623002/diff/40001/sky/sdk/lib/painting/bo...
sky/sdk/lib/painting/box_painter.dart:267: 
we're going to want to split this function into subfunctions for each thing it
does soon.

https://codereview.chromium.org/1217623002/diff/40001/sky/sdk/lib/painting/bo...
sky/sdk/lib/painting/box_painter.dart:298: // dst = // rect;
remove comments here

[abarth-chromium, 2015-06-26 23:35:58]

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

[abarth-chromium, 2015-06-26 23:35:59]

https://codereview.chromium.org/1217623002/diff/40001/sky/sdk/lib/rendering/b...
File sky/sdk/lib/rendering/box.dart (right):

https://codereview.chromium.org/1217623002/diff/40001/sky/sdk/lib/rendering/b...
sky/sdk/lib/rendering/box.dart:968:
_painter.decoration.backgroundImage.removeChangeListener(markNeedsPaint);
Don't we need to removeChangeListener even if value.backgroundImage == null (for
example, if _painter.decoration.backgroundImage is non-null)?

[jackson, 2015-06-29 16:53:25]

https://codereview.chromium.org/1217623002/diff/40001/sky/sdk/home.dart
File sky/sdk/home.dart (right):

https://codereview.chromium.org/1217623002/diff/40001/sky/sdk/home.dart#newco...
sky/sdk/home.dart:106: // new SkyDemo('Licences (Old)', 'LICENSES.sky'),
On 2015/06/26 23:13:26, Hixie wrote:
> we should get this working asap...

Agree, I'll file an issue

https://codereview.chromium.org/1217623002/diff/40001/sky/sdk/lib/painting/bo...
File sky/sdk/lib/painting/box_painter.dart (right):

https://codereview.chromium.org/1217623002/diff/40001/sky/sdk/lib/painting/bo...
sky/sdk/lib/painting/box_painter.dart:130: final String src;
On 2015/06/26 23:13:26, Hixie wrote:
> I don't think we should have the painter code working with URLs.
> 
> I think we should abstract this out into a separate class that handles the
image
> caching and so forth, which has callbacks for "size changed" and "image
> changed". This would also enable us to do animated images.
> 
> It's fine if you just file a bug for this or put a TODO or whatever.

I'll do this, added a TODO for now.

https://codereview.chromium.org/1217623002/diff/40001/sky/sdk/lib/painting/bo...
sky/sdk/lib/painting/box_painter.dart:162: String toString() =>
'BackgroundImage($image, $fit, $repeat)';
On 2015/06/26 23:13:27, Hixie wrote:
> what does Image.toString() do?

I changed it to $src, I think that's fine for now, but I'll update it when I
make the change above.

https://codereview.chromium.org/1217623002/diff/40001/sky/sdk/lib/painting/bo...
sky/sdk/lib/painting/box_painter.dart:267: 
On 2015/06/26 23:13:27, Hixie wrote:
> we're going to want to split this function into subfunctions for each thing it
> does soon.

Done.

https://codereview.chromium.org/1217623002/diff/40001/sky/sdk/lib/painting/bo...
sky/sdk/lib/painting/box_painter.dart:298: // dst = // rect;
On 2015/06/26 23:13:27, Hixie wrote:
> remove comments here

I think for this mode we can just say src = rect and dst = rect.

https://codereview.chromium.org/1217623002/diff/40001/sky/sdk/lib/rendering/b...
File sky/sdk/lib/rendering/box.dart (right):

https://codereview.chromium.org/1217623002/diff/40001/sky/sdk/lib/rendering/b...
sky/sdk/lib/rendering/box.dart:968:
_painter.decoration.backgroundImage.removeChangeListener(markNeedsPaint);
On 2015/06/26 23:35:59, abarth wrote:
> Don't we need to removeChangeListener even if value.backgroundImage == null
(for
> example, if _painter.decoration.backgroundImage is non-null)?

We do, thanks

[jackson, 2015-06-29 23:22:27]

Committed patchset #13 (id:240001) manually as
2004a15b5e54791a07cca9baac9573049a8fb41b (presubmit successful).