KTX-Software icon indicating copy to clipboard operation
KTX-Software copied to clipboard

ktx create not doing verbatim copy of EXR half_float. Also "Not enough precision" error message is unclear.

Open MarkCallow opened this issue 11 months ago • 3 comments

When loading an EXR file with half float channels and --format R32B32B32A32_SFLOAT ktx create raises an error

Not enough precision to convert 16 bit input to 32 bit output for R32B32B32A32_SFLOAT.

However ExrInput::ReadImage happily accepts a targetBitDepth of 32 which it passes on to LoadEXRImageFromMemory which returns float values. If I comment out this code https://github.com/KhronosGroup/KTX-Software/blob/b6190046bbf71e0fbf9e7a964f6ee39d726b16be/tools/ktx/command_create.cpp#L1406 where the error is raised then ktx create successfully creates the texture. So two questions:

  1. Why is it attempting to convert to float when it already has float? It requested float and received float.
  2. "Not enough precision"? TinyEXR is managing to do the conversion. Is this poor error message wording and ktx create simply does not implement such conversions?

The half-float file I've been using for testing is https://raw.githubusercontent.com/AcademySoftwareFoundation/openexr-images/main/ScanLines/Blobbies.jpg.

MarkCallow avatar Jan 01 '25 09:01 MarkCallow

It was an explicit requirement for the tool to do verbatim copies on create/extract, that is for a 16-bit float input file one must use a 16-bit float format. Pretty much it was a general goal to have 1:1 mapping whenever possible (there are exceptions like depth/stencil, packed float, certain integer, etc. formats, of course). Sure, it would be possible to convert, both up (FP32) or down (e.g. UNORM), so if it is deemed preferable to do conversion then it should be straightforward to add support.

aqnuep avatar Jan 03 '25 14:01 aqnuep

Verbatim copies is fine. With that in mind there are two issues with the current code:

  1. A verbatim copy is not being done. The half float input is being converted to float by TinyEXR then it is converted back to half-float by CommandCreate::convert. Float is being requested due to https://github.com/KhronosGroup/KTX-Software/blob/3038f7cae9c875c0060ebb3d809e031ce088707f/tools/ktx/command_create.cpp#L1271-L1274 inputImageFormat is exr_float because of https://github.com/KhronosGroup/KTX-Software/blob/3038f7cae9c875c0060ebb3d809e031ce088707f/tools/imageio/exr.imageio/exrinput.cc#L122-L126 No exr_half type is declared. Probably one needs to be added.
  2. The error message is misleading. My first reaction was "Why doesn't 16-bit input have enough precision to convert to 32-bit output? Something is not right in the code." Due to time pressure I immediately stopped thinking about it and used a half-float output format. Later I returned to investigate. The message must more clearly indicate that a different output format should be used.

I'll update the title of this issue.

MarkCallow avatar Jan 05 '25 08:01 MarkCallow

The reason why it is loaded as a FP32 is that some other parameters may require transforming the data in various fashions (e.g. generate mipmaps) and there's no native FP16 support to be able to implement it on the CPU, particularly in the image manipulation library KTX inherited from basisu. Adding that would require pulling in an IEEE compliant half library that is actually able to do math on halfs and even that would be less efficient than the current solution.

Functionally, this works today, i.e. you're able to create R16G16B16A16_SFLOAT from an FP16 EXR.

If you'd like to add "native" (of course, emulated) FP16 support to the imaging stack, you can use a library like the following: https://github.com/ROCm/HIP-CPU/blob/master/external/half/half.hpp

aqnuep avatar Jan 06 '25 14:01 aqnuep