Use Point, Size, and Rect in layout2.dart

Use Point, Size, and Rect in layout2.dart

R=ianh@google.com

Committed: https://chromium.googlesource.com/external/mojo/+/1b4a6bc5547448a51bfc61cc50892af639069f93

[Hixie, 2015-05-28 18:45:22]

https://codereview.chromium.org/1156303004/diff/1/sky/engine/core/painting/Po...
File sky/engine/core/painting/Point.dart (right):

https://codereview.chromium.org/1156303004/diff/1/sky/engine/core/painting/Po...
sky/engine/core/painting/Point.dart:12: Point([this.x = 0.0, this.y = 0.0]);
I think we should make these required. These don't really convey what they mean:

  new Point();
  new Point(5.0);

https://codereview.chromium.org/1156303004/diff/1/sky/engine/core/painting/Si...
File sky/engine/core/painting/Size.dart (right):

https://codereview.chromium.org/1156303004/diff/1/sky/engine/core/painting/Si...
sky/engine/core/painting/Size.dart:12: Size([this.width = 0.0, this.height =
0.0]);
Ditto.

https://codereview.chromium.org/1156303004/diff/1/sky/examples/raw/sector-lay...
File sky/examples/raw/sector-layout.dart (right):

https://codereview.chromium.org/1156303004/diff/1/sky/examples/raw/sector-lay...
sky/examples/raw/sector-layout.dart:267: canvas.paintChild(child, new
sky.Point());
I don't like the way this looks. It's not obvious that it's a zero point.
Maybe try sky.Point.ZERO and have that be a getter that returns a new 0,0 ?

https://codereview.chromium.org/1156303004/diff/1/sky/examples/raw/sector-lay...
sky/examples/raw/sector-layout.dart:466: canvas.paintChild(child, new
sky.Point(width/2.0, height/2.0));
We could replace width/height with a Rect and then use thatRect.center here.

https://codereview.chromium.org/1156303004/diff/1/sky/examples/raw/simple_ren...
File sky/examples/raw/simple_render_tree.dart (right):

https://codereview.chromium.org/1156303004/diff/1/sky/examples/raw/simple_ren...
sky/examples/raw/simple_render_tree.dart:13: RenderSolidColor(int
backgroundColor, { this.desiredSize })
If Size was a constant class, you could default it to INFINITY here.

https://codereview.chromium.org/1156303004/diff/1/sky/examples/raw/simple_ren...
sky/examples/raw/simple_render_tree.dart:20: height: desiredSize.height);
This would be simplified by withConstraints() taking a Rect. Not quite sure how
it'd work though.

https://codereview.chromium.org/1156303004/diff/1/sky/examples/raw/simple_ren...
sky/examples/raw/simple_render_tree.dart:44: RenderNode child = new
RenderSolidColor(backgroundColor, desiredSize: new Size.infinite());
I think the old code looked better (implying infinity). It's weird that you have
to explicitly ask for an infinite box.

https://codereview.chromium.org/1156303004/diff/1/sky/sdk/lib/framework/fn2.dart
File sky/sdk/lib/framework/fn2.dart (right):

https://codereview.chromium.org/1156303004/diff/1/sky/sdk/lib/framework/fn2.d...
sky/sdk/lib/framework/fn2.dart:949: RenderSolidColor(int backgroundColor, {
this.desiredSize })
If it's a required argument (which it appears it now is), we shouldn't have it
be optional.

But I'd rather we made Size constant and had it be optional, defaulting to
infinity.

https://codereview.chromium.org/1156303004/diff/1/sky/sdk/lib/framework/fn2.d...
sky/sdk/lib/framework/fn2.dart:954: return new
BoxDimensions.withConstraints(constraints,
What if desiredSize is null?

https://codereview.chromium.org/1156303004/diff/1/sky/sdk/lib/framework/layou...
File sky/sdk/lib/framework/layout2.dart (right):

https://codereview.chromium.org/1156303004/diff/1/sky/sdk/lib/framework/layou...
sky/sdk/lib/framework/layout2.dart:39: translate(position.x, position.y);
translate() should take a Point. :-)

https://codereview.chromium.org/1156303004/diff/1/sky/sdk/lib/framework/layou...
sky/sdk/lib/framework/layout2.dart:511: sky.Point position = new sky.Point();
It's tempting to suggest that BoxParentData should just mix Point in...

[Hixie, 2015-05-28 19:41:06]

lgtm

[abarth-chromium, 2015-05-28 19:42:59]

Committed patchset #3 (id:40001) manually as
1b4a6bc5547448a51bfc61cc50892af639069f93 (presubmit successful).