search

KhronosGroup / KTX-Specification

KTX file format source
Other
69 stars 12 forks source link

Refine mipPadding section #180

Closed lexaknyazev closed 2 years ago

lexaknyazev commented 2 years ago

Fixes #175.

MarkCallow commented 2 years ago

I have modified libktx to follow these new rules and a bunch of tests are failing. I have to investigate why...

MarkCallow commented 2 years ago

I have modified libktx to follow these new rules and a bunch of tests are failing. I have to investigate why...

It was a stupid error on my part. After fixing it, I added some tests for the depth stencil formats and immediately discovered that libktx was not giving them the KTX-specified texel block size. I've fixed that. I will create a PR. When it is ready to merge, I'll merge this spec update at the same time.

MarkCallow commented 2 years ago

I have realized that ktxTexture_VkUploadEx in libktx does not support the depth stencil formats. In looking at implementing support I find there is no easy way to upload these formats to Vulkan. VkCmdCopyBufferToImage does not have any way to specify a stride between pixels and the spec. says

When copying to or from a depth or stencil aspect, the data in buffer memory uses a layout that is a (mostly) tightly packed representation of the depth or stencil data.

Unless I am mistaken or missing some way to specify a stride the only way to upload such data to Vulkan is to have multiple planes. On the other hand the packed VK_FORMAT_D24_UNORM_S8_UINT and VK_FORMAT_D32_SFLOAT_S8_UINT formats can be uploaded to OpenGL.

Should we continue to support VK_FORMAT_D16_UNORM_S8_UINT and VK_FORMAT_X8_D24_UNORM_PACK32 which can't be uploaded to OpenGL?

Should we make all depth-stencil multiplane thus making the OpenGL loader rather than the Vulkan loader have to do pixel shuffling?

lexaknyazev commented 2 years ago

I find there is no easy way to upload these formats to Vulkan.

AFAIR, we agreed that Vulkan loaders would need to split components manually before upload in that case.

Should we continue to support VK_FORMAT_D16_UNORM_S8_UINT and VK_FORMAT_X8_D24_UNORM_PACK32 which can't be uploaded to OpenGL?

Overall, I think that KTX should not be too limited by GPU APIs, especially by OpenGL in 2022.

Our format mapping section suggests using GL_DEPTH_COMPONENT24 for VK_FORMAT_X8_D24_UNORM_PACK32, which may be incorrect. We should confirm what happens when an application makes a texture upload with the following args: internalFormat: GL_DEPTH_COMPONENT24, format: GL_DEPTH_COMPONENT, type: GL_UNSIGNED_INT. At this point, I think that GL drivers would rescale 32 bits to 24 thus leading to incorrect results.

lexaknyazev commented 2 years ago

Should we make all depth-stencil multiplane thus making the OpenGL loader rather than the Vulkan loader have to do pixel shuffling?

That would technically be a breaking change requiring bumping the major version to 3.

MarkCallow commented 2 years ago

AFAIR, we agreed that Vulkan loaders would need to split components manually before upload in that case.

Yes. The only alternative I see is to create a VkBufferImageCopy for each pixel, an even less attractive proposition that splitting before upload.

Our format mapping section suggests using GL_DEPTH_COMPONENT24 for VK_FORMAT_X8_D24_UNORM_PACK32, which may be incorrect.

I agree with your analysis of what GL drivers will do. Is there an alternative mapping?

That would technically be a breaking change requiring bumping the major version to 3.

Yes I know. We'll have to stick with what we've got. However I don't think anyone is using these formats yet given the lack of bug reports over the missing functionality in libktx.

lexaknyazev commented 2 years ago

Is there an alternative mapping?

No, I'll update the relevant section. Please see #182.