From d8e1ead6f4b53c0f62f2d42984cdebbcb09739cf Mon Sep 17 00:00:00 2001 From: Santo Shakil Date: Wed, 5 Aug 2026 00:37:49 +0600 Subject: [PATCH] fix(mobile): stale local renders after editing a photo on device (#30415) * fix(mobile): key local image caches by content so on-device edits re-render * use Object.hash and the local asset factory --- .../widgets/images/image_provider.dart | 10 +- .../widgets/images/local_image_provider.dart | 26 ++++-- .../images/local_image_provider_test.dart | 93 +++++++++++++++++++ 3 files changed, 119 insertions(+), 10 deletions(-) create mode 100644 mobile/test/presentation/widgets/images/local_image_provider_test.dart diff --git a/mobile/lib/presentation/widgets/images/image_provider.dart b/mobile/lib/presentation/widgets/images/image_provider.dart index 927734ca25..a2d7345b16 100644 --- a/mobile/lib/presentation/widgets/images/image_provider.dart +++ b/mobile/lib/presentation/widgets/images/image_provider.dart @@ -162,7 +162,13 @@ ImageProvider getFullImageProvider( provider = FileImage(File(localFilePath)); } else if (_shouldUseLocalAsset(asset)) { final id = asset is LocalAsset ? asset.id : (asset as RemoteAsset).localId!; - provider = LocalFullImageProvider(id: id, size: size, assetType: asset.type, isAnimated: asset.isAnimatedImage); + provider = LocalFullImageProvider( + id: id, + size: size, + assetType: asset.type, + isAnimated: asset.isAnimatedImage, + checksum: asset.checksum, + ); } else { final String assetId; final String thumbhash; @@ -190,7 +196,7 @@ ImageProvider getFullImageProvider( ImageProvider? getThumbnailImageProvider(BaseAsset asset, {Size size = kThumbnailResolution, 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); + return LocalThumbProvider(id: id, size: size, assetType: asset.type, checksum: asset.checksum); } final assetId = asset is RemoteAsset ? asset.id : (asset as LocalAsset).remoteId; diff --git a/mobile/lib/presentation/widgets/images/local_image_provider.dart b/mobile/lib/presentation/widgets/images/local_image_provider.dart index eba4f0a1cd..8a88e915ff 100644 --- a/mobile/lib/presentation/widgets/images/local_image_provider.dart +++ b/mobile/lib/presentation/widgets/images/local_image_provider.dart @@ -14,7 +14,10 @@ class LocalThumbProvider extends CancellableImageProvider final Size size; final AssetType assetType; - LocalThumbProvider({required this.id, required this.assetType, this.size = kThumbnailResolution}); + // an edit on the device keeps the id and changes the bytes, so the checksum is what separates two renders + final String? checksum; + + LocalThumbProvider({required this.id, required this.assetType, this.checksum, this.size = kThumbnailResolution}); @override Future obtainKey(ImageConfiguration configuration) { @@ -44,13 +47,13 @@ class LocalThumbProvider extends CancellableImageProvider return true; } if (other is LocalThumbProvider) { - return id == other.id; + return id == other.id && checksum == other.checksum; } return false; } @override - int get hashCode => id.hashCode; + int get hashCode => Object.hash(id, checksum); } class LocalFullImageProvider extends CancellableImageProvider @@ -59,8 +62,15 @@ class LocalFullImageProvider extends CancellableImageProvider obtainKey(ImageConfiguration configuration) { @@ -73,7 +83,7 @@ class LocalFullImageProvider extends CancellableImageProvider [ DiagnosticsProperty('Image provider', this), DiagnosticsProperty('Id', key.id), @@ -86,7 +96,7 @@ class LocalFullImageProvider extends CancellableImageProvider [ DiagnosticsProperty('Image provider', this), DiagnosticsProperty('Id', key.id), @@ -163,11 +173,11 @@ class LocalFullImageProvider extends CancellableImageProvider id.hashCode ^ size.hashCode ^ isAnimated.hashCode; + int get hashCode => Object.hash(id, size, isAnimated, 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 new file mode 100644 index 0000000000..ce9b15af9e --- /dev/null +++ b/mobile/test/presentation/widgets/images/local_image_provider_test.dart @@ -0,0 +1,93 @@ +import 'package:flutter/painting.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:immich_mobile/domain/models/asset/base_asset.model.dart'; +import 'package:immich_mobile/presentation/widgets/images/image_provider.dart'; +import 'package:immich_mobile/presentation/widgets/images/local_image_provider.dart'; + +import '../../../unit/factories/local_asset_factory.dart'; + +class _StubCompleter extends ImageStreamCompleter {} + +void main() { + late ImageCache cache; + late int loads; + + ImageStreamCompleter load() { + loads++; + return _StubCompleter(); + } + + setUp(() { + cache = ImageCache(); + loads = 0; + }); + + group('LocalThumbProvider caching', () { + test('editing on device re-renders the thumbnail', () { + cache.putIfAbsent(LocalThumbProvider(id: 'asset-1', assetType: AssetType.image, checksum: 'before'), load); + cache.putIfAbsent(LocalThumbProvider(id: 'asset-1', assetType: AssetType.image, checksum: 'after'), load); + + expect(loads, 2); + }); + + test('an unchanged thumbnail still comes from the cache', () { + cache.putIfAbsent(LocalThumbProvider(id: 'asset-1', assetType: AssetType.image, checksum: 'same'), load); + cache.putIfAbsent(LocalThumbProvider(id: 'asset-1', assetType: AssetType.image, checksum: 'same'), load); + + expect(loads, 1); + }); + + // The rehash clears the checksum before writing the new one, so the tile has to + // follow that step too or it waits for the hash to land before showing the edit. + test('re-renders while the checksum is still being recomputed', () { + cache.putIfAbsent(LocalThumbProvider(id: 'asset-1', assetType: AssetType.image, checksum: 'before'), load); + cache.putIfAbsent(LocalThumbProvider(id: 'asset-1', assetType: AssetType.image), load); + + expect(loads, 2); + }); + + test('stays cached while the checksum is missing', () { + cache.putIfAbsent(LocalThumbProvider(id: 'asset-1', assetType: AssetType.image), load); + cache.putIfAbsent(LocalThumbProvider(id: 'asset-1', assetType: AssetType.image), load); + + expect(loads, 1); + }); + }); + + group('factories', () { + test('thumbnails are keyed by the asset checksum', () { + final asset = LocalAssetFactory.create().copyWith(checksum: 'abc'); + + final provider = getThumbnailImageProvider(asset) as LocalThumbProvider; + + expect(provider.checksum, 'abc'); + }); + }); + + group('LocalFullImageProvider caching', () { + test('editing on device re-renders the full image', () { + cache.putIfAbsent( + LocalFullImageProvider( + id: 'asset-1', + assetType: AssetType.image, + size: const Size(100, 100), + isAnimated: false, + checksum: 'before', + ), + load, + ); + cache.putIfAbsent( + LocalFullImageProvider( + id: 'asset-1', + assetType: AssetType.image, + size: const Size(100, 100), + isAnimated: false, + checksum: 'after', + ), + load, + ); + + expect(loads, 2); + }); + }); +}