KhronosGroup / KTX-Software

KTX (Khronos Texture) Library and Tools
Other
854 stars 226 forks source link

ktx create --depth x have no use #826

Closed yangfengzzz closed 7 months ago

yangfengzzz commented 7 months ago

--depth x will not generate 3d texture with depth x, the ktx files have depth 1.

MarkCallow commented 7 months ago

Which tool are you using? Which version.

yangfengzzz commented 7 months ago

ktx version: v4.3.0-beta1~1 Macos

MarkCallow commented 7 months ago

I asked which tool.

Sorry. Didn't notice you'd changed the title to include this info.

yangfengzzz commented 7 months ago

I use https://github.com/donmccurdy/KTX2-Samples/blob/main/encode.sh to generate 3d texture, but If I change --layers 8 to --depth 8, the depth will still be 1. I can load https://github.com/KhronosGroup/KTX-Software/blob/main/tests/testimages/3dtex_7_reference_u.ktx2 well but I can't generate 3d texture ktx2 files by my own

MarkCallow commented 7 months ago

encode.sh uses --generate-mipmap on all calls to ktx create. If you changed --layers to --depth on, e.g. the "3d texture" case there would have been a fatal error from ktx create as it does not support mipmap generation for 3d textures. I do not see how any texture was created. You will have to post here the exact ktx create command issued.

@yangfengzzz You need to report the following issues to @donmccurdy:

yangfengzzz commented 7 months ago

Yes, I also remove --generate-mipmap but the result ktx still have problem. I don't know why @donmccurdy will use layers to generate 3d texture.

MarkCallow commented 7 months ago

@aqnuep, Please fix. Running with the current directory in the root of a clone of the KTX2-Samples repo, I tested using

ktx create --depth 8 --format R8G8B8A8_SRGB --zstd 18 --assign-oetf srgb --assign-primaries bt709 source/png/slice_*.png ktx2/3d_rgba8.ktx2

And as @yangfengzzz reports the created texture has a depth of 1.

You can ignore all the other arguments beyond depth and can pick any format. The issue is that target being passed to CommandCreate::createTexture has a depth of 1 and that depth is assigned to the created texture. options.depth is not looked at. Later when faceSlice of 1 is passed ktxTexture_GetImageOffset returns KTX_INVALID_OPERATION which, in the debug version, triggers the assert at line 1226 in command_create.cpp. In the release version, it will just continue with each 'faceSlice > 0' failing in the same way.

Clearly there is a hole in the tests here too.

donmccurdy commented 7 months ago

--layers makes an array texture. If he really wants to make a 3d texture as the message at line 32 suggests he needs to use --depth and to remove --generate-mipmap...

I think this was probably an oversight on my part. three.js does not support 3D textures in compressed formats yet, so I have limited ability to test in this area, but I'll fix the encode.sh script in any case. Thanks!

MarkCallow commented 7 months ago

I think this was probably an oversight on my part. three.js does not support 3D textures in compressed formats yet, so I have limited ability to test in this area, but I'll fix the encode.sh script in any case. Thanks!

@donmccurdy you'd best wait until this bug is fixed before fixing encode.sh.

Not sure why three.js would have issues with 3D textures in compressed formats. With the sole exception of the ASTC3D formats which aren't supported in WebGL, the format is independent of whether the texture is 2D or 3D. 3D textures have multiple images (slices) in the selected format.

donmccurdy commented 7 months ago

you'd best wait until this bug is fixed before fixing encode.sh.

:+1: Thanks!

Not sure why three.js would have issues with 3D textures in compressed formats...

No technical issue — we just have not created an API surface for them. Possibly there are still ways for three.js users to construct compressed 3D textures on their own. I've been working related features in https://github.com/mrdoob/three.js/pull/26642, which I think will need updated after the fix here as well.

aqnuep commented 7 months ago

I'll look into this one. We do have tests for the --depth parameter that correctly write the pixelDepth in the file, but there may be some interaction here that isn't handled correctly.