callstack / react-native-image-editor

A library providing an API for cropping images from the web and the local file system.
MIT License
383 stars 118 forks source link

Incorrect scaling on Android #150

Closed jthure closed 8 months ago

jthure commented 8 months ago

Environment

Android

Description

I'm experiencing an issue when trying to scale a large image down to a lot smaller size. No cropping involved, although I don't think that matter.

If I'm not misunderstanding how things work, the code related to sample size in cropAndResizeTask is incorrect. outOptions.inSampleSize is calculated and set, and then the parameters to Bitmap.createBitmap are modified based on the resulting value, but the bitmap is never decoded, from which the new resulting is created from, is never decoded using the options object and specifically the inSampleSize. Therefore, if getDecodeSampleSize returns a sampleSize greater than 1, the parameters to Bitmap.createBitmap is downscaled, which results in a undesired "cropping" of the resulting bitmap

Reproducible Demo

Try to scale down (no cropping) an image to such an target size that the getDecodeSampleSize method returns a value greater than 1.

jthure commented 8 months ago

Again if I'm not completely understanding whats happening, it looks like there was some thought about doing the bitmap management more efficient, but it was not completed. Removing the calculations based on inSampleSize fixes the issue

diff --git a/node_modules/@react-native-community/image-editor/android/src/main/java/com/reactnativecommunity/imageeditor/ImageEditorModuleImpl.kt b/node_modules/@react-native-community/image-editor/android/src/main/java/com/reactnativecommunity/imageeditor/ImageEditorModuleImpl.kt
index ff78c15..435b154 100644
--- a/node_modules/@react-native-community/image-editor/android/src/main/java/com/reactnativecommunity/imageeditor/ImageEditorModuleImpl.kt
+++ b/node_modules/@react-native-community/image-editor/android/src/main/java/com/reactnativecommunity/imageeditor/ImageEditorModuleImpl.kt
@@ -307,12 +307,11 @@ class ImageEditorModuleImpl(private val reactContext: ReactApplicationContext) {
         // Is there a way to just continue reading from the stream?
         outOptions.inSampleSize = getDecodeSampleSize(width, height, targetWidth, targetHeight)

-        val cropX = (newX / outOptions.inSampleSize.toFloat()).roundToInt()
-        val cropY = (newY / outOptions.inSampleSize.toFloat()).roundToInt()
-        val cropWidth = (newWidth / outOptions.inSampleSize.toFloat()).roundToInt()
-        val cropHeight = (newHeight / outOptions.inSampleSize.toFloat()).roundToInt()
-        val cropScale = scale * outOptions.inSampleSize
-        val scaleMatrix = Matrix().apply { setScale(cropScale, cropScale) }
+        val cropX = newX.roundToInt()
+        val cropY = newY.roundToInt()
+        val cropWidth = newWidth.roundToInt()
+        val cropHeight = newHeight.roundToInt()
+        val scaleMatrix = Matrix().apply { setScale(scale, scale) }
         val filter = true

         return Bitmap.createBitmap(bitmap, cropX, cropY, cropWidth, cropHeight, scaleMatrix, filter)
retyui commented 8 months ago

can create a simple demo to demonstrate the issue

or share your params that you used to scale down an image

jthure commented 8 months ago

Here's a simple demo which shows the issue. Interestingly, I'm also seeing an issue with rotation in the case of not doing any scaling

retyui commented 8 months ago

@jthure Thanks so much!

jthure commented 8 months ago

Just pushed another commit. The orientation problem with the middle picture was due to an error in the demo code. But the scaling issue remains

jthure commented 8 months ago

@retyui I opened a PR #151. From some light testing it seems to fix the issue, but would need a review to make sure it makes sense. The PR also adds a "scale" slider to the example app, which can be used to reproduce the issue in the current state