Fix KTX2 header initialization in included BasisU code

[IceLuna]

KTX2 header wasn't initialized correctly which led to bugs. In my case, this code was succeeding, but ktxTexture->baseDepth was containing garbage because of it.

basisu::basis_compressor::error_code basisResult = basisCompressor.process();

if (basisResult == basisu::basis_compressor::cECSuccess)
{
    const basisu::vector<uint8_t>* ktx2Data = &basisCompressor.get_output_ktx2_file();
				
    ktxTexture2* ktxTexture;
    KTX_error_code result = ktxTexture2_CreateFromMemory(ktx2Data->data(), ktx2Data->size(), KTX_TEXTURE_CREATE_LOAD_IMAGE_DATA_BIT, &ktxTexture);
    if (result != KTX_SUCCESS)
    {
        throw std::runtime_error("Could not load the requested image file.");
    }
}

I'm not sure why this was changed though
https://github.com/KhronosGroup/KTX-Software/pull/687/files#diff-00fb4e838b1a5a861eb92b498116262bbf956d06d6ae0bab54b1b30fb4e2acbc

[CLAassistant]

All committers have signed the CLA.

[MarkCallow]

Two questions:

1. How are you encountering this code. The function you have changed is not used by KTX-Software which long pre-dates addition of ktx2 support to BasisU. We did not update KTX-Software on the basis of "if it ain't broke don't fix it."
2. The change was prompted by compiler warnings. basist::ktx2_header header = {}; is now the recommended way of initializing structures in C++. If the header is truly not being initialized in your case that prompts a third question
3. What compiler are you using?

[IceLuna]

I cloned this repository, built it as a static library, and linked it to my project. Since KTX already contains a copy of BasisU, I'm using it to convert a .png to .ktx2 for further compression by KTX.

As far as I understand, the struct is not initialized to 0 because it's not a POD struct, because struct ktx2_header contains struct packed_uint. And packed_uint has user defined constructors and copy operators, and that's the reason = {} does nothing.

I'm using visual studio 2022

[MarkCallow]

I still do not understand how you end up calling this code. libktx does not use it. When it uses one of the BasisU codecs it has it create a .basis file in memory which it converts to a ktxTexture2 object. It does not call function containing the line of code you have changed. If you are calling this API directly that is unsupported. It is not exposed in ktx.h nor is it exported when building a DLL. Since libktx doesn't use this code it may very possibly be removed in a future release.

You can use ktx create to make a ktx2 file from png file(s).

[IceLuna]

I guess I misused the library. I didn't find a way to create a .ktx2 file from a .png using ktx (none of the create functions are taking a pointer to raw png data, at least I didn't find one).

And that's why I thought I needed to convert it to KTX2 using BasisU which is inside KTX-Software\lib\basisu. So I added includes from this BasisU folder to my project and it compiled/linked just fine.

[MarkCallow]

What you are doing could never have worked if you are using this repo's root CMakeLists.txt to build libktx. It does not even include BasisU's PNG loader in the project's source file list and it sets a define when building libktx that causes the BasisU encoder's load_image function to become a no-op that returns false. Did you create your own build set up?

We would not use BasisU's load_image/load_png because it ignores the color space information in the file and leaves telling the compressor whether the data is sRGB or linear up to the user. We have no plan to include image loading in libktx. As libktx supports the full range of Vulkan formats it would need multiple image file format loaders to provide all the necessary possible input formats plus some extensive image processing capabilities. In short, it would be a lot of code. This is best left to applications such as ktx create.

I am curious why there was no compiler warning or error due to the non-POD struct issue. We have warnings as errors in our CI builds, though we disable some in clang and gcc. Are you using the MSVC or clang toolset in VS 2022? The disabled clang warnings do not seem to be any of those I would expect to see due to the non-POD struct issue.

I'm going to merge this so we don't inadvertently push the broken code upstream to the BasisU repo.

[IceLuna]

I didn't use any custom builds. I cloned the repo, generated the project files using CMake (as static library), built the project, copied KTX and BasisU include files to my project, and also linked my project to KTX .lib files.

Ah, I see. Sorry, I incorrectly expressed myself in the last message. I wasn't trying to feed the libraries a pointer to raw png data.
The png data is decoded by stb_image. And those decoded pixels are written to basisu::image for KTX2 file generation, which is later passed to KTX library. So, basically, I didn't use any loaders from BasisU.

Here's what I'm doing currently:

uvec2 size = ...; // Size of an input texture
Buffer imageData = ...; // Decoded data from stbi_image

// Copy stbi data to basisu::image
basisu::image img;
img.resize(size.x, size.y);
auto& pixels = img.get_pixels();
memcpy(pixels.data(), imageData.Data, imageData.Size);

basisu::basis_compressor_params params;
// Set params and init compressor
basisCompressor.process(); // generate ktx2 file
auto ktxFileData = basisCompressor.get_output_ktx2_file(); // get ktx2 data to be passed to `CreateFromMemory`
ktxTexture2_CreateFromMemory(ktxFileData.Data, ktxFileData.Size, KTX_TEXTURE_CREATE_LOAD_IMAGE_DATA_BIT, &texture);

I think I need to make myself more familiar with other KTX tools such as ktx create. Thanks!

Yes, that's a bit strange, I also didn't see any warnings. I'm using MSVC.

[MarkCallow]

The png data is decoded by stb_image. And those decoded pixels are written to basisu::image for KTX2 file generation, which is later passed to KTX library. So, basically, I didn't use any loaders from BasisU.

I see. That makes sense. You can do something very similar via the libktx API. You first have to determine a VkFormat that matches the color type of the PNG file, as modified by whatever parameters you pass to the stb_image decoder, and the color space (sRGB or not) of the PNG file. You create an empty ktxTexture2 in that format using ktxTexture2_Create. Then you can add the image data from your imageData buffer with ktxTexture2_SetImageFromMemory.

stb_image does not provide access to the color space information of a PNG file which is why the KTX tools do not use it. You will have to determine the color space from some other tool. E.g., on macOS you can open the file in Preview then choose Tools->Show Inspector.

Using the KTX tools will be much easier than writing your own.

Yes, that's a bit strange, I also didn't see any warnings. I'm using MSVC.

All level 4 warnings are enabled in MSVC builds so it must be that MSVC does not warn in such circumstance which I would consider a bug.

[IceLuna]

Do I understand correctly, that all I need to do is this?

ktxTextureCreateInfo createInfo{};
createInfo.vkFormat = myVkFormat; // VK_FORMAT_R8G8B8A8_UNORM
createInfo.baseWidth = size.x;
createInfo.baseHeight = size.y;
createInfo.baseDepth = 1;
createInfo.numDimensions = 2;
createInfo.numLevels = mipsCount;
createInfo.numLayers = 1;
createInfo.numFaces = 1;
createInfo.isArray = KTX_FALSE;
createInfo.generateMipmaps = false;

ktxTexture2_Create(&createInfo, KTX_TEXTURE_CREATE_ALLOC_STORAGE, &texture);

uint32_t level = 0, layer = 0, faceSlice = 0;
ktxTexture_SetImageFromMemory(ktxTexture(texture), level, layer, faceSlice, stbiData, size);

// params
ktxTexture2_CompressBasisEx(texture, &params);
ktxTexture2_TranscodeBasis(texture, format, 0);

Seems like it's working. But how do I generate compressed mips? Here I set the image data from stbi_data to be the base level. And now I want to generate, let's say, mip level 1, and mip level 2. Doesn't seem like generateMipmaps is what I want because then I can't specify the desired mip count.

Do I need to generate mips on my side and pass the data using ktxTexture_SetImageFromMemory but to levels 1 and 2?

Thanks!

[MarkCallow]

Do I need to generate mips on my side and pass the data using ktxTexture_SetImageFromMemory but to levels 1 and 2?

Yes. Open issue #464 has requested mipmapping capabilities be added to libktx. We haven't decided yet. It's a big chunk of code, even bigger if it mipmap generation for 3d textures is added. Currently ktx create has the mipmap generation code but only for 2d.

generateMipmaps tells the loader it should generate mipmaps when uploading to a GPU. Sometimes it is done by code in the loader (the case for Vulkan though it does use Vulkan to do the filtering), sometimes by the GPU API (the case for most OpenGL implementations). It will generate a level for every level needed based on the size of the base level.

createInfo.vkFormat = myVkFormat; // VK_FORMAT_R8G8B8A8_UNORM

Be careful that you match the format, UNORM or SRGB to the transfer function of your input image. If they don't match rendering results will look very bad. If you are doing color space conversion avoid doing sRGB to linear (UNORM) conversion as you will likely see banding in the resulting texture.

[IceLuna]

Ok, much thanks for the help!

One last question: currently, compression of HDR textures is not supported, right? I couldn't find any information on how to do it

[MarkCallow]

currently, compression of HDR textures is not supported, right?

Right. We plan to support compression to ASTC HDR soon and I believe Binomial is working on a universal codec for HDR. I've no idea of its ETA.

[IceLuna]

Ok. Thanks again!