KhronosGroup / KTX-Software

KTX (Khronos Texture) Library and Tools
Other
885 stars 232 forks source link

No option to apply `zstd`/`zlib` compression via JNI #877

Closed javagl closed 7 months ago

javagl commented 8 months ago

There are methods for ktxTexture2_DeflateZstd (and ZLIB, respectively). They are members of the ktxTexture2 class (even though, for whatever reason, they are in some writer2.c file...).

It looks like these methods are not exposed via the Java KtxTexture2 class. This means that there does not seem to be a way to apply ZLIB/zstd compression via Java.

Should/could these methods be exposed? Or are there any caveats with that, or specific reasons why they should not be exported?

(Otherwise, I might give this a try, pragmatically, following the same patterns as the existing (native) methods in the KtxTexture2 class...)

MarkCallow commented 8 months ago

I'm surprised ktxTexture2_DeflateZstd is not exposed as it pre-dates the Java wrapper. I must have overlooked it during my review. ktxTexture2_DeflateZlib on the other hand was added after the wrapper. Yes these methods should be exposed and a PR would be welcome.

The native methods are in writer2.c because that file contains all the methods related to creating new and writing ktxTexture2 objects and deflate is only needed when writing.

javagl commented 8 months ago

I'm surprised ktxTexture2_DeflateZstd is not exposed as it pre-dates the Java wrapper.

When you phrase it like that, then I hope that it's OK to ask this a bit more broadly.

(With a huge disclaimer: I just started looking more closely into all of this, so I often don't have and idea how everything around KTX is supposed to work, at all).


Some functions should certainly not be exposed to Java, because they are "too specific for the language". Other functions currently are exposed, but one could make a strong case for not exposing them.

For example: I wouldn't have included createFromNamedFile on the Java side. File I/O is very platform- and language specific. Conversely, the createFromMemory function should definitely be exposed, because it is the most generic form of IO, and the only way to read KTX data when it's not in a file.

(You might see the similarities to the point of having to twiddle with 'temp' files in the Maven issue : There should definitely be a way to load raw KTX data from some buffer/array, without having to put it into a temp file...)


So the broader question is: Which functions should be exposed?

I won't claim to know the answer here. But I'll try to allocate some time to create a PR for DeflateZstd and DeflateZLIB at least (and maybe I'll boldly sneak in that CreateFromMemory there as well...), and we can see where to go from there.

The general approach for this is: "When in doubt, leave it out". One can always expose additional functions, but changing or removing one that was previously exposed always is a breaking change.

MarkCallow commented 8 months ago

I agree CreateFromMemory should be exposed. Again I'm surprised its not. I must not have been firing on all cylinders when I reviewed the PR. I also agree with the general approach you state.

I last worked with Java when I was part of JSR184 and JSR257 (3D graphics for J2ME, aka M3G) and developed an M3G engine more than 15 years ago. I no longer remember much of the nitty-gritty. I am happy to receive guidance from experts with current Java knowledge.

@ShukantPal would you like to comment?

javagl commented 8 months ago

I'm also not entirely up to date here. I started https://github.com/jcuda and https://github.com/gpu/JOCL (also ~15 years ago), where I blindly dumped out the whole API (with automatic JNI code generation and such). In the meantime, a lot has happened in this area - mainly in the context of https://openjdk.org/projects/panama/ , which recently has become part of the latest releases, but I have not yet looked at the details.

I'll try to go through the list of functions in KtxTexture(1/2) and see whether there are functions that should be exposed. But it's difficult for someone who just got started with all this.

For example (and this does not really refer to the Java/JNI part):

Imagine I have a KTX file, load it into a ktxTexture2, and then want to obtain the ("decoded") pixels, as a stupid byte[w*h*4] array with RGBA values. How to do that? I started reading through the KTX source code (i.e. through the actual command line function implementations). But I would have a hard time to list the exact sequence of function calls that are necessary for this (and which consequently should also be available on the Java side, eventually...)

MarkCallow commented 8 months ago

When it comes to anything to do with creating KTX files, Java should be limited to KTX v2.

Imagine I have a KTX file, load it into a ktxTexture2, and then want to obtain the ("decoded") pixels, as a stupid byte[wh4] array with RGBA values.

What do you mean by decoded here? Do you just mean get the image payload data from the file or do you really want RGBA? libktx does not contain decoders for any block-compressed formats except ETC[12]. So you can't get RGBA for BC1-7, ASTC, etc. These are usually used by uploading to a GPU. The BasisU formats can be decoded to RGBA with ktxTexture2_TranscodeBasis at which point you have an uncompressed texture. Read on. Uncompressed formats can be accessed via the data pointer in the ktxTexture2 struct. Not all have 4 components though so you'd have to read those pixels and expand them to RGBA. The format field in the header and the DFD provide all the information needed to understand what format you are dealing with. Many are not 1 byte/component.

javagl commented 8 months ago

(I'd probably have to insert too many disclaimers here, and could hardly have a profound discussion, because I'm in no way involved in the technicalities of the format itself (or rather, the formatS - from what I heard, KTX itself is merely a "container"...)).

But yes, I mean the actual RGBA pixel values. I know that this is not what KTX is supposed to be used for. But in that regard, one might have to ask what the purpose of the JNI bindings should be in the first place. The ktxTexture2_GLUpload is not available there. But it could (and maybe should) be available, so that this could be used together with https://jogamp.org/jogl/www/ or https://www.lwjgl.org/ to actually use KTX in the context of rendering... For now, my main use case would be on the naive level (in the context of glTF), where I (suggestively!!!) need two functions like byte[] createKtxData(byte rgbaPixels[], Options options) and byte[] extractRgbaPixels(byte ktxFileData[])

I know, I know, that's naive, and it's not that simple. But maybe there are some steps that at least go in that direction, and make it easier for "average" developers to handle KTX...

ShukantPal commented 8 months ago

The Java bindings are just a one to one mapping for the functionality libktx provides in the C/C++ interface. Uncompressing textures is not implemented in the native library, which is probably why it won’t be in Java; otherwise we’d have to re implement that logic in each other language binding too.

The reason why it’s not implemented is probably because KTX is a container file format, and not an encoding format. But it would certainly make sense to support manipulation of the encoding if there’s sufficient demand.

In the case of GL2 upload, we would want to be careful not to be prescriptive of which Java binding you use with Vulcan. Usually you can simply upload the byte array to these APIs so there’s not much value add to adding a GL2 upload helper. But I’m certainly not against it.

javagl commented 8 months ago

The approach of a 1:1 mapping does make sense (on a low level - I used this for JCuda and JOCL as well). And I cannot say anything about the ~"uncompression logic" (I'd have to spend faaaar more time in the source code for this). But from a high-level, naive perspective: There currently seems to be a command-line functionality in the KTX software that converts a KTX into one or multiple PNG files. And I wondered whether this could, theoretically be implemented in Java, using only the JNI bindings. (I think that there are some functions that are not exposed in the KtxTexture2 class, but I do not (yet) know whether these are necessary in order to implement that...)

javagl commented 8 months ago

@ShukantPal I have to ask for a quick confirmation (actually, for the unit test that is supposed to be added to https://github.com/KhronosGroup/KTX-Software/pull/876 ) :

I assume that the string "libktx-jni" in https://github.com/KhronosGroup/KTX-Software/blob/5414bd0ebfdfdf4ecb8c613a6edb21372303c2b9/interface/java_binding/src/test/java/org/khronos/ktx/test/KtxTestLibraryLoader.java#L46 should actually be "ktx-jni" (the name of the DLL that is generated).

Is that correct?

Otherwise, I don't really understand the purpose of this class...

ShukantPal commented 8 months ago

I think on Linux it's libktx-jni.so

javagl commented 8 months ago

Right, I forgot. That explains it.

Depending on whether something like the LibUtils will (have to) be added as part of a Maven release in the context of https://github.com/KhronosGroup/KTX-Software/issues/624 , this might already cover the functionality of figuring out the actual name of the library ([lib]ktx-jni.[so|dll|dylib]), so maybe that class can then be generalized to work on Win/Linux/MacOS transparently.

MarkCallow commented 8 months ago

For now, my main use case would be on the naive level (in the context of glTF), where I (suggestively!!!) need two functions like byte[] createKtxData(byte rgbaPixels[], Options options) and byte[] extractRgbaPixels(byte ktxFileData[])

createKtxData is easy to implement using libktx functions. But it violates the 1:1 mapping mentioned. It it is more Java-friendly then perhaps its okay to violate the 1:1 mapping. In the libktx API implementation of this involves 2 functions

ktxTexture2_Create(createInfo, storageAllocation, &newKtx2);
// This has to be called for each image. The number of images depends
// on the type and size of the texture as specified in createInfo.
ktxTexture2_SetImageFromMemory(newKtx2, level, layer, faceSlice,
                               src, srcByteCount);

You can pass KTX_FACESLICE_WHOLE_LEVEL as faceSlice and upload an entire mip level at one. This value has probably not been exposed in the JNI as it postdates the JNI. It should be exposed.

extractRgbaPixels is more complicated. The first thing to recognize, which likely affects the API design, is that not all pixels are byte arrays. A Vulkan VkFormat can be stored in a KTX v2 file which encompasses 8-, 16-, 32- and 64-bit values, floats, half-floats and various packed pixel formats such as RGB565. Some of these are reasonable to convert to bytes during extraction, others not.

The second thing to recognize is that many formats are compressed and we do not have decoders for all of them. Generally they are just uploaded to the GPU in compressed form.

ktx extract, which you mentioned, can likely be implemented using the JNI bindings with the following caveats:

It does not decompress BC1-7 or ETC. Those formats can only be saved to raw files.

libktx has functions to obtain pointers to any layer, level or faceSlice in the file from which you can then copy the data. ktx extract uses these and the transcode function.

In the case of GL2 upload, we would want to be careful not to be prescriptive of which Java binding you use with Vulcan. Usually you can simply upload the byte array to these APIs so there’s not much value add to adding a GL2 upload helper. But I’m certainly not against it.

Is there a Vulkan binding for Java?

ktxTexture{,1,2}_GLUpload are completely independent of the GL version. They will work for ES or desktop. The application has to create and make current a GL context before calling them. Uploading a texture is not so simple as uploading a byte array. You have to determine the shape of the texture to create appropriate texture objects and then you have to upload each image individually. Vulkan is even more of a pain. The JNI binding should absolutely expose ktxTexture2_GLUpload and, if there is a Java Vulkan wrapper, ktxTexture2_VkUpload.

Otherwise, I don't really understand the purpose of this class... (KtxTestLibraryLoader)

Libraries for both GNU/Linux and macOS are named libktx-jni.{so.dylib}. I think this class is used to find the library, via the environment variable LIBKTX_BINARY_DIR during testing when the library is in the build folder. But, if that is the case, I don't know how testing works on Windows or how it works if you want to run the tests using the installed library. @ShukantPal?

javagl commented 8 months ago

There are several comments in various places (the issues and PRs that I opened recently - and sorry that this is not more organized, but I just started diving into that, there are many loose ends to tie up). I'll try to keep the response here short, and move some of the more general questions to the https://github.com/KhronosGroup/KTX-Software/issues/880 where appropriate.


createKtxData is easy to implement using libktx functions. But it violates the 1:1 mapping mentioned.

This was a misunderstanding. These createKtxData and extractRgbaPixels functions are functions that I would like to implement, on top of the KTX JNI functions, in my own ("pure Java") library. The KTX JNI part should only offer the functions of the KTX Software, with the 1:1 mapping (as far as reasonable).

The first thing to recognize, which likely affects the API design, is that not all pixels are byte arrays. ... ... ktx extract, which you mentioned, can likely be implemented using the JNI bindings

These are all points to keep in mind when I actually try to create this library (on top of the JNI bindings). But this library should initially be stupidly simple, with not much more than these two functions, focussed on ~"converting files" (and not on uploading things like 'half-float' to the GPU or so).

I know that for the goal of having such a extractRgbaPixels(ktxData) function, there will be the question of which input formats can be decompressed, and how the result of that can be converted to 32bit RGBA pixel values. But none of this is in the responsibility of the JNI bindings. The JNI bindings only have to provide the functionality of the KTX-Software (and what others build on top of that is left to the developers).

But... coincidentally, the comment that you casually threw in at https://github.com/KhronosGroup/KTX-Software/pull/876#discussion_r1537192266 may be an important one here: It sounds like my library could just call transcode(KTX_TTF_RGBA32...) to obtain "pixel values". (Yeah, with caveats and restrictions, but ... I do not yet understand these constraints, and users (of my library) should not have to care about that...)


Is there a Vulkan binding for Java?

There is, as part of https://www.lwjgl.org/ - for example, https://github.com/LWJGL/lwjgl3/blob/master/modules/samples/src/test/java/org/lwjgl/demo/vulkan/HelloVulkan.java

I have not used these Vulkan bindings yet. But for OpenGL, I did ~"mix different JNI libraries" that only communicated via some GLuint someOpenGLObject: One library uploads data to "this object", and the other uses "this object" (for rendering). One has to be careful in terms of threads and making sure that the context is current and all that. But it does work.


I think this class is used to find the library, via the environment variable LIBKTX_BINARY_DIR during testing when the library is in the build folder. But, if that is the case, I don't know how testing works on Windows or how it works if you want to run the tests using the installed library.

Yes, I figured that out in the meantime: By default, and independent of the OS, the tests are run with the installed version of the libraries (by calling System.loadLibrary("ktx-jni"), which goes through the usual search process, like the PATH on Windows).

But as you said, for tests, it is imortant to be able to refer to the build output, by setting an environment variable like LIBKTX_BINARY_DIR = C:\KTX-Software\build\Debug It will then search this directory, and use the full path to "manually" load the library, like System.load("C:\KTX-Software\build\Debug\ktx-jni.dll").

But right now this class assumes that the name of the file starts with libktx-jni, wheras on Windows, it starts with ktx-jni. So this class does not work on Windows, with the (trivial) fix, in pseudocode

if (os.equals("Windows")) {
    loadFileStartingWith("ktx-jni");
}  else {
    loadFileStartingWith("libktx-jni");
}

(I'll try to sneak that into one of the upcoming PRs...)

MarkCallow commented 8 months ago

I know that for the goal of having such a extractRgbaPixels(ktxData) function, there will be the question of which input formats can be decompressed, and how the result of that can be converted to 32bit RGBA pixel values. But none of this is in the responsibility of the JNI bindings.

Indeed. You can follow https://github.com/KhronosGroup/KTX-Software/blob/main/tools/ktx/command_extract.cpp as a model. It does all the things you mention including which input formats it can decompressed, or not. By the way, you should not limit your thinking to 32-bit RGBA pixels.

. But right now this class assumes that the name of the file starts with libktx-jni, wheras on Windows, it starts with ktx-jni. So this class does not work on Windows, with the (trivial) fix, in pseudocode

That is my analysis too but somehow the tests are working our Windows CI. Best to figure out how before attempting to fix the class.

javagl commented 8 months ago

That is my analysis too but somehow the tests are working our Windows CI. Best to figure out how before attempting to fix the class.

I think that just means that the test is run after the actual installation procedure. The installation is putting the binaries into the default installation directory of the build machine and sets up the PATH. During the Java tests, the usual mechanisms for picking up these binaries kicks in.
(The approach of the "manual search" and the attempt to load the file directly is only taken when the LIBKTX_BINARY_DIR environment variable is defined, and this certainly isn't the case in CI...)

ShukantPal commented 8 months ago

Libraries for both GNU/Linux and macOS are named libktx-jni.{so.dylib}. I think this class is used to find the library, via the environment variable LIBKTX_BINARY_DIR during testing when the library is in the build folder. But, if that is the case, I don't know how testing works on Windows or how it works if you want to run the tests using the installed library. @ShukantPal?

The approach of the "manual search" and the attempt to load the file directly is only taken when the LIBKTX_BINARY_DIR environment variable is defined, and this certainly isn't the case in CI...

That's right. There were probably a bunch of small mistakes like this in my original implementation, and I'm glad that they are being fixed based on actual usage of the Java bindings! It was hard for me to support Windows fully since I don't develop on Windows.

javagl commented 8 months ago

@ShukantPal Ideally, this class will not be required (or rather: significantly changed) in the future, because ideally, it should not be necessary to call System.loadLibrary manually. However, there still has to be a mechanism for picking up the libraries from a build directory, temporarily, during development. And this will likely involve similar structures (i.e. some environment variable or so - details to be sorted out as part of the path towards a Maven release...)

MarkCallow commented 8 months ago

I think that just means that the test is run after the actual installation procedure. The installation is putting the binaries into the default installation directory of the build machine and sets up the PATH. During the Java tests, the usual mechanisms for picking up these binaries kicks in.

Our CI does not build any CMake install targets. They are only run as dependencies of packaging which sets its own root to install the files in a temporary hierarchy. Perhaps the POM build for the Java target does an install. I don't think that is true though because I can build Java locally without having to do sudo which would be needed for an install to complete.

javagl commented 8 months ago

About the KtxLibraryLoader: I don't know the build scipts, CI, and details of the current Java POM (fortunately, "everything worked" for me locally - and there's nothing better than not having to care about stuff like that 😌 ).

But whether or not the KtxLibraryLoader is actually kicking in can be checked easily:

Maybe I'll have another look (e.g. at CI build logs). I locally started generalizing the KtxTestLibraryLoader to also work on windows (and ... added some comments and cleanups). This should not affect the functionality on any other environment. But why it "looked like it worked on Windows" (although it couldn't have worked) will have to be investigated.

MarkCallow commented 8 months ago

Maybe I'll have another look (e.g. at CI build logs).

The test is run under CMake's ctest which means the output of the commands that are run is not visible in the CI logs. ctest only prints the pass/fail status. It saves command output to a log file which disappears at the end of the CI run, when all tests are successful. Talk to you in 10 days.

javagl commented 7 months ago

This has been fixed via https://github.com/KhronosGroup/KTX-Software/pull/879

A broader PR built on top of this is still pending in https://github.com/KhronosGroup/KTX-Software/pull/886