From 00e813ba84b66dc69519fa4897140afb0ab6179e Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Wed, 5 Aug 2026 01:08:27 +0600 Subject: [PATCH] fix(mobile): stop long images squishing on ios (#29367) * fix(mobile): stop long images squishing on ios * fix(mobile): bound the original to 16384 with fast resize * refactor(mobile): reuse the shared request options for the original * fix(mobile): fix squished long image previews on ios * fix(mobile): retry with exact resize when fast resize overshoots the texture bound * fix(mobile): clamp preview targets to a minimum of one pixel * fix nullable cast flagged by the stricter lints --- .../ios/Runner/Images/LocalImagesImpl.swift | 40 +++++++++++++-- .../widgets/images/image_provider.dart | 2 + .../widgets/images/local_image_provider.dart | 41 +++++++++++++-- .../images/local_image_provider_test.dart | 50 ++++++++++++++++++- 4 files changed, 124 insertions(+), 9 deletions(-) diff --git a/mobile/ios/Runner/Images/LocalImagesImpl.swift b/mobile/ios/Runner/Images/LocalImagesImpl.swift index 9c142da054..0d023ad466 100644 --- a/mobile/ios/Runner/Images/LocalImagesImpl.swift +++ b/mobile/ios/Runner/Images/LocalImagesImpl.swift @@ -20,6 +20,7 @@ class LocalImageApiImpl: LocalImageApi { requestOptions.version = .current return requestOptions }() + private static let maxPixelSize: CGFloat = 16384 private static let registry = RequestRegistry() @@ -108,11 +109,16 @@ class LocalImageApiImpl: LocalImageApi { ])) } + let isOriginal = !(width > 0 && height > 0) + let targetSize = isOriginal + ? CGSize(width: Self.maxPixelSize, height: Self.maxPixelSize) + : CGSize(width: Double(width), height: Double(height)) + let contentMode: PHImageContentMode = isOriginal ? .aspectFit : .aspectFill var image: UIImage? Self.imageManager.requestImage( for: asset, - targetSize: width > 0 && height > 0 ? CGSize(width: Double(width), height: Double(height)) : PHImageManagerMaximumSize, - contentMode: .aspectFill, + targetSize: targetSize, + contentMode: contentMode, options: Self.requestOptions, resultHandler: { (_image, info) -> Void in image = _image @@ -123,12 +129,38 @@ class LocalImageApiImpl: LocalImageApi { return request.completion(ImageProcessing.cancelledResult) } - guard let image = image, - let cgImage = image.cgImage else { + guard let fastImage = image, + var cgImage = fastImage.cgImage else { Self.registry.remove(requestId: requestId) return request.completion(.failure(PigeonError(code: "", message: "Could not get pixel data for \(assetId)", details: nil))) } + // .fast can return larger than the target, so retry with .exact to guarantee the bound. + if max(cgImage.width, cgImage.height) > Int(Self.maxPixelSize) { + let exactOptions = Self.requestOptions.copy() as! PHImageRequestOptions + exactOptions.resizeMode = .exact + image = nil + Self.imageManager.requestImage( + for: asset, + targetSize: targetSize, + contentMode: contentMode, + options: exactOptions, + resultHandler: { (_image, info) -> Void in + image = _image + } + ) + + if request.isCancelled { + return request.completion(ImageProcessing.cancelledResult) + } + + guard let exactImage = image?.cgImage else { + Self.registry.remove(requestId: requestId) + return request.completion(.failure(PigeonError(code: "", message: "Could not resize image for \(assetId)", details: nil))) + } + cgImage = exactImage + } + if request.isCancelled { return request.completion(ImageProcessing.cancelledResult) } diff --git a/mobile/lib/presentation/widgets/images/image_provider.dart b/mobile/lib/presentation/widgets/images/image_provider.dart index a2d7345b16..aaf3e0dbaf 100644 --- a/mobile/lib/presentation/widgets/images/image_provider.dart +++ b/mobile/lib/presentation/widgets/images/image_provider.dart @@ -167,6 +167,8 @@ ImageProvider getFullImageProvider( size: size, assetType: asset.type, isAnimated: asset.isAnimatedImage, + width: asset.width, + height: asset.height, checksum: asset.checksum, ); } else { diff --git a/mobile/lib/presentation/widgets/images/local_image_provider.dart b/mobile/lib/presentation/widgets/images/local_image_provider.dart index 8a88e915ff..46e8eda85b 100644 --- a/mobile/lib/presentation/widgets/images/local_image_provider.dart +++ b/mobile/lib/presentation/widgets/images/local_image_provider.dart @@ -1,3 +1,5 @@ +import 'dart:math' as math; + import 'package:flutter/foundation.dart'; import 'package:flutter/widgets.dart'; import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; @@ -8,6 +10,9 @@ 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'; +// iOS GPU textures max out at 16384px; larger images squish. +const _kMaxPixelSize = 16384; + class LocalThumbProvider extends CancellableImageProvider with CancellableImageProviderMixin { final String id; @@ -62,6 +67,8 @@ class LocalFullImageProvider extends CancellableImageProvider + previewTargetSize(size.width * dpr, size.height * dpr, width, height, previewIsFinal: previewIsFinal); + + // Use an aspect-correct target when aspectFill would exceed the texture limit. + @visibleForTesting + static Size previewTargetSize(double boxW, double boxH, int? width, int? height, {required bool previewIsFinal}) { + if (width == null || height == null || width <= 0 || height <= 0) { + return Size(boxW, boxH); + } + final imgLong = math.max(width, height).toDouble(); + final coverLong = imgLong * math.max(boxW / width, boxH / height); + if (coverLong <= _kMaxPixelSize) { + return Size(boxW, boxH); + } + final bound = previewIsFinal ? _kMaxPixelSize.toDouble() : math.max(boxW, boxH); + final scale = math.min(1.0, bound / imgLong); + return Size(math.max(1.0, width * scale), math.max(1.0, height * scale)); + } + @override Future obtainKey(ImageConfiguration configuration) { return SynchronousFuture(this); @@ -118,7 +146,7 @@ class LocalFullImageProvider extends CancellableImageProvider Object.hash(id, size, isAnimated, checksum); + int get hashCode => Object.hash(id, size, isAnimated, width, height, checksum); } diff --git a/mobile/test/presentation/widgets/images/local_image_provider_test.dart b/mobile/test/presentation/widgets/images/local_image_provider_test.dart index ce9b15af9e..5b556f8a41 100644 --- a/mobile/test/presentation/widgets/images/local_image_provider_test.dart +++ b/mobile/test/presentation/widgets/images/local_image_provider_test.dart @@ -22,6 +22,54 @@ void main() { loads = 0; }); + group('LocalFullImageProvider.previewTargetSize', () { + const box = Size(1179, 2556); + const cases = <(String, Size, int?, int?, bool, Size)>[ + ('normal', box, 4032, 3024, true, box), + ('missing dimensions', box, null, null, true, box), + ('invalid dimensions', box, 1000, 0, true, box), + ('long final preview', box, 1000, 30000, true, Size(16384 / 30, 16384)), + ('long first preview', box, 30000, 1000, false, Size(2556, 2556 / 30)), + ('ultra-thin preview', box, 10, 50000, false, Size(1, 2556)), + ('small source', box, 50, 2000, true, Size(50, 2000)), + ('at limit', Size(16384, 100), 1000, 1000, true, Size(16384, 100)), + ('over limit', Size(16385, 100), 1000, 1000, true, Size(1000, 1000)), + ]; + + for (final (name, box, width, height, previewIsFinal, expected) in cases) { + test(name, () { + final actual = LocalFullImageProvider.previewTargetSize( + box.width, + box.height, + width, + height, + previewIsFinal: previewIsFinal, + ); + expect(actual.width, closeTo(expected.width, 1e-6)); + expect(actual.height, closeTo(expected.height, 1e-6)); + }); + } + }); + + group('LocalFullImageProvider equality', () { + LocalFullImageProvider make({int? width, int? height}) => LocalFullImageProvider( + id: 'a', + assetType: AssetType.image, + size: const Size(100, 200), + isAnimated: false, + width: width, + height: height, + ); + + test('uses dimensions in the cache key', () { + final a = make(width: 100, height: 200); + final b = make(width: 100, height: 200); + expect(a, b); + expect(a.hashCode, b.hashCode); + expect(a == make(width: 200, height: 100), isFalse); + }); + }); + group('LocalThumbProvider caching', () { test('editing on device re-renders the thumbnail', () { cache.putIfAbsent(LocalThumbProvider(id: 'asset-1', assetType: AssetType.image, checksum: 'before'), load); @@ -58,7 +106,7 @@ void main() { test('thumbnails are keyed by the asset checksum', () { final asset = LocalAssetFactory.create().copyWith(checksum: 'abc'); - final provider = getThumbnailImageProvider(asset) as LocalThumbProvider; + final provider = getThumbnailImageProvider(asset)! as LocalThumbProvider; expect(provider.checksum, 'abc'); });