From 94fefaa49d86fde856146c05d6817382a2fe7b3c Mon Sep 17 00:00:00 2001 From: Viren Khatri <44755140+werainkhatri@users.noreply.github.com> Date: Wed, 6 Apr 2022 17:17:34 +0530 Subject: [PATCH] Fixes `FadeInImage` to follow gapless playback (#94601) * renovated and added a test * fixes nits and tests. * revert commits * make FadeInImage follow gapless image playback * refactor: never dispose _AnimatedFadeOutFadeIn * add assert --- .../lib/src/widgets/fade_in_image.dart | 33 +++-- .../test/widgets/fade_in_image_test.dart | 118 +++++++++++------- 2 files changed, 90 insertions(+), 61 deletions(-) diff --git a/packages/flutter/lib/src/widgets/fade_in_image.dart b/packages/flutter/lib/src/widgets/fade_in_image.dart index ce60c7ca8a..76f977d4d2 100644 --- a/packages/flutter/lib/src/widgets/fade_in_image.dart +++ b/packages/flutter/lib/src/widgets/fade_in_image.dart @@ -371,6 +371,7 @@ class FadeInImage extends StatefulWidget { class _FadeInImageState extends State { static const Animation _kOpaqueAnimation = AlwaysStoppedAnimation(1.0); + bool targetLoaded = false; // These ProxyAnimations are changed to the fade in animation by // [_AnimatedFadeOutFadeInState]. Otherwise these animations are reset to @@ -378,11 +379,6 @@ class _FadeInImageState extends State { final ProxyAnimation _imageAnimation = ProxyAnimation(_kOpaqueAnimation); final ProxyAnimation _placeholderAnimation = ProxyAnimation(_kOpaqueAnimation); - void _resetAnimations() { - _imageAnimation.parent = _kOpaqueAnimation; - _placeholderAnimation.parent = _kOpaqueAnimation; - } - Image _image({ required ImageProvider image, ImageErrorWidgetBuilder? errorBuilder, @@ -415,9 +411,8 @@ class _FadeInImageState extends State { opacity: _imageAnimation, fit: widget.fit, frameBuilder: (BuildContext context, Widget child, int? frame, bool wasSynchronouslyLoaded) { - if (wasSynchronouslyLoaded) { - _resetAnimations(); - return child; + if (wasSynchronouslyLoaded || frame != null) { + targetLoaded = true; } return _AnimatedFadeOutFadeIn( target: child, @@ -429,7 +424,8 @@ class _FadeInImageState extends State { fit: widget.placeholderFit ?? widget.fit, ), placeholderProxyAnimation: _placeholderAnimation, - isTargetLoaded: frame != null, + isTargetLoaded: targetLoaded, + wasSynchronouslyLoaded: wasSynchronouslyLoaded, fadeInDuration: widget.fadeInDuration, fadeOutDuration: widget.fadeOutDuration, fadeInCurve: widget.fadeInCurve, @@ -463,6 +459,7 @@ class _AnimatedFadeOutFadeIn extends ImplicitlyAnimatedWidget { required this.fadeOutCurve, required this.fadeInDuration, required this.fadeInCurve, + required this.wasSynchronouslyLoaded, }) : assert(target != null), assert(placeholder != null), assert(isTargetLoaded != null), @@ -470,6 +467,7 @@ class _AnimatedFadeOutFadeIn extends ImplicitlyAnimatedWidget { assert(fadeOutCurve != null), assert(fadeInDuration != null), assert(fadeInCurve != null), + assert(!wasSynchronouslyLoaded || isTargetLoaded), super(key: key, duration: fadeInDuration + fadeOutDuration); final Widget target; @@ -481,6 +479,7 @@ class _AnimatedFadeOutFadeIn extends ImplicitlyAnimatedWidget { final Duration fadeOutDuration; final Curve fadeInCurve; final Curve fadeOutCurve; + final bool wasSynchronouslyLoaded; @override _AnimatedFadeOutFadeInState createState() => _AnimatedFadeOutFadeInState(); @@ -508,6 +507,11 @@ class _AnimatedFadeOutFadeInState extends ImplicitlyAnimatedWidgetState<_Animate @override void didUpdateTweens() { + if (widget.wasSynchronouslyLoaded) { + // Opacity animations should not be reset if image was synchronously loaded. + return; + } + _placeholderOpacityAnimation = animation.drive(TweenSequence(>[ TweenSequenceItem( tween: _placeholderOpacity!.chain(CurveTween(curve: widget.fadeOutCurve)), @@ -534,23 +538,14 @@ class _AnimatedFadeOutFadeInState extends ImplicitlyAnimatedWidgetState<_Animate weight: widget.fadeInDuration.inMilliseconds.toDouble(), ), ])); - if (!widget.isTargetLoaded && _isValid(_placeholderOpacity!) && _isValid(_targetOpacity!)) { - // Jump (don't fade) back to the placeholder image, so as to be ready - // for the full animation when the new target image becomes ready. - controller.value = controller.upperBound; - } widget.targetProxyAnimation.parent = _targetOpacityAnimation; widget.placeholderProxyAnimation.parent = _placeholderOpacityAnimation; } - bool _isValid(Tween tween) { - return tween.begin != null && tween.end != null; - } - @override Widget build(BuildContext context) { - if (_placeholderOpacityAnimation!.isCompleted) { + if (widget.wasSynchronouslyLoaded || _placeholderOpacityAnimation!.isCompleted) { return widget.target; } diff --git a/packages/flutter/test/widgets/fade_in_image_test.dart b/packages/flutter/test/widgets/fade_in_image_test.dart index d7cb081fb6..53c98f498a 100644 --- a/packages/flutter/test/widgets/fade_in_image_test.dart +++ b/packages/flutter/test/widgets/fade_in_image_test.dart @@ -156,6 +156,82 @@ Future main() async { expect(findFadeInImage(tester).target.opacity, 1); }); + testWidgets("FadeInImage's image obeys gapless playback", (WidgetTester tester) async { + final TestImageProvider placeholderProvider = TestImageProvider(placeholderImage); + final TestImageProvider imageProvider = TestImageProvider(targetImage); + final TestImageProvider secondImageProvider = TestImageProvider(replacementImage); + + await tester.pumpWidget(FadeInImage( + placeholder: placeholderProvider, + image: imageProvider, + fadeOutDuration: animationDuration, + fadeInDuration: animationDuration, + )); + + imageProvider.complete(); + placeholderProvider.complete(); + await tester.pump(); + await tester.pump(animationDuration * 2); + // Calls setState after the animation, which removes the placeholder image. + await tester.pump(const Duration(milliseconds: 100)); + + await tester.pumpWidget(FadeInImage( + placeholder: placeholderProvider, + image: secondImageProvider, + )); + await tester.pump(); + + FadeInImageParts parts = findFadeInImage(tester); + // Continually shows previously loaded image, + expect(parts.placeholder, isNull); + expect(parts.target.rawImage.image!.isCloneOf(targetImage), isTrue); + expect(parts.target.opacity, 1); + + // Until the new image provider provides the image. + secondImageProvider.complete(); + await tester.pump(); + + parts = findFadeInImage(tester); + expect(parts.target.rawImage.image!.isCloneOf(replacementImage), isTrue); + expect(parts.target.opacity, 1); + }); + + testWidgets("FadeInImage's placeholder obeys gapless playback", (WidgetTester tester) async { + final TestImageProvider placeholderProvider = TestImageProvider(placeholderImage); + final TestImageProvider secondPlaceholderProvider = TestImageProvider(replacementImage); + final TestImageProvider imageProvider = TestImageProvider(targetImage); + + await tester.pumpWidget(FadeInImage( + placeholder: placeholderProvider, + image: imageProvider, + )); + + placeholderProvider.complete(); + await tester.pump(); + + FadeInImageParts parts = findFadeInImage(tester); + expect(parts.placeholder!.rawImage.image!.isCloneOf(placeholderImage), true); + expect(parts.placeholder!.opacity, 1); + + await tester.pumpWidget(FadeInImage( + placeholder: secondPlaceholderProvider, + image: imageProvider, + )); + + parts = findFadeInImage(tester); + // continually shows previously loaded image. + expect(parts.placeholder!.rawImage.image!.isCloneOf(placeholderImage), true); + expect(parts.placeholder!.opacity, 1); + + // Until the new image provider provides the image. + secondPlaceholderProvider.complete(); + await tester.pump(); + + parts = findFadeInImage(tester); + expect(parts.placeholder!.rawImage.image!.isCloneOf(replacementImage), true); + expect(parts.placeholder!.opacity, 1); + }); + testWidgets('shows a cached image immediately when skipFadeOnSynchronousLoad=true', (WidgetTester tester) async { final TestImageProvider placeholderProvider = TestImageProvider(placeholderImage); final TestImageProvider imageProvider = TestImageProvider(targetImage); @@ -226,48 +302,6 @@ Future main() async { expect(find.byType(Image), findsOneWidget); }); - testWidgets('re-fades in the image when the target image is updated', (WidgetTester tester) async { - final TestImageProvider placeholderProvider = TestImageProvider(placeholderImage); - final TestImageProvider imageProvider = TestImageProvider(targetImage); - final TestImageProvider secondImageProvider = TestImageProvider(replacementImage); - - await tester.pumpWidget(FadeInImage( - placeholder: placeholderProvider, - image: imageProvider, - fadeOutDuration: animationDuration, - fadeInDuration: animationDuration, - excludeFromSemantics: true, - )); - - final State? state = findFadeInImage(tester).state; - placeholderProvider.complete(); - imageProvider.complete(); - await tester.pump(); - await tester.pump(animationDuration * 2); - - await tester.pumpWidget(FadeInImage( - placeholder: placeholderProvider, - image: secondImageProvider, - fadeOutDuration: animationDuration, - fadeInDuration: animationDuration, - excludeFromSemantics: true, - )); - - secondImageProvider.complete(); - await tester.pump(); - - expect(findFadeInImage(tester).target.rawImage.image!.isCloneOf(replacementImage), true); - expect(findFadeInImage(tester).state, same(state)); - expect(findFadeInImage(tester).placeholder!.opacity, moreOrLessEquals(1)); - expect(findFadeInImage(tester).target.opacity, moreOrLessEquals(0)); - await tester.pump(animationDuration); - expect(findFadeInImage(tester).placeholder!.opacity, moreOrLessEquals(0)); - expect(findFadeInImage(tester).target.opacity, moreOrLessEquals(0)); - await tester.pump(animationDuration); - expect(findFadeInImage(tester).placeholder!.opacity, moreOrLessEquals(0)); - expect(findFadeInImage(tester).target.opacity, moreOrLessEquals(1)); - }); - testWidgets("doesn't interrupt in-progress animation when animation values are updated", (WidgetTester tester) async { final TestImageProvider placeholderProvider = TestImageProvider(placeholderImage); final TestImageProvider imageProvider = TestImageProvider(targetImage);