Commit 2d32f1f1 authored by Ian Hickson's avatar Ian Hickson

Fix fallout from intrinsics changes (#4299)

* Fix fallout from intrinsics changes

* Move away from isInfinite in favor of isFinite
parent c26fcfdf
...@@ -77,11 +77,11 @@ class RenderBlock extends RenderBox ...@@ -77,11 +77,11 @@ class RenderBlock extends RenderBox
assert(() { assert(() {
switch (mainAxis) { switch (mainAxis) {
case Axis.horizontal: case Axis.horizontal:
if (constraints.maxWidth.isInfinite) if (!constraints.hasBoundedWidth)
return true; return true;
break; break;
case Axis.vertical: case Axis.vertical:
if (constraints.maxHeight.isInfinite) if (!constraints.hasBoundedHeight)
return true; return true;
break; break;
} }
...@@ -97,11 +97,11 @@ class RenderBlock extends RenderBox ...@@ -97,11 +97,11 @@ class RenderBlock extends RenderBox
assert(() { assert(() {
switch (mainAxis) { switch (mainAxis) {
case Axis.horizontal: case Axis.horizontal:
if (!constraints.maxHeight.isInfinite) if (constraints.hasBoundedHeight)
return true; return true;
break; break;
case Axis.vertical: case Axis.vertical:
if (!constraints.maxWidth.isInfinite) if (constraints.hasBoundedWidth)
return true; return true;
break; break;
} }
......
...@@ -916,19 +916,22 @@ abstract class RenderBox extends RenderObject { ...@@ -916,19 +916,22 @@ abstract class RenderBox extends RenderObject {
return result; return result;
} }
final double minIntrinsicWidth = testIntrinsic(getMinIntrinsicWidth, 'getMinIntrinsicWidth', constraints.maxWidth); void testIntrinsicsForValues(double getMin(double extent), double getMax(double extent), String name, double constraint) {
final double maxIntrinsicWidth = testIntrinsic(getMaxIntrinsicWidth, 'getMaxIntrinsicWidth', constraints.maxWidth); final double min = testIntrinsic(getMin, 'getMinIntrinsic$name', constraint);
if (minIntrinsicWidth > maxIntrinsicWidth) { final double max = testIntrinsic(getMax, 'getMaxIntrinsic$name', constraint);
failures.writeln(' * getMinIntrinsicWidth(${constraints.maxWidth}) returned a larger value ($minIntrinsicWidth) than getMaxIntrinsicWidth(${constraints.maxWidth}) ($maxIntrinsicWidth)'); if (min > max) {
failureCount += 1; failures.writeln(' * getMinIntrinsic$name($constraint) returned a larger value ($min) than getMaxIntrinsic$name($constraint) ($max)');
} failureCount += 1;
final double minIntrinsicHeight = testIntrinsic(getMinIntrinsicHeight, 'getMinIntrinsicHeight', constraints.maxHeight); }
final double maxIntrinsicHeight = testIntrinsic(getMaxIntrinsicHeight, 'getMaxIntrinsicHeight', constraints.maxHeight);
if (minIntrinsicHeight > maxIntrinsicHeight) {
failures.writeln(' * getMinIntrinsicHeight(${constraints.maxHeight}) returned a larger value ($minIntrinsicHeight) than getMaxIntrinsicHeight(${constraints.maxHeight}) ($maxIntrinsicHeight)');
failureCount += 1;
} }
testIntrinsicsForValues(getMinIntrinsicWidth, getMaxIntrinsicWidth, 'Width', double.INFINITY);
testIntrinsicsForValues(getMinIntrinsicHeight, getMaxIntrinsicHeight, 'Height', double.INFINITY);
if (constraints.hasBoundedWidth)
testIntrinsicsForValues(getMinIntrinsicWidth, getMaxIntrinsicWidth, 'Width', constraints.maxWidth);
if (constraints.hasBoundedHeight)
testIntrinsicsForValues(getMinIntrinsicHeight, getMaxIntrinsicHeight, 'Height', constraints.maxHeight);
// TODO(ianh): Test that values are internally consistent in more ways than the above. // TODO(ianh): Test that values are internally consistent in more ways than the above.
RenderObject.debugCheckingIntrinsics = false; RenderObject.debugCheckingIntrinsics = false;
......
...@@ -179,40 +179,77 @@ abstract class GridDelegate { ...@@ -179,40 +179,77 @@ abstract class GridDelegate {
return getGridSpecification(constraints, childCount).gridSize; return getGridSpecification(constraints, childCount).gridSize;
} }
// TODO(ianh): It's a bit dubious to be using the getSize function from the delegate to
// figure out the intrinsic dimensions. We really should either not support intrinsics,
// or we should expose intrinsic delegate callbacks and throw if they're not implemented.
/// Returns the minimum width that this grid could be without failing to paint /// Returns the minimum width that this grid could be without failing to paint
/// its contents within itself. /// its contents within itself.
/// ///
/// Override this to provide a more efficient solution. The default
/// implementation actually instantiates a grid specification and measures the
/// grid at the given height and child count.
///
/// For more details, see [RenderBox.getMinIntrinsicWidth]. /// For more details, see [RenderBox.getMinIntrinsicWidth].
double getMinIntrinsicWidth(double height, int childCount) { double getMinIntrinsicWidth(double height, int childCount) {
return _getGridSize(new BoxConstraints.tightForFinite(height: height), childCount).width; final double width = _getGridSize(new BoxConstraints.tightForFinite(height: height), childCount).width;
if (width.isFinite)
return width;
return 0.0;
} }
/// Returns the smallest width beyond which increasing the width never /// Returns the smallest width beyond which increasing the width never
/// decreases the preferred height. /// decreases the preferred height.
/// ///
/// Override this to provide a more efficient solution. The default
/// implementation actually instantiates a grid specification and measures the
/// grid at the given height and child count.
///
/// For more details, see [RenderBox.getMaxIntrinsicWidth]. /// For more details, see [RenderBox.getMaxIntrinsicWidth].
double getMaxIntrinsicWidth(double height, int childCount) { double getMaxIntrinsicWidth(double height, int childCount) {
return _getGridSize(new BoxConstraints.tightForFinite(height: height), childCount).width; final double width = _getGridSize(new BoxConstraints.tightForFinite(height: height), childCount).width;
if (width.isFinite)
return width;
return 0.0;
} }
/// Return the minimum height that this grid could be without failing to paint /// Return the minimum height that this grid could be without failing to paint
/// its contents within itself. /// its contents within itself.
/// ///
/// Override this to provide a more efficient solution. The default
/// implementation actually instantiates a grid specification and measures the
/// grid at the given width and child count.
///
/// For more details, see [RenderBox.getMinIntrinsicHeight]. /// For more details, see [RenderBox.getMinIntrinsicHeight].
double getMinIntrinsicHeight(double width, int childCount) { double getMinIntrinsicHeight(double width, int childCount) {
return _getGridSize(new BoxConstraints.tightForFinite(width: width), childCount).height; final double height = _getGridSize(new BoxConstraints.tightForFinite(width: width), childCount).height;
if (height.isFinite)
return height;
return 0.0;
} }
/// Returns the smallest height beyond which increasing the height never /// Returns the smallest height beyond which increasing the height never
/// decreases the preferred width. /// decreases the preferred width.
/// ///
/// Override this to provide a more efficient solution. The default
/// implementation actually instantiates a grid specification and measures the
/// grid at the given width and child count.
///
/// For more details, see [RenderBox.getMaxIntrinsicHeight]. /// For more details, see [RenderBox.getMaxIntrinsicHeight].
double getMaxIntrinsicHeight(double width, int childCount) { double getMaxIntrinsicHeight(double width, int childCount) {
return _getGridSize(new BoxConstraints.tightForFinite(width: width), childCount).height; final double height = _getGridSize(new BoxConstraints.tightForFinite(width: width), childCount).height;
if (height.isFinite)
return height;
return 0.0;
} }
} }
/// A [GridDelegate] the places its children in order throughout the grid. /// A [GridDelegate] the places its children in order throughout the grid.
///
/// Subclasses must still provide a mechanism for sizing the grid by
/// implementing [getGridSpecification], and should also provide efficent
/// versions of the intrinsic sizing functions ([getMinIntrinsicWidth] and
/// company).
abstract class GridDelegateWithInOrderChildPlacement extends GridDelegate { abstract class GridDelegateWithInOrderChildPlacement extends GridDelegate {
/// Initializes [columnSpacing], [rowSpacing], and [padding] for subclasses. /// Initializes [columnSpacing], [rowSpacing], and [padding] for subclasses.
/// ///
...@@ -254,7 +291,7 @@ abstract class GridDelegateWithInOrderChildPlacement extends GridDelegate { ...@@ -254,7 +291,7 @@ abstract class GridDelegateWithInOrderChildPlacement extends GridDelegate {
} }
/// A [GridDelegate] that divides the grid's width evenly amount a fixed number of columns. /// A [GridDelegate] that divides the grid's width evenly for a fixed number of columns.
class FixedColumnCountGridDelegate extends GridDelegateWithInOrderChildPlacement { class FixedColumnCountGridDelegate extends GridDelegateWithInOrderChildPlacement {
/// Creates a grid delegate that uses a fixed column count. /// Creates a grid delegate that uses a fixed column count.
/// ///
...@@ -302,13 +339,17 @@ class FixedColumnCountGridDelegate extends GridDelegateWithInOrderChildPlacement ...@@ -302,13 +339,17 @@ class FixedColumnCountGridDelegate extends GridDelegateWithInOrderChildPlacement
@override @override
double getMinIntrinsicWidth(double height, int childCount) { double getMinIntrinsicWidth(double height, int childCount) {
// TODO(ianh): Strictly, this should examine the children.
return 0.0; return 0.0;
} }
@override @override
double getMaxIntrinsicWidth(double height, int childCount) { double getMaxIntrinsicWidth(double height, int childCount) {
// TODO(ianh): Strictly, this should examine the children.
return 0.0; return 0.0;
} }
// TODO(ianh): Provide efficient intrinsic height functions.
} }
/// A [GridDelegate] that fills the width with a variable number of tiles. /// A [GridDelegate] that fills the width with a variable number of tiles.
...@@ -318,7 +359,6 @@ class FixedColumnCountGridDelegate extends GridDelegateWithInOrderChildPlacement ...@@ -318,7 +359,6 @@ class FixedColumnCountGridDelegate extends GridDelegateWithInOrderChildPlacement
/// ///
/// - The tile width evenly divides the width of the grid. /// - The tile width evenly divides the width of the grid.
/// - The tile width is at most [maxTileWidth]. /// - The tile width is at most [maxTileWidth].
///
class MaxTileWidthGridDelegate extends GridDelegateWithInOrderChildPlacement { class MaxTileWidthGridDelegate extends GridDelegateWithInOrderChildPlacement {
/// Creates a grid delegate that uses a max tile width. /// Creates a grid delegate that uses a max tile width.
/// ///
...@@ -342,7 +382,18 @@ class MaxTileWidthGridDelegate extends GridDelegateWithInOrderChildPlacement { ...@@ -342,7 +382,18 @@ class MaxTileWidthGridDelegate extends GridDelegateWithInOrderChildPlacement {
@override @override
GridSpecification getGridSpecification(BoxConstraints constraints, int childCount) { GridSpecification getGridSpecification(BoxConstraints constraints, int childCount) {
assert(constraints.maxWidth < double.INFINITY); if (!constraints.maxWidth.isFinite) {
// if we're unbounded, just shrink-wrap around a single line of tiles
return new GridSpecification.fromRegularTiles(
tileWidth: maxTileWidth,
tileHeight: maxTileWidth / tileAspectRatio,
columnCount: childCount,
rowCount: 1,
columnSpacing: columnSpacing,
rowSpacing: rowSpacing,
padding: padding
);
}
final double gridWidth = math.max(0.0, constraints.maxWidth - padding.horizontal); final double gridWidth = math.max(0.0, constraints.maxWidth - padding.horizontal);
int columnCount = (gridWidth / maxTileWidth).ceil(); int columnCount = (gridWidth / maxTileWidth).ceil();
int rowCount = (childCount / columnCount).ceil(); int rowCount = (childCount / columnCount).ceil();
...@@ -368,6 +419,7 @@ class MaxTileWidthGridDelegate extends GridDelegateWithInOrderChildPlacement { ...@@ -368,6 +419,7 @@ class MaxTileWidthGridDelegate extends GridDelegateWithInOrderChildPlacement {
@override @override
double getMinIntrinsicWidth(double height, int childCount) { double getMinIntrinsicWidth(double height, int childCount) {
// TODO(ianh): Strictly, this should examine the children.
return 0.0; return 0.0;
} }
...@@ -375,6 +427,8 @@ class MaxTileWidthGridDelegate extends GridDelegateWithInOrderChildPlacement { ...@@ -375,6 +427,8 @@ class MaxTileWidthGridDelegate extends GridDelegateWithInOrderChildPlacement {
double getMaxIntrinsicWidth(double height, int childCount) { double getMaxIntrinsicWidth(double height, int childCount) {
return maxTileWidth * childCount; return maxTileWidth * childCount;
} }
// TODO(ianh): Provide efficient intrinsic height functions.
} }
/// Parent data for use with [RenderGrid] /// Parent data for use with [RenderGrid]
......
...@@ -554,7 +554,7 @@ class RenderIntrinsicWidth extends RenderProxyBox { ...@@ -554,7 +554,7 @@ class RenderIntrinsicWidth extends RenderProxyBox {
if (child == null) if (child == null)
return 0.0; return 0.0;
double childResult = child.getMaxIntrinsicWidth(height); double childResult = child.getMaxIntrinsicWidth(height);
assert(!childResult.isInfinite); assert(childResult.isFinite);
return _applyStep(childResult, _stepWidth); return _applyStep(childResult, _stepWidth);
} }
......
...@@ -717,7 +717,7 @@ class RenderTable extends RenderBox { ...@@ -717,7 +717,7 @@ class RenderTable extends RenderBox {
// winner of the 2016 world's most expensive intrinsic dimension function award // winner of the 2016 world's most expensive intrinsic dimension function award
// honorable mention, most likely to improve if taught about memoization award // honorable mention, most likely to improve if taught about memoization award
assert(_children.length == rows * columns); assert(_children.length == rows * columns);
final List<double> widths = _computeColumnWidths(constraints); final List<double> widths = _computeColumnWidths(new BoxConstraints.tightForFinite(width: width));
double rowTop = 0.0; double rowTop = 0.0;
for (int y = 0; y < rows; y += 1) { for (int y = 0; y < rows; y += 1) {
double rowHeight = 0.0; double rowHeight = 0.0;
...@@ -773,6 +773,7 @@ class RenderTable extends RenderBox { ...@@ -773,6 +773,7 @@ class RenderTable extends RenderBox {
} }
List<double> _computeColumnWidths(BoxConstraints constraints) { List<double> _computeColumnWidths(BoxConstraints constraints) {
assert(constraints != null);
assert(_children.length == rows * columns); assert(_children.length == rows * columns);
// We apply the constraints to the column widths in the order of // We apply the constraints to the column widths in the order of
// least important to most important: // least important to most important:
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment