Update Java bindings for HDR support#1147
Conversation
The first two are not specific to HDR. |
|
I have just create PR #1150 to add everything to the Python binding. Letting you know as it may be helpful. |
|
I'll have a closer look at the changes and will try add any changes that might be missing here. |
|
Most of the updates that are specific for the added HDR support should be done now. (I stumbled over the missing I'll add some further tests for the new functionality. One thing that is "unrelated" to the HDR changes: I noticed that the bindings for are missing, including the corresponding I'll try to allocate some time to add them (this could be a new PR, but 🤷♂️ it probably makes sense to just add them here...). |
MarkCallow
left a comment
There was a problem hiding this comment.
Looks good. Thanks. Some test coverage using the new tokens would be great, if time allows.
I am waiting for fixes to two other issues. I will hold off on merging this until those arrive to give you a window for adding ktxTexture2_GetColorModel_e and ktxTexture2_GetPrimaries_e to this PR. They should indeed be included in the Java wrapper.
|
A PR has been submitted with the fixes I mentioned. I plan to merge this PR after that PR is merged. I expect to do so within the next 3 days. Please add |
|
@MarkCallow There currently seem to be two PRs, #1155 #1157 , that might affect the API or the Java bindings. I'm in no way deeply enough involved to know what's going on there. Much of that (more than I'd like) just consists of "looking at the code, and mimicking that in Java". However: When I manage to allocate the time for the remaining update, I'll look at 'the current state (at that time)' and do the update accordingly, but will keep in mind that theye PRs may be merged in the next few days and the update here may have to take them into account. |
|
#1155 is a name change. It will only affect this PR if the DFD's channelType is exposed in the Java wrapper. It is not exposed in the Python wrapper so I think it is not exposed in Java either. It is a very low level thing. The only part of #1157 will affect this is the removal of the rec2020 field. The rest of it is new functionality in I'd prefer to prepare this for #1157 now. After #1157 and this are merged I will rebase #1155 and take care of any necessary fixes in the wrappers. Since I didn't need any in Python or JavaScript I'll be surprised to find any in Java. |
|
I've just merged #1157. Please remove the rec2020 field from this and remove the draft status so I can merge it. |
I think the remaining tasks here are
and I'll try to do this "soon" (maybe during the weekend), and drop a note here when it's done. |
Agreed. |
|
The last commits...
I wanted to add some test that at least "touch" the new HDR functionality in some way. The commit 73db397 adds a test that reads HDR input and checks for the Unrelated note:
at KTX-Software/lib/src/texture2.c Line 1998 in 20211a2 Given that it's only a minor comment fix, I can change this as part of this PR, if that's OK. |
|
Thank you very much for your hard work on this. |
The PR #1100 added HDR support.
This PR updates the Java bindings accordingly so fixes #1144.
What it does:
UASTC_HDR_6X6_INTERMEDIATEsupercompression scheme enum valueRGBA_HALF/ASTC_HDR_4x4_RGBA/ASTC_HDR_6x6_RGBAtranscode format enum valuesKtxBasisCodecclass, corresponding to the codec enum, as requested in a review commentktxBasisParams(likeuastcHDRUltraQuantetc) to theKtxBasisParamsclassKTX_API ktx_bool_t KTX_APIENTRY ktxTexture1_IsTranscodable(ktxTexture1* This);functions (for
KtxTexture1andKtxTexture2)getColorModel,getTransferFunction,isHDR.