From 9862e50aab90f47d5c2d6daafef5c381a8fbc3c6 Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Mon, 10 Aug 2026 21:14:23 +0600 Subject: [PATCH] fix(mobile): decode remote thumbnails at displayed size (#29965) * fix(mobile): decode remote thumbnails at displayed size * use nullable decode size and reuse the tapped thumbnail in the viewer * clean up decode size naming and guards --- .../alextran/immich/images/LocalImagesImpl.kt | 22 +++- .../immich/images/RemoteImagesImpl.kt | 13 +- .../ios/Runner/Images/RemoteImagesImpl.swift | 52 ++++++-- .../infrastructure/loaders/image_request.dart | 27 +++- .../loaders/remote_image_request.dart | 27 +++- .../asset_viewer/asset_page.widget.dart | 13 +- .../widgets/asset_viewer/asset_preloader.dart | 12 +- .../asset_viewer/asset_viewer.page.dart | 19 +-- .../widgets/images/image_provider.dart | 15 ++- .../widgets/images/remote_image_provider.dart | 34 +++-- .../widgets/images/thumbnail.widget.dart | 13 +- .../widgets/images/thumbnail_tile.widget.dart | 6 +- .../widgets/timeline/constants.dart | 2 +- .../widgets/timeline/fixed/segment.model.dart | 46 ++++--- .../asset_viewer/asset_viewer.provider.dart | 14 ++- mobile/pigeon/remote_image_api.dart | 9 +- .../loaders/remote_image_request_test.dart | 117 ++++++++++++++++++ .../asset_viewer_provider_test.dart | 16 +++ 18 files changed, 386 insertions(+), 71 deletions(-) create mode 100644 mobile/test/infrastructure/loaders/remote_image_request_test.dart diff --git a/mobile/android/app/src/main/kotlin/app/alextran/immich/images/LocalImagesImpl.kt b/mobile/android/app/src/main/kotlin/app/alextran/immich/images/LocalImagesImpl.kt index 7b49d8ca67..af953612cf 100644 --- a/mobile/android/app/src/main/kotlin/app/alextran/immich/images/LocalImagesImpl.kt +++ b/mobile/android/app/src/main/kotlin/app/alextran/immich/images/LocalImagesImpl.kt @@ -33,12 +33,28 @@ data class Request( val callback: (Result?>) -> Unit ) +/** Set [exactSize] to decode at the smallest size that covers [target] without upscaling. */ @RequiresApi(Build.VERSION_CODES.Q) -inline fun ImageDecoder.Source.decodeBitmap(target: Size = Size(0, 0)): Bitmap { +inline fun ImageDecoder.Source.decodeBitmap(target: Size = Size(0, 0), exactSize: Boolean = false): Bitmap { return ImageDecoder.decodeBitmap(this) { decoder, info, _ -> if (target.width > 0 && target.height > 0) { - val sample = max(1, min(info.size.width / target.width, info.size.height / target.height)) - decoder.setTargetSampleSize(sample) + if (exactSize) { + val fillScale = max( + target.width.toDouble() / info.size.width, + target.height.toDouble() / info.size.height + ) + val scale = min(1.0, fillScale) + if (scale < 1) { + val width = ceil(info.size.width * scale).toInt() + val height = ceil(info.size.height * scale).toInt() + if (width > 0 && height > 0) { + decoder.setTargetSize(width, height) + } + } + } else { + val sample = max(1, min(info.size.width / target.width, info.size.height / target.height)) + decoder.setTargetSampleSize(sample) + } } decoder.allocator = ImageDecoder.ALLOCATOR_SOFTWARE decoder.setTargetColorSpace(ColorSpace.get(ColorSpace.Named.SRGB)) diff --git a/mobile/android/app/src/main/kotlin/app/alextran/immich/images/RemoteImagesImpl.kt b/mobile/android/app/src/main/kotlin/app/alextran/immich/images/RemoteImagesImpl.kt index 1623c9cb09..cdc82c871c 100644 --- a/mobile/android/app/src/main/kotlin/app/alextran/immich/images/RemoteImagesImpl.kt +++ b/mobile/android/app/src/main/kotlin/app/alextran/immich/images/RemoteImagesImpl.kt @@ -5,6 +5,7 @@ import android.graphics.ImageDecoder import android.os.Build import android.os.CancellationSignal import android.os.OperationCanceledException +import android.util.Size import androidx.exifinterface.media.ExifInterface import app.alextran.immich.INITIAL_BUFFER_SIZE import app.alextran.immich.NativeBuffer @@ -79,6 +80,8 @@ class RemoteImagesImpl(context: Context) : RemoteImageApi { url: String, requestId: Long, preferEncoded: Boolean, + width: Long?, + height: Long?, callback: (Result?>) -> Unit ) { val signal = CancellationSignal() @@ -102,13 +105,21 @@ class RemoteImagesImpl(context: Context) : RemoteImageApi { if (!preferEncoded && Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { decodeExecutor.execute { val res = if (signal.isCanceled) null else try { - val bitmap = ImageDecoder.createSource(NativeBuffer.wrap(buffer.pointer, buffer.offset)).decodeBitmap() // The embedded preview a raw decodes to has no orientation, so read the container's. val orientation = if (isRawMime(contentType)) { readRawOrientation(NativeBuffer.wrap(buffer.pointer, buffer.offset), buffer.offset) } else { ExifInterface.ORIENTATION_NORMAL } + val target = when (orientation) { + ExifInterface.ORIENTATION_TRANSPOSE, + ExifInterface.ORIENTATION_ROTATE_90, + ExifInterface.ORIENTATION_TRANSVERSE, + ExifInterface.ORIENTATION_ROTATE_270 -> Size(height?.toInt() ?: 0, width?.toInt() ?: 0) + else -> Size(width?.toInt() ?: 0, height?.toInt() ?: 0) + } + val source = ImageDecoder.createSource(NativeBuffer.wrap(buffer.pointer, buffer.offset)) + val bitmap = source.decodeBitmap(target, exactSize = true) if (orientation == ExifInterface.ORIENTATION_NORMAL || orientation == ExifInterface.ORIENTATION_UNDEFINED) { bitmap.toNativeBuffer() } else { diff --git a/mobile/ios/Runner/Images/RemoteImagesImpl.swift b/mobile/ios/Runner/Images/RemoteImagesImpl.swift index de1f6dec89..b4b70f0b30 100644 --- a/mobile/ios/Runner/Images/RemoteImagesImpl.swift +++ b/mobile/ios/Runner/Images/RemoteImagesImpl.swift @@ -1,5 +1,6 @@ import Accelerate import Flutter +import ImageIO import MobileCoreServices import Photos @@ -27,21 +28,21 @@ class RemoteImageApiImpl: NSObject, RemoteImageApi { bitmapInfo: CGBitmapInfo(rawValue: CGImageAlphaInfo.premultipliedLast.rawValue), renderingIntent: .perceptual )! - private static let decodeOptions = [ + private static let decodeOptions: [NSString: Bool] = [ kCGImageSourceShouldCache: false, kCGImageSourceShouldCacheImmediately: true, kCGImageSourceCreateThumbnailWithTransform: true, kCGImageSourceCreateThumbnailFromImageAlways: true - ] as CFDictionary + ] - func requestImage(url: String, requestId: Int64, preferEncoded: Bool, completion: @escaping (Result<[String : Int64]?, any Error>) -> Void) { + func requestImage(url: String, requestId: Int64, preferEncoded: Bool, width: Int64?, height: Int64?, completion: @escaping (Result<[String : Int64]?, any Error>) -> Void) { var urlRequest = URLRequest(url: URL(string: url)!) urlRequest.cachePolicy = .returnCacheDataElseLoad let request = RemoteImageRequest(id: requestId, completion: completion) let task = URLSessionManager.shared.session.dataTask(with: urlRequest) { data, response, error in - Self.handleCompletion(request: request, encoded: preferEncoded, data: data, response: response, error: error) + Self.handleCompletion(request: request, encoded: preferEncoded, width: width, height: height, data: data, response: response, error: error) } request.task = task @@ -49,7 +50,7 @@ class RemoteImageApiImpl: NSObject, RemoteImageApi { task.resume() } - private static func handleCompletion(request: RemoteImageRequest, encoded: Bool, data: Data?, response: URLResponse?, error: Error?) { + private static func handleCompletion(request: RemoteImageRequest, encoded: Bool, width: Int64?, height: Int64?, data: Data?, response: URLResponse?, error: Error?) { if request.isCancelled { return request.completion(ImageProcessing.cancelledResult) } @@ -87,8 +88,17 @@ class RemoteImageApiImpl: NSObject, RemoteImageApi { return request.completion(ImageProcessing.cancelledResult) } - guard let imageSource = CGImageSourceCreateWithData(data as CFData, nil), - let cgImage = CGImageSourceCreateThumbnailAtIndex(imageSource, 0, decodeOptions) else { + guard let imageSource = CGImageSourceCreateWithData(data as CFData, nil) else { + registry.remove(requestId: request.id) + return request.completion(.failure(PigeonError(code: "", message: "Failed to decode image for request", details: nil))) + } + + var options: [NSString: Any] = decodeOptions + if let maxPixelSize = targetThumbnailRenderSize(imageSource: imageSource, width: width, height: height) { + options[kCGImageSourceThumbnailMaxPixelSize] = maxPixelSize + } + + guard let cgImage = CGImageSourceCreateThumbnailAtIndex(imageSource, 0, options as CFDictionary) else { registry.remove(requestId: request.id) return request.completion(.failure(PigeonError(code: "", message: "Failed to decode image for request", details: nil))) } @@ -120,6 +130,34 @@ class RemoteImageApiImpl: NSObject, RemoteImageApi { } } + /// Returns the longest rendered edge needed to cover the requested size. + private static func targetThumbnailRenderSize(imageSource: CGImageSource, width: Int64?, height: Int64?) -> Int? { + guard let width, + let height, + width > 0, + height > 0, + let properties = CGImageSourceCopyPropertiesAtIndex(imageSource, 0, nil) as? [CFString: Any], + let pixelWidth = properties[kCGImagePropertyPixelWidth] as? NSNumber, + let pixelHeight = properties[kCGImagePropertyPixelHeight] as? NSNumber else { + return nil + } + + let orientation = (properties[kCGImagePropertyOrientation] as? NSNumber) + .flatMap { CGImagePropertyOrientation(rawValue: $0.uint32Value) } ?? .up + let swapsDimensions: Bool + switch orientation { + case .leftMirrored, .right, .rightMirrored, .left: + swapsDimensions = true + default: + swapsDimensions = false + } + let sourceWidth = swapsDimensions ? pixelHeight.doubleValue : pixelWidth.doubleValue + let sourceHeight = swapsDimensions ? pixelWidth.doubleValue : pixelHeight.doubleValue + let fillScale = max(Double(width) / sourceWidth, Double(height) / sourceHeight) + let scale = min(1, fillScale) + return scale < 1 ? Int(ceil(max(sourceWidth * scale, sourceHeight * scale))) : nil + } + func cancelRequest(requestId: Int64) { Self.registry.remove(requestId: requestId)?.cancel() } diff --git a/mobile/lib/infrastructure/loaders/image_request.dart b/mobile/lib/infrastructure/loaders/image_request.dart index 8b7cdc7621..6b2be218dc 100644 --- a/mobile/lib/infrastructure/loaders/image_request.dart +++ b/mobile/lib/infrastructure/loaders/image_request.dart @@ -1,5 +1,6 @@ import 'dart:async'; import 'dart:ffi'; +import 'dart:math' as math; import 'dart:ui' as ui; import 'package:ffi/ffi.dart'; @@ -36,7 +37,11 @@ abstract class ImageRequest { void _onCancelled(); - Future<(ui.Codec, ui.ImageDescriptor)?> _codecFromEncodedPlatformImage(int address, int length) async { + Future<(ui.Codec, ui.ImageDescriptor)?> _codecFromEncodedPlatformImage( + int address, + int length, { + ui.Size? decodeSize, + }) async { final pointer = Pointer.fromAddress(address); if (_isCancelled) { malloc.free(pointer); @@ -62,7 +67,8 @@ abstract class ImageRequest { return null; } - final codec = await descriptor.instantiateCodec(); + final target = _targetSize(descriptor.width, descriptor.height, decodeSize); + final codec = await descriptor.instantiateCodec(targetWidth: target?.$1, targetHeight: target?.$2); if (_isCancelled) { descriptor.dispose(); codec.dispose(); @@ -72,8 +78,8 @@ abstract class ImageRequest { return (codec, descriptor); } - Future _fromEncodedPlatformImage(int address, int length) async { - final result = await _codecFromEncodedPlatformImage(address, length); + Future _fromEncodedPlatformImage(int address, int length, {ui.Size? decodeSize}) async { + final result = await _codecFromEncodedPlatformImage(address, length, decodeSize: decodeSize); if (result == null) { return null; } @@ -96,6 +102,19 @@ abstract class ImageRequest { return frame; } + (int, int)? _targetSize(int width, int height, ui.Size? decodeSize) { + if (width <= 0 || height <= 0 || decodeSize == null || decodeSize.width <= 0 || decodeSize.height <= 0) { + return null; + } + + final scale = math.max(decodeSize.width / width, decodeSize.height / height); + if (scale >= 1) { + return null; + } + + return ((width * scale).ceil(), (height * scale).ceil()); + } + Future _fromDecodedPlatformImage(int address, int width, int height, int rowBytes) async { final pointer = Pointer.fromAddress(address); if (_isCancelled) { diff --git a/mobile/lib/infrastructure/loaders/remote_image_request.dart b/mobile/lib/infrastructure/loaders/remote_image_request.dart index d6a25753fb..7b504697ac 100644 --- a/mobile/lib/infrastructure/loaders/remote_image_request.dart +++ b/mobile/lib/infrastructure/loaders/remote_image_request.dart @@ -3,7 +3,10 @@ part of 'image_request.dart'; class RemoteImageRequest extends ImageRequest { final String uri; - RemoteImageRequest({required this.uri}); + /// Physical size to decode, or null for the source size. + final ui.Size? decodeSize; + + RemoteImageRequest({required this.uri, this.decodeSize}); @override Future load(ImageDecoderCallback decode, {double scale = 1.0}) async { @@ -11,10 +14,20 @@ class RemoteImageRequest extends ImageRequest { return null; } - final info = await remoteImageApi.requestImage(uri, requestId: requestId, preferEncoded: false); + final info = await remoteImageApi.requestImage( + uri, + requestId: requestId, + preferEncoded: false, + width: decodeSize?.width.ceil(), + height: decodeSize?.height.ceil(), + ); // Android falls back to encoded data if native decoding fails, so check for both shapes of the response. final frame = switch (info) { - {'pointer': final int pointer, 'length': final int length} => await _fromEncodedPlatformImage(pointer, length), + {'pointer': final int pointer, 'length': final int length} => await _fromEncodedPlatformImage( + pointer, + length, + decodeSize: decodeSize, + ), { 'pointer': final int pointer, 'width': final int width, @@ -33,7 +46,13 @@ class RemoteImageRequest extends ImageRequest { return null; } - final info = await remoteImageApi.requestImage(uri, requestId: requestId, preferEncoded: true); + final info = await remoteImageApi.requestImage( + uri, + requestId: requestId, + preferEncoded: true, + width: null, + height: null, + ); if (info == null) { return null; } 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 c887f8e65a..7556505158 100644 --- a/mobile/lib/presentation/widgets/asset_viewer/asset_page.widget.dart +++ b/mobile/lib/presentation/widgets/asset_viewer/asset_page.widget.dart @@ -333,9 +333,15 @@ class _AssetPageState extends ConsumerState { required bool isCurrent, required bool isPlayingMotionVideo, required String? localFilePath, + required Size? remoteThumbnailSize, }) { final size = context.sizeData; - final imageProvider = getFullImageProvider(asset, size: size, localFilePath: localFilePath); + final imageProvider = getFullImageProvider( + asset, + size: size, + localFilePath: localFilePath, + remoteThumbnailSize: remoteThumbnailSize, + ); if (asset.isImage && !isPlayingMotionVideo) { return PhotoView( @@ -397,7 +403,9 @@ class _AssetPageState extends ConsumerState { @override Widget build(BuildContext context) { - final currentAsset = ref.watch(assetViewerProvider.select((s) => s.currentAsset)); + final (currentAsset, thumbnailSize) = ref.watch( + assetViewerProvider.select((s) => (s.currentAsset, s.thumbnailSize)), + ); _showingDetails = ref.watch(assetViewerProvider.select((s) => s.showingDetails)); final stackIndex = ref.watch(assetViewerProvider.select((s) => s.stackIndex)); final isPlayingMotionVideo = ref.watch(isPlayingMotionVideoProvider); @@ -454,6 +462,7 @@ class _AssetPageState extends ConsumerState { isCurrent: isCurrent, isPlayingMotionVideo: isPlayingMotionVideo, localFilePath: viewIntentFilePath, + remoteThumbnailSize: thumbnailSize, ), ), if (showingOcr && displayAsset.width != null && displayAsset.height != null) diff --git a/mobile/lib/presentation/widgets/asset_viewer/asset_preloader.dart b/mobile/lib/presentation/widgets/asset_viewer/asset_preloader.dart index 4d1856b90d..8c153e9995 100644 --- a/mobile/lib/presentation/widgets/asset_viewer/asset_preloader.dart +++ b/mobile/lib/presentation/widgets/asset_viewer/asset_preloader.dart @@ -17,7 +17,8 @@ class AssetPreloader { AssetPreloader({required this.timelineService, required this.mounted}); - void preload(int index, Size size) { + /// Preloads adjacent images with the current thumbnail size. + void preload(int index, Size size, {Size? thumbnailSize}) { unawaited(timelineService.preloadAssets(index)); _timer?.cancel(); _timer = Timer(Durations.medium4, () async { @@ -33,13 +34,14 @@ class AssetPreloader { } _prevStream?.removeListener(_dummyListener); _nextStream?.removeListener(_dummyListener); - _prevStream = prev != null ? _resolveImage(prev, size) : null; - _nextStream = next != null ? _resolveImage(next, size) : null; + _prevStream = prev != null ? _resolveImage(prev, size, thumbnailSize) : null; + _nextStream = next != null ? _resolveImage(next, size, thumbnailSize) : null; }); } - ImageStream _resolveImage(BaseAsset asset, Size size) { - return getFullImageProvider(asset, size: size).resolve(ImageConfiguration.empty)..addListener(_dummyListener); + ImageStream _resolveImage(BaseAsset asset, Size size, Size? thumbnailSize) { + return getFullImageProvider(asset, size: size, remoteThumbnailSize: thumbnailSize).resolve(ImageConfiguration.empty) + ..addListener(_dummyListener); } void dispose() { diff --git a/mobile/lib/presentation/widgets/asset_viewer/asset_viewer.page.dart b/mobile/lib/presentation/widgets/asset_viewer/asset_viewer.page.dart index 23cc6119fb..731d8f3481 100644 --- a/mobile/lib/presentation/widgets/asset_viewer/asset_viewer.page.dart +++ b/mobile/lib/presentation/widgets/asset_viewer/asset_viewer.page.dart @@ -65,7 +65,8 @@ class AssetViewer extends ConsumerStatefulWidget { @override ConsumerState createState() => _AssetViewerState(); - static void setAsset(WidgetRef ref, BaseAsset asset) { + /// Sets the asset and thumbnail size before opening the viewer. + static void setAsset(WidgetRef ref, BaseAsset asset, {Size? thumbnailSize}) { ref.read(assetViewerProvider.notifier).reset(); // Hide controls by default for videos @@ -73,11 +74,7 @@ class AssetViewer extends ConsumerStatefulWidget { ref.read(assetViewerProvider.notifier).setControls(false); } - _setAsset(ref, asset); - } - - static void _setAsset(WidgetRef ref, BaseAsset asset) { - ref.read(assetViewerProvider.notifier).setAsset(asset); + ref.read(assetViewerProvider.notifier).setAsset(asset, thumbnailSize: thumbnailSize); } } @@ -163,7 +160,11 @@ class _AssetViewerState extends ConsumerState { } void _onAssetInit(Duration timeStamp) { - _preloader.preload(widget.initialIndex, context.sizeData); + _preloader.preload( + widget.initialIndex, + context.sizeData, + thumbnailSize: ref.read(assetViewerProvider).thumbnailSize, + ); _handleCasting(); } @@ -181,8 +182,8 @@ class _AssetViewerState extends ConsumerState { return; } - AssetViewer._setAsset(ref, asset); - _preloader.preload(index, context.sizeData); + ref.read(assetViewerProvider.notifier).setAsset(asset); + _preloader.preload(index, context.sizeData, thumbnailSize: ref.read(assetViewerProvider).thumbnailSize); _handleCasting(); _stackChildrenKeepAlive?.close(); _stackChildrenKeepAlive = ref.read(stackChildrenNotifier(asset).notifier).ref.keepAlive(); diff --git a/mobile/lib/presentation/widgets/images/image_provider.dart b/mobile/lib/presentation/widgets/images/image_provider.dart index aaf3e0dbaf..bdedd29d45 100644 --- a/mobile/lib/presentation/widgets/images/image_provider.dart +++ b/mobile/lib/presentation/widgets/images/image_provider.dart @@ -155,6 +155,7 @@ ImageProvider getFullImageProvider( Size size = const Size(1080, 1920), bool edited = true, String? localFilePath, + Size? remoteThumbnailSize, }) { // Create new provider and cache it final ImageProvider provider; @@ -189,13 +190,21 @@ ImageProvider getFullImageProvider( assetType: asset.type, isAnimated: asset.isAnimatedImage, edited: edited, + thumbnailSize: remoteThumbnailSize, ); } return provider; } -ImageProvider? getThumbnailImageProvider(BaseAsset asset, {Size size = kThumbnailResolution, bool edited = true}) { +ImageProvider? getThumbnailImageProvider( + BaseAsset asset, { + Size size = kThumbnailResolution, + + /// Physical size to decode for remote thumbnails, or null for the source size. + Size? remoteSize, + bool edited = true, +}) { if (_shouldUseLocalAsset(asset)) { final id = asset is LocalAsset ? asset.id : (asset as RemoteAsset).localId!; return LocalThumbProvider(id: id, size: size, assetType: asset.type, checksum: asset.checksum); @@ -203,7 +212,9 @@ ImageProvider? getThumbnailImageProvider(BaseAsset asset, {Size size = kThumbnai final assetId = asset is RemoteAsset ? asset.id : (asset as LocalAsset).remoteId; final thumbhash = asset is RemoteAsset ? asset.thumbHash ?? "" : ""; - return assetId != null ? RemoteImageProvider.thumbnail(assetId: assetId, thumbhash: thumbhash, edited: edited) : null; + return assetId != null + ? RemoteImageProvider.thumbnail(assetId: assetId, thumbhash: thumbhash, edited: edited, decodeSize: remoteSize) + : null; } bool _shouldUseLocalAsset(BaseAsset asset) => diff --git a/mobile/lib/presentation/widgets/images/remote_image_provider.dart b/mobile/lib/presentation/widgets/images/remote_image_provider.dart index 9618ee72ad..b533fd8cf5 100644 --- a/mobile/lib/presentation/widgets/images/remote_image_provider.dart +++ b/mobile/lib/presentation/widgets/images/remote_image_provider.dart @@ -14,10 +14,17 @@ class RemoteImageProvider extends CancellableImageProvider final String url; final bool edited; - RemoteImageProvider({required this.url, this.edited = true}); + /// Physical size to decode, or null for the source size. + final Size? decodeSize; - RemoteImageProvider.thumbnail({required String assetId, required String thumbhash, this.edited = true}) - : url = getThumbnailUrlForRemoteId(assetId, thumbhash: thumbhash, edited: edited); + RemoteImageProvider({required this.url, this.edited = true, this.decodeSize}); + + RemoteImageProvider.thumbnail({ + required String assetId, + required String thumbhash, + this.edited = true, + this.decodeSize, + }) : url = getThumbnailUrlForRemoteId(assetId, thumbhash: thumbhash, edited: edited); @override Future obtainKey(ImageConfiguration configuration) { @@ -37,7 +44,7 @@ class RemoteImageProvider extends CancellableImageProvider } Stream _codec(RemoteImageProvider key, ImageDecoderCallback decode) { - final request = this.request = RemoteImageRequest(uri: key.url); + final request = this.request = RemoteImageRequest(uri: key.url, decodeSize: key.decodeSize); return loadRequest(request, decode, isFinal: true); } @@ -47,13 +54,13 @@ class RemoteImageProvider extends CancellableImageProvider return true; } if (other is RemoteImageProvider) { - return url == other.url && edited == other.edited; + return url == other.url && edited == other.edited && decodeSize == other.decodeSize; } return false; } @override - int get hashCode => url.hashCode ^ edited.hashCode; + int get hashCode => url.hashCode ^ edited.hashCode ^ decodeSize.hashCode; } class RemoteFullImageProvider extends CancellableImageProvider @@ -64,12 +71,16 @@ class RemoteFullImageProvider extends CancellableImageProvider [ DiagnosticsProperty('Image provider', this), DiagnosticsProperty('Asset Id', key.assetId), @@ -96,7 +109,12 @@ class RemoteFullImageProvider extends CancellableImageProvider [ DiagnosticsProperty('Image provider', this), diff --git a/mobile/lib/presentation/widgets/images/thumbnail.widget.dart b/mobile/lib/presentation/widgets/images/thumbnail.widget.dart index 90bb79cced..5bfeb25977 100644 --- a/mobile/lib/presentation/widgets/images/thumbnail.widget.dart +++ b/mobile/lib/presentation/widgets/images/thumbnail.widget.dart @@ -26,17 +26,22 @@ class Thumbnail extends StatefulWidget { required String remoteId, required String thumbhash, this.fit = BoxFit.cover, - Size size = kThumbnailResolution, + + /// Physical size to decode, or null for the source size. + Size? decodeSize, super.key, - }) : imageProvider = RemoteImageProvider.thumbnail(assetId: remoteId, thumbhash: thumbhash), + }) : imageProvider = RemoteImageProvider.thumbnail(assetId: remoteId, thumbhash: thumbhash, decodeSize: decodeSize), thumbhashProvider = null; Thumbnail.fromAsset({ required BaseAsset? asset, this.fit = BoxFit.cover, - /// The logical UI size of the thumbnail. This is only used to determine the ideal image resolution and does not affect the widget size. + /// Decode size for local thumbnails. This does not affect the widget size. Size size = kThumbnailResolution, + + /// Physical size to decode for remote thumbnails. + Size? remoteSize, super.key, }) : thumbhashProvider = switch (asset) { RemoteAsset() when asset.thumbHash != null && asset.localId == null => ThumbHashProvider( @@ -44,7 +49,7 @@ class Thumbnail extends StatefulWidget { ), _ => null, }, - imageProvider = asset == null ? null : getThumbnailImageProvider(asset, size: size); + imageProvider = asset == null ? null : getThumbnailImageProvider(asset, size: size, remoteSize: remoteSize); @override State createState() => _ThumbnailState(); diff --git a/mobile/lib/presentation/widgets/images/thumbnail_tile.widget.dart b/mobile/lib/presentation/widgets/images/thumbnail_tile.widget.dart index 1d3dcb2cf0..953efc144d 100644 --- a/mobile/lib/presentation/widgets/images/thumbnail_tile.widget.dart +++ b/mobile/lib/presentation/widgets/images/thumbnail_tile.widget.dart @@ -16,6 +16,7 @@ class ThumbnailTile extends ConsumerStatefulWidget { const ThumbnailTile( this.asset, { this.size = kThumbnailResolution, + this.remoteSize, this.fit = BoxFit.cover, this.showStorageIndicator = false, this.lockSelection = false, @@ -26,6 +27,9 @@ class ThumbnailTile extends ConsumerStatefulWidget { final BaseAsset? asset; final Size size; + + /// Physical size to decode for remote thumbnails. + final Size? remoteSize; final BoxFit fit; final bool showStorageIndicator; final bool lockSelection; @@ -108,7 +112,7 @@ class _ThumbnailTileState extends ConsumerState { // but other solutions have failed thus far. key: ValueKey(isCurrentAsset), tag: '${asset?.heroTag}_$heroIndex', - child: Thumbnail.fromAsset(asset: asset, size: widget.size), + child: Thumbnail.fromAsset(asset: asset, size: widget.size, remoteSize: widget.remoteSize), // Placeholderbuilder used to hide indicators on first hero animation, since flightShuttleBuilder isn't called until both source and destination hero exist in widget tree. placeholderBuilder: (context, heroSize, child) { if (!_hideIndicators) { diff --git a/mobile/lib/presentation/widgets/timeline/constants.dart b/mobile/lib/presentation/widgets/timeline/constants.dart index 84892c79f7..86c44d4a7f 100644 --- a/mobile/lib/presentation/widgets/timeline/constants.dart +++ b/mobile/lib/presentation/widgets/timeline/constants.dart @@ -9,5 +9,5 @@ const double kScrubberThumbHeight = 48.0; const Duration kTimelineScrubberFadeInDuration = Duration(milliseconds: 300); const Duration kTimelineScrubberFadeOutDuration = Duration(milliseconds: 800); -const Size kThumbnailResolution = Size.square(320); // TODO: make the resolution vary based on actual tile size +const Size kThumbnailResolution = Size.square(320); const kThumbnailDiskCacheSize = 1024 << 20; // 1GiB diff --git a/mobile/lib/presentation/widgets/timeline/fixed/segment.model.dart b/mobile/lib/presentation/widgets/timeline/fixed/segment.model.dart index b108acde3b..32bbffa95e 100644 --- a/mobile/lib/presentation/widgets/timeline/fixed/segment.model.dart +++ b/mobile/lib/presentation/widgets/timeline/fixed/segment.model.dart @@ -144,19 +144,6 @@ class _FixedSegmentRow extends ConsumerWidget { TimelineService timelineService, bool isDynamicLayout, ) { - final children = [ - for (int i = 0; i < assets.length; i++) - TimelineAssetIndexWrapper( - assetIndex: assetIndex + i, - segmentIndex: 0, // For simplicity, using 0 for now - child: _AssetTileWidget( - key: ValueKey(Object.hash(assets[i].heroTag, assetIndex + i, timelineService.hashCode)), - asset: assets[i], - assetIndex: assetIndex + i, - ), - ), - ]; - final widths = List.filled(assets.length, tileHeight); if (isDynamicLayout) { @@ -186,6 +173,20 @@ class _FixedSegmentRow extends ConsumerWidget { } } + final children = [ + for (int i = 0; i < assets.length; i++) + TimelineAssetIndexWrapper( + assetIndex: assetIndex + i, + segmentIndex: 0, // For simplicity, using 0 for now + child: _AssetTileWidget( + key: ValueKey(Object.hash(assets[i].heroTag, assetIndex + i, timelineService.hashCode)), + asset: assets[i], + assetIndex: assetIndex + i, + size: Size(widths[i], tileHeight), + ), + ), + ]; + return TimelineDragRegion( child: TimelineRow( height: tileHeight, @@ -201,10 +202,18 @@ class _FixedSegmentRow extends ConsumerWidget { class _AssetTileWidget extends ConsumerWidget { final BaseAsset asset; final int assetIndex; + final Size size; - const _AssetTileWidget({super.key, required this.asset, required this.assetIndex}); + const _AssetTileWidget({super.key, required this.asset, required this.assetIndex, required this.size}); - Future _handleOnTap(BuildContext ctx, WidgetRef ref, int assetIndex, BaseAsset asset, int? heroOffset) async { + Future _handleOnTap( + BuildContext ctx, + WidgetRef ref, + int assetIndex, + BaseAsset asset, + int? heroOffset, + Size remoteSize, + ) async { final multiSelectState = ref.read(multiSelectProvider); if (multiSelectState.forceEnable || multiSelectState.isEnabled) { @@ -216,7 +225,7 @@ class _AssetTileWidget extends ConsumerWidget { } ref.read(isPlayingMotionVideoProvider.notifier).playing = false; - AssetViewer.setAsset(ref, asset); + AssetViewer.setAsset(ref, asset, thumbnailSize: remoteSize); unawaited( ctx.pushRoute( AssetViewerRoute( @@ -256,6 +265,8 @@ class _AssetTileWidget extends ConsumerWidget { @override Widget build(BuildContext context, WidgetRef ref) { + final remoteSize = size * MediaQuery.devicePixelRatioOf(context); + final heroOffset = TabsRouterScope.of(context)?.controller.activeIndex ?? 0; final lockSelection = _getLockSelectionStatus(ref); @@ -265,10 +276,11 @@ class _AssetTileWidget extends ConsumerWidget { return RepaintBoundary( child: GestureDetector( - onTap: () => lockSelection ? null : _handleOnTap(context, ref, assetIndex, asset, heroOffset), + onTap: () => lockSelection ? null : _handleOnTap(context, ref, assetIndex, asset, heroOffset, remoteSize), onLongPress: () => lockSelection || isReadonlyModeEnabled ? null : _handleOnLongPress(ref, asset), child: ThumbnailTile( asset, + remoteSize: remoteSize, lockSelection: lockSelection, showStorageIndicator: showStorageIndicator, showStackIndicator: showStackIndicator, diff --git a/mobile/lib/providers/asset_viewer/asset_viewer.provider.dart b/mobile/lib/providers/asset_viewer/asset_viewer.provider.dart index ac929d2b6a..fd7fbebe36 100644 --- a/mobile/lib/providers/asset_viewer/asset_viewer.provider.dart +++ b/mobile/lib/providers/asset_viewer/asset_viewer.provider.dart @@ -1,4 +1,5 @@ import 'dart:async'; +import 'dart:ui'; import 'package:flutter/foundation.dart'; import 'package:freezed_annotation/freezed_annotation.dart'; @@ -18,6 +19,9 @@ abstract class AssetViewerState with _$AssetViewerState { @Default(false) bool isZoomed, @Default(false) bool showingOcr, BaseAsset? currentAsset, + + /// Physical thumbnail size retained while paging through the viewer. + Size? thumbnailSize, @Default(0) int stackIndex, }) = _AssetViewerState; } @@ -37,11 +41,17 @@ class AssetViewerStateNotifier extends Notifier { state = const AssetViewerState(); } - void setAsset(BaseAsset asset) { + void setAsset(BaseAsset asset, {Size? thumbnailSize}) { if (asset == state.currentAsset) { return; } - state = state.copyWith(currentAsset: asset, stackIndex: 0, showingOcr: false); + // Swiping to a neighbor passes no size; keep the tapped tile's so neighbors reuse it. + state = state.copyWith( + currentAsset: asset, + thumbnailSize: thumbnailSize ?? state.thumbnailSize, + stackIndex: 0, + showingOcr: false, + ); _watchCurrentAsset(asset); } diff --git a/mobile/pigeon/remote_image_api.dart b/mobile/pigeon/remote_image_api.dart index 7f0135acb8..277b7016fd 100644 --- a/mobile/pigeon/remote_image_api.dart +++ b/mobile/pigeon/remote_image_api.dart @@ -13,8 +13,15 @@ import 'package:pigeon/pigeon.dart'; ) @HostApi() abstract class RemoteImageApi { + /// Width and height are the physical decode size, or null for the source size. @async - Map? requestImage(String url, {required int requestId, required bool preferEncoded}); + Map? requestImage( + String url, { + required int requestId, + required bool preferEncoded, + int? width, + int? height, + }); void cancelRequest(int requestId); diff --git a/mobile/test/infrastructure/loaders/remote_image_request_test.dart b/mobile/test/infrastructure/loaders/remote_image_request_test.dart new file mode 100644 index 0000000000..97bdf3e2bf --- /dev/null +++ b/mobile/test/infrastructure/loaders/remote_image_request_test.dart @@ -0,0 +1,117 @@ +import 'dart:ffi'; +import 'dart:io'; +import 'dart:ui' as ui; + +import 'package:ffi/ffi.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:immich_mobile/infrastructure/loaders/image_request.dart'; +import 'package:immich_mobile/platform/remote_image_api.g.dart'; +import 'package:immich_mobile/presentation/widgets/images/remote_image_provider.dart'; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + const channel = BasicMessageChannel( + 'dev.flutter.pigeon.immich_mobile.RemoteImageApi.requestImage', + RemoteImageApi.pigeonChannelCodec, + ); + late List args; + + setUp(() { + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger.setMockDecodedMessageHandler(channel, ( + message, + ) async { + args = message! as List; + return [null]; + }); + }); + + tearDown(() { + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger.setMockDecodedMessageHandler(channel, null); + }); + + Future loadEncoded(String path, ui.Size decodeSize) async { + final bytes = await File(path).readAsBytes(); + final pointer = malloc(bytes.length)..asTypedList(bytes.length).setAll(0, bytes); + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger.setMockDecodedMessageHandler(channel, ( + message, + ) async { + return [ + {'pointer': pointer.address, 'length': bytes.length}, + ]; + }); + final request = RemoteImageRequest(uri: 'https://example.test/fallback', decodeSize: decodeSize); + + return (await request.load((_, {getTargetSize}) => throw UnimplementedError()))!.image; + } + + test('passes the requested decode size to the platform', () async { + final request = RemoteImageRequest(uri: 'https://example.test/thumbnail', decodeSize: const ui.Size(319.1, 179.1)); + + await request.load((_, {getTargetSize}) => throw UnimplementedError()); + + expect(args[0], 'https://example.test/thumbnail'); + expect(args[2], isFalse); + expect(args[3], 320); + expect(args[4], 180); + }); + + test('leaves requests without a size unbounded', () async { + final request = RemoteImageRequest(uri: 'https://example.test/thumbnail'); + + await request.load((_, {getTargetSize}) => throw UnimplementedError()); + + expect(args[3], isNull); + expect(args[4], isNull); + }); + + test('leaves encoded animation requests unbounded', () async { + final request = RemoteImageRequest(uri: 'https://example.test/animation', decodeSize: const ui.Size(320, 180)); + + await request.loadCodec(); + + expect(args[2], isTrue); + expect(args[3], isNull); + expect(args[4], isNull); + }); + + test('keeps portrait cover quality in a wide tile', () async { + final image = await loadEncoded('assets/feature_message/ocr.webp', const ui.Size(963, 642)); + + expect(image.width, 963); + expect(image.height, 1453); + image.dispose(); + }); + + test('preserves cover quality for extreme aspect ratios', () async { + final image = await loadEncoded('assets/immich-logo-inline-light.png', const ui.Size.square(320)); + + expect(image.width, 1311); + expect(image.height, 320); + image.dispose(); + }); + + test('does not upscale encoded fallback', () async { + final image = await loadEncoded('assets/feature_message/ocr.webp', const ui.Size.square(2000)); + + expect(image.width, 1206); + expect(image.height, 1819); + image.dispose(); + }); + + test('uses the decode size in the provider cache key', () { + final small = RemoteImageProvider(url: 'https://example.test/thumbnail', decodeSize: const ui.Size.square(160)); + final large = RemoteImageProvider(url: 'https://example.test/thumbnail', decodeSize: const ui.Size.square(320)); + + expect(small, isNot(large)); + }); + + test('shares the cache key when no decode size is set', () { + final first = RemoteImageProvider(url: 'https://example.test/thumbnail'); + final second = RemoteImageProvider(url: 'https://example.test/thumbnail'); + + expect(first, second); + expect(first.hashCode, second.hashCode); + }); +} diff --git a/mobile/test/providers/asset_viewer/asset_viewer_provider_test.dart b/mobile/test/providers/asset_viewer/asset_viewer_provider_test.dart index 27033a5774..454923affa 100644 --- a/mobile/test/providers/asset_viewer/asset_viewer_provider_test.dart +++ b/mobile/test/providers/asset_viewer/asset_viewer_provider_test.dart @@ -1,4 +1,5 @@ import 'dart:async'; +import 'dart:ui'; import 'package:flutter_test/flutter_test.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; @@ -107,5 +108,20 @@ void main() { await pumpEventQueue(); expect(container.read(assetViewerProvider).currentAsset, updatedTwo); }); + + test('keeps the tapped tile size while swiping', () { + final first = RemoteAssetFactory.create(); + final second = RemoteAssetFactory.create(); + final notifier = container.read(assetViewerProvider.notifier); + + notifier.setAsset(first, thumbnailSize: const Size.square(642)); + expect(container.read(assetViewerProvider).thumbnailSize, const Size.square(642)); + + notifier.setAsset(second); + expect(container.read(assetViewerProvider).thumbnailSize, const Size.square(642)); + + notifier.reset(); + expect(container.read(assetViewerProvider).thumbnailSize, isNull); + }); }); }