[mob][photos] Use file-derived dimensions for large images#10393
Open
AmanRajSinghMourya wants to merge 2 commits into
Open
[mob][photos] Use file-derived dimensions for large images#10393AmanRajSinghMourya wants to merge 2 commits into
AmanRajSinghMourya wants to merge 2 commits into
Conversation
Member
|
Let's also test for this #10429 |
Member
|
Whenever we resume work on this, we should also think about how (in the future), we can handle just rotation edit, without re-uploading or saving duplicate for file. We don't have to make that change right away, but we should have clarity about the way forward. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
Some Samsung Galaxy S24/S25 Ultra 200MP portrait photos can render squished in the mobile Photos viewer.
The viewer already avoids full-resolution decode for very large images to prevent Flutter OOMs. That path passes
cacheWidthandcacheHeighttoImage.file. For affected portrait images, the storedw/hmetadata can be wrong or missing, so the viewer computes a landscape-shaped decode target for a portrait image.Observed cases:
16320x12240instead of display12240x16320.0x0.0x0bypasses the large-image guard and can attempt a full-resolution decode.Root Cause
Android MediaStore/photo manager dimensions are not always a reliable source of display dimensions for rotated media. On affected Samsung files, MediaStore can already report display-oriented dimensions, and then the Android upload path applies another EXIF-orientation swap.
For normal images this usually goes unnoticed. For 200MP images, the viewer takes the large-image path and uses stored dimensions to compute
cacheWidth/cacheHeight. If those dimensions are wrong, Flutter decodes/scales with the wrong aspect ratio and the image appears squished.Fix
This adds an internal-only path to derive image dimensions from the file itself instead of relying only on stored MediaStore dimensions.
Behind
useFileDerivedImageDimensions:w/h.rwrhrotHow It Works
For new uploads, the upload path now prefers EXIF/file-derived dimensions when the internal flag is enabled.
Example Samsung 200MP portrait:
The uploaded public metadata becomes:
{ "w": 12240, "h": 16320, "rw": 16320, "rh": 12240, "rot": 90 }If EXIF raw dimension tags are unavailable, we fall back to the previous asset dimensions. On Android, if EXIF orientation is still available, we apply the same orientation swap behavior to the fallback dimensions so the existing behavior does not regress.
For existing files, the viewer resolves dimensions in this order:
w/h.For large images, the viewer computes
cacheWidth/cacheHeightfrom the resolved display dimensions, so portrait images stay portrait-shaped during reduced-resolution decode.Backfill is intentionally conservative:
Shared files are not mutated by viewers.
Tests
flutter test test/utils/image_dimension_util_test.dart test/models/metadata/file_magic_test.dartflutter analyzegit diff --checkNote: the clean PR branch is based on
main, which expects Flutter 3.32.8. Local validation was done on the Flutter-up branch where the current local Flutter SDK resolves.References