fix(ios+android): crop/resize/rotate correctness (5 bugs)#117
Open
rayabelcode wants to merge 2 commits intomrousavy:mainfrom
Open
fix(ios+android): crop/resize/rotate correctness (5 bugs)#117rayabelcode wants to merge 2 commits intomrousavy:mainfrom
rayabelcode wants to merge 2 commits intomrousavy:mainfrom
Conversation
Four related bugs in iOS caused Image.crop() / Image.resize() (and, by extension, Image.rotate() slow path and ImageFactory.createBlankImage) to produce incorrect output. They surface most visibly through the vision-camera v5 flow Photo.toImageAsync() -> crop -> resize -> saveToTemporaryFileAsync(), but repro from any source image. ios/Public/NativeImage.swift: 1. crop: the targetRect used CGSize(uiImage.size.width, uiImage.size.height) instead of the computed (targetWidth, targetHeight). CGImage.cropping intersects the rect with the image bounds, so callers got "image minus origin offset" instead of an actual crop of the requested region. 2. crop: the pre-crop UIImage could have non-.up imageOrientation and/or scale != 1 (both common for the Photo.toImageAsync() bridge, which constructs UIImage(cgImage:, scale: 1, orientation: uiOrientation)). uiImage.size is display-space while cgImage is sensor-space, so the caller's rect landed in the wrong region of cgImage. UIImage(cgImage:) also defaulted the output to .up orientation, dropping the source's orientation and producing sensor-oriented pixels. Fix: if orientation != .up or scale != 1, draw through a scale=1 UIGraphicsImageRenderer to bake both into pixels before cropping, then wrap the cropped CGImage with explicit scale=1 and .up so downstream consumers see consistent state. 3. resize + rotate (slow path): UIGraphicsImageRenderer(size:) without a UIGraphicsImageRendererFormat defaults to UIScreen.main.scale. On 2x/3x devices "resize to 2000x1500" produced 4000x3000 or 6000x4500-pixel output, while uiImage.size still reported (2000, 1500) in points. JS callers saw correct Image.width/height but the saved JPEG was 2-3x too large in each dimension. Fix: set UIGraphicsImageRendererFormat().scale = 1 before constructing the renderer. ios/HybridImageFactory.swift: 4. createBlankImage had the same default-scale issue; applied the same scale = 1 fix. Android (HybridImage.kt) is unchanged -- its Bitmap-based path has no point/pixel distinction and was already correct. Verified on an iPhone 15 Pro (3x screen scale) via a vision-camera v5 photo-scanning app. Before: cropped region was wrong, orientation dropped to landscape sensor-space, saved files 3x larger than requested. After: crop produces the requested region in portrait orientation, and saved files match the requested pixel dimensions.
64b9896 to
43a7280
Compare
The rotate slow path on iOS (NativeImage.swift) and Android (HybridImage.kt) sized the output canvas to the input image's dimensions. When degrees is not a multiple of 180 (e.g., a 90 degree rotation of a portrait image into a landscape), the rotated content extends beyond the canvas and gets clipped -- the caller sees an image at the input dimensions with half the rotated content missing. Fix: compute the rotated bounding box as outputW = abs(cos(r)) * w + abs(sin(r)) * h outputH = abs(sin(r)) * w + abs(cos(r)) * h Size the renderer / destination bitmap to that, then translate so the rotated content lands centered on the output canvas. Verified end-to-end on iPhone 15 Pro (3x) and a Pixel-class Android device via a vision-camera v5 photo-scanning app: rotating a portrait card 90 degrees now produces a correctly sized landscape JPEG with complete content, rather than a portrait JPEG with a clipped landscape rectangle inside. Works for any angle (the cos/sin formula gives a tight bounding box for arbitrary rotations), but the bug is most visible at 90/270 where the canvas is transposed from the content.
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.
Summary
Fixes five related bugs that make
Image.crop(),Image.resize(),Image.rotate(), andImageFactory.createBlankImage()produce incorrect output. They compound - chaining crop + rotate + resize (a common flow, including vision-camera v5's advertisedPhoto.toImageAsync() -> crop -> resize -> saveToTemporaryFileAsync()) gets the wrong region, the wrong orientation, pixel-inflated files, AND clipped rotation output.Bugs 1-4 are iOS-only. Bug 5 affects both iOS and Android.
Commits in this PR:
fix(ios): correct crop rect size, orientation, and renderer scale- bugs 1-4fix: rotate slow path clipped content by keeping canvas at input size- bug 5 (both platforms)The bugs
All iOS fixes are in
packages/react-native-nitro-image/ios/Public/NativeImage.swift(plus one inHybridImageFactory.swift). The Android fix is inpackages/react-native-nitro-image/android/src/main/java/com/margelo/nitro/image/HybridImage.kt.1.
crop- rect size used full image dims instead of the crop size (iOS)CGImage.cropping(to:)intersects the rect with the image bounds, so a full-image-sized rect returned "image minus origin offset" rather than a crop of(targetWidth, targetHeight).2.
crop- orientation / scale mismatch between rect and cgImage (iOS)Two linked issues:
uiImage.imageOrientation != .up(common for images from vision-camera v5'sPhoto.toImageAsync(), which constructsUIImage(cgImage:, scale: 1, orientation: uiOrientation)),uiImage.sizeis display-space butuiImage.cgImageis sensor-space. The caller's(startX, startY, endX, endY)rect is in display-space, socgImage.cropping(to:)landed in the wrong region of the bitmap.UIImage(cgImage: croppedCgImage)defaulted the output to.up, dropping the source's orientation and surfacing raw sensor-oriented pixels to callers who expected display-space output.3.
resize- device-scale-factor pixel inflation (iOS)UIGraphicsImageRenderer(size:)defaults toUIScreen.main.scale(2 on standard iPhones, 3 on Pro). Soresize(2000, 1500)on a 3x device produced a 6000x4500-pixel bitmap.uiImage.sizestill returned(2000, 1500)in points, so JS saw correctImage.width/Image.height- butsaveToTemporaryFileAsync/toEncodedImageDataencode at the CGImage's actual pixel dimensions, so saved files were 3x too large in each dimension.4. Same default-scale issue in
rotate()slow path andcreateBlankImage(iOS)Both use
UIGraphicsImageRenderer(size:)/UIGraphicsImageRendererFormat()without setting scale.5.
rotate()slow path - output canvas sized to input, clips rotated content (iOS + Android)Both platforms' slow-path rotate used the input's dimensions as the output buffer size:
NativeImage.swift):UIGraphicsImageRenderer(size: uiImage.size, ...)- canvas stays at input size.HybridImage.kt):createBitmap(bitmap.width, bitmap.height)- destination bitmap stays at input size.When
degreesis not a multiple of 180 (e.g., a portrait 1500x2000 image rotated -90 degrees), the rotated content wants a 2000x1500 canvas but gets a 1500x2000 one.cgImage.cropping(to:)/Canvas.drawBitmapclip the extents: the caller receives an image at the input dimensions with half the rotated content missing. On the JS side,Image.width/Image.heightstill report the (unchanged) input dims, so the bug is invisible from metadata - but the visible content is wrong.Symptom observed in a production vision-camera v5 app: "image is cropped to the right size but rotated 90 degrees so you only see part of it" - exactly the canvas-clipping fingerprint.
The fixes
crop- normalize input, use actual crop size, wrap output with explicit scale+orientation (iOS)resize/rotateslow path /createBlankImage- force scale=1 (iOS)rotateslow path - size canvas to rotated bounding box (iOS + Android)Compute the rotated bounding box via
|cos|*w + |sin|*hand|sin|*w + |cos|*h. Size the renderer / destination bitmap to that. Translate so the rotated content lands centered on the output canvas.iOS:
Android:
Works for any angle (the cos/sin formula gives a tight bounding box for arbitrary rotations), but the bug is most visible at 90/270 where the canvas transposes from the content.
Reproduction
Test plan
node_modules/react-native-nitro-imagein an Expo SDK 55 app (vision-camera v5.0.1, nitro-modules 0.35.x)