From fb97905231470e14a93c85d75615ef81fbe4bcab Mon Sep 17 00:00:00 2001 From: Hixie Date: Mon, 19 Oct 2015 12:46:52 -0700 Subject: [PATCH] Stop AnimatedContainer from animating every build It turns out that an AnimatedContainer with a BoxDecoration would start animating every single time it was built, whether or not the container's decoration had changed. --- .../sky/lib/src/animation/scheduler.dart | 2 + .../sky/lib/src/painting/box_painter.dart | 249 ++++++++++++++++-- .../lib/src/widgets/animated_container.dart | 3 +- .../sky/lib/src/widgets/framework.dart | 6 +- .../test/widget/animated_container_test.dart | 48 ++++ 5 files changed, 279 insertions(+), 29 deletions(-) diff --git a/sky/packages/sky/lib/src/animation/scheduler.dart b/sky/packages/sky/lib/src/animation/scheduler.dart index 699ced48303..88d2b2b3119 100644 --- a/sky/packages/sky/lib/src/animation/scheduler.dart +++ b/sky/packages/sky/lib/src/animation/scheduler.dart @@ -30,6 +30,8 @@ class Scheduler { Map _transientCallbacks = new LinkedHashMap(); final Set _removedIds = new Set(); + int get transientCallbackCount => _transientCallbacks.length; + /// Called by the engine to produce a new frame. /// /// This function first calls all the callbacks registered by diff --git a/sky/packages/sky/lib/src/painting/box_painter.dart b/sky/packages/sky/lib/src/painting/box_painter.dart index 59831383440..5aa148c9622 100644 --- a/sky/packages/sky/lib/src/painting/box_painter.dart +++ b/sky/packages/sky/lib/src/painting/box_painter.dart @@ -51,7 +51,7 @@ class EdgeDims { /// The size that this edge dims would occupy with an empty interior. Size get collapsedSize => new Size(left + right, top + bottom); - EdgeDims operator-(EdgeDims other) { + EdgeDims operator -(EdgeDims other) { return new EdgeDims.TRBL( top - other.top, right - other.right, @@ -60,7 +60,7 @@ class EdgeDims { ); } - EdgeDims operator+(EdgeDims other) { + EdgeDims operator +(EdgeDims other) { return new EdgeDims.TRBL( top + other.top, right + other.right, @@ -69,7 +69,7 @@ class EdgeDims { ); } - EdgeDims operator*(double other) { + EdgeDims operator *(double other) { return new EdgeDims.TRBL( top * other, right * other, @@ -78,7 +78,7 @@ class EdgeDims { ); } - EdgeDims operator/(double other) { + EdgeDims operator /(double other) { return new EdgeDims.TRBL( top / other, right / other, @@ -87,7 +87,7 @@ class EdgeDims { ); } - EdgeDims operator~/(double other) { + EdgeDims operator ~/(double other) { return new EdgeDims.TRBL( (top ~/ other).toDouble(), (right ~/ other).toDouble(), @@ -96,7 +96,7 @@ class EdgeDims { ); } - EdgeDims operator%(double other) { + EdgeDims operator %(double other) { return new EdgeDims.TRBL( top % other, right % other, @@ -166,12 +166,23 @@ class BorderSide { /// A black border side of zero width static const none = const BorderSide(width: 0.0); + bool operator ==(dynamic other) { + if (identical(this, other)) + return true; + if (other is! BorderSide) + return false; + final BorderSide typedOther = other; + return color == typedOther.color && + width == typedOther.width; + } + int get hashCode { int value = 373; - value = 37 * value * color.hashCode; - value = 37 * value * width.hashCode; + value = 37 * value + color.hashCode; + value = 37 * value + width.hashCode; return value; } + String toString() => 'BorderSide($color, $width)'; } @@ -210,14 +221,27 @@ class Border { return new EdgeDims.TRBL(top.width, right.width, bottom.width, left.width); } + bool operator ==(dynamic other) { + if (identical(this, other)) + return true; + if (other is! Border) + return false; + final Border typedOther = other; + return top == typedOther.top && + right == typedOther.right && + bottom == typedOther.bottom && + left == typedOther.left; + } + int get hashCode { int value = 373; - value = 37 * value * top.hashCode; - value = 37 * value * right.hashCode; - value = 37 * value * bottom.hashCode; - value = 37 * value * left.hashCode; + value = 37 * value + top.hashCode; + value = 37 * value + right.hashCode; + value = 37 * value + bottom.hashCode; + value = 37 * value + left.hashCode; return value; } + String toString() => 'Border($top, $right, $bottom, $left)'; } @@ -290,17 +314,37 @@ class BoxShadow { return result; } + bool operator ==(dynamic other) { + if (identical(this, other)) + return true; + if (other is! BoxShadow) + return false; + final BoxShadow typedOther = other; + return color == typedOther.color && + offset == typedOther.offset && + blur == typedOther.blur; + } + + int get hashCode { + int value = 373; + value = 37 * value + color.hashCode; + value = 37 * value + offset.hashCode; + value = 37 * value + blur.hashCode; + return value; + } + String toString() => 'BoxShadow($color, $offset, $blur)'; } /// A 2D gradient abstract class Gradient { + const Gradient(); ui.Shader createShader(); } /// A 2D linear gradient class LinearGradient extends Gradient { - LinearGradient({ + const LinearGradient({ this.begin, this.end, this.colors, @@ -332,6 +376,57 @@ class LinearGradient extends Gradient { return new ui.Gradient.linear([begin, end], this.colors, this.stops, this.tileMode); } + bool operator ==(dynamic other) { + if (identical(this, other)) + return true; + if (other is! LinearGradient) + return false; + final LinearGradient typedOther = other; + if (begin != typedOther.begin || + end != typedOther.end || + tileMode != typedOther.tileMode || + colors?.length != typedOther.colors?.length || + stops?.length != typedOther.stops?.length) + return false; + if (colors != null) { + assert(typedOther.colors != null); + assert(colors.length == typedOther.colors.length); + for (int i = 0; i < colors.length; i += 1) { + if (colors[i] != typedOther.colors[i]) + return false; + } + } + if (stops != null) { + assert(typedOther.stops != null); + assert(stops.length == typedOther.stops.length); + for (int i = 0; i < stops.length; i += 1) { + if (stops[i] != typedOther.stops[i]) + return false; + } + } + return true; + } + + int get hashCode { + int value = 373; + value = 37 * value + begin.hashCode; + value = 37 * value + end.hashCode; + value = 37 * value + tileMode.hashCode; + if (colors != null) { + for (int i = 0; i < colors.length; i += 1) + value = 37 * value + colors[i].hashCode; + } else { + value = 37 * value + null.hashCode; + } + if (stops != null) { + for (int i = 0; i < stops.length; i += 1) + value = 37 * value + stops[i].hashCode; + } else { + value = 37 * value + null.hashCode; + } + return value; + } + String toString() { return 'LinearGradient($begin, $end, $colors, $stops, $tileMode)'; } @@ -339,7 +434,7 @@ class LinearGradient extends Gradient { /// A 2D radial gradient class RadialGradient extends Gradient { - RadialGradient({ + const RadialGradient({ this.center, this.radius, this.colors, @@ -373,6 +468,57 @@ class RadialGradient extends Gradient { return new ui.Gradient.radial(center, radius, colors, stops, tileMode); } + bool operator ==(dynamic other) { + if (identical(this, other)) + return true; + if (other is! RadialGradient) + return false; + final RadialGradient typedOther = other; + if (center != typedOther.center || + radius != typedOther.radius || + tileMode != typedOther.tileMode || + colors?.length != typedOther.colors?.length || + stops?.length != typedOther.stops?.length) + return false; + if (colors != null) { + assert(typedOther.colors != null); + assert(colors.length == typedOther.colors.length); + for (int i = 0; i < colors.length; i += 1) { + if (colors[i] != typedOther.colors[i]) + return false; + } + } + if (stops != null) { + assert(typedOther.stops != null); + assert(stops.length == typedOther.stops.length); + for (int i = 0; i < stops.length; i += 1) { + if (stops[i] != typedOther.stops[i]) + return false; + } + } + return true; + } + + int get hashCode { + int value = 373; + value = 37 * value + center.hashCode; + value = 37 * value + radius.hashCode; + value = 37 * value + tileMode.hashCode; + if (colors != null) { + for (int i = 0; i < colors.length; i += 1) + value = 37 * value + colors[i].hashCode; + } else { + value = 37 * value + null.hashCode; + } + if (stops != null) { + for (int i = 0; i < stops.length; i += 1) + value = 37 * value + stops[i].hashCode; + } else { + value = 37 * value + null.hashCode; + } + return value; + } + String toString() { return 'RadialGradient($center, $radius, $colors, $stops, $tileMode)'; } @@ -498,6 +644,14 @@ typedef void BackgroundImageChangeListener(); /// A background image for a box. class BackgroundImage { + BackgroundImage({ + ImageResource image, + this.fit, + this.repeat: ImageRepeat.noRepeat, + this.centerSlice, + this.colorFilter + }) : _imageResource = image; + /// How the background image should be inscribed into the box. final ImageFit fit; @@ -516,22 +670,14 @@ class BackgroundImage { /// A color filter to apply to the background image before painting it. final ui.ColorFilter colorFilter; - BackgroundImage({ - ImageResource image, - this.fit, - this.repeat: ImageRepeat.noRepeat, - this.centerSlice, - this.colorFilter - }) : _imageResource = image; - /// The image to be painted into the background. ui.Image get image => _image; ui.Image _image; - ImageResource _imageResource; + final ImageResource _imageResource; final List _listeners = - new List(); + new List(); /// Call listener when the background images changes (e.g., arrives from the network). void addChangeListener(BackgroundImageChangeListener listener) { @@ -557,10 +703,32 @@ class BackgroundImage { return; _image = resolvedImage; final List localListeners = - new List.from(_listeners); - for (BackgroundImageChangeListener listener in localListeners) { + new List.from(_listeners); + for (BackgroundImageChangeListener listener in localListeners) listener(); - } + } + + bool operator ==(dynamic other) { + if (identical(this, other)) + return true; + if (other is! BackgroundImage) + return false; + final BackgroundImage typedOther = other; + return fit == typedOther.fit && + repeat == typedOther.repeat && + centerSlice == typedOther.centerSlice && + colorFilter == typedOther.colorFilter && + _imageResource == typedOther._imageResource; + } + + int get hashCode { + int value = 373; + value = 37 * value + fit.hashCode; + value = 37 * value + repeat.hashCode; + value = 37 * value + centerSlice.hashCode; + value = 37 * value + colorFilter.hashCode; + value = 37 * value + _imageResource.hashCode; + return value; } String toString() => 'BackgroundImage($fit, $repeat)'; @@ -650,6 +818,33 @@ class BoxDecoration { ); } + bool operator ==(dynamic other) { + if (identical(this, other)) + return true; + if (other is! BoxDecoration) + return false; + final BoxDecoration typedOther = other; + return backgroundColor == typedOther.backgroundColor && + backgroundImage == typedOther.backgroundImage && + border == typedOther.border && + borderRadius == typedOther.borderRadius && + boxShadow == typedOther.boxShadow && + gradient == typedOther.gradient && + shape == typedOther.shape; + } + + int get hashCode { + int value = 373; + value = 37 * value + backgroundColor.hashCode; + value = 37 * value + backgroundImage.hashCode; + value = 37 * value + border.hashCode; + value = 37 * value + borderRadius.hashCode; + value = 37 * value + boxShadow.hashCode; + value = 37 * value + gradient.hashCode; + value = 37 * value + shape.hashCode; + return value; + } + String toString([String prefix = '']) { List result = []; if (backgroundColor != null) diff --git a/sky/packages/sky/lib/src/widgets/animated_container.dart b/sky/packages/sky/lib/src/widgets/animated_container.dart index 8017dbaa16b..f205fdd2f9b 100644 --- a/sky/packages/sky/lib/src/widgets/animated_container.dart +++ b/sky/packages/sky/lib/src/widgets/animated_container.dart @@ -66,6 +66,7 @@ class AnimatedContainer extends StatefulComponent { } final Widget child; + final BoxConstraints constraints; final BoxDecoration decoration; final BoxDecoration foregroundDecoration; @@ -95,7 +96,7 @@ class _AnimatedContainerState extends State { void initState() { super.initState(); - _performance = new Performance(duration: config.duration) + _performance = new Performance(duration: config.duration, debugLabel: '${config.toStringShort()}') ..timing = new AnimationTiming(curve: config.curve) ..addListener(_updateAllVariables); _configAllVariables(); diff --git a/sky/packages/sky/lib/src/widgets/framework.dart b/sky/packages/sky/lib/src/widgets/framework.dart index 891f7ebd713..2a5aedc7b85 100644 --- a/sky/packages/sky/lib/src/widgets/framework.dart +++ b/sky/packages/sky/lib/src/widgets/framework.dart @@ -202,8 +202,12 @@ abstract class Widget { /// Inflates this configuration to a concrete instance. Element createElement(); + String toStringShort() { + return key == null ? '$runtimeType' : '$runtimeType-$key'; + } + String toString() { - final String name = key == null ? '$runtimeType' : '$runtimeType-$key'; + final String name = toStringShort(); final List data = []; debugFillDescription(data); if (data.isEmpty) diff --git a/sky/unit/test/widget/animated_container_test.dart b/sky/unit/test/widget/animated_container_test.dart index c54beb01ea0..653f20f6029 100644 --- a/sky/unit/test/widget/animated_container_test.dart +++ b/sky/unit/test/widget/animated_container_test.dart @@ -1,3 +1,4 @@ +import 'package:flutter/animation.dart'; import 'package:flutter/rendering.dart'; import 'package:flutter/widgets.dart'; import 'package:test/test.dart'; @@ -45,4 +46,51 @@ void main() { }); }); + + test('AnimatedContainer overanimate test', () { + testWidgets((WidgetTester tester) { + tester.pumpWidget( + new AnimatedContainer( + duration: const Duration(milliseconds: 200), + decoration: new BoxDecoration( + backgroundColor: new Color(0xFF00FF00) + ) + ) + ); + expect(scheduler.transientCallbackCount, 0); + tester.pump(new Duration(seconds: 1)); + expect(scheduler.transientCallbackCount, 0); + tester.pumpWidget( + new AnimatedContainer( + duration: const Duration(milliseconds: 200), + decoration: new BoxDecoration( + backgroundColor: new Color(0xFF00FF00) + ) + ) + ); + expect(scheduler.transientCallbackCount, 0); + tester.pump(new Duration(seconds: 1)); + expect(scheduler.transientCallbackCount, 0); + tester.pumpWidget( + new AnimatedContainer( + duration: const Duration(milliseconds: 200), + decoration: new BoxDecoration( + backgroundColor: new Color(0xFF0000FF) + ) + ) + ); + expect(scheduler.transientCallbackCount, 1); // this is the only time an animation should have started! + tester.pump(new Duration(seconds: 1)); + expect(scheduler.transientCallbackCount, 0); + tester.pumpWidget( + new AnimatedContainer( + duration: const Duration(milliseconds: 200), + decoration: new BoxDecoration( + backgroundColor: new Color(0xFF0000FF) + ) + ) + ); + expect(scheduler.transientCallbackCount, 0); + }); + }); }