Extend Sky's RenderFlex with intrinsic sizes and compute cross-size height

sky/sdk/lib/framework/rendering/flex.dart

Index: sky/sdk/lib/framework/rendering/flex.dart
diff --git a/sky/sdk/lib/framework/rendering/flex.dart b/sky/sdk/lib/framework/rendering/flex.dart
index 683d94786872da404f9be4aeeda6b9016cd31b85..bc4e0ee621590cf4654c317cd283f57e66306894 100644
--- a/sky/sdk/lib/framework/rendering/flex.dart
+++ b/sky/sdk/lib/framework/rendering/flex.dart
@@ -27,6 +27,8 @@ enum FlexJustifyContent {
   spaceAround,
 }
 
+typedef double ChildSizingFunction(RenderBox child, BoxConstraints constraints);

[abarth-chromium, 2015/06/10 04:23:20]

_ChildSizingFunction

The _ scopes the definition to this file.

[jackson, 2015/06/10 04:44:46]

Acknowledged.

[kulakowski, 2015/06/10 16:07:14]

Pedantry hopefully useful in the future: There is only library level visibility
in Dart, not file level.

+
 class RenderFlex extends RenderBox with ContainerRenderObjectMixin<RenderBox, FlexBoxParentData>,
                                         RenderBoxContainerDefaultsMixin<RenderBox, FlexBoxParentData> {
   // lays out RenderBox children using flexible layout
@@ -59,35 +61,170 @@ class RenderFlex extends RenderBox with ContainerRenderObjectMixin<RenderBox, Fl
       child.parentData = new FlexBoxParentData();
   }
 
-  // We don't currently support this for RenderFlex
+  double _getIntrinsicSize(BoxConstraints constraints,
+                           FlexDirection sizingDirection,
+                           ChildSizingFunction childSize) {
+    // http://www.w3.org/TR/2015/WD-css-flexbox-1-20150514/#intrinsic-sizes
+    if (_direction == sizingDirection) {
+      // INTRINSIC MAIN SIZE
+      // Intrinsic main size is the smallest size the flex container can take
+      // while maintaining the min/max-content contributions of its flex items.
+      BoxConstraints childConstraints;
+      switch(_direction) {
+        case FlexDirection.horizontal:
+          childConstraints = new BoxConstraints(maxHeight: constraints.maxHeight);
+          break;
+        case FlexDirection.vertical:
+          childConstraints = new BoxConstraints(maxWidth: constraints.maxWidth);
+          break;
+      }
+
+      double totalFlex = 0.0;
+      double inflexibleSpace = 0.0;
+      double maxFlexFractionSoFar = 0.0;
+      RenderBox child = firstChild;
+      while (child != null) {
+        int flex = _getFlex(child);
+        totalFlex += flex;
+        if (flex > 0) {
+          double flexFraction = childSize(child, childConstraints) / _getFlex(child);
+          maxFlexFractionSoFar = math.max(maxFlexFractionSoFar, flexFraction);
+        } else {
+          inflexibleSpace += childSize(child, childConstraints);
+        }
+        child = child.parentData.nextSibling;
+      }
+      double mainSize = maxFlexFractionSoFar * totalFlex + inflexibleSpace;
+
+      // Ensure that we don't violate the given constraints with our result
+      switch(_direction) {
+        case FlexDirection.horizontal:
+          return constraints.constrainWidth(mainSize);
+        case FlexDirection.vertical:
+          return constraints.constrainHeight(mainSize);
+      }
+    } else {
+      // INTRINSIC CROSS SIZE
+      // The spec wants us to perform layout into the given available main-axis
+      // space and return the cross size. That's too expensive, so instead we
+      // size inflexible children according to their max intrinsic size in the
+      // main direction and use those constraints to determine their max
+      // intrinsic size in the cross direction. We don't care if the caller
+      // asked for max or min -- the answer is always computed using the
+      // max size in the main direction.
+
+      double availableMainSpace;
+      BoxConstraints childConstraints;
+      switch(_direction) {
+        case FlexDirection.horizontal:
+          childConstraints = new BoxConstraints(maxWidth: constraints.maxWidth);
+          availableMainSpace = innerConstraints.maxWidth;
+          break;
+        case FlexDirection.vertical:
+          childConstraints = new BoxConstraints(maxHeight: constraints.maxHeight);
+          availableMainSpace = innerConstraints.maxHeight;
+          break;
+      }
+
+      // Get inflexible space using the max in the main direction
+      int totalFlex = 0;
+      double inflexibleSpace = 0.0;
+      double maxCrossSize = 0.0;
+      RenderBox child = firstChild;
+      while (child != null) {
+        int flex = _getFlex(child);
+        totalFlex += flex;
+        double mainSize;
+        double crossSize;
+        if (flex == 0) {
+          switch (_direction) {
+              case FlexDirection.horizontal:
+                mainSize = child.getMaxIntrinsicWidth(childConstraints);
+                BoxConstraints widthConstraints = 
+                  new BoxConstraints(minWidth: mainSize, maxWidth: mainSize);
+                crossSize = child.getMinIntrinsicHeight(widthConstraints);
+                break;
+              case FlexDirection.vertical:
+                mainSize = child.getMaxIntrinsicHeight(childConstraints);
+                BoxConstraints heightConstraints = 
+                  new BoxConstraints(minWidth: mainSize, maxWidth: mainSize);
+                crossSize = child.getMinIntrinsicWidth(heightConstraints);
+                inflexibleSpace += mainSize;

[abarth-chromium, 2015/06/10 04:23:20]

Is this line missing from the other case?

[jackson, 2015/06/10 04:44:46]

Acknowledged.

+                break;
+          }
+          inflexibleSpace += mainSize;

[abarth-chromium, 2015/06/10 04:23:20]

Wait, it seems like we're doing this twice in the vertical case.

[jackson, 2015/06/10 04:44:46]

Acknowledged.

+          maxCrossSize = math.max(maxCrossSize, crossSize);
+        }
+        child = child.parentData.nextSibling;
+      }
+
+      // Determine the spacePerFlex by allocating the remaining available space
+      double spacePerFlex = (availableMainSpace - inflexibleSpace) / totalFlex;
+
+      // Size remaining items, take the maximum
+      child = firstChild;
+      while (child != null) {
+        int flex = _getFlex(child);
+        if (flex > 0) {
+          double childMainSize = spacePerFlex * flex;
+          double crossSize;
+          switch (_direction) {
+            case FlexDirection.horizontal:
+              BoxConstraints childConstraints = 
+                new BoxConstraints(minWidth: childMainSize, maxWidth: childMainSize);
+              crossSize = child.getMaxIntrinsicHeight(childConstraints);

[abarth-chromium, 2015/06/10 04:23:20]

It's odd that we use getMaxIntrinsicHeight here but getMinIntrinsicHeight in the
inflexible case.  I understand that it shouldn't matter...  Maybe we should use
getMaxIntrinsicHeight for consistency and then assert that getMinIntrinsicHeight
gives the same answer.

[jackson, 2015/06/10 04:44:46]

Acknowledged.

+              maxCrossSize = math.max(maxCrossSize, crossSize);
+              break;
+            case FlexDirection.vertical:
+              BoxConstraints childConstraints =
+                new BoxConstraints(minHeight: childMainSize, maxHeight: childMainSize);
+              crossSize = child.getMaxIntrinsicWidth(childConstraints);
+              break;
+          }
+          maxCrossSize = math.max(maxCrossSize, crossSize);

[abarth-chromium, 2015/06/10 04:23:20]

Again, we seem to have the problem with this line being duplicated above (on
line 176).  In this case, it doesn't matter for correctness, but should probably
be fixed anyway.

[jackson, 2015/06/10 04:44:46]

Acknowledged.

+        }
+        child = child.parentData.nextSibling;
+      }
+
+      // Ensure that we don't violate the given constraints with our result
+      switch(_direction) {
+        case FlexDirection.horizontal:
+          return innerConstraints.constrainHeight(maxCrossSize);
+        case FlexDirection.vertical:
+          return innerConstraints.constrainWidth(maxCrossSize);
+      }
+    }
+  }
+
   double getMinIntrinsicWidth(BoxConstraints constraints) {
-    assert(false);
-    return constraints.constrainWidth(0.0);
+    return _getIntrinsicSize(
+      constraints,
+      FlexDirection.horizontal,

[abarth-chromium, 2015/06/10 04:23:20]

Shouldn't we use the flex direction of the container?

[jackson, 2015/06/10 04:44:46]

This argument (sizingDirection) is compared with the container's direction at
the top of _getIntrinsicSize to figure out whether we're determining the main
size or the cross size. Does that make sense? Maybe it would be less confusing
if rather than passing it as an argument, split _getIntrinsicSize into a main
size and cross size method, and decide which to call in getMinIntrinsicWidth.

[abarth-chromium, 2015/06/10 04:52:07]

Ah, I see.  I think it's fine this way.  Another choice would be to use a named
argument so that the call site would be self-documenting.

+      (c, innerConstraints) => c.getMinIntrinsicWidth(innerConstraints)
+    );
   }
 
-  // We don't currently support this for RenderFlex
   double getMaxIntrinsicWidth(BoxConstraints constraints) {
-    assert(false);
-    return constraints.constrainWidth(0.0);
+    return _getIntrinsicSize(
+      constraints,
+      FlexDirection.horizontal,

[abarth-chromium, 2015/06/10 04:23:20]

ditto

+      (c, innerConstraints) => c.getMaxIntrinsicWidth(innerConstraints)
+    );
   }
 
-  // We don't currently support this for RenderFlex
   double getMinIntrinsicHeight(BoxConstraints constraints) {
-    assert(false);
-    return constraints.constrainHeight(0.0);
+    return _getIntrinsicSize(
+      constraints,
+      FlexDirection.vertical,

[abarth-chromium, 2015/06/10 04:23:20]

ditto

+      (c, innerConstraints) => c.getMinIntrinsicHeight(innerConstraints)
+    );
   }
 
-  // We don't currently support this for RenderFlex
   double getMaxIntrinsicHeight(BoxConstraints constraints) {
-    assert(false);
-    return constraints.constrainHeight(0.0);
-  }
-
-  bool get sizedByParent => true;
-  void performResize() {
-    size = constraints.constrain(new Size(constraints.maxWidth, constraints.maxHeight));
-    assert(size.height < double.INFINITY);
-    assert(size.width < double.INFINITY);
+    return _getIntrinsicSize(
+      constraints,
+      FlexDirection.vertical,

[abarth-chromium, 2015/06/10 04:23:20]

ditto

+      (c, innerConstraints) => c.getMaxIntrinsicHeight(innerConstraints));
   }
 
   int _getFlex(RenderBox child) {
@@ -101,7 +238,9 @@ class RenderFlex extends RenderBox with ContainerRenderObjectMixin<RenderBox, Fl
     int totalFlex = 0;
     int totalChildren = 0;
     assert(constraints != null);
-    double freeSpace = (_direction == FlexDirection.horizontal) ? constraints.maxWidth : constraints.maxHeight;
+    final double mainSize = (_direction == FlexDirection.horizontal) ? constraints.maxWidth : constraints.maxHeight;
+    double crossSize = 0.0;  // This will be determined after laying out the children
+    double freeSpace = mainSize;
     RenderBox child = firstChild;
     while (child != null) {
       totalChildren++;
@@ -131,15 +270,19 @@ class RenderFlex extends RenderBox with ContainerRenderObjectMixin<RenderBox, Fl
             innerConstraints = new BoxConstraints(maxHeight: constraints.maxHeight,
                                                   minWidth: spaceForChild,
                                                   maxWidth: spaceForChild);
+            child.layout(innerConstraints, parentUsesSize: true);
+            usedSpace += child.size.width;
+            crossSize = math.max(crossSize, child.size.height);
             break;
           case FlexDirection.vertical:
             innerConstraints = new BoxConstraints(minHeight: spaceForChild,
                                                   maxHeight: spaceForChild,
                                                   maxWidth: constraints.maxWidth);
+            child.layout(innerConstraints, parentUsesSize: true);
+            usedSpace += child.size.height;
+            crossSize = math.max(crossSize, child.size.width);
             break;
         }
-        child.layout(innerConstraints, parentUsesSize: true);
-        usedSpace += _direction == FlexDirection.horizontal ? child.size.width : child.size.height;
       }
       child = child.parentData.nextSibling;
     }
@@ -172,6 +315,15 @@ class RenderFlex extends RenderBox with ContainerRenderObjectMixin<RenderBox, Fl
         break;
     }
 
+    switch (_direction) {
+      case FlexDirection.horizontal:
+        size = constraints.constrain(new Size(mainSize, crossSize));
+        break;
+      case FlexDirection.vertical:
+        size = constraints.constrain(new Size(crossSize, mainSize));
+        break;
+    }
+
     // Position elements. For now, center the flex items in the cross direction
     double mainDimPosition = leadingSpace;
     child = firstChild;