Fill out some more documentation about building RenderBox subclasses.

sky/sdk/lib/rendering/README.md

Index: sky/sdk/lib/rendering/README.md
diff --git a/sky/sdk/lib/rendering/README.md b/sky/sdk/lib/rendering/README.md
index 77ea6d3de4a2c564fc854329e52c37c390aab9a1..86cd5032b64b895f780fdfd2f88ae7a7a7948cdd 100644
--- a/sky/sdk/lib/rendering/README.md
+++ b/sky/sdk/lib/rendering/README.md
@@ -121,6 +121,98 @@ Writing new subclasses
 
 ### The RenderBox contract
 
+A RenderBox subclass is required to implement the following contract:
+
+* If its constructor takes any nodes to be used as children, the

[hansmuller, 2015/06/24 19:53:30]

I wasn't sure what "nodes" were here. It looks like they are
RenderObject "ChildType" subclasses per mixins like
ContainerRenderObjectMixin<ChildType extends RenderObject, ...>

Do children need to be RenderObjects or RenderBoxes or can they be any
AbstractNode subclass?

[Hixie, 2015/06/25 19:15:31]

done

+  constructor must call add() for each such child.
+
+* If it has any data to store on its children, define a BoxParentData

[hansmuller, 2015/06/24 19:53:30]

"it" is the the RenderBox subclass

The setParentData() method below implies that each child must be a RenderBox?

[Hixie, 2015/06/25 19:15:30]

Fixed the sample code, clarified the text.

+  subclass and override setParentData() to initialise the child's
+  parent data appropriately, as in:
+
+```dart
+  class FooParentData extends BoxParentData { ... }
+
+  // In RenderFoo
+  void setParentData(RenderBox child) {

[hansmuller, 2015/06/24 19:53:30]

I found this method name confusing; I'd expected the parameters to be (RenderBox
child, ParentData data). Maybe it should be called initParentData()?

Who's responsible for calling setParentData() on each child?

[Hixie, 2015/06/25 19:15:31]

Added a paragraph about this. In general parentData isn't well documented yet.

+    if (child.parentData is! FooParentData)
+      child.parentData = new FooParentData();
+  }
+```
+
+* The class must encapsulate a layout algorithm that has the following
+  features:
+
+** It uses as input a set of constraints, described by a
+   BoxConstraints object, and a set of zero or more children, as
+   determined by the class itself, and has as output a Size, and
+   positions for each child.

[hansmuller, 2015/06/24 19:53:31]

It also causes each child's size to be set?

[Hixie, 2015/06/25 19:15:31]

elaborated.

+
+** The algorithm can decide the Size in one of two ways: either

[hansmuller, 2015/06/24 19:53:30]

It would be nice to explain why RenderBoxes make the sizedByParent distinction.

[Hixie, 2015/06/25 19:15:31]

done.

+   exclusively based on the given constraints (i.e. it is effectively
+   sized entirely by its parent), or based on those constraints and
+   the dimensions of the children.
+
+   In the former case, the class must have a sizedByParent getter that
+   returns true, and it must have a `performResize()` method that uses

[hansmuller, 2015/06/24 19:53:30]

I'm not entirely clear on what a performResize() override needs to do. Not clear
what the name is trying to convey either.  

[Hixie, 2015/06/25 19:15:31]

I don't understand. Doesn't this paragraph exactly describe what it needs to do?
I don't know how to make it clearer...

+   the object's `constraints` member to size itself by setting the
+   `size` member. The size must be consistent, a given set of
+   constraints must always result in the same size.
+
+   In the latter case, it will inherit the default `sizedByParent`
+   getter that returns false, and it will size itself in the
+   `performLayout()` function described below.
+
+* The following methods must report numbers consistent with the output
+  of the layout algorithm used:
+
+** `double getMinIntrinsicWidth(BoxConstraints constraints)` must
+   return the width that fits within the given constraints below which

[hansmuller, 2015/06/24 19:53:30]

This is somewhat hard to parse. Maybe you could substitute "constrained width"
for "the width that fits within the given constraints". Using horizontal text as
an example might clarify this; like the minimum intrinsic width of "a b cd e"
would be the width of "cd" ...

[Hixie, 2015/06/25 19:15:31]

Added example. "constrained width" doesn't say constrained by what.

+   making the width constraint smaller would not increase the
+   resulting height, or, to put it another way, the narrowest width at
+   which the box can be rendered without failing to lay the children
+   out within itself.
+
+** `double getMaxIntrinsicWidth(BoxConstraints constraints)` must
+   return the width that fits within the given constraints above which
+   making the width constraint larger would not decrease the resulting
+   height.
+
+** `double getMinIntrinsicHeight(BoxConstraints constraints)` must
+   return the height that fits within the given constraints below
+   which making the height constraint smaller would not increase the
+   resulting width, or, to put it another way, the shortest height at
+   which the box can be rendered without failing to lay the children
+   out within itself.

[hansmuller, 2015/06/24 19:53:31]

This constraint might also be explained with a text layout example. I assume the
min intrinsic height of "a b cd e" is the line height.

[Hixie, 2015/06/25 19:15:31]

Added example.

+
+** `double getMaxIntrinsicHeight(BoxConstraints constraints)` must
+   return the height that fits within the given constraints above
+   which making the height constraint larger would not decrease the
+   resulting width. If the height depends exclusively on the width,
+   and the width does not depend on the height, then
+   `getMinIntrinsicHeight()` and `getMaxIntrinsicHeight()` will return the
+   same number given the same constraints.
+
+* The box must have a `performLayout()` method that walks over the

[hansmuller, 2015/06/24 19:53:30]

A slightly simplified version of RenderBlock's performLayout might help clarify
this section. For example it would clarify what you mean by "walk over the
object's children".

Also: I think it would helpful to start this section by saying that the
performLayout() is responsible for computing the size and position of each child
and setting its own size.

[Hixie, 2015/06/25 19:15:31]

I've added a TODO to document how to walk over a ContainerRenderObjectMixin
subclass' children. In general I don't know how to clarify it further, though.
It depends entirely on how the class represents its children.
I've added text explaining the responsibilities.

+  object's children, if any, and for each one calls `child.layout()`

[hansmuller, 2015/06/24 19:53:30]

It would be helpful to note here that child.layout() causes the child's
performLayout() method to run, after which child.size is the child's preferred
(?) size. I assume that the box is free to reset the child's size?

[Hixie, 2015/06/25 19:15:31]

I've clarified that layout() sometimes results in performLayout() being called
recursively on the child.

I've clarified that the parent must not set the child's size.

+  with a BoxConstraints object as the first argument, and optionally a
+  second argument named `parentUsesSize` which is set to true if the
+  child's resulting size will in any way influence the layout, and
+  must be false if the child's resulting size is ignored. The
+  children's positions (`child.parentData.position`) must also be set.
+
+  If `sizedByParent` is false, then `performLayout()` must also size
+  the object (by setting `size`), otherwise, the size must be left

[hansmuller, 2015/06/24 19:53:30]

...must also size the child?

If sizedByParent is false then the parent must set the child's size?

[Hixie, 2015/06/25 19:15:31]

This is about the parent's size, not the child's size. I've tried to clarify it.

+  untouched.
+
+* The `size` member must never be set to an infinite value.

[hansmuller, 2015/06/24 19:53:30]

At some point I think we're going to need to explain how size units relate to
pixels and font dimensions.

[Hixie, 2015/06/25 19:15:31]

Added.

+
+* The box must also implement `hitTestChildren()`.
+  TODO(ianh): Define this better
+
+* The box must also implement `paint()`.
+  TODO(ianh): Define this better
+
 #### Using RenderProxyBox
 
 ### The Hit Testing contract