[Bitmap Utils@findCropPosition()] Fix: Must not calculate x or y as m…

…inus
guhungry · Jun 27, 2024 · 88130f5 · 88130f5 · guhungry · Jun 27, 2024
1 parent 44f392f
commit 88130f5
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 4 deletions.
diff --git a/photomanipulator/src/main/java/com/guhungry/photomanipulator/BitmapUtils.kt b/photomanipulator/src/main/java/com/guhungry/photomanipulator/BitmapUtils.kt
@@ -7,6 +7,7 @@ import com.guhungry.photomanipulator.factory.AndroidFactory
 import com.guhungry.photomanipulator.factory.AndroidConcreteFactory
 import java.io.IOException
 import java.io.InputStream
+import kotlin.math.floor
 
 object BitmapUtils {
     @JvmStatic
@@ -97,7 +98,12 @@ object BitmapUtils {
     /**
      * Find Crop Position result as Resize Mode = Cover
      */
-    private fun findCropPosition(rect: CGRect, targetSize: CGSize, sampleSize: Int): CGRect {
+    internal fun findCropPosition(
+        rect: CGRect,
+        targetSize: CGSize,
+        sampleSize: Int,
+        factory: AndroidFactory = AndroidConcreteFactory()
+    ): CGRect {
         val newWidth: Float
         val newHeight: Float
         val newX: Float
@@ -106,18 +112,18 @@ object BitmapUtils {
         val targetRatio = targetSize.ratio()
 
         if (cropRectRatio > targetRatio) { // e.g. source is landscape, target is portrait
-            newWidth = rect.size.height * targetRatio
+            newWidth = floor(rect.size.height * targetRatio)
             newHeight = rect.size.height.toFloat()
             newX = rect.origin.x + (rect.size.width - newWidth) / 2
             newY = rect.origin.y.toFloat()
         } else { // e.g. source is landscape, target is portrait
             newWidth = rect.size.width.toFloat()
-            newHeight = rect.size.width / targetRatio
+            newHeight = floor(rect.size.width / targetRatio)
             newX = rect.origin.x.toFloat()
             newY = rect.origin.y + (rect.size.height - newHeight) / 2
         }
 
-        return CGRect(applyScale(newX, sampleSize), applyScale(newY, sampleSize), applyScale(newWidth, sampleSize), applyScale(newHeight, sampleSize))
+        return CGRect(applyScale(newX, sampleSize), applyScale(newY, sampleSize), applyScale(newWidth, sampleSize), applyScale(newHeight, sampleSize), factory)
     }
     private fun applyScale(value: Float, sampleSize: Int) = kotlin.math.floor(value / sampleSize).toInt()
 

diff --git a/photomanipulator/src/test/java/com/guhungry/photomanipulator/BitmapUtilsTest.kt b/photomanipulator/src/test/java/com/guhungry/photomanipulator/BitmapUtilsTest.kt
@@ -2,6 +2,7 @@ package com.guhungry.photomanipulator
 
 import android.graphics.*
 import com.guhungry.photomanipulator.factory.AndroidFactory
+import org.hamcrest.CoreMatchers.equalTo
 import org.hamcrest.CoreMatchers.sameInstance
 import org.hamcrest.MatcherAssert.assertThat
 import org.junit.Test
@@ -119,4 +120,32 @@ internal class BitmapUtilsTest {
 
         assertThat(actual, sameInstance(background))
     }
+
+    /**
+     * Error due to calculate origin y = -1 instead of 0
+     * When
+     * cropRegion: {"x":0,"y":0,"height":4025,"width":3060}
+     * targetSize: {"height":4025,"width":3060}
+     * https://github.com/guhungry/react-native-photo-manipulator/issues/837
+     */
+    @Test
+    fun `findCropPosition when issue 837 should return correctly`() {
+        val factory = mock(AndroidFactory::class.java)
+        `when`(factory.makePoint(anyInt(), anyInt())).thenAnswer {
+            mock(Point::class.java).apply {
+                x = it.arguments[0] as Int
+                y = it.arguments[1] as Int
+            }
+        }
+        var expected = CGRect(0, 0, 3060, 4025, factory)
+        val cropRegion = CGRect(0, 0, 3060, 4025, factory)
+        val targetSize = CGSize(3060, 4025)
+        val sampleSize = 1
+
+        val actual = BitmapUtils.findCropPosition(cropRegion, targetSize, sampleSize, factory)
+
+        assertThat(actual.size, equalTo(expected.size))
+        assertThat(actual.origin.x, equalTo(expected.origin.x))
+        assertThat(actual.origin.y, equalTo(expected.origin.y))
+    }
 }