fix: asset_viewer page viewing experience (#20889)

* fix: zoomed in effect on swiped when bottom sheet is open

* fix: memory leaked

* fix: asset out of range when swiping in asset_viewer
This commit is contained in:
Alex 2025-08-13 11:35:42 -05:00 committed by GitHub
parent b3fb831994
commit aefa62b234
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 126 additions and 43 deletions

View File

@ -169,6 +169,36 @@ class TimelineService {
return _buffer.elementAt(index - _bufferOffset);
}
/// Gets an asset at the given index, automatically loading the buffer if needed.
/// This is an async version that can handle out-of-range indices by loading the appropriate buffer.
Future<BaseAsset?> getAssetAsync(int index) async {
if (index < 0 || index >= _totalAssets) {
return null;
}
if (hasRange(index, 1)) {
return _buffer.elementAt(index - _bufferOffset);
}
// Load the buffer containing the requested index
try {
final assets = await loadAssets(index, 1);
return assets.isNotEmpty ? assets.first : null;
} catch (e) {
return null;
}
}
/// Safely gets an asset at the given index without throwing a RangeError.
/// Returns null if the index is out of bounds or not currently in the buffer.
/// For automatic buffer loading, use getAssetAsync instead.
BaseAsset? getAssetSafe(int index) {
if (index < 0 || index >= _totalAssets || !hasRange(index, 1)) {
return null;
}
return _buffer.elementAt(index - _bufferOffset);
}
Future<void> dispose() async {
await _bucketSubscription?.cancel();
_bucketSubscription = null;

View File

@ -127,20 +127,21 @@ class _AssetViewerState extends ConsumerState<AssetViewer> {
_delayedOperations.clear();
}
// This is used to calculate the scale of the asset when the bottom sheet is showing.
// It is a small increment to ensure that the asset is slightly zoomed in when the
// bottom sheet is showing, which emulates the zoom effect.
double get _getScaleForBottomSheet => (viewController?.prevValue.scale ?? viewController?.value.scale ?? 1.0) + 0.01;
double _getVerticalOffsetForBottomSheet(double extent) =>
(context.height * extent) - (context.height * _kBottomSheetMinimumExtent);
Future<void> _precacheImage(int index) async {
if (!mounted || index < 0 || index >= totalAssets) {
if (!mounted) {
return;
}
final timelineService = ref.read(timelineServiceProvider);
final asset = await timelineService.getAssetAsync(index);
if (asset == null || !mounted) {
return;
}
final asset = ref.read(timelineServiceProvider).getAsset(index);
final screenSize = Size(context.width, context.height);
// Precache both thumbnail and full image for smooth transitions
@ -152,8 +153,15 @@ class _AssetViewerState extends ConsumerState<AssetViewer> {
);
}
void _onAssetChanged(int index) {
final asset = ref.read(timelineServiceProvider).getAsset(index);
void _onAssetChanged(int index) async {
// Validate index bounds and try to get asset, loading buffer if needed
final timelineService = ref.read(timelineServiceProvider);
final asset = await timelineService.getAssetAsync(index);
if (asset == null) {
return;
}
// Always holds the current asset from the timeline
ref.read(assetViewerProvider.notifier).setAsset(asset);
// The currentAssetNotifier actually holds the current asset that is displayed
@ -217,19 +225,15 @@ class _AssetViewerState extends ConsumerState<AssetViewer> {
final verticalOffset =
(context.height * bottomSheetController.size) - (context.height * _kBottomSheetMinimumExtent);
controller.position = Offset(0, -verticalOffset);
// Apply the zoom effect when the bottom sheet is showing
initialScale = controller.scale;
controller.scale = (controller.scale ?? 1.0) + 0.01;
}
}
void _onPageChanged(int index, PhotoViewControllerBase? controller) {
_onAssetChanged(index);
viewController = controller;
// If the bottom sheet is showing, we need to adjust scale the asset to
// emulate the zoom effect
if (showingBottomSheet) {
initialScale = controller?.scale;
controller?.scale = _getScaleForBottomSheet;
}
}
void _onDragStart(
@ -412,16 +416,22 @@ class _AssetViewerState extends ConsumerState<AssetViewer> {
}
}
void _onAssetReloadEvent() {
setState(() {
final index = pageController.page?.round() ?? 0;
final newAsset = ref.read(timelineServiceProvider).getAsset(index);
final currentAsset = ref.read(currentAssetNotifier);
// Do not reload / close the bottom sheet if the asset has not changed
if (newAsset.heroTag == currentAsset?.heroTag) {
return;
}
void _onAssetReloadEvent() async {
final index = pageController.page?.round() ?? 0;
final timelineService = ref.read(timelineServiceProvider);
final newAsset = await timelineService.getAssetAsync(index);
if (newAsset == null) {
return;
}
final currentAsset = ref.read(currentAssetNotifier);
// Do not reload / close the bottom sheet if the asset has not changed
if (newAsset.heroTag == currentAsset?.heroTag) {
return;
}
setState(() {
_onAssetChanged(pageController.page!.round());
sheetCloseController?.close();
});
@ -430,7 +440,7 @@ class _AssetViewerState extends ConsumerState<AssetViewer> {
void _openBottomSheet(BuildContext ctx, {double extent = _kBottomSheetMinimumExtent}) {
ref.read(assetViewerProvider.notifier).setBottomSheet(true);
initialScale = viewController?.scale;
viewController?.updateMultiple(scale: _getScaleForBottomSheet);
// viewController?.updateMultiple(scale: (viewController?.scale ?? 1.0) + 0.01);
previousExtent = _kBottomSheetMinimumExtent;
sheetCloseController = showBottomSheet(
context: ctx,
@ -468,16 +478,29 @@ class _AssetViewerState extends ConsumerState<AssetViewer> {
}
Widget _placeholderBuilder(BuildContext ctx, ImageChunkEvent? progress, int index) {
BaseAsset asset = ref.read(timelineServiceProvider).getAsset(index);
final timelineService = ref.read(timelineServiceProvider);
final asset = timelineService.getAssetSafe(index);
// If asset is not available in buffer, show a loading container
if (asset == null) {
return Container(
width: double.infinity,
height: double.infinity,
color: backgroundColor,
child: const Center(child: CircularProgressIndicator()),
);
}
BaseAsset displayAsset = asset;
final stackChildren = ref.read(stackChildrenNotifier(asset)).valueOrNull;
if (stackChildren != null && stackChildren.isNotEmpty) {
asset = stackChildren.elementAt(ref.read(assetViewerProvider.select((s) => s.stackIndex)));
displayAsset = stackChildren.elementAt(ref.read(assetViewerProvider.select((s) => s.stackIndex)));
}
return Container(
width: double.infinity,
height: double.infinity,
color: backgroundColor,
child: Thumbnail(asset: asset, fit: BoxFit.contain),
child: Thumbnail(asset: displayAsset, fit: BoxFit.contain),
);
}
@ -493,18 +516,34 @@ class _AssetViewerState extends ConsumerState<AssetViewer> {
PhotoViewGalleryPageOptions _assetBuilder(BuildContext ctx, int index) {
scaffoldContext ??= ctx;
BaseAsset asset = ref.read(timelineServiceProvider).getAsset(index);
final timelineService = ref.read(timelineServiceProvider);
final asset = timelineService.getAssetSafe(index);
// If asset is not available in buffer, return a placeholder
if (asset == null) {
return PhotoViewGalleryPageOptions.customChild(
heroAttributes: PhotoViewHeroAttributes(tag: 'loading_$index'),
child: Container(
width: ctx.width,
height: ctx.height,
color: backgroundColor,
child: const Center(child: CircularProgressIndicator()),
),
);
}
BaseAsset displayAsset = asset;
final stackChildren = ref.read(stackChildrenNotifier(asset)).valueOrNull;
if (stackChildren != null && stackChildren.isNotEmpty) {
asset = stackChildren.elementAt(ref.read(assetViewerProvider.select((s) => s.stackIndex)));
displayAsset = stackChildren.elementAt(ref.read(assetViewerProvider.select((s) => s.stackIndex)));
}
final isPlayingMotionVideo = ref.read(isPlayingMotionVideoProvider);
if (asset.isImage && !isPlayingMotionVideo) {
return _imageBuilder(ctx, asset);
if (displayAsset.isImage && !isPlayingMotionVideo) {
return _imageBuilder(ctx, displayAsset);
}
return _videoBuilder(ctx, asset);
return _videoBuilder(ctx, displayAsset);
}
PhotoViewGalleryPageOptions _imageBuilder(BuildContext ctx, BaseAsset asset) {
@ -515,8 +554,6 @@ class _AssetViewerState extends ConsumerState<AssetViewer> {
heroAttributes: PhotoViewHeroAttributes(tag: '${asset.heroTag}_$heroOffset'),
filterQuality: FilterQuality.high,
tightMode: true,
initialScale: PhotoViewComputedScale.contained * 0.999,
minScale: PhotoViewComputedScale.contained * 0.999,
disableScaleGestures: showingBottomSheet,
onDragStart: _onDragStart,
onDragUpdate: _onDragUpdate,
@ -545,9 +582,7 @@ class _AssetViewerState extends ConsumerState<AssetViewer> {
onTapDown: _onTapDown,
heroAttributes: PhotoViewHeroAttributes(tag: '${asset.heroTag}_$heroOffset'),
filterQuality: FilterQuality.high,
initialScale: PhotoViewComputedScale.contained * 0.99,
maxScale: 1.0,
minScale: PhotoViewComputedScale.contained * 0.99,
basePosition: Alignment.center,
child: SizedBox(
width: ctx.width,

View File

@ -11,7 +11,6 @@ import 'package:immich_mobile/domain/services/setting.service.dart';
import 'package:immich_mobile/extensions/codec_extensions.dart';
import 'package:immich_mobile/infrastructure/repositories/asset_media.repository.dart';
import 'package:immich_mobile/infrastructure/repositories/storage.repository.dart';
import 'package:immich_mobile/presentation/widgets/images/image_provider.dart';
import 'package:immich_mobile/presentation/widgets/images/one_frame_multi_image_stream_completer.dart';
import 'package:immich_mobile/presentation/widgets/timeline/constants.dart';
import 'package:immich_mobile/providers/image/cache/thumbnail_image_cache_manager.dart';
@ -107,7 +106,6 @@ class LocalFullImageProvider extends ImageProvider<LocalFullImageProvider> {
ImageStreamCompleter loadImage(LocalFullImageProvider key, ImageDecoderCallback decode) {
return OneFramePlaceholderImageStreamCompleter(
_codec(key, decode),
initialImage: getCachedImage(LocalThumbProvider(id: key.id, updatedAt: key.updatedAt)),
informationCollector: () => <DiagnosticsNode>[
DiagnosticsProperty<String>('Id', key.id),
DiagnosticsProperty<DateTime>('Updated at', key.updatedAt),
@ -117,13 +115,30 @@ class LocalFullImageProvider extends ImageProvider<LocalFullImageProvider> {
}
// Streams in each stage of the image as we ask for it
Stream<ImageInfo> _codec(LocalFullImageProvider key, ImageDecoderCallback decode) {
Stream<ImageInfo> _codec(LocalFullImageProvider key, ImageDecoderCallback decode) async* {
try {
return switch (key.type) {
// First, yield the thumbnail image from LocalThumbProvider
final thumbProvider = LocalThumbProvider(id: key.id, updatedAt: key.updatedAt);
try {
final thumbCodec = await thumbProvider._codec(
thumbProvider,
thumbProvider.cacheManager ?? ThumbnailImageCacheManager(),
decode,
);
final thumbImageInfo = await thumbCodec.getImageInfo();
yield thumbImageInfo;
} catch (_) {}
// Then proceed with the main image loading stream
final mainStream = switch (key.type) {
AssetType.image => _decodeProgressive(key, decode),
AssetType.video => _getThumbnailCodec(key, decode),
_ => throw StateError('Unsupported asset type ${key.type}'),
};
await for (final imageInfo in mainStream) {
yield imageInfo;
}
} catch (error, stack) {
Logger('ImmichLocalImageProvider').severe('Error loading local image ${key.id}', error, stack);
throw const ImageLoadingException('Could not load image from local storage');

View File

@ -86,6 +86,7 @@ class _ImageWrapperState extends State<ImageWrapper> {
Size? _imageSize;
Object? _lastException;
StackTrace? _lastStack;
bool _didLoadSynchronously = false;
@override
void dispose() {
@ -130,9 +131,11 @@ class _ImageWrapperState extends State<ImageWrapper> {
_loadingProgress = null;
_lastException = null;
_lastStack = null;
_didLoadSynchronously = synchronousCall;
}
synchronousCall ? setupCB() : setState(setupCB);
synchronousCall && !_didLoadSynchronously ? setupCB() : setState(setupCB);
}
void handleError(dynamic error, StackTrace? stackTrace) {