Commit 60d9ab7e authored by Ian Hickson's avatar Ian Hickson

Fix some intrinsic constraints contract violations.

RenderBlock wasn't constraining the results.
RenderPadding wasn't constraining the results (which matters
especially when the constraints can't fit the padding in the first
place).
RenderViewport wasn't constraining the results.

Add a test for the block case.

To catch this kind of thing in the future, add some asserts to
debugDoesMeetConstraints() that all four intrinsic functions return
values that are within the constraints.

RenderBlockViewport doesn't support returning intrinsics, so turn off
the "no intrinsic support" asserts (and return zero) when we're doing
this new assert.

This new assert screwed up the custom layout classes' tests, so adjust
those tests to ignore the callbacks invoked from these asserts.

Add to the _debugReportException() method a short summary of the
descendants of this node. It's important to have this information when
debugging errors like these intrinsic constraints contract violations
because often nodes just pass the values through to their child so you
have to go several steps down to find the actual problem.

Fixes https://github.com/flutter/flutter/issues/1210
parent aa701d44
......@@ -131,7 +131,7 @@ class RenderBlock extends RenderBlockBase {
double minExtent: 0.0
}) : super(children: children, direction: direction, itemExtent: itemExtent, minExtent: minExtent);
double _getIntrinsicCrossAxis(BoxConstraints constraints, _ChildSizingFunction childSize) {
double _getIntrinsicCrossAxis(BoxConstraints constraints, _ChildSizingFunction childSize, _Constrainer constrainer) {
double extent = 0.0;
BoxConstraints innerConstraints = isVertical ? constraints.widthConstraints() : constraints.heightConstraints();
RenderBox child = firstChild;
......@@ -140,10 +140,10 @@ class RenderBlock extends RenderBlockBase {
final BlockParentData childParentData = child.parentData;
child = childParentData.nextSibling;
}
return extent;
return constrainer(extent);
}
double _getIntrinsicMainAxis(BoxConstraints constraints) {
double _getIntrinsicMainAxis(BoxConstraints constraints, _Constrainer constrainer) {
double extent = 0.0;
BoxConstraints innerConstraints = _getInnerConstraints(constraints);
RenderBox child = firstChild;
......@@ -160,7 +160,7 @@ class RenderBlock extends RenderBlockBase {
final BlockParentData childParentData = child.parentData;
child = childParentData.nextSibling;
}
return math.max(extent, minExtent);
return constrainer(math.max(extent, minExtent));
}
double getMinIntrinsicWidth(BoxConstraints constraints) {
......@@ -168,10 +168,11 @@ class RenderBlock extends RenderBlockBase {
if (isVertical) {
return _getIntrinsicCrossAxis(
constraints,
(RenderBox child, BoxConstraints innerConstraints) => child.getMinIntrinsicWidth(innerConstraints)
(RenderBox child, BoxConstraints innerConstraints) => child.getMinIntrinsicWidth(innerConstraints),
constraints.constrainWidth
);
}
return _getIntrinsicMainAxis(constraints);
return _getIntrinsicMainAxis(constraints, constraints.constrainWidth);
}
double getMaxIntrinsicWidth(BoxConstraints constraints) {
......@@ -179,29 +180,32 @@ class RenderBlock extends RenderBlockBase {
if (isVertical) {
return _getIntrinsicCrossAxis(
constraints,
(RenderBox child, BoxConstraints innerConstraints) => child.getMaxIntrinsicWidth(innerConstraints)
(RenderBox child, BoxConstraints innerConstraints) => child.getMaxIntrinsicWidth(innerConstraints),
constraints.constrainWidth
);
}
return _getIntrinsicMainAxis(constraints);
return _getIntrinsicMainAxis(constraints, constraints.constrainWidth);
}
double getMinIntrinsicHeight(BoxConstraints constraints) {
assert(constraints.isNormalized);
if (isVertical)
return _getIntrinsicMainAxis(constraints);
return _getIntrinsicMainAxis(constraints, constraints.constrainHeight);
return _getIntrinsicCrossAxis(
constraints,
(RenderBox child, BoxConstraints innerConstraints) => child.getMinIntrinsicWidth(innerConstraints)
(RenderBox child, BoxConstraints innerConstraints) => child.getMinIntrinsicWidth(innerConstraints),
constraints.constrainHeight
);
}
double getMaxIntrinsicHeight(BoxConstraints constraints) {
assert(constraints.isNormalized);
if (isVertical)
return _getIntrinsicMainAxis(constraints);
return _getIntrinsicMainAxis(constraints, constraints.constrainHeight);
return _getIntrinsicCrossAxis(
constraints,
(RenderBox child, BoxConstraints innerConstraints) => child.getMaxIntrinsicWidth(innerConstraints)
(RenderBox child, BoxConstraints innerConstraints) => child.getMaxIntrinsicWidth(innerConstraints),
constraints.constrainHeight
);
}
......@@ -346,7 +350,7 @@ class RenderBlockViewport extends RenderBlockBase {
if (intrinsicCallback == null) {
assert(() {
'RenderBlockViewport does not support returning intrinsic dimensions if the relevant callbacks have not been specified.';
return false;
return RenderObject.debugInDebugDoesMeetConstraints;
});
return constrainer(0.0);
}
......
......@@ -531,15 +531,30 @@ abstract class RenderBox extends RenderObject {
/// The box constraints most recently received from the parent.
BoxConstraints get constraints => super.constraints;
bool debugDoesMeetConstraints() {
assert(!RenderObject.debugInDebugDoesMeetConstraints);
RenderObject.debugInDebugDoesMeetConstraints = true;
assert(constraints != null);
// verify that the size is not infinite
assert(_size != null);
assert(() {
'See https://flutter.io/layout/#unbounded-constraints';
return !_size.isInfinite;
});
// verify that the size is within the constraints
bool result = constraints.isSatisfiedBy(_size);
if (!result)
debugPrint("${this.runtimeType} does not meet its constraints. Constraints: $constraints, size: $_size");
// verify that the intrinsics are also within the constraints
double intrinsic;
intrinsic = getMinIntrinsicWidth(constraints);
assert(intrinsic == constraints.constrainWidth(intrinsic));
intrinsic = getMaxIntrinsicWidth(constraints);
assert(intrinsic == constraints.constrainWidth(intrinsic));
intrinsic = getMinIntrinsicHeight(constraints);
assert(intrinsic == constraints.constrainHeight(intrinsic));
intrinsic = getMaxIntrinsicHeight(constraints);
assert(intrinsic == constraints.constrainHeight(intrinsic));
RenderObject.debugInDebugDoesMeetConstraints = false;
return result;
}
......
......@@ -436,18 +436,51 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
dynamic debugOwner;
void _debugReportException(String method, dynamic exception, StackTrace stack) {
if (debugRenderingExceptionHandler != null) {
debugRenderingExceptionHandler(this, method, exception, stack);
} else {
debugPrint('-- EXCEPTION CAUGHT BY RENDERING LIBRARY -------------------------------');
debugPrint('The following exception was raised during $method():');
debugPrint('$exception');
debugPrint('The following RenderObject was being processed when the exception was fired:\n${this}');
if (debugOwner != null)
debugPrint('That RenderObject had the following owner:\n$debugOwner');
debugPrint('Stack trace:');
debugPrint('$stack');
debugPrint('------------------------------------------------------------------------');
try {
if (debugRenderingExceptionHandler != null) {
debugRenderingExceptionHandler(this, method, exception, stack);
} else {
debugPrint('-- EXCEPTION CAUGHT BY RENDERING LIBRARY -------------------------------');
debugPrint('The following exception was raised during $method():');
debugPrint('$exception');
debugPrint('The following RenderObject was being processed when the exception was fired:\n${this}');
if (debugOwner != null)
debugPrint('This RenderObject had the following owner:\n$debugOwner');
int depth = 0;
List<String> descendants = <String>[];
void visitor(RenderObject child) {
descendants.add('${" " * depth}$child');
depth += 1;
if (depth < 5)
child.visitChildren(visitor);
depth -= 1;
}
visitChildren(visitor);
if (descendants.length > 1) {
debugPrint('This RenderObject had the following descendants:');
} else if (descendants.length == 1) {
debugPrint('This RenderObject had the following child:');
} else {
debugPrint('This RenderObject has no descendants.');
}
descendants.forEach(debugPrint);
assert(() {
if (debugInDebugDoesMeetConstraints) {
debugPrint('This exception was thrown while debugDoesMeetConstraints() was running.');
debugPrint('debugDoesMeetConstraints() verifies that some invariants are not being');
debugPrint('violated. For example, it verifies that RenderBox objects are sized in');
debugPrint('a manner consistent with the constraints provided, and, in addition, that');
debugPrint('the getMinIntrinsicWidth(), getMaxIntrinsicWidth(), etc, functions all');
debugPrint('return consistent values within the same constraints.');
}
return true;
});
debugPrint('Stack trace:');
debugPrint('$stack');
debugPrint('------------------------------------------------------------------------');
}
} catch (exception) {
debugPrint('(exception during exception handler: $exception)');
}
}
......@@ -486,6 +519,13 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
Constraints get constraints => _constraints;
/// Override this function in a subclass to verify that your state matches the constraints object.
bool debugDoesMeetConstraints();
/// When true, debugDoesMeetConstraints() is currently executing.
///
/// This should be set by implementations of debugDoesMeetConstraints() so that
/// tests can selectively ignore custom layout callbacks. It should not be set
/// outside of debugDoesMeetConstraints() implementations and should not be used
/// for purposes other than tests.
static bool debugInDebugDoesMeetConstraints = false;
bool debugAncestorsAlreadyMarkedNeedsLayout() {
if (_relayoutSubtreeRoot == null)
return true; // we haven't yet done layout even once, so there's nothing for us to do
......
......@@ -110,7 +110,7 @@ class RenderPadding extends RenderShiftedBox {
assert(constraints.isNormalized);
double totalPadding = padding.left + padding.right;
if (child != null)
return child.getMinIntrinsicWidth(constraints.deflate(padding)) + totalPadding;
return constraints.constrainWidth(child.getMinIntrinsicWidth(constraints.deflate(padding)) + totalPadding);
return constraints.constrainWidth(totalPadding);
}
......@@ -118,7 +118,7 @@ class RenderPadding extends RenderShiftedBox {
assert(constraints.isNormalized);
double totalPadding = padding.left + padding.right;
if (child != null)
return child.getMaxIntrinsicWidth(constraints.deflate(padding)) + totalPadding;
return constraints.constrainWidth(child.getMaxIntrinsicWidth(constraints.deflate(padding)) + totalPadding);
return constraints.constrainWidth(totalPadding);
}
......@@ -126,7 +126,7 @@ class RenderPadding extends RenderShiftedBox {
assert(constraints.isNormalized);
double totalPadding = padding.top + padding.bottom;
if (child != null)
return child.getMinIntrinsicHeight(constraints.deflate(padding)) + totalPadding;
return constraints.constrainHeight(child.getMinIntrinsicHeight(constraints.deflate(padding)) + totalPadding);
return constraints.constrainHeight(totalPadding);
}
......@@ -134,7 +134,7 @@ class RenderPadding extends RenderShiftedBox {
assert(constraints.isNormalized);
double totalPadding = padding.top + padding.bottom;
if (child != null)
return child.getMaxIntrinsicHeight(constraints.deflate(padding)) + totalPadding;
return constraints.constrainHeight(child.getMaxIntrinsicHeight(constraints.deflate(padding)) + totalPadding);
return constraints.constrainHeight(totalPadding);
}
......
......@@ -88,28 +88,28 @@ class RenderViewport extends RenderBox with RenderObjectWithChildMixin<RenderBox
double getMinIntrinsicWidth(BoxConstraints constraints) {
assert(constraints.isNormalized);
if (child != null)
return child.getMinIntrinsicWidth(_getInnerConstraints(constraints));
return constraints.constrainWidth(child.getMinIntrinsicWidth(_getInnerConstraints(constraints)));
return super.getMinIntrinsicWidth(constraints);
}
double getMaxIntrinsicWidth(BoxConstraints constraints) {
assert(constraints.isNormalized);
if (child != null)
return child.getMaxIntrinsicWidth(_getInnerConstraints(constraints));
return constraints.constrainWidth(child.getMaxIntrinsicWidth(_getInnerConstraints(constraints)));
return super.getMaxIntrinsicWidth(constraints);
}
double getMinIntrinsicHeight(BoxConstraints constraints) {
assert(constraints.isNormalized);
if (child != null)
return child.getMinIntrinsicHeight(_getInnerConstraints(constraints));
return constraints.constrainHeight(child.getMinIntrinsicHeight(_getInnerConstraints(constraints)));
return super.getMinIntrinsicHeight(constraints);
}
double getMaxIntrinsicHeight(BoxConstraints constraints) {
assert(constraints.isNormalized);
if (child != null)
return child.getMaxIntrinsicHeight(_getInnerConstraints(constraints));
return constraints.constrainHeight(child.getMaxIntrinsicHeight(_getInnerConstraints(constraints)));
return super.getMaxIntrinsicHeight(constraints);
}
......
......@@ -156,7 +156,7 @@ class _MixedViewportElement extends RenderObjectElement<MixedViewport> {
'MixedViewport does not support returning intrinsic dimensions. ' +
'Calculating the intrinsic dimensions would require walking the entire child list, ' +
'which defeats the entire point of having a lazily-built list of children.';
return false;
return RenderObject.debugInDebugDoesMeetConstraints;
});
return null;
}
......
......@@ -13,6 +13,54 @@ class TestBlockPainter extends Painter {
}
void main() {
test('block intrinsics', () {
RenderParagraph paragraph = new RenderParagraph(
new StyledTextSpan(
new TextStyle(
height: 1.0
),
<TextSpan>[new PlainTextSpan('Hello World')]
)
);
const BoxConstraints unconstrained = const BoxConstraints();
double textWidth = paragraph.getMaxIntrinsicWidth(unconstrained);
double oneLineTextHeight = paragraph.getMinIntrinsicHeight(unconstrained);
final BoxConstraints constrained = new BoxConstraints(maxWidth: textWidth * 0.9);
double wrappedTextWidth = paragraph.getMinIntrinsicWidth(unconstrained);
double twoLinesTextHeight = paragraph.getMinIntrinsicHeight(constrained);
// controls
expect(wrappedTextWidth, lessThan(textWidth));
expect(paragraph.getMinIntrinsicWidth(unconstrained), equals(wrappedTextWidth));
expect(paragraph.getMaxIntrinsicWidth(constrained), equals(constrained.maxWidth));
expect(oneLineTextHeight, lessThan(twoLinesTextHeight));
expect(twoLinesTextHeight, lessThan(oneLineTextHeight * 3.0));
expect(paragraph.getMaxIntrinsicHeight(unconstrained), equals(oneLineTextHeight));
expect(paragraph.getMaxIntrinsicHeight(constrained), equals(twoLinesTextHeight));
RenderBox testBlock = new RenderBlock(
children: <RenderBox>[
paragraph,
]
);
expect(testBlock.getMinIntrinsicWidth(unconstrained), equals(wrappedTextWidth));
expect(testBlock.getMinIntrinsicWidth(constrained), equals(wrappedTextWidth));
expect(testBlock.getMaxIntrinsicWidth(unconstrained), equals(textWidth));
expect(testBlock.getMaxIntrinsicWidth(constrained), equals(constrained.maxWidth));
expect(testBlock.getMinIntrinsicHeight(unconstrained), equals(oneLineTextHeight));
expect(testBlock.getMinIntrinsicHeight(constrained), equals(twoLinesTextHeight));
expect(testBlock.getMaxIntrinsicHeight(unconstrained), equals(oneLineTextHeight));
expect(testBlock.getMaxIntrinsicHeight(constrained), equals(twoLinesTextHeight));
final BoxConstraints empty = new BoxConstraints.tight(Size.zero);
expect(testBlock.getMinIntrinsicWidth(empty), equals(0.0));
expect(testBlock.getMaxIntrinsicWidth(empty), equals(0.0));
expect(testBlock.getMinIntrinsicHeight(empty), equals(0.0));
expect(testBlock.getMaxIntrinsicHeight(empty), equals(0.0));
});
test('overlay painters can attach and detach', () {
TestBlockPainter first = new TestBlockPainter();
TestBlockPainter second = new TestBlockPainter();
......
......@@ -11,7 +11,8 @@ class TestMultiChildLayoutDelegate extends MultiChildLayoutDelegate {
BoxConstraints getSizeConstraints;
Size getSize(BoxConstraints constraints) {
getSizeConstraints = constraints;
if (!RenderObject.debugInDebugDoesMeetConstraints)
getSizeConstraints = constraints;
return new Size(200.0, 300.0);
}
......@@ -22,6 +23,7 @@ class TestMultiChildLayoutDelegate extends MultiChildLayoutDelegate {
bool performLayoutIsChild;
void performLayout(Size size, BoxConstraints constraints) {
assert(!RenderObject.debugInDebugDoesMeetConstraints);
expect(() {
performLayoutSize = size;
performLayoutConstraints = constraints;
......@@ -34,6 +36,7 @@ class TestMultiChildLayoutDelegate extends MultiChildLayoutDelegate {
bool shouldRelayoutCalled = false;
bool shouldRelayoutValue = false;
bool shouldRelayout(_) {
assert(!RenderObject.debugInDebugDoesMeetConstraints);
shouldRelayoutCalled = true;
return shouldRelayoutValue;
}
......
......@@ -3,6 +3,7 @@
// found in the LICENSE file.
import 'package:flutter_test/flutter_test.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/widgets.dart';
import 'package:test/test.dart';
......@@ -13,11 +14,13 @@ class TestOneChildLayoutDelegate extends OneChildLayoutDelegate {
Size childSizeFromGetPositionForChild;
Size getSize(BoxConstraints constraints) {
constraintsFromGetSize = constraints;
if (!RenderObject.debugInDebugDoesMeetConstraints)
constraintsFromGetSize = constraints;
return new Size(200.0, 300.0);
}
BoxConstraints getConstraintsForChild(BoxConstraints constraints) {
assert(!RenderObject.debugInDebugDoesMeetConstraints);
constraintsFromGetConstraintsForChild = constraints;
return new BoxConstraints(
minWidth: 100.0,
......@@ -28,6 +31,7 @@ class TestOneChildLayoutDelegate extends OneChildLayoutDelegate {
}
Offset getPositionForChild(Size size, Size childSize) {
assert(!RenderObject.debugInDebugDoesMeetConstraints);
sizeFromGetPositionForChild = size;
childSizeFromGetPositionForChild = childSize;
return Offset.zero;
......@@ -36,6 +40,7 @@ class TestOneChildLayoutDelegate extends OneChildLayoutDelegate {
bool shouldRelayoutCalled = false;
bool shouldRelayoutValue = false;
bool shouldRelayout(_) {
assert(!RenderObject.debugInDebugDoesMeetConstraints);
shouldRelayoutCalled = true;
return shouldRelayoutValue;
}
......
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