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.

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

Fix JNI function name for getDataSizeUncompressed #957

Merged
merged 2 commits into from
Nov 15, 2024

Conversation

javagl
Copy link
Contributor

@javagl javagl commented Nov 12, 2024

A small fix for the JNI bindings:

The function signature for getDataSizeUncompressed was given as
Java_org_khronos_KTXTexture_getDataSizeUncompressed but must be
Java_org_khronos_ktx_KtxTexture_getDataSizeUncompressed (matching the full package- and class name)

Currently, calling this function would result in a
java.lang.UnsatisfiedLinkError: org.khronos.ktx.KtxTexture.getDataSizeUncompressed()J

I also added a test that might look useless at the first glance: It just creates a texture, and calls all the native "getter" functions on that (without any assertion). The point of that test is that it would have prevented this kind of error...

@@ -599,4 +599,37 @@ public void testSupercompressionZLIB() throws IOException {

t.destroy();
}

@Test
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a comment similar to what you wrote in the PR description about this test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That certainly makes sense (otherwise, people might wonder what the purpose of the test is).

(An aside: In another context, I automated this kind of test, using some reflection magic - but here, we only have ~20 native functions, whereas for JCuda, it was about (literally) thousands of native methods in dozens of classes...)

@MarkCallow MarkCallow merged commit f3902db into KhronosGroup:main Nov 15, 2024
17 checks passed
@MarkCallow
Copy link
Collaborator

Thanks @javagl.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants