From c2d840089963233e646f383f32a2bde3f4717431 Mon Sep 17 00:00:00 2001 From: Thomas <9749173+uhthomas@users.noreply.github.com> Date: Fri, 27 Feb 2026 12:09:38 +0000 Subject: [PATCH] fix(mobile): prevent video player from being recreated unnecessarily (#26553) The changes in #25952 inadvertently removed an optimisation which prevents the video player from being recreated when the tree changed. This happens surprisingly often, namely when the hero animation finishes. The widget is particularly expensive, so recreating it 2-3 in a short period not only feels sluggish, but also causes the video to hitch and restart. The solution is to bring the global key back for the native video player. Unlike before, we are using a custom global key which compares the values of hero tags directly. This means we don't need to maintain a map of hero tags to global keys in the state, and also means we don't have to pass the global key down multiple layers. This also fixes #25981. --- .../asset_viewer/asset_page.widget.dart | 29 ++++++++++++++++--- .../asset_viewer/video_viewer.widget.dart | 6 ++-- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/mobile/lib/presentation/widgets/asset_viewer/asset_page.widget.dart b/mobile/lib/presentation/widgets/asset_viewer/asset_page.widget.dart index b62f579762..18fd8846b4 100644 --- a/mobile/lib/presentation/widgets/asset_viewer/asset_page.widget.dart +++ b/mobile/lib/presentation/widgets/asset_viewer/asset_page.widget.dart @@ -307,7 +307,7 @@ class _AssetPageState extends ConsumerState { if (displayAsset.isImage && !isPlayingMotionVideo) { final size = context.sizeData; return PhotoView( - key: ValueKey(displayAsset.heroTag), + key: Key(displayAsset.heroTag), index: widget.index, imageProvider: getFullImageProvider(displayAsset, size: size), heroAttributes: heroAttributes, @@ -335,7 +335,7 @@ class _AssetPageState extends ConsumerState { } return PhotoView.customChild( - key: ValueKey(displayAsset), + key: Key(displayAsset.heroTag), onDragStart: _onDragStart, onDragUpdate: _onDragUpdate, onDragEnd: _onDragEnd, @@ -351,12 +351,11 @@ class _AssetPageState extends ConsumerState { enablePanAlways: true, backgroundDecoration: backgroundDecoration, child: NativeVideoViewer( - key: ValueKey(displayAsset), + key: _NativeVideoViewerKey(displayAsset.heroTag), asset: displayAsset, scaleStateNotifier: _videoScaleStateNotifier, disableScaleGestures: showingDetails, image: Image( - key: ValueKey(displayAsset.heroTag), image: getFullImageProvider(displayAsset, size: context.sizeData), height: context.height, width: context.width, @@ -460,3 +459,25 @@ class _AssetPageState extends ConsumerState { ); } } + +// A global key is used for video viewers to prevent them from being +// unnecessarily recreated. They're quite expensive, and maintain internal +// state. This can cause videos to restart multiple times during normal usage, +// like a hero animation. +// +// A plain ValueKey is insufficient, as it does not allow widgets to reparent. A +// GlobalObjectKey is fragile, as it checks if the given objects are identical, +// rather than equal. Hero tags are created with string interpolation, which +// prevents Dart from interning them. As such, hero tags are not identical, even +// if they are equal. +class _NativeVideoViewerKey extends GlobalKey { + final String value; + + const _NativeVideoViewerKey(this.value) : super.constructor(); + + @override + bool operator ==(Object other) => other is _NativeVideoViewerKey && other.value == value; + + @override + int get hashCode => value.hashCode; +} diff --git a/mobile/lib/presentation/widgets/asset_viewer/video_viewer.widget.dart b/mobile/lib/presentation/widgets/asset_viewer/video_viewer.widget.dart index 0f6568e8fd..30889835f6 100644 --- a/mobile/lib/presentation/widgets/asset_viewer/video_viewer.widget.dart +++ b/mobile/lib/presentation/widgets/asset_viewer/video_viewer.widget.dart @@ -420,20 +420,18 @@ class NativeVideoViewer extends HookConsumerWidget { child: Stack( children: [ // Hide thumbnail once video is visible to avoid it showing in background when zooming out on video. - if (!isVisible.value || controller.value == null) Center(key: ValueKey(asset.heroTag), child: image), + if (!isVisible.value || controller.value == null) Center(child: image), if (aspectRatio.value != null && !isCasting && isCurrent) Visibility.maintain( - key: ValueKey(asset), visible: isVisible.value, child: PhotoView.customChild( - key: ValueKey(asset), enableRotation: false, disableScaleGestures: disableScaleGestures, // Transparent to avoid a black flash when viewer becomes visible but video isn't loaded yet. backgroundDecoration: const BoxDecoration(color: Colors.transparent), scaleStateChangedCallback: (state) => scaleStateNotifier?.value = state, childSize: videoContextSize(aspectRatio.value, context), - child: NativeVideoPlayerView(key: ValueKey(asset), onViewReady: initController), + child: NativeVideoPlayerView(onViewReady: initController), ), ), if (showControls) const Center(child: VideoViewerControls()),