From e50d0e622b0f65fcbd91ebe4c98a08c2c4b1ab4a Mon Sep 17 00:00:00 2001 From: Adam Barth Date: Mon, 8 Jun 2015 16:20:11 -0700 Subject: [PATCH] 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 | 21 ++--- sdk/lib/framework/fn2.dart | 8 ++ sdk/lib/framework/rendering/block.dart | 11 ++- sdk/lib/framework/rendering/box.dart | 77 +++++++++++++++++-- sdk/lib/framework/rendering/paragraph.dart | 72 ++++++++++++----- 5 files changed, 149 insertions(+), 40 deletions(-) diff --git a/sdk/lib/framework/components2/popup_menu.dart b/sdk/lib/framework/components2/popup_menu.dart index 71559a528a..a398980bf5 100644 --- a/sdk/lib/framework/components2/popup_menu.dart +++ b/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 + ) + ); } } diff --git a/sdk/lib/framework/fn2.dart b/sdk/lib/framework/fn2.dart index 727f56adcb..197f1d0f74 100644 --- a/sdk/lib/framework/fn2.dart +++ b/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; diff --git a/sdk/lib/framework/rendering/block.dart b/sdk/lib/framework/rendering/block.dart index cb9a5396ec..2e925e3611 100644 --- a/sdk/lib/framework/rendering/block.dart +++ b/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 { } @@ -49,11 +50,14 @@ class RenderBlock extends RenderBox with ContainerRenderObjectMixin '${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 } 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() { diff --git a/sdk/lib/framework/rendering/paragraph.dart b/sdk/lib/framework/rendering/paragraph.dart index 551045fe6e..834a534930 100644 --- a/sdk/lib/framework/rendering/paragraph.dart +++ b/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); } -- GitLab