From cc5648532f66063a91487440066abfc1918b069c Mon Sep 17 00:00:00 2001 From: Marco Hutter Date: Sun, 14 Apr 2024 04:06:52 +0200 Subject: [PATCH] Properly convert Java char to C char in inputSwizzle (#876) Fixes https://github.com/KhronosGroup/KTX-Software/issues/875 As detailed in the issue: The Java `char[]` array for the input swizzle was casted to a `jbyteArray` in the JNI layer, in order to write it to the C/C++ `char[4]` array. This cast was invalid and caused data corruption. There could be two ways of tackling this: - One could change the type of the `inputSwizzle` in the Java layer to `byte[]` (which has 8 bits, and would make the cast valid). This would be a breaking change, and may require the inconvenient casting at the call site like `p.setInputSwizzle(new byte[]{ (byte)'b', (byte)'r', (byte)'g', (byte)'a' });` - One could fetch the Java `char[]` array as a `jcharArray`, and cast the elements to (C/C++) `char` individually This PR takes the second option (but we could do this either way, depending on the preferences from others). --- .../java_binding/src/main/cpp/libktx-jni.cpp | 30 +++-- .../ktx/test/KtxTestLibraryLoader.java | 3 - .../org/khronos/ktx/test/KtxTexture2Test.java | 106 +++++++++++++++++- .../java/org/khronos/ktx/test/TestUtils.java | 67 +++++++++++ 4 files changed, 186 insertions(+), 20 deletions(-) create mode 100644 interface/java_binding/src/test/java/org/khronos/ktx/test/TestUtils.java diff --git a/interface/java_binding/src/main/cpp/libktx-jni.cpp b/interface/java_binding/src/main/cpp/libktx-jni.cpp index d88233b989..150be38fca 100644 --- a/interface/java_binding/src/main/cpp/libktx-jni.cpp +++ b/interface/java_binding/src/main/cpp/libktx-jni.cpp @@ -97,12 +97,14 @@ void copy_ktx_astc_params(JNIEnv *env, jobject params, ktxAstcParams &out) out.normalMap = env->GetBooleanField(params, normalMap); out.perceptual = env->GetBooleanField(params, perceptual); - env->GetByteArrayRegion( - static_cast(env->GetObjectField(params, inputSwizzle)), - 0, - 4, - reinterpret_cast(&out.inputSwizzle) - ); + jobject inputSwizzleObject = env->GetObjectField(params, inputSwizzle); + jcharArray inputSwizzleArray = static_cast(inputSwizzleObject); + jchar *inputSwizzleValues = env->GetCharArrayElements( inputSwizzleArray, NULL); + for (int i=0; i<4; i++) + { + out.inputSwizzle[i] = static_cast(inputSwizzleValues[i]); + } + env->ReleaseCharArrayElements(inputSwizzleArray, inputSwizzleValues, JNI_ABORT); } void copy_ktx_basis_params(JNIEnv *env, jobject params, ktxBasisParams &out) @@ -145,12 +147,16 @@ void copy_ktx_basis_params(JNIEnv *env, jobject params, ktxBasisParams &out) out.endpointRDOThreshold = env->GetFloatField(params, endpointRDOThreshold); out.maxSelectors = env->GetIntField(params, maxSelectors); out.selectorRDOThreshold = env->GetFloatField(params, selectorRDOThreshold); - env->GetByteArrayRegion( - static_cast(env->GetObjectField(params, inputSwizzle)), - 0, - 4, - reinterpret_cast(&out.inputSwizzle) - ); + + jobject inputSwizzleObject = env->GetObjectField(params, inputSwizzle); + jcharArray inputSwizzleArray = static_cast(inputSwizzleObject); + jchar *inputSwizzleValues = env->GetCharArrayElements( inputSwizzleArray, NULL); + for (int i=0; i<4; i++) + { + out.inputSwizzle[i] = static_cast(inputSwizzleValues[i]); + } + env->ReleaseCharArrayElements(inputSwizzleArray, inputSwizzleValues, JNI_ABORT); + out.normalMap = env->GetBooleanField(params, normalMap); out.preSwizzle = env->GetBooleanField(params, preSwizzle); out.noEndpointRDO = env->GetBooleanField(params, noEndpointRDO); diff --git a/interface/java_binding/src/test/java/org/khronos/ktx/test/KtxTestLibraryLoader.java b/interface/java_binding/src/test/java/org/khronos/ktx/test/KtxTestLibraryLoader.java index 2187ef5d35..1964739f5d 100644 --- a/interface/java_binding/src/test/java/org/khronos/ktx/test/KtxTestLibraryLoader.java +++ b/interface/java_binding/src/test/java/org/khronos/ktx/test/KtxTestLibraryLoader.java @@ -11,9 +11,6 @@ import java.io.File; import java.nio.file.Files; import java.nio.file.Path; -import java.util.Arrays; - -import static org.junit.jupiter.api.extension.ExtensionContext.Namespace.GLOBAL; public class KtxTestLibraryLoader implements BeforeAllCallback, ExtensionContext.Store.CloseableResource { diff --git a/interface/java_binding/src/test/java/org/khronos/ktx/test/KtxTexture2Test.java b/interface/java_binding/src/test/java/org/khronos/ktx/test/KtxTexture2Test.java index 09d958c54a..48db332084 100644 --- a/interface/java_binding/src/test/java/org/khronos/ktx/test/KtxTexture2Test.java +++ b/interface/java_binding/src/test/java/org/khronos/ktx/test/KtxTexture2Test.java @@ -5,17 +5,26 @@ package org.khronos.ktx.test; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.khronos.ktx.*; +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertNotNull; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.khronos.ktx.KtxBasisParams; +import org.khronos.ktx.KtxCreateStorage; +import org.khronos.ktx.KtxErrorCode; +import org.khronos.ktx.KtxSupercmpScheme; +import org.khronos.ktx.KtxTexture2; +import org.khronos.ktx.KtxTextureCreateFlagBits; +import org.khronos.ktx.KtxTextureCreateInfo; +import org.khronos.ktx.KtxTranscodeFormat; +import org.khronos.ktx.VkFormat; @ExtendWith({ KtxTestLibraryLoader.class }) public class KtxTexture2Test { @@ -227,4 +236,91 @@ public void testCreate() { texture.destroy(); } + + @Test + public void testInputSwizzleBasisEx() throws IOException { + + int sizeX = 32; + int sizeY = 32; + int outputFormat = KtxTranscodeFormat.RGBA32; + int transcodeFlags = 0; + + // Create the actual texture data: + // - create RGBA pixels + // - create texture + // - compress with BRGA input swizzling + // - obtain resulting RGBA values + + // Create a RGBA pixels for an image filled with + // 8 rows of red pixels + // 8 rows of green pixels + // 8 rows of blue pixels + // 8 rows of white pixels + byte[] input = new byte[sizeX * sizeY * 4]; + TestUtils.fillRows(input, sizeX, sizeY, 0, 8, 255, 0, 0, 255); // Red + TestUtils.fillRows(input, sizeX, sizeY, 8, 16, 0, 255, 0, 255); // Green + TestUtils.fillRows(input, sizeX, sizeY, 16, 24, 0, 0, 255, 255); // Blue + TestUtils.fillRows(input, sizeX, sizeY, 24, 32, 255, 255, 255, 255); // White + + // Create the input texture from the pixels + KtxTextureCreateInfo inputInfo = new KtxTextureCreateInfo(); + inputInfo.setBaseWidth(sizeX); + inputInfo.setBaseHeight(sizeY); + inputInfo.setVkFormat(VkFormat.VK_FORMAT_R8G8B8A8_SRGB); + KtxTexture2 inputTexture = KtxTexture2.create(inputInfo, KtxCreateStorage.ALLOC); + inputTexture.setImageFromMemory(0, 0, 0, input); + + // Apply basis compression to the input, with an input swizzle BRGA, + // so that + // the former B channel becomes the R channel + // the former R channel becomes the G channel + // the former G channel becomes the B channel + // the former A channel remains the A channel + KtxBasisParams inputParams = new KtxBasisParams(); + inputParams.setUastc(false); + inputParams.setInputSwizzle(new char[] { 'b', 'r', 'g', 'a' }); + inputTexture.compressBasisEx(inputParams); + + // Transcode the input texture to RGBA32 + inputTexture.transcodeBasis(outputFormat, transcodeFlags); + byte[] actualRgba = inputTexture.getData(); + + // Create the expected reference data: + // - create RGBA pixels, swizzled with BRGA + // - create texture + // - compress without input swizzling + // - obtain resulting RGBA values + + // Create "golden" reference pixels, where a BRGA + // swizzling was already applied + byte[] gold = new byte[sizeX * sizeY * 4]; + TestUtils.fillRows(gold, sizeX, sizeY, 0, 8, 0, 255, 0, 255); // Green + TestUtils.fillRows(gold, sizeX, sizeY, 8, 16, 0, 0, 255, 255); // Blue + TestUtils.fillRows(gold, sizeX, sizeY, 16, 24, 255, 0, 0, 255); // Red + TestUtils.fillRows(gold, sizeX, sizeY, 24, 32, 255, 255, 255, 255); // White + + // Create the reference texture from the swizzled pixels + KtxTextureCreateInfo goldInfo = new KtxTextureCreateInfo(); + goldInfo.setBaseWidth(sizeX); + goldInfo.setBaseHeight(sizeY); + goldInfo.setVkFormat(VkFormat.VK_FORMAT_R8G8B8A8_SRGB); + KtxTexture2 goldTexture = KtxTexture2.create(goldInfo, KtxCreateStorage.ALLOC); + goldTexture.setImageFromMemory(0, 0, 0, gold); + + // Apply basis compression to the reference, without swizzling + KtxBasisParams goldParams = new KtxBasisParams(); + goldParams.setUastc(false); + goldTexture.compressBasisEx(goldParams); + + // Transcode the reference texture to RGBA32 + goldTexture.transcodeBasis(outputFormat, transcodeFlags); + byte[] expectedRgba = goldTexture.getData(); + + // Compare the resulting data to the expected RGBA values. + assertArrayEquals(expectedRgba, actualRgba); + + inputTexture.destroy(); + goldTexture.destroy(); + } } + diff --git a/interface/java_binding/src/test/java/org/khronos/ktx/test/TestUtils.java b/interface/java_binding/src/test/java/org/khronos/ktx/test/TestUtils.java new file mode 100644 index 0000000000..1e1c95e66f --- /dev/null +++ b/interface/java_binding/src/test/java/org/khronos/ktx/test/TestUtils.java @@ -0,0 +1,67 @@ +/* + * Copyright (c) 2024, Khronos Group and Contributors + * SPDX-License-Identifier: Apache-2.0 + */ +package org.khronos.ktx.test; + +import java.util.Locale; + +/** + * Utilities for the test package + */ +class TestUtils { + + /** + * Fill the specified range of rows of the given RGBA pixels array with the + * given RGBA components + * + * @param rgba The RGBA pixels array + * @param sizeX The size of the image in x-direction + * @param sizeY The size of the image in y-direction + * @param minRow The minimum row, inclusive + * @param maxRow The maximum row, exclusive + * @param r The red component (in [0,255]) + * @param g The green component (in [0,255]) + * @param b The blue component (in [0,255]) + * @param a The alpha component (in [0,255]) + */ + static void fillRows(byte rgba[], int sizeX, int sizeY, + int minRow, int maxRow, + int r, int g, int b, int a) { + for (int y = minRow; y < maxRow; y++) { + for (int x = 0; x < sizeX; x++) { + int index = (y * sizeX) + x; + rgba[index * 4 + 0] = (byte) r; + rgba[index * 4 + 1] = (byte) g; + rgba[index * 4 + 2] = (byte) b; + rgba[index * 4 + 3] = (byte) a; + } + } + } + + /** + * Create a string representation of the RGBA components of the specified pixel + * in the given RGBA pixels array. + * + * This is mainly intended for debugging. Some details of the resulting string + * are not specified. + * + * @param rgba The RGBA pixels array + * @param pixelIndex The pixel index + * @return The string + */ + static String createRgbaString(byte rgba[], int pixelIndex) { + byte r = rgba[pixelIndex * 4 + 0]; + byte g = rgba[pixelIndex * 4 + 1]; + byte b = rgba[pixelIndex * 4 + 2]; + byte a = rgba[pixelIndex * 4 + 3]; + int ir = Byte.toUnsignedInt(r); + int ig = Byte.toUnsignedInt(g); + int ib = Byte.toUnsignedInt(b); + int ia = Byte.toUnsignedInt(a); + String s = String.format(Locale.ENGLISH, "%3d, %3d, %3d, %3d", ir, ig, ib, ia); + return s; + + } + +}