commonize ColorTest.kt and DataTest.kt (#255)

* commonize ColorTest.kt and DataTest.kt

* Add check for offset+length when reading Data bytes. Add test `failsToReadMoreBytesThanPossible`
Co-authored-by: Oleksandr Karpovich <oleksandr.karpovich@jetbrains.com>

skiko/src/commonMain/cpp/generated/Data.cc

@@ -24,8 +24,8 @@ SKIKO_EXPORT KInteropPointer org_jetbrains_skia_Data__1nToByteBuffer
   (KNativePointer ptr) {
     TODO("implement org_jetbrains_skia_Data__1nToByteBuffer");
 }
-     
-#if 0 
+
+#if 0
 SKIKO_EXPORT KInteropPointer org_jetbrains_skia_Data__1nToByteBuffer
   (KNativePointer ptr) {
     SkData* instance = reinterpret_cast<SkData*>((ptr));
@@ -35,9 +35,11 @@ SKIKO_EXPORT KInteropPointer org_jetbrains_skia_Data__1nToByteBuffer
 
 
 
-SKIKO_EXPORT KByte* org_jetbrains_skia_Data__1nBytes
-  (KNativePointer ptr, KNativePointer offset, KNativePointer length) {
-    TODO("implement org_jetbrains_skia_Data__1nBytes");
+SKIKO_EXPORT void org_jetbrains_skia_Data__1nBytes
+  (KNativePointer ptr, int offset, int length, KByte* destBytes) {
+    SkData* instance = reinterpret_cast<SkData*>(ptr);
+    const KByte* source = reinterpret_cast<const KByte*>(instance->bytes() + offset);
+    memcpy(destBytes, source, length);
 }
 
 SKIKO_EXPORT KBoolean org_jetbrains_skia_Data__1nEquals
@@ -71,4 +73,4 @@ SKIKO_EXPORT KNativePointer org_jetbrains_skia_Data__1nMakeSubset
 SKIKO_EXPORT KNativePointer org_jetbrains_skia_Data__1nMakeEmpty() {
     SkData* instance = SkData::MakeEmpty().release();
     return reinterpret_cast<KNativePointer>(instance);
-}
\ No newline at end of file
+}

skiko/src/commonMain/kotlin/org/jetbrains/skia/Data.kt

@@ -40,7 +40,7 @@ class Data internal constructor(ptr: NativePointer) : Managed(ptr, _FinalizerHol
         }
     }
 
-    val size: Long
+    val size: Int
         get() = try {
             Stats.onNativeCall()
             _nSize(_ptr)
@@ -50,15 +50,25 @@ class Data internal constructor(ptr: NativePointer) : Managed(ptr, _FinalizerHol
     val bytes: ByteArray
         get() = getBytes(0, size)
 
-    fun getBytes(offset: Long, length: Long): ByteArray {
+    fun getBytes(offset: Int, length: Int): ByteArray {
         return try {
             Stats.onNativeCall()
-            _nBytes(_ptr, offset, length)
+            check(_nSize(_ptr) >= offset + length) {
+                "Data=${_ptr}: Can't getBytes with offset=$offset and length=$length"
+            }
+            withResult(ByteArray(length)) {
+                _nBytes(_ptr, offset, length, it)
+            }
         } finally {
             reachabilityBarrier(this)
         }
     }
 
+    override fun equals(other: Any?): Boolean {
+        val otherData = other as? Data ?: return false
+        return _nativeEquals(otherData)
+    }
+
     /**
      * Returns true if these two objects have the same length and contents,
      * effectively returning 0 == memcmp(...)
@@ -77,7 +87,7 @@ class Data internal constructor(ptr: NativePointer) : Managed(ptr, _FinalizerHol
      * Create a new dataref using a subset of the data in the specified
      * src dataref.
      */
-    fun makeSubset(offset: Long, length: Long): Data {
+    fun makeSubset(offset: Int, length: Int): Data {
         return try {
             Stats.onNativeCall()
             Data(_nMakeSubset(_ptr, offset, length))
@@ -114,10 +124,10 @@ class Data internal constructor(ptr: NativePointer) : Managed(ptr, _FinalizerHol
 private external fun Data_nGetFinalizer(): NativePointer
 
 @ExternalSymbolName("org_jetbrains_skia_Data__1nSize")
-private external fun _nSize(ptr: NativePointer): Long
+private external fun _nSize(ptr: NativePointer): Int
 
 @ExternalSymbolName("org_jetbrains_skia_Data__1nBytes")
-private external fun _nBytes(ptr: NativePointer, offset: Long, length: Long): ByteArray
+private external fun _nBytes(ptr: NativePointer, offset: Int, length: Int, destBytes: InteropPointer)
 
 @ExternalSymbolName("org_jetbrains_skia_Data__1nEquals")
 private external fun _nEquals(ptr: NativePointer, otherPtr: NativePointer): Boolean
@@ -132,7 +142,7 @@ private external fun _nMakeFromBytes(bytes: InteropPointer, offset: Int, length:
 private external fun _nMakeFromFileName(path: String?): NativePointer
 
 @ExternalSymbolName("org_jetbrains_skia_Data__1nMakeSubset")
-private external fun _nMakeSubset(ptr: NativePointer, offset: Long, length: Long): NativePointer
+private external fun _nMakeSubset(ptr: NativePointer, offset: Int, length: Int): NativePointer
 
 @ExternalSymbolName("org_jetbrains_skia_Data__1nMakeEmpty")
 private external fun _nMakeEmpty(): NativePointer

skiko/src/jvmTest/kotlin/org/jetbrains/skiko/ColorTest.kt → skiko/src/commonTest/kotlin/org/jetbrains/skia/ColorTest.kt

-package org.jetbrains.skiko
+package org.jetbrains.skia
 
-import org.jetbrains.skia.Color
-import org.jetbrains.skia.Color4f
+import org.jetbrains.skiko.tests.runTest
 import kotlin.test.Test
 
 import kotlin.test.assertEquals
 
 class ColorTest {
     @Test
-    fun testColor() {
+    fun testColor() = runTest {
         val cases = mapOf(
-        0x00000000 to Color4f(0f, 0f, 0f, 0f),
-        -0x1000000 to Color4f(0f, 0f, 0f, 1f),
-        0x00FF0000 to Color4f(1f, 0f, 0f, 0f),
-        0x0000FF00 to Color4f(0f, 1f, 0f, 0f),
-        0x000000FF to Color4f(0f, 0f, 1f, 0f),
-        -0x7f7f7f80 to Color4f(128 / 255f, 128 / 255f, 128 / 255f, 128 / 255f)
+            0x00000000 to Color4f(0f, 0f, 0f, 0f),
+            -0x1000000 to Color4f(0f, 0f, 0f, 1f),
+            0x00FF0000 to Color4f(1f, 0f, 0f, 0f),
+            0x0000FF00 to Color4f(0f, 1f, 0f, 0f),
+            0x000000FF to Color4f(0f, 0f, 1f, 0f),
+            -0x7f7f7f80 to Color4f(128 / 255f, 128 / 255f, 128 / 255f, 128 / 255f)
         )
 
         cases.forEach { (value, color) ->
@@ -32,4 +31,4 @@ class ColorTest {
         assertEquals(0x1234FE78, Color.withG(0x12345678, 0xFE))
         assertEquals(0x123456FE, Color.withB(0x12345678, 0xFE))
     }
-}
\ No newline at end of file
+}

skiko/src/commonTest/kotlin/org/jetbrains/skia/DataTest.kt

+package org.jetbrains.skia
+
+import org.jetbrains.skia.impl.use
+import org.jetbrains.skiko.tests.runTest
+import kotlin.test.*
+
+class DataTest {
+
+    @Test
+    fun dataTest() = runTest {
+        Data.makeEmpty().use { data ->
+            assertEquals(0, data.size)
+            assertContentEquals(ByteArray(0), data.bytes)
+
+            Data.makeEmpty().use { data2 -> assertEquals(data, data2) }
+        }
+
+        val bytes = "abcdef".toCharArray().map { it.code.toByte() }.toByteArray()
+        val bytesSubset = "bcde".toCharArray().map { it.code.toByte() }.toByteArray()
+
+        Data.makeFromBytes(bytes).use { data ->
+            assertEquals(6, data.size)
+            assertContentEquals(bytes, data.bytes)
+            assertContentEquals(bytesSubset, data.getBytes(1, 4))
+
+            Data.makeFromBytes(bytes).use { data2 -> assertEquals(data, data2) }
+            data.makeCopy().use { data2 -> assertEquals(data, data2) }
+
+            Data.makeFromBytes(bytes, 1, 4).use { data3 ->
+                assertEquals(4, data3.size)
+                assertContentEquals(bytesSubset, data3.bytes)
+                assertNotEquals(data, data3)
+            }
+
+            data.makeSubset(1, 4).use { data4 ->
+                assertEquals(4, data4.size)
+                assertContentEquals(bytesSubset, data4.bytes)
+                assertNotEquals(data, data4)
+            }
+        }
+    }
+
+    @Test
+    fun failsToReadMoreBytesThanPossible() = runTest {
+        val bytes = byteArrayOf(1, 2, 3, 4, 5)
+        val data = Data.makeFromBytes(bytes)
+
+
+        data.use {
+            assertFailsWith<IllegalStateException> {
+                it.getBytes(2, 10)
+            }
+
+            assertFailsWith<IllegalStateException> {
+                it.getBytes(0, 6)
+            }
+
+            assertContentEquals(bytes, it.getBytes(0, 5))
+            assertContentEquals(bytes.takeLast(4).toByteArray(), it.getBytes(1, 4))
+            assertContentEquals(bytes.takeLast(3).toByteArray(), it.getBytes(2, 3))
+            assertContentEquals(bytes.takeLast(2).toByteArray(), it.getBytes(3, 2))
+            assertContentEquals(bytes.takeLast(1).toByteArray(), it.getBytes(4, 1))
+            assertContentEquals(byteArrayOf(), it.getBytes(5, 0))
+        }
+    }
+}

skiko/src/jsMain/kotlin/org/jetbrains/skia/impl/Native.js.kt

@@ -56,7 +56,7 @@ actual class InteropScope actual constructor() {
     }
 
     actual fun toInterop(array: ByteArray?): InteropPointer {
-        return if (array != null) {
+        return if (array != null && array.isNotEmpty()) {
             val data = _malloc(array.size)
             elements.add(data)
             toWasm(data, array)
@@ -70,7 +70,7 @@ actual class InteropScope actual constructor() {
     }
 
     actual fun toInterop(array: ShortArray?): InteropPointer {
-        return if (array != null) {
+        return if (array != null && array.isNotEmpty()) {
             val data = _malloc(array.size * 2)
             elements.add(data)
             toWasm(data, array)
@@ -85,7 +85,7 @@ actual class InteropScope actual constructor() {
     }
 
     actual fun toInterop(array: IntArray?): InteropPointer {
-        return if (array != null) {
+        return if (array != null && array.isNotEmpty()) {
             val data = _malloc(array.size * 4)
             elements.add(data)
             toWasm(data, array)
@@ -108,7 +108,7 @@ actual class InteropScope actual constructor() {
     }
 
     actual fun toInterop(array: FloatArray?): InteropPointer {
-        return if (array != null) {
+        return if (array != null && array.isNotEmpty()) {
             val data = _malloc(array.size * 4)
             elements.add(data)
             toWasm(data, array)
@@ -123,7 +123,7 @@ actual class InteropScope actual constructor() {
     }
 
     actual fun toInterop(array: DoubleArray?): InteropPointer {
-        return if (array != null) {
+        return if (array != null && array.isNotEmpty()) {
             val data = _malloc(array.size * 8)
             elements.add(data)
             toWasm(data, array)
@@ -138,7 +138,7 @@ actual class InteropScope actual constructor() {
     }
 
     actual fun toInterop(array: NativePointerArray?): InteropPointer {
-        return if (array != null) {
+        return if (array != null && array.size > 0) {
             val data = _malloc(array.size * 4)
             elements.add(data)
             toWasm(data, array.backing)
@@ -153,7 +153,7 @@ actual class InteropScope actual constructor() {
     }
 
     actual fun toInterop(stringArray: Array<String>?): InteropPointer {
-        return if (stringArray != null) {
+        return if (stringArray != null && stringArray.isNotEmpty()) {
             val ptrs = stringArray.map {
                 toInterop(it)
             }.toIntArray()

skiko/src/jvmMain/cpp/common/Data.cc

@@ -11,7 +11,7 @@ extern "C" JNIEXPORT jlong JNICALL Java_org_jetbrains_skia_DataKt_Data_1nGetFina
     return static_cast<jlong>(reinterpret_cast<uintptr_t>(&deleteData));
 }
 
-extern "C" JNIEXPORT jlong JNICALL Java_org_jetbrains_skia_DataKt__1nSize
+extern "C" JNIEXPORT jint JNICALL Java_org_jetbrains_skia_DataKt__1nSize
   (JNIEnv* env, jclass jclass, jlong ptr) {
     SkData* instance = reinterpret_cast<SkData*>(static_cast<uintptr_t>(ptr));
     return instance->size();
@@ -23,13 +23,11 @@ extern "C" JNIEXPORT jobject JNICALL Java_org_jetbrains_skia_DataKt__1nToByteBuf
     return env->NewDirectByteBuffer(instance->writable_data(), instance->size());
 }
 
-extern "C" JNIEXPORT jbyteArray JNICALL Java_org_jetbrains_skia_DataKt__1nBytes
-  (JNIEnv* env, jclass jclass, jlong ptr, jlong offset, jlong length) {
+extern "C" JNIEXPORT void JNICALL Java_org_jetbrains_skia_DataKt__1nBytes
+  (JNIEnv* env, jclass jclass, jlong ptr, jint offset, jint length, jbyteArray destBytes) {
     SkData* instance = reinterpret_cast<SkData*>(static_cast<uintptr_t>(ptr));
-    jbyteArray bytesArray = env->NewByteArray((jsize) length);
     const jbyte* bytes = reinterpret_cast<const jbyte*>(instance->bytes() + offset);
-    env->SetByteArrayRegion(bytesArray, 0, (jsize) length, bytes);
-    return bytesArray;
+    env->SetByteArrayRegion(destBytes, 0, (jsize) length, bytes);
 }
 
 extern "C" JNIEXPORT jboolean JNICALL Java_org_jetbrains_skia_DataKt__1nEquals
@@ -56,7 +54,7 @@ extern "C" JNIEXPORT jlong JNICALL Java_org_jetbrains_skia_DataKt__1nMakeFromFil
 }
 
 extern "C" JNIEXPORT jlong JNICALL Java_org_jetbrains_skia_DataKt__1nMakeSubset
-  (JNIEnv* env, jclass jclass, jlong ptr, jlong offset, jlong length) {
+  (JNIEnv* env, jclass jclass, jlong ptr, jint offset, jint length) {
     SkData* instance = reinterpret_cast<SkData*>(static_cast<uintptr_t>(ptr));
     SkData* subset = SkData::MakeSubset(instance, offset, length).release();
     return reinterpret_cast<jlong>(subset);

skiko/src/jvmTest/kotlin/org/jetbrains/skiko/DataTest.kt

-package org.jetbrains.skiko
-
-import org.jetbrains.skia.Data
-import org.junit.Assert.assertArrayEquals
-import java.nio.charset.StandardCharsets
-import kotlin.test.Test
-
-import kotlin.test.assertEquals
-import kotlin.test.assertNotEquals
-
-class DataTest {
-    @Test
-    fun dataTest() {
-        Data.makeEmpty().use { data ->
-            assertEquals(0L, data.size)
-            assertArrayEquals(ByteArray(0), data.bytes)
-
-            Data.makeEmpty().use { data2 -> assertEquals(data, data2) }
-        }
-
-        val bytes = "abcdef".toByteArray(StandardCharsets.UTF_8)
-        val bytesSubset = "bcde".toByteArray(StandardCharsets.UTF_8)
-
-        Data.makeFromBytes(bytes).use { data ->
-            assertEquals(6L, data.size)
-            assertArrayEquals(bytes, data.bytes)
-            assertArrayEquals(bytesSubset, data.getBytes(1, 4))
-
-            Data.makeFromBytes(bytes).use { data2 -> assertEquals(data, data2) }
-            data.makeCopy().use { data2 -> assertEquals(data, data2) }
-
-            Data.makeFromBytes(bytes, 1, 4).use { data3 ->
-                assertEquals(4L, data3.size)
-                assertArrayEquals(bytesSubset, data3.bytes)
-                assertNotEquals(data, data3)
-            }
-
-            data.makeSubset(1, 4).use { data4 ->
-                assertEquals(4L, data4.size)
-                assertArrayEquals(bytesSubset, data4.bytes)
-                assertNotEquals(data, data4)
-            }
-        }
-    }
-}
\ No newline at end of file

skiko/src/nativeMain/kotlin/org/jetbrains/skia/impl/Native.native.kt

@@ -57,7 +57,7 @@ actual class InteropScope actual constructor() {
     actual fun InteropPointer.fromInterop(result: CharArray) {}
 
     actual fun toInterop(array: ByteArray?): InteropPointer {
-        return if (array != null) {
+        return if (array != null && array.isNotEmpty()) {
             val pinned = array.pin()
             elements.add(pinned)
             val result = pinned.addressOf(0).rawValue
@@ -70,7 +70,7 @@ actual class InteropScope actual constructor() {
     actual fun InteropPointer.fromInterop(result: ByteArray) {}
 
     actual fun toInterop(array: ShortArray?): InteropPointer {
-        return if (array != null) {
+        return if (array != null && array.isNotEmpty()) {
             val pinned = array.pin()
             elements.add(pinned)
             val result = pinned.addressOf(0).rawValue
@@ -83,7 +83,7 @@ actual class InteropScope actual constructor() {
     actual fun InteropPointer.fromInterop(result: ShortArray) {}
 
     actual fun toInterop(array: IntArray?): InteropPointer {
-        return if (array != null) {
+        return if (array != null && array.isNotEmpty()) {
             val pinned = array.pin()
             elements.add(pinned)
             val result = pinned.addressOf(0).rawValue
@@ -96,7 +96,7 @@ actual class InteropScope actual constructor() {
     actual fun InteropPointer.fromInterop(result: IntArray) {}
 
     actual fun toInterop(array: LongArray?): InteropPointer {
-        return if (array != null) {
+        return if (array != null && array.isNotEmpty()) {
             val pinned = array.pin()
             elements.add(pinned)
             val result = pinned.addressOf(0).rawValue
@@ -109,7 +109,7 @@ actual class InteropScope actual constructor() {
     actual fun InteropPointer.fromInterop(result: LongArray) {}
 
     actual fun toInterop(array: FloatArray?): InteropPointer {
-        return if (array != null) {
+        return if (array != null && array.isNotEmpty()) {
             val pinned = array.pin()
             elements.add(pinned)
             val result = pinned.addressOf(0).rawValue
@@ -122,7 +122,7 @@ actual class InteropScope actual constructor() {
     actual fun InteropPointer.fromInterop(result: FloatArray) {}
 
     actual fun toInterop(array: DoubleArray?): InteropPointer {
-        return if (array != null) {
+        return if (array != null && array.isNotEmpty()) {
             val pinned = array.pin()
             elements.add(pinned)
             val result = pinned.addressOf(0).rawValue
@@ -135,7 +135,7 @@ actual class InteropScope actual constructor() {
     actual fun InteropPointer.fromInterop(result: DoubleArray) {}
 
     actual fun toInterop(array: NativePointerArray?): InteropPointer {
-        return if (array != null) {
+        return if (array != null && array.size > 0) {
             // We pass it as LongArray via boundary.
             val pinned = array.backing.pin()
             elements.add(pinned)
@@ -149,7 +149,7 @@ actual class InteropScope actual constructor() {
     actual fun InteropPointer.fromInterop(result: NativePointerArray) {}
 
     actual fun toInterop(stringArray: Array<String>?): InteropPointer {
-        if (stringArray == null) return NativePtr.NULL
+        if (stringArray == null || stringArray.isEmpty()) return NativePtr.NULL
 
         val pins = stringArray.toList()
             .map { it.encodeToByteArray().pin() }
@@ -197,4 +197,4 @@ actual class NativePointerArray actual constructor(size: Int) {
 
     actual val size: Int
         get() = backing.size
-}
\ No newline at end of file
+}