fix: honor EXIF orientation when decoding caller-supplied images - #184
Merged
Conversation
hm21
marked this pull request as ready for review
July 30, 2026 16:22
A `ChromaKey.backgroundImage` or an image `VideoLayer` given a photo straight from the gallery rendered sideways. A phone stores a portrait shot as landscape pixels plus an EXIF `Orientation` tag, and the decoders on the render path dropped that tag — so the same input came out upright on macOS (`NSImage` bakes the tag in) but rotated 90° on Android (`BitmapFactory`) and iOS (`UIImage` parses the tag, `.cgImage` then hands back the stored pixels). That cross-platform disagreement was the core defect; the contract is now that an encoded image renders the way the user sees it in their photo library. - Android: extract `StopMotionGenerator`'s existing orientation handling into a shared `ImageOrientation` helper and decode the chroma-key background and image layers through it. `probe()` returns the orientation-corrected bounds plus the orientation, which is what `ChromaKeyEffect`'s header-only probe needs to size its background texture. `read()` normalizes anything outside the eight defined values — including the `UNDEFINED` a tagless file reports — to `NORMAL`, so callers get a definite orientation. - Darwin: add `decodeOrientedImage` and use it for both sites. It decodes via `CGImageSource`, which returns the stored pixels on iOS and macOS alike, and applies the tag itself, so the `#if os()` split disappears instead of being patched per half. The extent origin is renormalized to zero, which the overlay positioning already assumes. - Swap `android.media.ExifInterface` for `androidx.exifinterface`: it covers HEIF/AVIF/WebP as well as JPEG, and being plain Java it is reachable from the JVM unit tests, where the framework class is stubbed to return 0. Tests build their fixtures at runtime, because the usual encoders bake the orientation into the pixels and drop the tag: a minimal APP1 EXIF segment is spliced into a JPEG by hand instead. The Swift tests use a four-colour quadrant image so they pin which way the pixels turned, not just that the extent swapped, and both platforms have a counter-test that an untagged image is left alone — otherwise "rotate everything" would satisfy the first assertion. Verified non-vacuous: neutering the Darwin helper fails exactly the two orientation tests while both counter-tests keep passing. The Android unit tests run on the JVM with no Robolectric, where `BitmapFactory` returns null, so `ImageOrientationTest` drives `probeOf` — the seam `probe` hands its header-decoded size to. It covers the real EXIF parse and the size contract; the pixel decode itself is covered by the Swift tests and the example integration tests.
Decode image layers no larger than `prepareOverlay` is about to scale them to. A full-resolution decode of a phone photo cost a second full-resolution copy once the EXIF orientation had to be turned, which is where an overlay runs out of memory — and the result was scaled down immediately afterwards anyway. `overlayDecodeSize` mirrors `prepareOverlay`'s own branching so a positioned layer, which is laid out from its own pixel dimensions, still keeps its full resolution. `ImageOrientation.decode` now maps the requested size back into stored space before choosing `inSampleSize`, which measures the stored pixels; on a rotated image the two were compared across mismatched axes and settled one step too conservative. Darwin's stop-motion decode applies the orientation on its fallback too. `CGImageSourceCreateImageAtIndex` returns the stored pixels with the tag dropped, so a failed thumbnail decode put a portrait frame back on its side — the one path left contradicting the contract this branch establishes. Also: `apply` renamed to `orient`, since it took ownership of its argument and read as Kotlin's scope function at the call site; the two identical "failed to decode" log lines separated; `DEFINED_ORIENTATIONS` unboxed; the CHANGELOG entry cut to the user-facing effect; exifinterface to 1.4.2. Tests: `OverlayDecodeSizeTest` pins the decode-size branching against `prepareOverlay`, and a grayscale fixture covers the one case `NSImage` used to hide on macOS — `CGImageSource` hands back the file's own color space, and the render path's `CIContext` does no color management. Android 24 pass, macOS 27, iOS 15, flutter analyze/format/test clean.
hm21
force-pushed
the
fix/exif-orientation
branch
from
July 30, 2026 16:33
01affc1 to
56662cb
Compare
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
1 participant
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.
The bug
A
ChromaKey.backgroundImageor an imageVideoLayergiven a photo straight from a phone's gallery renders sideways. A portrait photo is stored as landscape pixels plus an EXIFOrientationtag, and the render path's decoders drop that tag.Reported against a real app: a portrait JPEG used as a chroma-key background came out rotated 90° in the exported video.
Two surfaces were affected, and they disagreed per platform — which was the core defect:
BitmapFactory, no EXIFUIImage(data:)parses the tag,.cgImagediscards itNSImageapplies itBitmapFactory, no EXIF.cgImagedropSo the same input rendered upright on macOS and rotated on Android and iOS. The contract is now uniform: an encoded image renders the way the user sees it in their photo library. Callers hand over encoded bytes and can't be expected to pre-normalise them for one backend's decoder.
The fix
Android —
StopMotionGeneratorhad already solved this, so itsdecodeOriented/orientedBounds/readExifOrientation/applyOrientationmoved into a sharedImageOrientationhelper rather than getting a second implementation.probe()returns the orientation-corrected bounds plus the orientation, which is whatChromaKeyEffect's header-only probe needs to size its background texture (and it saves a second EXIF read at decode time).read()normalizes anything outside the eight defined values — including theUNDEFINEDa tagless file reports — toNORMAL, so callers get a definite orientation instead of a value they have to range-check.Darwin —
decodeOrientedImagedecodes viaCGImageSource, which returns the stored pixels on both platforms, then applies the tag itself. That removes the#if os()split rather than patching each half, and macOS no longer depends onNSImagehappening to bake the tag in. It renormalizes the extent origin to zero, which the overlay positioning atVideoCompositor.swift:828already assumes.Dependency —
androidx.exifinterfacereplacesandroid.media.ExifInterface: it covers HEIF/AVIF/WebP as well as JPEG (increasingly what a gallery hands over), and being plain Java it is reachable from the JVM unit tests, where the framework class is stubbed to return0.Out of scope and untouched: the chroma-key maths, the composition layout, the public API. GIF layers are unaffected — GIF carries no EXIF. Darwin's stop-motion path was already correct via
kCGImageSourceCreateThumbnailWithTransform.Tests
Fixtures are built at runtime, because the usual encoders bake the orientation into the pixels and drop the tag — a "landscape pixels +
Orientation=6" image can't be produced by round-tripping one. A minimal APP1 EXIF segment is spliced in by hand (TIFF header, one IFD entry, tag0x0112, type SHORT, value 6). On Swift the splice goes after any leading JFIF APP0 so the file stays well-formed, and a guard test asserts the fixture really declares orientation 6 — without it, a no-op splice would make every counter-test pass for the wrong reason.The Swift tests use a four-colour quadrant image, so they pin which way the pixels turned (top-left → top-right for orientation 6), not just that the extent swapped. Each platform has a counter-test that an untagged image is left alone — otherwise "rotate everything" would satisfy the first assertion.
Verified non-vacuous: neutering
decodeOrientedImagefails exactlytestExifOrientationSwapsTheDecodedDimensionsandtestExifOrientationRotatesThePixelsAndNotJustTheExtent, while both untagged counter-tests keep passing.ImageOrientationTest— 7 cases, passflutter analyzeclean on the plugin and the exampleCoverage limit worth knowing: the Android unit tests run on the JVM with no Robolectric, where
BitmapFactoryreturns null for everything. SoImageOrientationTestdrivesprobeOf— the seamprobehands its header-decoded size to. It covers the real EXIF parse and the orientation→size contract, but notBitmapFactory+Bitmap.createBitmap(matrix)themselves; that link is exercised by the Swift tests' equivalents and the example integration tests. Adding Robolectric would need the JUnit vintage engine on top of this module'suseJUnitPlatform(), which is a larger build change than a FIX warrants.Unrelated friction hit while verifying
example/*/Flutter/ephemeral/.../Package.swiftis regenerated with a stale deployment target whenever the other Darwin platform is built, so aflutter build <platform>is needed beforexcodebuild test.fvppackage intermittently fails to link (passed on retry, twice).