diff --git a/packages/flutter/lib/src/rendering/animated_size.dart b/packages/flutter/lib/src/rendering/animated_size.dart index 28f34a2231..61fe33b68f 100644 --- a/packages/flutter/lib/src/rendering/animated_size.dart +++ b/packages/flutter/lib/src/rendering/animated_size.dart @@ -156,14 +156,18 @@ class RenderAnimatedSize extends RenderAligningShiftedBox { _lastValue = _controller.value; _hasVisualOverflow = false; - if (child == null) { + if (child == null || constraints.isTight) { + _controller.stop(); size = _sizeTween.begin = _sizeTween.end = constraints.smallest; + _state = RenderAnimatedSizeState.start; + child?.layout(constraints); return; } child.layout(constraints, parentUsesSize: true); - switch(_state) { + assert(_state != null); + switch (_state) { case RenderAnimatedSizeState.start: _layoutStart(); break; @@ -176,8 +180,6 @@ class RenderAnimatedSize extends RenderAligningShiftedBox { case RenderAnimatedSizeState.unstable: _layoutUnstable(); break; - default: - throw new StateError('$runtimeType is in an invalid state $_state'); } size = constraints.constrain(_animatedSize); @@ -198,7 +200,7 @@ class RenderAnimatedSize extends RenderAligningShiftedBox { /// We have the initial size to animate from, but we do not have the target /// size to animate to, so we set both ends to child's size. void _layoutStart() { - _sizeTween.begin = _sizeTween.end = child.size; + _sizeTween.begin = _sizeTween.end = debugAdoptSize(child.size); _state = RenderAnimatedSizeState.stable; } @@ -209,12 +211,12 @@ class RenderAnimatedSize extends RenderAligningShiftedBox { /// animation. void _layoutStable() { if (_sizeTween.end != child.size) { - _sizeTween.end = child.size; + _sizeTween.end = debugAdoptSize(child.size); _restartAnimation(); _state = RenderAnimatedSizeState.changed; } else if (_controller.value == _controller.upperBound) { // Animation finished. Reset target sizes. - _sizeTween.begin = _sizeTween.end = child.size; + _sizeTween.begin = _sizeTween.end = debugAdoptSize(child.size); } } @@ -227,7 +229,7 @@ class RenderAnimatedSize extends RenderAligningShiftedBox { void _layoutChanged() { if (_sizeTween.end != child.size) { // Child size changed again. Match the child's size and restart animation. - _sizeTween.begin = _sizeTween.end = child.size; + _sizeTween.begin = _sizeTween.end = debugAdoptSize(child.size); _restartAnimation(); _state = RenderAnimatedSizeState.unstable; } else { @@ -242,7 +244,7 @@ class RenderAnimatedSize extends RenderAligningShiftedBox { void _layoutUnstable() { if (_sizeTween.end != child.size) { // Still unstable. Continue tracking the child. - _sizeTween.begin = _sizeTween.end = child.size; + _sizeTween.begin = _sizeTween.end = debugAdoptSize(child.size); _restartAnimation(); } else { // Child size stabilized. diff --git a/packages/flutter/lib/src/rendering/box.dart b/packages/flutter/lib/src/rendering/box.dart index fcdb7acf24..5f80fe9400 100644 --- a/packages/flutter/lib/src/rendering/box.dart +++ b/packages/flutter/lib/src/rendering/box.dart @@ -15,7 +15,7 @@ import 'object.dart'; // This class should only be used in debug builds. class _DebugSize extends Size { - _DebugSize(Size source, this._owner, this._canBeUsedByParent): super.copy(source); + _DebugSize(Size source, this._owner, this._canBeUsedByParent) : super.copy(source); final RenderBox _owner; final bool _canBeUsedByParent; } @@ -856,7 +856,7 @@ class _IntrinsicDimensionsCacheEntry { /// constraints would be growing to fit the parent. /// /// Sizing purely based on the constraints allows the system to make some -/// significant optimisations. Classes that use this approach should override +/// significant optimizations. Classes that use this approach should override /// [sizedByParent] to return true, and then override [performResize] to set the /// [size] using nothing but the constraints, e.g.: /// @@ -882,7 +882,7 @@ class _IntrinsicDimensionsCacheEntry { /// child, passing it a [BoxConstraints] object describing the constraints /// within which the child can render. Passing tight constraints (see /// [BoxConstraints.isTight]) to the child will allow the rendering library to -/// apply some optimisations, as it knows that if the constraints are tight, the +/// apply some optimizations, as it knows that if the constraints are tight, the /// child's dimensions cannot change even if the layout of the child itself /// changes. /// @@ -892,7 +892,7 @@ class _IntrinsicDimensionsCacheEntry { /// then it must specify the `parentUsesSize` argument to the child's [layout] /// function, setting it to true. /// -/// This flag turns off some optimisations; algorithms that do not rely on the +/// This flag turns off some optimizations; algorithms that do not rely on the /// children's sizes will be more efficient. (In particular, relying on the /// child's [size] means that if the child is marked dirty for layout, the /// parent will probably also be marked dirty for layout, unless the @@ -910,7 +910,7 @@ class _IntrinsicDimensionsCacheEntry { /// subclass, and instead of reading the child's size, the parent would read /// whatever the output of [layout] is for that layout protocol. The /// `parentUsesSize` flag is still used to indicate whether the parent is going -/// to read that output, and optimisations still kick in if the child has tight +/// to read that output, and optimizations still kick in if the child has tight /// constraints (as defined by [Constraints.isTight]). /// /// ### Painting @@ -1484,20 +1484,74 @@ abstract class RenderBox extends RenderObject { ); }); assert(() { - if (value is _DebugSize) { - if (value._owner != this) { - assert(value._owner.parent == this); - assert(value._canBeUsedByParent); - } - } + value = debugAdoptSize(value); return true; }); _size = value; + assert(() { debugAssertDoesMeetConstraints(); return true; }); + } + + /// Claims ownership of the given [Size]. + /// + /// In debug mode, the [RenderBox] class verifies that [Size] objects obtained + /// from other [RenderBox] objects are only used according to the semantics of + /// the [RenderBox] protocol, namely that a [Size] from a [RenderBox] can only + /// be used by its parent, and then only if `parentUsesSize` was set. + /// + /// Sometimes, a [Size] that can validly be used ends up no longer being valid + /// over time. The common example is a [Size] taken from a child that is later + /// removed from the parent. In such cases, this method can be called to first + /// check whether the size can legitimately be used, and if so, to then create + /// a new [Size] that can be used going forward, regardless of what happens to + /// the original owner. + Size debugAdoptSize(Size value) { + Size result = value; assert(() { - _size = new _DebugSize(_size, this, debugCanParentUseSize); + if (value is _DebugSize) { + if (value._owner != this) { + if (value._owner.parent != this) { + throw new FlutterError( + 'The size property was assigned a size inappropriately.\n' + 'The following render object:\n' + ' $this\n' + '...was assigned a size obtained from:\n' + ' ${value._owner}\n' + 'However, this second render object is not, or is no longer, a ' + 'child of the first, and it is therefore a violation of the ' + 'RenderBox layout protocol to use that size in the layout of the ' + 'first render object.\n' + 'If the size was obtained at a time where it was valid to read ' + 'the size (because the second render object above was a child ' + 'of the first at the time), then it should be adopted using ' + 'debugAdoptSize at that time.\n' + 'If the size comes from a grandchild or a render object from an ' + 'entirely different part of the render tree, then there is no ' + 'way to be notified when the size changes and therefore attempts ' + 'to read that size are almost certainly a source of bugs. A different ' + 'approach should be used.' + ); + } + if (!value._canBeUsedByParent) { + throw new FlutterError( + 'A child\'s size was used without setting parentUsesSize.\n' + 'The following render object:\n' + ' $this\n' + '...was assigned a size obtained from its child:\n' + ' ${value._owner}\n' + 'However, when the child was laid out, the parentUsesSize argument ' + 'was not set or set to false. Subsequently this transpired to be ' + 'inaccurate: the size was nonetheless used by the parent.\n' + 'It is important to tell the framework if the size will be used or not ' + 'as several important performance optimizations can be made if the ' + 'size will not be used by the parent.' + ); + } + } + } + result = new _DebugSize(value, this, debugCanParentUseSize); return true; }); - assert(() { debugAssertDoesMeetConstraints(); return true; }); + return result; } @override diff --git a/packages/flutter/lib/src/widgets/animated_cross_fade.dart b/packages/flutter/lib/src/widgets/animated_cross_fade.dart index 307ea068de..7311a1df47 100644 --- a/packages/flutter/lib/src/widgets/animated_cross_fade.dart +++ b/packages/flutter/lib/src/widgets/animated_cross_fade.dart @@ -24,6 +24,42 @@ enum CrossFadeState { showSecond, } +/// Signature for the [AnimatedCrossFade.layoutBuilder] callback. +/// +/// The `topChild` is the child fading in, which is normally drawn on top. The +/// `bottomChild` is the child fading out, normally drawn on the bottom. +/// +/// For good performance, the returned widget tree should contain both the +/// `topChild` and the `bottomChild`; the depth of the tree, and the types of +/// the widgets in the tree, from the returned widget to each of the children +/// should be the same; and where there is a widget with multiple children, the +/// top child and the bottom child should be keyed using the provided +/// `topChildKey` and `bottomChildKey` keys respectively. +/// +/// ## Sample code +/// +/// ```dart +/// Widget defaultLayoutBuilder(Widget topChild, Key topChildKey, Widget bottomChild, Key bottomChildKey) { +/// return new Stack( +/// fit: StackFit.loose, +/// children: [ +/// new Positioned( +/// key: bottomChildKey, +/// left: 0.0, +/// top: 0.0, +/// right: 0.0, +/// child: bottomChild, +/// ), +/// new Positioned( +/// key: topChildKey, +/// child: topChild, +/// ) +/// ], +/// ); +/// } +/// ``` +typedef Widget AnimatedCrossFadeBuilder(Widget topChild, Key topChildKey, Widget bottomChild, Key bottomChildKey); + /// A widget that cross-fades between two given children and animates itself /// between their sizes. /// @@ -70,6 +106,8 @@ class AnimatedCrossFade extends StatefulWidget { /// The [duration] of the animation is the same for all components (fade in, /// fade out, and size), and you can pass [Interval]s instead of [Curve]s in /// order to have finer control, e.g., creating an overlap between the fades. + /// + /// All the arguments other than [key] must be non-null. const AnimatedCrossFade({ Key key, @required this.firstChild, @@ -79,10 +117,17 @@ class AnimatedCrossFade extends StatefulWidget { this.sizeCurve: Curves.linear, this.alignment: FractionalOffset.topCenter, @required this.crossFadeState, - @required this.duration - }) : assert(firstCurve != null), + @required this.duration, + this.layoutBuilder: defaultLayoutBuilder, + }) : assert(firstChild != null), + assert(secondChild != null), + assert(firstCurve != null), assert(secondCurve != null), assert(sizeCurve != null), + assert(alignment != null), + assert(crossFadeState != null), + assert(duration != null), + assert(layoutBuilder != null), super(key: key); /// The child that is visible when [crossFadeState] is @@ -123,6 +168,49 @@ class AnimatedCrossFade extends StatefulWidget { /// Defaults to [FractionalOffset.topCenter]. final FractionalOffset alignment; + /// A builder that positions the [firstChild] and [secondChild] widgets. + /// + /// The widget returned by this method is wrapped in an [AnimatedSize]. + /// + /// By default, this uses [AnimatedCrossFade.defaultLayoutBuilder], which uses + /// a [Stack] and aligns the `bottomChild` to the top of the stack while + /// providing the `topChild` as the non-positioned child to fill the provided + /// constraints. This works well when the [AnimatedCrossFade] is in a position + /// to change size and when the children are not flexible. However, if the + /// children are less fussy about their sizes (for example a + /// [CircularProgressIndicator] inside a [Center]), or if the + /// [AnimatedCrossFade] is being forced to a particular size, then it can + /// result in the widgets jumping about when the cross-fade state is changed. + final AnimatedCrossFadeBuilder layoutBuilder; + + /// The default layout algorithm used by [AnimatedCrossFade]. + /// + /// The top child is placed in a stack that sizes itself to match the top + /// child. The bottom child is positioned at the top of the same stack, sized + /// to fit its width but without forcing the height. The stack is then + /// clipped. + /// + /// This is the default value for [layoutBuilder]. It implements + /// [AnimatedCrossFadeBuilder]. + static Widget defaultLayoutBuilder(Widget topChild, Key topChildKey, Widget bottomChild, Key bottomChildKey) { + return new Stack( + overflow: Overflow.visible, + children: [ + new Positioned( + key: bottomChildKey, + left: 0.0, + top: 0.0, + right: 0.0, + child: bottomChild, + ), + new Positioned( + key: topChildKey, + child: topChild, + ) + ], + ); + } + @override _AnimatedCrossFadeState createState() => new _AnimatedCrossFadeState(); @@ -203,7 +291,8 @@ class _AnimatedCrossFadeState extends State with TickerProvid /// Whether we're in the middle of cross-fading this frame. bool get _isTransitioning => _controller.status == AnimationStatus.forward || _controller.status == AnimationStatus.reverse; - List _buildCrossFadedChildren() { + @override + Widget build(BuildContext context) { const Key kFirstChildKey = const ValueKey(CrossFadeState.showFirst); const Key kSecondChildKey = const ValueKey(CrossFadeState.showSecond); final bool transitioningForwards = _controller.status == AnimationStatus.completed || _controller.status == AnimationStatus.forward; @@ -230,54 +319,35 @@ class _AnimatedCrossFadeState extends State with TickerProvid bottomAnimation = _secondAnimation; } - return [ - new TickerMode( - key: bottomKey, - enabled: _isTransitioning, - child: new Positioned( - // TODO(dragostis): Add a way to crop from top right for - // right-to-left languages. - left: 0.0, - top: 0.0, - right: 0.0, - child: new ExcludeSemantics( - excluding: true, // always exclude the semantics of the widget that's fading out - child: new FadeTransition( - opacity: bottomAnimation, - child: bottomChild, - ), - ), + bottomChild = new TickerMode( + key: bottomKey, + enabled: _isTransitioning, + child: new ExcludeSemantics( + excluding: true, // Always exclude the semantics of the widget that's fading out. + child: new FadeTransition( + opacity: bottomAnimation, + child: bottomChild, ), ), - new TickerMode( - key: topKey, - enabled: true, // top widget always has its animations enabled - child: new Positioned( - child: new ExcludeSemantics( - excluding: false, // always publish semantics for the widget that's fading in - child: new FadeTransition( - opacity: topAnimation, - child: topChild, - ), - ), + ); + topChild = new TickerMode( + key: topKey, + enabled: true, // Top widget always has its animations enabled. + child: new ExcludeSemantics( + excluding: false, // Always publish semantics for the widget that's fading in. + child: new FadeTransition( + opacity: topAnimation, + child: topChild, ), ), - ]; - } - - @override - Widget build(BuildContext context) { + ); return new ClipRect( child: new AnimatedSize( - key: new ValueKey(widget.key), alignment: widget.alignment, duration: widget.duration, curve: widget.sizeCurve, vsync: this, - child: new Stack( - overflow: Overflow.visible, - children: _buildCrossFadedChildren(), - ), + child: widget.layoutBuilder(topChild, topKey, bottomChild, bottomKey), ), ); } diff --git a/packages/flutter/test/widgets/animated_cross_fade_test.dart b/packages/flutter/test/widgets/animated_cross_fade_test.dart index 18c2a2afa8..14b14f6b96 100644 --- a/packages/flutter/test/widgets/animated_cross_fade_test.dart +++ b/packages/flutter/test/widgets/animated_cross_fade_test.dart @@ -21,7 +21,7 @@ void main() { height: 200.0 ), duration: const Duration(milliseconds: 200), - crossFadeState: CrossFadeState.showFirst + crossFadeState: CrossFadeState.showFirst, ) ) ); @@ -43,7 +43,7 @@ void main() { height: 200.0 ), duration: const Duration(milliseconds: 200), - crossFadeState: CrossFadeState.showSecond + crossFadeState: CrossFadeState.showSecond, ) ) ); @@ -69,7 +69,7 @@ void main() { height: 200.0 ), duration: const Duration(milliseconds: 200), - crossFadeState: CrossFadeState.showSecond + crossFadeState: CrossFadeState.showSecond, ) ) ); @@ -183,6 +183,35 @@ void main() { expect(state.ticker.muted, true); expect(findSemantics().excluding, true); }); + + testWidgets('AnimatedCrossFade.layoutBuilder', (WidgetTester tester) async { + await tester.pumpWidget(const AnimatedCrossFade( + firstChild: const Text('AAA'), + secondChild: const Text('BBB'), + crossFadeState: CrossFadeState.showFirst, + duration: const Duration(milliseconds: 50), + )); + expect(find.text('AAA'), findsOneWidget); + expect(find.text('BBB'), findsOneWidget); + await tester.pumpWidget(new AnimatedCrossFade( + firstChild: const Text('AAA'), + secondChild: const Text('BBB'), + crossFadeState: CrossFadeState.showFirst, + duration: const Duration(milliseconds: 50), + layoutBuilder: (Widget a, Key aKey, Widget b, Key bKey) => a, + )); + expect(find.text('AAA'), findsOneWidget); + expect(find.text('BBB'), findsNothing); + await tester.pumpWidget(new AnimatedCrossFade( + firstChild: const Text('AAA'), + secondChild: const Text('BBB'), + crossFadeState: CrossFadeState.showSecond, + duration: const Duration(milliseconds: 50), + layoutBuilder: (Widget a, Key aKey, Widget b, Key bKey) => a, + )); + expect(find.text('BBB'), findsOneWidget); + expect(find.text('AAA'), findsNothing); + }); } class _TickerWatchingWidget extends StatefulWidget {