#1172 viewer: hero pop/push diversion animation issue workaround

lib/widgets/collection/collection_grid.dart

@@ -249,6 +249,11 @@ class _CollectionGridContentState extends State<_CollectionGridContent> {
   Future<void> _goToViewer(CollectionLens collection, AvesEntry entry) async {
     // track viewer entry for dynamic hero placeholder
     final viewerEntryNotifier = context.read<ViewerEntryNotifier>();
+
+    // prevent navigating again to the same entry until fully back,
+    // as a workaround for the hero pop/push diversion animation issue
+    // (cf `ThumbnailImage` `Hero` usage)
+    if (viewerEntryNotifier.value == entry) return;
     WidgetsBinding.instance.addPostFrameCallback((_) => viewerEntryNotifier.value = entry);
 
     final selection = context.read<Selection<AvesEntry>>();

lib/widgets/common/fx/transition_image.dart

@@ -3,7 +3,6 @@ import 'dart:ui' as ui;
 import 'package:flutter/foundation.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter/scheduler.dart';
-import 'package:flutter/semantics.dart';
 
 // adapted from Flutter `_ImageState` in `/widgets/image.dart`
 // and `DecorationImagePainter` in `/painting/decoration_image.dart`
@@ -15,13 +14,6 @@ class TransitionImage extends StatefulWidget {
   final ImageProvider image;
   final ValueListenable<double> animation;
   final BoxFit thumbnailFit, viewerFit;
-  final ImageFrameBuilder? frameBuilder;
-  final ImageLoadingBuilder? loadingBuilder;
-  final ImageErrorWidgetBuilder? errorBuilder;
-  final String? semanticLabel;
-  final bool excludeFromSemantics;
-  final double? width, height;
-  final bool gaplessPlayback = false;
   final Color? background;
 
   const TransitionImage({
@@ -30,13 +22,6 @@ class TransitionImage extends StatefulWidget {
     required this.animation,
     required this.thumbnailFit,
     required this.viewerFit,
-    this.frameBuilder,
-    this.loadingBuilder,
-    this.errorBuilder,
-    this.semanticLabel,
-    this.excludeFromSemantics = false,
-    this.width,
-    this.height,
     this.background,
   });
 
@@ -47,13 +32,9 @@ class TransitionImage extends StatefulWidget {
 class _TransitionImageState extends State<TransitionImage> with WidgetsBindingObserver {
   ImageStream? _imageStream;
   ImageInfo? _imageInfo;
-  ImageChunkEvent? _loadingProgress;
   bool _isListeningToStream = false;
-  int? _frameNumber;
   bool _wasSynchronouslyLoaded = false;
   late DisposableBuildContext<State<TransitionImage>> _scrollAwareContext;
-  Object? _lastException;
-  StackTrace? _lastStack;
   ImageStreamCompleterHandle? _completerHandle;
 
   @override
@@ -90,11 +71,6 @@ class _TransitionImageState extends State<TransitionImage> with WidgetsBindingOb
   @override
   void didUpdateWidget(TransitionImage oldWidget) {
     super.didUpdateWidget(oldWidget);
-    if (_isListeningToStream && (widget.loadingBuilder == null) != (oldWidget.loadingBuilder == null)) {
-      final ImageStreamListener oldListener = _getListener();
-      _imageStream!.addListener(_getListener(recreateListener: true));
-      _imageStream!.removeListener(oldListener);
-    }
     if (widget.image != oldWidget.image) {
       _resolveImage();
     }
@@ -111,10 +87,7 @@ class _TransitionImageState extends State<TransitionImage> with WidgetsBindingOb
       context: _scrollAwareContext,
       imageProvider: widget.image,
     );
-    final ImageStream newStream = provider.resolve(createLocalImageConfiguration(
-      context,
-      size: widget.width != null && widget.height != null ? Size(widget.width!, widget.height!) : null,
-    ));
+    final ImageStream newStream = provider.resolve(createLocalImageConfiguration(context));
     _updateSourceStream(newStream);
   }
 
@@ -122,27 +95,7 @@ class _TransitionImageState extends State<TransitionImage> with WidgetsBindingOb
 
   ImageStreamListener _getListener({bool recreateListener = false}) {
     if (_imageStreamListener == null || recreateListener) {
-      _lastException = null;
-      _lastStack = null;
-      _imageStreamListener = ImageStreamListener(
-        _handleImageFrame,
-        onChunk: widget.loadingBuilder == null ? null : _handleImageChunk,
-        onError: widget.errorBuilder != null || kDebugMode
-            ? (error, stackTrace) {
-                setState(() {
-                  _lastException = error;
-                  _lastStack = stackTrace;
-                });
-                assert(() {
-                  if (widget.errorBuilder == null) {
-                    // ignore: only_throw_errors, since we're just proxying the error.
-                    throw error; // Ensures the error message is printed to the console.
-                  }
-                  return true;
-                }());
-              }
-            : null,
-      );
+      _imageStreamListener = ImageStreamListener(_handleImageFrame);
     }
     return _imageStreamListener!;
   }
@@ -150,23 +103,10 @@ class _TransitionImageState extends State<TransitionImage> with WidgetsBindingOb
   void _handleImageFrame(ImageInfo imageInfo, bool synchronousCall) {
     setState(() {
       _replaceImage(info: imageInfo);
-      _loadingProgress = null;
-      _lastException = null;
-      _lastStack = null;
-      _frameNumber = _frameNumber == null ? 0 : _frameNumber! + 1;
       _wasSynchronouslyLoaded = _wasSynchronouslyLoaded | synchronousCall;
     });
   }
 
-  void _handleImageChunk(ImageChunkEvent event) {
-    assert(widget.loadingBuilder != null);
-    setState(() {
-      _loadingProgress = event;
-      _lastException = null;
-      _lastStack = null;
-    });
-  }
-
   void _replaceImage({required ImageInfo? info}) {
     final ImageInfo? oldImageInfo = _imageInfo;
     SchedulerBinding.instance.addPostFrameCallback((_) => oldImageInfo?.dispose());
@@ -185,15 +125,8 @@ class _TransitionImageState extends State<TransitionImage> with WidgetsBindingOb
       _imageStream!.removeListener(_getListener());
     }
 
-    if (!widget.gaplessPlayback) {
-      setState(() {
-        _replaceImage(info: null);
-      });
-    }
-
     setState(() {
-      _loadingProgress = null;
-      _frameNumber = null;
+      _replaceImage(info: null);
       _wasSynchronouslyLoaded = false;
     });
 
@@ -235,46 +168,9 @@ class _TransitionImageState extends State<TransitionImage> with WidgetsBindingOb
     _isListeningToStream = false;
   }
 
-  Widget _debugBuildErrorWidget(BuildContext context, Object error) {
-    return Stack(
-      alignment: Alignment.center,
-      children: <Widget>[
-        const Positioned.fill(
-          child: Placeholder(
-            color: Color(0xCF8D021F),
-          ),
-        ),
-        Padding(
-          padding: const EdgeInsets.all(4.0),
-          child: FittedBox(
-            child: Text(
-              '$error',
-              textAlign: TextAlign.center,
-              textDirection: TextDirection.ltr,
-              style: const TextStyle(
-                shadows: <Shadow>[
-                  Shadow(blurRadius: 1.0),
-                ],
-              ),
-            ),
-          ),
-        ),
-      ],
-    );
-  }
-
   @override
   Widget build(BuildContext context) {
-    if (_lastException != null) {
-      if (widget.errorBuilder != null) {
-        return widget.errorBuilder!(context, _lastException!, _lastStack);
-      }
-      if (kDebugMode) {
-        return _debugBuildErrorWidget(context, _lastException!);
-      }
-    }
-
-    Widget result = ValueListenableBuilder<double>(
+    return ValueListenableBuilder<double>(
       valueListenable: widget.animation,
       builder: (context, t, child) => CustomPaint(
         painter: _TransitionImagePainter(
@@ -287,35 +183,6 @@ class _TransitionImageState extends State<TransitionImage> with WidgetsBindingOb
         ),
       ),
     );
-
-    if (!widget.excludeFromSemantics) {
-      result = Semantics(
-        container: widget.semanticLabel != null,
-        image: true,
-        label: widget.semanticLabel ?? '',
-        child: result,
-      );
-    }
-
-    if (widget.frameBuilder != null) {
-      result = widget.frameBuilder!(context, result, _frameNumber, _wasSynchronouslyLoaded);
-    }
-
-    if (widget.loadingBuilder != null) {
-      result = widget.loadingBuilder!(context, result, _loadingProgress);
-    }
-
-    return result;
-  }
-
-  @override
-  void debugFillProperties(DiagnosticPropertiesBuilder description) {
-    super.debugFillProperties(description);
-    description.add(DiagnosticsProperty<ImageStream>('stream', _imageStream));
-    description.add(DiagnosticsProperty<ImageInfo>('pixels', _imageInfo));
-    description.add(DiagnosticsProperty<ImageChunkEvent>('loadingProgress', _loadingProgress));
-    description.add(DiagnosticsProperty<int>('frameNumber', _frameNumber));
-    description.add(DiagnosticsProperty<bool>('wasSynchronouslyLoaded', _wasSynchronouslyLoaded));
   }
 }
 
@@ -325,6 +192,11 @@ class _TransitionImagePainter extends CustomPainter {
   final Color? background;
   final BoxFit thumbnailFit, viewerFit;
 
+  static final _paint = Paint()
+    ..isAntiAlias = false
+    ..filterQuality = FilterQuality.low;
+  static const _alignment = Alignment.center;
+
   const _TransitionImagePainter({
     required this.image,
     required this.scale,
@@ -336,20 +208,15 @@ class _TransitionImagePainter extends CustomPainter {
 
   @override
   void paint(Canvas canvas, Size size) {
-    if (image == null) return;
+    final _image = image;
+    if (_image == null) return;
 
-    final paint = Paint()
-      ..isAntiAlias = false
-      ..filterQuality = FilterQuality.low;
-    const alignment = Alignment.center;
-
-    final rect = Offset.zero & size;
-    if (rect.isEmpty) {
+    if (size.isEmpty) {
       return;
     }
 
-    final outputSize = rect.size;
-    final inputSize = Size(image!.width.toDouble(), image!.height.toDouble());
+    final outputSize = size;
+    final inputSize = Size(_image.width.toDouble(), _image.height.toDouble());
 
     final thumbnailSizes = applyBoxFit(thumbnailFit, inputSize / scale, size);
     final viewerSizes = applyBoxFit(viewerFit, inputSize / scale, size);
@@ -358,11 +225,12 @@ class _TransitionImagePainter extends CustomPainter {
 
     final halfWidthDelta = (outputSize.width - destinationSize.width) / 2.0;
     final halfHeightDelta = (outputSize.height - destinationSize.height) / 2.0;
-    final dx = halfWidthDelta + alignment.x * halfWidthDelta;
-    final dy = halfHeightDelta + alignment.y * halfHeightDelta;
-    final destinationPosition = rect.topLeft.translate(dx, dy);
+    final dx = halfWidthDelta + _alignment.x * halfWidthDelta;
+    final dy = halfHeightDelta + _alignment.y * halfHeightDelta;
+    final destinationPosition = Offset(dx, dy);
+
     final destinationRect = destinationPosition & destinationSize;
-    final sourceRect = alignment.inscribe(
+    final sourceRect = _alignment.inscribe(
       sourceSize,
       Offset.zero & inputSize,
     );
@@ -370,7 +238,7 @@ class _TransitionImagePainter extends CustomPainter {
       // deflate to avoid background artifact around opaque image
       canvas.drawRect(destinationRect.deflate(1), Paint()..color = background!);
     }
-    canvas.drawImageRect(image!, sourceRect, destinationRect, paint);
+    canvas.drawImageRect(_image, sourceRect, destinationRect, _paint);
   }
 
   @override

lib/widgets/common/thumbnail/image.dart

@@ -270,6 +270,9 @@ class _ThumbnailImageState extends State<ThumbnailImage> {
       image = Hero(
         tag: heroTag,
         flightShuttleBuilder: (flightContext, animation, flightDirection, fromHeroContext, toHeroContext) {
+          // as of Flutter v3.27.0-0.1.pre, the flight `animation` is incorrect when diverting a pop:
+          // - diverting a push (t = 0 -> 1) with a pop (t = 1 -> 0) works as expected (t = 0 -> [0,1] -> 0)
+          // - diverting a pop (t = 1 -> 0) with a push (t = 0 -> 1) finishes the pop (t = 1 -> [0,1] -> 0) instead of diverting (t = 1 -> [0,1] -> 1)
           Widget child = TransitionImage(
             image: entry.bestCachedThumbnail,
             animation: animation,