ShrinkWrap the Stocks menu

The popup menu in the stocks app is supposed to size its width to the max
intrinsic width of the menu. This CL teaches it how to do that.

It's a shame that we need to ceilToDouble the output of RenderParagraph. If we
don't do that, we run into floating point layout trouble and the menu triggers
a line break. The correct fix is to do layout in fixed point.

R=ianh@google.com

Review URL: https://codereview.chromium.org/1168113005

sdk/lib/framework/components2/popup_menu.dart

@@ -98,14 +98,17 @@ class PopupMenu extends AnimatedComponent {
       return new PopupMenuItem(key: i++, children: item, opacity: opacity);
     }));
 
-    return new Material(
-      content: new Container(
-        padding: const EdgeDims.all(8.0),
-        // border-radius: 2px
-        decoration: new BoxDecoration(backgroundColor: Grey[50]),
-        // inlineStyle: _inlineStyle(),
-        child: new BlockContainer(children: children)
-      ),
-      level: level);
+    return new ShrinkWrapWidth(
+      child: new Material(
+        content: new Container(
+          padding: const EdgeDims.all(8.0),
+          // border-radius: 2px
+          decoration: new BoxDecoration(backgroundColor: Grey[50]),
+          // inlineStyle: _inlineStyle(),
+          child: new BlockContainer(children: children)
+        ),
+        level: level
+      )
+    );
   }
 }

sdk/lib/framework/fn2.dart

@@ -437,6 +437,14 @@ class ConstrainedBox extends OneChildRenderObjectWrapper {
   }
 }
 
+class ShrinkWrapWidth extends OneChildRenderObjectWrapper {
+  RenderShrinkWrapWidth root;
+
+  ShrinkWrapWidth({ UINode child, Object key }) : super(child: child, key: key);
+
+  RenderShrinkWrapWidth createNode() => new RenderShrinkWrapWidth();
+}
+
 class Transform extends OneChildRenderObjectWrapper {
   RenderTransform root;
   final Matrix4 transform;

sdk/lib/framework/rendering/block.dart

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import 'box.dart';
+import 'dart:math' as math;
 import 'object.dart';
 
 class BlockParentData extends BoxParentData with ContainerParentDataMixin<RenderBox> { }
@@ -49,11 +50,14 @@ class RenderBlock extends RenderBox with ContainerRenderObjectMixin<RenderBox, B
     return width;
   }
 
+  BoxConstraints _getInnerConstraintsForWidth(double width) {
+    return new BoxConstraints(minWidth: width, maxWidth: width);
+  }
+
   double _getIntrinsicHeight(BoxConstraints constraints) {
     double height = 0.0;
     double width = constraints.constrainWidth(constraints.maxWidth);
-    BoxConstraints innerConstraints = new BoxConstraints(minWidth: width,
-                                                         maxWidth: width);
+    BoxConstraints innerConstraints = _getInnerConstraintsForWidth(width);
     RenderBox child = firstChild;
     while (child != null) {
       double childHeight = child.getMinIntrinsicHeight(innerConstraints);
@@ -75,10 +79,11 @@ class RenderBlock extends RenderBox with ContainerRenderObjectMixin<RenderBox, B
   void performLayout() {
     assert(constraints is BoxConstraints);
     double width = constraints.constrainWidth(constraints.maxWidth);
+    BoxConstraints innerConstraints = _getInnerConstraintsForWidth(width);
     double y = 0.0;
     RenderBox child = firstChild;
     while (child != null) {
-      child.layout(new BoxConstraints(minWidth: width, maxWidth: width), parentUsesSize: true);
+      child.layout(innerConstraints, parentUsesSize: true);
       assert(child.parentData is BlockParentData);
       child.parentData.position = new Point(0.0, y);
       y += child.size.height;

sdk/lib/framework/rendering/box.dart

@@ -85,6 +85,20 @@ class BoxConstraints {
       maxHeight: math.min(maxHeight, constraints.maxHeight));
   }
 
+  BoxConstraints applyWidth(double width) {
+    return new BoxConstraints(minWidth: width,
+                              maxWidth: width,
+                              minHeight: minHeight,
+                              maxHeight: maxHeight);
+  }
+
+  BoxConstraints applyHeight(double height) {
+    return new BoxConstraints(minWidth: minWidth,
+                              maxWidth: maxWidth,
+                              minHeight: height,
+                              maxHeight: height);
+  }
+
   final double minWidth;
   final double maxWidth;
   final double minHeight;
@@ -329,6 +343,49 @@ class RenderConstrainedBox extends RenderProxyBox {
   String debugDescribeSettings(String prefix) => '${super.debugDescribeSettings(prefix)}${prefix}additionalConstraints: ${additionalConstraints}\n';
 }
 
+class RenderShrinkWrapWidth extends RenderProxyBox {
+  RenderShrinkWrapWidth({ RenderBox child }) : super(child);
+
+  BoxConstraints _getInnerConstraints(BoxConstraints constraints) {
+    double width = child.getMaxIntrinsicWidth(constraints);
+    assert(width == constraints.constrainWidth(width));
+    return constraints.applyWidth(width);
+  }
+
+  double getMinIntrinsicWidth(BoxConstraints constraints) {
+    if (child != null)
+      return child.getMaxIntrinsicWidth(constraints);
+    return constraints.constrainWidth(0.0);
+  }
+
+  double getMaxIntrinsicWidth(BoxConstraints constraints) {
+    if (child != null)
+      return child.getMaxIntrinsicWidth(constraints);
+    return constraints.constrainWidth(0.0);
+  }
+
+  double getMinIntrinsicHeight(BoxConstraints constraints) {
+    if (child != null)
+      return child.getMinIntrinsicHeight(_getInnerConstraints(constraints));
+    return constraints.constrainWidth(0.0);
+  }
+
+  double getMaxIntrinsicHeight(BoxConstraints constraints) {
+    if (child != null)
+      return child.getMaxIntrinsicHeight(_getInnerConstraints(constraints));
+    return constraints.constrainWidth(0.0);
+  }
+
+  void performLayout() {
+    if (child != null) {
+      child.layout(_getInnerConstraints(constraints));
+      size = child.size;
+    } else {
+      performResize();
+    }
+  }
+}
+
 class RenderClip extends RenderProxyBox {
   RenderClip({ RenderBox child }) : super(child);
 
@@ -361,27 +418,31 @@ class RenderPadding extends RenderBox with RenderObjectWithChildMixin<RenderBox>
   }
 
   double getMinIntrinsicWidth(BoxConstraints constraints) {
+    double totalPadding = padding.left + padding.right;
     if (child != null)
-      return child.getMinIntrinsicWidth(constraints.deflate(padding));
-    return constraints.constrainWidth(padding.left + padding.right);
+      return child.getMinIntrinsicWidth(constraints.deflate(padding)) + totalPadding;
+    return constraints.constrainWidth(totalPadding);
   }
 
   double getMaxIntrinsicWidth(BoxConstraints constraints) {
+    double totalPadding = padding.left + padding.right;
     if (child != null)
-      return child.getMaxIntrinsicWidth(constraints.deflate(padding));
-    return constraints.constrainWidth(padding.left + padding.right);
+      return child.getMaxIntrinsicWidth(constraints.deflate(padding)) + totalPadding;
+    return constraints.constrainWidth(totalPadding);
   }
 
   double getMinIntrinsicHeight(BoxConstraints constraints) {
+    double totalPadding = padding.top + padding.bottom;
     if (child != null)
-      return child.getMinIntrinsicHeight(constraints.deflate(padding));
-    return constraints.constrainHeight(padding.top + padding.bottom);
+      return child.getMinIntrinsicHeight(constraints.deflate(padding)) + totalPadding;
+    return constraints.constrainHeight(totalPadding);
   }
 
   double getMaxIntrinsicHeight(BoxConstraints constraints) {
+    double totalPadding = padding.top + padding.bottom;
     if (child != null)
-      return child.getMaxIntrinsicHeight(constraints.deflate(padding));
-    return constraints.constrainHeight(padding.top + padding.bottom);
+      return child.getMaxIntrinsicHeight(constraints.deflate(padding)) + totalPadding;
+    return constraints.constrainHeight(totalPadding);
   }
 
   void performLayout() {

sdk/lib/framework/rendering/paragraph.dart

@@ -12,6 +12,17 @@ class RenderInline extends RenderObject {
   RenderInline(this.data);
 }
 
+// Unfortunately, using full precision floating point here causes bad layouts
+// because floating point math isn't associative. If we add and subtract
+// padding, for example, we'll get different values when we estimate sizes and
+// when we actually compute layout because the operations will end up associated
+// differently. To work around this problem for now, we round fractional pixel
+// values up to the nearest whole pixel value. The right long-term fix is to do
+// layout using fixed precision arithmetic.
+double _applyFloatingPointHack(double layoutValue) {
+  return layoutValue.ceilToDouble();
+}
+
 class RenderParagraph extends RenderBox {
 
   RenderParagraph({
@@ -40,45 +51,66 @@ class RenderParagraph extends RenderBox {
     }
   }
 
-  // We don't currently support this for RenderParagraph
+  BoxConstraints _constraintsForCurrentLayout;
+
+  sky.Element _layout(BoxConstraints constraints) {
+    _layoutRoot.maxWidth = constraints.maxWidth;
+    _layoutRoot.minWidth = constraints.minWidth;
+    _layoutRoot.minHeight = constraints.minHeight;
+    _layoutRoot.maxHeight = constraints.maxHeight;
+    _layoutRoot.layout();
+    _constraintsForCurrentLayout = constraints;
+  }
+
   double getMinIntrinsicWidth(BoxConstraints constraints) {
-    assert(false);
-    return constraints.constrainWidth(0.0);
+    _layout(constraints);
+    return constraints.constrainWidth(
+        _applyFloatingPointHack(_layoutRoot.rootElement.minContentWidth));
   }
 
-  // We don't currently support this for RenderParagraph
   double getMaxIntrinsicWidth(BoxConstraints constraints) {
-    assert(false);
-    return constraints.constrainWidth(0.0);
+    _layout(constraints);
+    return constraints.constrainWidth(
+        _applyFloatingPointHack(_layoutRoot.rootElement.maxContentWidth));
+  }
+
+  double _getIntrinsicHeight(BoxConstraints constraints) {
+    _layout(constraints);
+    return constraints.constrainHeight(
+        _applyFloatingPointHack(_layoutRoot.rootElement.height.ceilToDouble));
   }
 
-  // We don't currently support this for RenderParagraph
   double getMinIntrinsicHeight(BoxConstraints constraints) {
-    assert(false);
-    return constraints.constrainHeight(0.0);
+    return _getIntrinsicHeight(constraints);
   }
 
-  // We don't currently support this for RenderParagraph
   double getMaxIntrinsicHeight(BoxConstraints constraints) {
-    assert(false);
-    return constraints.constrainHeight(0.0);
+    return _getIntrinsicHeight(constraints);
   }
 
   void performLayout() {
-    _layoutRoot.maxWidth = constraints.maxWidth;
-    _layoutRoot.minWidth = constraints.minWidth;
-    _layoutRoot.minHeight = constraints.minHeight;
-    _layoutRoot.maxHeight = constraints.maxHeight;
-    _layoutRoot.layout();
-    // rootElement.width always expands to fill, use maxContentWidth instead.
+    _layout(constraints);
     sky.Element root = _layoutRoot.rootElement;
-    size = constraints.constrain(new Size(root.maxContentWidth, root.height));
+    // rootElement.width always expands to fill, use maxContentWidth instead.
+    size = constraints.constrain(new Size(_applyFloatingPointHack(root.maxContentWidth),
+                                          _applyFloatingPointHack(root.height)));
   }
 
   void paint(RenderObjectDisplayList canvas) {
-    if (_color != null)
+    // Ideally we could compute the min/max intrinsic width/height with a
+    // non-destructive operation. However, currently, computing these values
+    // will destroy state inside the layout root. If that happens, we need to
+    // get back the correct state by calling _layout again.
+    //
+    // TODO(abarth): Make computing the min/max intrinsic width/height a
+    //               non-destructive operation.
+    if (_constraintsForCurrentLayout != constraints && constraints != null)
+      _layout(constraints);
+
+    if (_color != null) {
       _layoutRoot.rootElement.style['color'] =
           'rgba(${_color.red}, ${_color.green}, ${_color.blue}, ${_color.alpha / 255.0 })';
+    }
     _layoutRoot.paint(canvas);
   }