Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Properly convert Java char to C char in inputSwizzle #876

Merged
merged 8 commits into from
Apr 14, 2024
Merged

Properly convert Java char to C char in inputSwizzle #876

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5eea92e
Properly convert Java char to C char in inputSwizzle
javagl Mar 22, 2024
0bedf8d
Add basic test for JNI input swizzle
javagl Mar 23, 2024
4638ca0
Removed unused imports
javagl Mar 23, 2024
0ff0788
Compare result of swizzle to reference
javagl Mar 24, 2024
7da5422
Test swizzle without reference image
javagl Mar 26, 2024
89b29cb
Organize imports
javagl Mar 26, 2024
13a571c
Merge remote-tracking branch 'upstream/main' into fix-jni-input-swizzle
javagl Mar 26, 2024
647a5fc
Fix test for input swizzling in JNI
javagl Apr 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions interface/java_binding/src/main/cpp/libktx-jni.cpp
View file
Open in desktop
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]);
MarkCallow marked this conversation as resolved.
Show resolved Hide resolved
}
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
3 changes: 0 additions & 3 deletions interface/java_binding/src/test/java/org/khronos/ktx/test/KtxTestLibraryLoader.java
View file
Open in desktop
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
69 changes: 69 additions & 0 deletions interface/java_binding/src/test/java/org/khronos/ktx/test/KtxTexture2Test.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import java.nio.file.Path;
import java.nio.file.Paths;

import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;

Expand Down Expand Up @@ -227,4 +228,72 @@ public void testCreate() {

texture.destroy();
}

@Test
public void testInputSwizzleBasisEx() throws IOException {

// Create RGBA pixels for an image with 32x32 pixels,
// filled with
// 8 rows of red pixels
// 8 rows of green pixels
// 8 rows of blue pixels
// 8 rows of white pixels
int sizeX = 32;
int sizeY = 32;
byte[] rgba = new byte[sizeX * sizeY * 4];
fillRows(rgba, sizeX, sizeY, 0, 8, 255, 0, 0, 255); // Red
fillRows(rgba, sizeX, sizeY, 8, 16, 0, 255, 0, 255); // Green
fillRows(rgba, sizeX, sizeY, 16, 24, 0, 0, 255, 255); // Blue
fillRows(rgba, sizeX, sizeY, 24, 32, 255, 255, 255, 255); // White

// Create a texture and fill it with the RGBA pixel data
KtxTextureCreateInfo info = new KtxTextureCreateInfo();
info.setBaseWidth(sizeX);
info.setBaseHeight(sizeY);
info.setVkFormat(VkFormat.VK_FORMAT_R8G8B8A8_SRGB);
KtxTexture2 t = KtxTexture2.create(info, KtxCreateStorage.ALLOC);
t.setImageFromMemory(0, 0, 0, rgba);

// Apply basis compression 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 p = new KtxBasisParams();
p.setUastc(false);
p.setInputSwizzle(new char[] { 'b', 'r', 'g', 'a' });
t.compressBasisEx(p);

// Obtain the texture data, and compare it to the
// expected data of the reference texture
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you are creating the texture in the test, I'd prefer not to need a reference file. You can transcode the swizzled t to KTX_TTF_RGBA32 and compare those pixels with the pixels in rgba. Given the full on colors you are using I don't think compression & transcode will change them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd also like to avoid that reference file. (One reason: The tests/testimages directory already contains >100 files, without any information about what these files are, where they come from, and what they are used for. I'd like to avoid throwing more images on that pile...)

And what you suggested might ... nearly work: From a quick test, converting the resulting data to KTX_TTF_RGBA32 gave the following input/output pxiel values:

Pixel 0 in input  255,   0,   0, 255
Pixel 0 in output   2, 255,   2, 255
...
Pixel 256 in input    0, 255,   0, 255
Pixel 256 in output   0,   0, 253, 255
...
Pixel 512 in input    0,   0, 255, 255
Pixel 512 in output 253,   0,   0, 255

The swizzling is applied correctly, but there is a slight deviation in the formerly "pure" colors, of +/- 2 in some components. I don't know where this might come from - maybe something about SRGB, or it's a compression artifact, or whatnot. If you have an idea what might be causing this (or how to avoid this), I could change it accordingly.

Regarldless of that, the test could be changed to not use a reference image, by comparing the result of converting the image to KTX_TTF_RGBA32 to pixel values that are created programmatically (using the same functions that are used for creating the input image).

This could then be

  • A fixed comparison to the values that I currently see in the output (2, 255, 2)
  • Or a comparison with expected=(0, 255, 0) but with some "epsilon" of maybe +/-5 to account for the slight changes...

Any preference?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer the fixed comparison as the test will not then be sensitive to changes in the encoder. You could also try ktxTexture2_CompressAstcEx. Perhaps that will have less artifacts.

Copy link
Contributor Author

@javagl javagl Mar 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could also try ktxTexture2_CompressAstcEx. Perhaps that will have less artifacts.

Actually, in order to have proper test coverage, I can not only 'try' this (as an alternative), but actually have to test this in addition to the current test. Both of them (basis and astc) are going through different code paths (even though they are doing the same sort of conversion for the swizzling).

byte[] data = t.getData();
Path referenceKtxFile = Paths.get("")
.resolve("../../tests/testimages/swizzle-brga-reference.ktx")
.toAbsolutePath()
.normalize();
KtxTexture2 referenceTexture = KtxTexture2.createFromNamedFile(referenceKtxFile.toString(),
KtxTextureCreateFlagBits.LOAD_IMAGE_DATA_BIT);
byte[] referenceData = referenceTexture.getData();

assertArrayEquals(referenceData, data);

t.destroy();
referenceTexture.destroy();
}

// Fill the specified range of rows of the given RGBA pixels
// array with the given RGBA components
private 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;
}
}
}
}
3 changes: 3 additions & 0 deletions tests/testimages/swizzle-brga-reference.ktx
View file
Open in desktop
Git LFS file not shown
Loading