From 15fb5c4ca656e852cd7930469fed151a0be5f4ea Mon Sep 17 00:00:00 2001 From: Ian Hickson Date: Tue, 15 Nov 2016 17:55:51 -0800 Subject: [PATCH] Clean up some RenderObject layer stuff (#6883) More idiomatic use of constraints in performResize. Trivial fixes to comments. Make ProxyBox not use BoxParentData since it ignores the field. Make applyPaintTransform more helpful if you use a different ParentData subclass than RenderBox expects. Make debugAssertIsValid actually fulfill its contract in RenderObject as documented. Add a childBefore for symmetry (we already had childAfter). Fix the way we dump the child list when there's no children in a multichild render object. More asserts in the rendering test library. --- packages/flutter/lib/src/rendering/box.dart | 29 +++++++++-- .../flutter/lib/src/rendering/object.dart | 33 ++++++++++--- .../flutter/lib/src/rendering/proxy_box.dart | 11 +++++ packages/flutter/test/rendering/box_test.dart | 49 ++++++++++--------- .../test/rendering/rendering_tester.dart | 2 + 5 files changed, 89 insertions(+), 35 deletions(-) diff --git a/packages/flutter/lib/src/rendering/box.dart b/packages/flutter/lib/src/rendering/box.dart index b395f6032c..d912d54057 100644 --- a/packages/flutter/lib/src/rendering/box.dart +++ b/packages/flutter/lib/src/rendering/box.dart @@ -1631,7 +1631,7 @@ abstract class RenderBox extends RenderObject { @override void performResize() { // default behavior for subclasses that have sizedByParent = true - size = constraints.constrain(Size.zero); + size = constraints.smallest; assert(!size.isInfinite); } @@ -1657,7 +1657,7 @@ abstract class RenderBox extends RenderObject { /// given hit test result. /// /// The caller is responsible for transforming [position] into the local - /// coordinate space of the callee. The callee is responsible for checking + /// coordinate space of the callee. The callee is responsible for checking /// whether the given position is within its bounds. /// /// Hit testing requires layout to be up-to-date but does not require painting @@ -1712,7 +1712,7 @@ abstract class RenderBox extends RenderObject { /// Override this method to check whether any children are located at the /// given position. /// - /// Typically children should be hit tested in reverse paint order so that + /// Typically children should be hit-tested in reverse paint order so that /// hit tests at locations where children overlap hit the child that is /// visually "on top" (i.e., paints later). /// @@ -1733,9 +1733,28 @@ abstract class RenderBox extends RenderObject { /// child's [parentData] in the [BoxParentData.offset] field. @override void applyPaintTransform(RenderObject child, Matrix4 transform) { + assert(child != null); assert(child.parent == this); - BoxParentData childParentData = child.parentData; - Offset offset = childParentData.offset; + assert(() { + if (child.parentData is! BoxParentData) { + throw new FlutterError( + '$runtimeType does not implement applyPaintTransform.\n' + 'The following $runtimeType object:\n' + ' ${this.toStringShallow()}\n' + '...did not use a BoxParentData class for the parentData field of the following child:\n' + ' ${child.toStringShallow()}\n' + 'The $runtimeType class inherits from RenderBox. ' + 'The default applyPaintTransform implementation provided by RenderBox assumes that the ' + 'children all use BoxParentData objects for their parentData field. ' + 'Since $runtimeType does not in fact use that ParentData class for its children, it must ' + 'provide an implementation of applyPaintTransform that supports the specific ParentData ' + 'subclass used by its children (which apparently is ${child.parentData.runtimeType}).' + ); + } + return true; + }); + final BoxParentData childParentData = child.parentData; + final Offset offset = childParentData.offset; transform.translate(offset.dx, offset.dy); } diff --git a/packages/flutter/lib/src/rendering/object.dart b/packages/flutter/lib/src/rendering/object.dart index 8d0958c63a..32597171d4 100644 --- a/packages/flutter/lib/src/rendering/object.dart +++ b/packages/flutter/lib/src/rendering/object.dart @@ -515,7 +515,10 @@ abstract class Constraints { bool debugAssertIsValid({ bool isAppliedConstraint: false, InformationCollector informationCollector - }); + }) { + assert(isNormalized); + return isNormalized; + } } /// Signature for a function that is called for each [RenderObject]. @@ -1296,7 +1299,7 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { /// Calls visitor for each immediate child of this render object. /// - /// Override in subclasses with children and call the visitor for each child + /// Override in subclasses with children and call the visitor for each child. void visitChildren(RenderObjectVisitor visitor) { } /// The object responsible for creating this render object. @@ -2606,6 +2609,9 @@ abstract class ContainerRenderObjectMixin _lastChild; + /// The previous child before the given child in the child list. + ChildType childBefore(ChildType child) { + assert(child != null); + assert(child.parent == this); + final ParentDataType childParentData = child.parentData; + return childParentData.previousSibling; + } + /// The next child after the given child in the child list. ChildType childAfter(ChildType child) { + assert(child != null); + assert(child.parent == this); final ParentDataType childParentData = child.parentData; return childParentData.nextSibling; } @override String debugDescribeChildren(String prefix) { - String result = '$prefix \u2502\n'; - if (_firstChild != null) { - ChildType child = _firstChild; + if (firstChild != null) { + String result = '$prefix \u2502\n'; + ChildType child = firstChild; int count = 1; - while (child != _lastChild) { + while (child != lastChild) { result += '${child.toStringDeep("$prefix \u251C\u2500child $count: ", "$prefix \u2502")}'; count += 1; final ParentDataType childParentData = child.parentData; child = childParentData.nextSibling; } if (child != null) { - assert(child == _lastChild); + assert(child == lastChild); result += '${child.toStringDeep("$prefix \u2514\u2500child $count: ", "$prefix ")}'; } + return result; } - return result; + return ''; } } diff --git a/packages/flutter/lib/src/rendering/proxy_box.dart b/packages/flutter/lib/src/rendering/proxy_box.dart index dc07adbb37..d7b0889ea5 100644 --- a/packages/flutter/lib/src/rendering/proxy_box.dart +++ b/packages/flutter/lib/src/rendering/proxy_box.dart @@ -51,6 +51,14 @@ class RenderProxyBox extends RenderBox with RenderObjectWithChildMixin { + @override + void setupParentData(RenderObject child) { + // We don't actually use the offset argument in BoxParentData, so let's + // avoid allocating it at all. + if (child.parentData is! ParentData) + child.parentData = new ParentData(); + } + @override double computeMinIntrinsicWidth(double height) { if (child != null) @@ -101,6 +109,9 @@ abstract class RenderProxyBoxMixin implements RenderBox, RenderObjectWithChildMi return child?.hitTest(result, position: position) ?? false; } + @override + void applyPaintTransform(RenderObject child, Matrix4 transform) { } + @override void paint(PaintingContext context, Offset offset) { if (child != null) diff --git a/packages/flutter/test/rendering/box_test.dart b/packages/flutter/test/rendering/box_test.dart index 6152cad9aa..dc03fd4420 100644 --- a/packages/flutter/test/rendering/box_test.dart +++ b/packages/flutter/test/rendering/box_test.dart @@ -15,8 +15,10 @@ void main() { backgroundColor: const Color(0xFF00FF00), gradient: new RadialGradient( center: FractionalOffset.topLeft, radius: 1.8, - colors: [Colors.yellow[500], Colors.blue[500]]), - boxShadow: kElevationToShadow[3]) + colors: [Colors.yellow[500], Colors.blue[500]], + ), + boxShadow: kElevationToShadow[3], + ), ); layout(root); expect(root.size.width, equals(800.0)); @@ -25,28 +27,28 @@ void main() { test('Flex and padding', () { RenderBox size = new RenderConstrainedBox( - additionalConstraints: new BoxConstraints().tighten(height: 100.0) + additionalConstraints: new BoxConstraints().tighten(height: 100.0), ); RenderBox inner = new RenderDecoratedBox( decoration: new BoxDecoration( - backgroundColor: const Color(0xFF00FF00) + backgroundColor: const Color(0xFF00FF00), ), - child: size + child: size, ); RenderBox padding = new RenderPadding( padding: new EdgeInsets.all(50.0), - child: inner + child: inner, ); RenderBox flex = new RenderFlex( children: [padding], direction: Axis.vertical, - crossAxisAlignment: CrossAxisAlignment.stretch + crossAxisAlignment: CrossAxisAlignment.stretch, ); RenderBox outer = new RenderDecoratedBox( decoration: new BoxDecoration( backgroundColor: const Color(0xFF0000FF) ), - child: flex + child: flex, ); layout(outer); @@ -65,13 +67,15 @@ void main() { test("should not have a 0 sized colored Box", () { RenderBox coloredBox = new RenderDecoratedBox( - decoration: new BoxDecoration() + decoration: new BoxDecoration(), + ); + RenderBox paddingBox = new RenderPadding( + padding: const EdgeInsets.all(10.0), + child: coloredBox, ); - RenderBox paddingBox = new RenderPadding(padding: const EdgeInsets.all(10.0), - child: coloredBox); RenderBox root = new RenderDecoratedBox( decoration: new BoxDecoration(), - child: paddingBox + child: paddingBox, ); layout(root); expect(coloredBox.size.width, equals(780.0)); @@ -80,22 +84,23 @@ void main() { test("reparenting should clear position", () { RenderDecoratedBox coloredBox = new RenderDecoratedBox( - decoration: new BoxDecoration()); + decoration: new BoxDecoration(), + ); + RenderPadding paddedBox = new RenderPadding( - child: coloredBox, padding: const EdgeInsets.all(10.0)); - + child: coloredBox, + padding: const EdgeInsets.all(10.0), + ); layout(paddedBox); - BoxParentData parentData = coloredBox.parentData; expect(parentData.offset.dx, isNot(equals(0.0))); - paddedBox.child = null; + RenderConstrainedBox constraintedBox = new RenderConstrainedBox( - child: coloredBox, additionalConstraints: const BoxConstraints()); - + child: coloredBox, + additionalConstraints: const BoxConstraints(), + ); layout(constraintedBox); - - parentData = coloredBox.parentData; - expect(parentData.offset.dx, equals(0.0)); + expect(coloredBox.parentData?.runtimeType, ParentData); }); } diff --git a/packages/flutter/test/rendering/rendering_tester.dart b/packages/flutter/test/rendering/rendering_tester.dart index 32ecaf6b92..34504d6234 100644 --- a/packages/flutter/test/rendering/rendering_tester.dart +++ b/packages/flutter/test/rendering/rendering_tester.dart @@ -70,6 +70,8 @@ void layout(RenderBox box, { void pumpFrame({ EnginePhase phase: EnginePhase.layout }) { assert(renderer != null); + assert(renderer.renderView != null); + assert(renderer.renderView.child != null); // call layout() first! renderer.phase = phase; renderer.beginFrame(); }