Skip to content

Commit

Permalink
Properly convert Java char to C char in inputSwizzle (#876)
Browse files Browse the repository at this point in the history
Fixes #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).
  • Loading branch information
javagl authored Apr 14, 2024
1 parent e31b3e5 commit cc56485
Show file tree
Hide file tree
Showing 4 changed files with 186 additions and 20 deletions.
30 changes: 18 additions & 12 deletions interface/java_binding/src/main/cpp/libktx-jni.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<jbyteArray>(env->GetObjectField(params, inputSwizzle)),
0,
4,
reinterpret_cast<jbyte*>(&out.inputSwizzle)
);
jobject inputSwizzleObject = env->GetObjectField(params, inputSwizzle);
jcharArray inputSwizzleArray = static_cast<jcharArray>(inputSwizzleObject);
jchar *inputSwizzleValues = env->GetCharArrayElements( inputSwizzleArray, NULL);
for (int i=0; i<4; i++)
{
out.inputSwizzle[i] = static_cast<char>(inputSwizzleValues[i]);
}
env->ReleaseCharArrayElements(inputSwizzleArray, inputSwizzleValues, JNI_ABORT);
}

void copy_ktx_basis_params(JNIEnv *env, jobject params, ktxBasisParams &out)
Expand Down Expand Up @@ -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<jbyteArray>(env->GetObjectField(params, inputSwizzle)),
0,
4,
reinterpret_cast<jbyte*>(&out.inputSwizzle)
);

jobject inputSwizzleObject = env->GetObjectField(params, inputSwizzle);
jcharArray inputSwizzleArray = static_cast<jcharArray>(inputSwizzleObject);
jchar *inputSwizzleValues = env->GetCharArrayElements( inputSwizzleArray, NULL);
for (int i=0; i<4; i++)
{
out.inputSwizzle[i] = static_cast<char>(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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
}
}

Original file line number Diff line number Diff line change
@@ -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;

}

}

0 comments on commit cc56485

Please sign in to comment.