bkaradzic / bgfx

Cross-platform, graphics API agnostic, "Bring Your Own Engine/Framework" style rendering library.
https://bkaradzic.github.io/bgfx/overview.html
BSD 2-Clause "Simplified" License
15.05k stars 1.94k forks source link

Example 08-update: Non 1x1 texel block size ImageFormats get copied incorrectly from buffer to image in Vulkan. #3311

Open mcourteaux opened 4 months ago

mcourteaux commented 4 months ago

Describe the bug For example BC2 has a texel block size of 4x4, however it seems that BGFX and BIMG are trying to produce mipmaps for this format all the way until 1x1. This gets detected by the Vulkan validation layer as an error.

To Reproduce Steps to reproduce the behavior:

  1. Compile with make clean && make -j8 config=debug64 example-08-update
  2. Run the Vulkan backend with validation layer on. If you wish, you may put a breakpoint on renderer_vk.cpp:695.

You should see excerpts like this:

For BC2:

../../../src/renderer_vk.cpp (683): BGFX ---E-       CommandBuffer, Validation, 0: Validation Error: [ VUID-vkCmdCopyBufferToImage-imageSubresource-07971 ] Object 0: handle = 0x7fffe8442b50, type = VK_OBJECT_TYPE_COMMAND_BUFFER; Object 1: handle = 0xb8a507000000007c, name = TH 14: textures/texture_compression_bc2.ktx, type = VK_OBJECT_TYPE_IMAGE; | MessageID = 0x9a6a1fcf | vkCmdCopyBufferToImage(): pRegions[0] imageOffset.x (0) and (imageExtent.width + imageOffset.x) (4) must be >= zero or <= image subresource width (2). The Vulkan spec states: For each element of pRegions, imageOffset.x and (imageExtent.width + imageOffset.x) must both be greater than or equal to 0 and less than or equal to the width of the specified imageSubresource of dstImage (https://vulkan.lunarg.com/doc/view/1.3.283.0/linux/1.3-extensions/vkspec.html#VUID-vkCmdCopyBufferToImage-imageSubresource-07971)
../../../src/renderer_vk.cpp (683): BGFX ---E-       CommandBuffer, Validation, 0: Validation Error: [ VUID-vkCmdCopyBufferToImage-imageSubresource-07972 ] Object 0: handle = 0x7fffe8442b50, type = VK_OBJECT_TYPE_COMMAND_BUFFER; Object 1: handle = 0xb8a507000000007c, name = TH 14: textures/texture_compression_bc2.ktx, type = VK_OBJECT_TYPE_IMAGE; | MessageID = 0x12f1c9cc | vkCmdCopyBufferToImage(): pRegions[0] imageOffset.y (0) and (imageExtent.height + imageOffset.y) (4) must be >= zero or <= image subresource height (2). The Vulkan spec states: For each element of pRegions, imageOffset.y and (imageExtent.height + imageOffset.y) must both be greater than or equal to 0 and less than or equal to the height of the specified imageSubresource of dstImage (https://vulkan.lunarg.com/doc/view/1.3.283.0/linux/1.3-extensions/vkspec.html#VUID-vkCmdCopyBufferToImage-imageSubresource-07972)
../../../src/renderer_vk.cpp (683): BGFX ---E-       CommandBuffer, Validation, 0: Validation Error: [ VUID-vkCmdCopyBufferToImage-imageSubresource-07971 ] Object 0: handle = 0x7fffe8442b50, type = VK_OBJECT_TYPE_COMMAND_BUFFER; Object 1: handle = 0xb8a507000000007c, name = TH 14: textures/texture_compression_bc2.ktx, type = VK_OBJECT_TYPE_IMAGE; | MessageID = 0x9a6a1fcf | vkCmdCopyBufferToImage(): pRegions[0] imageOffset.x (0) and (imageExtent.width + imageOffset.x) (4) must be >= zero or <= image subresource width (1). The Vulkan spec states: For each element of pRegions, imageOffset.x and (imageExtent.width + imageOffset.x) must both be greater than or equal to 0 and less than or equal to the width of the specified imageSubresource of dstImage (https://vulkan.lunarg.com/doc/view/1.3.283.0/linux/1.3-extensions/vkspec.html#VUID-vkCmdCopyBufferToImage-imageSubresource-07971)
../../../src/renderer_vk.cpp (683): BGFX ---E-       CommandBuffer, Validation, 0: Validation Error: [ VUID-vkCmdCopyBufferToImage-imageSubresource-07972 ] Object 0: handle = 0x7fffe8442b50, type = VK_OBJECT_TYPE_COMMAND_BUFFER; Object 1: handle = 0xb8a507000000007c, name = TH 14: textures/texture_compression_bc2.ktx, type = VK_OBJECT_TYPE_IMAGE; | MessageID = 0x12f1c9cc | vkCmdCopyBufferToImage(): pRegions[0] imageOffset.y (0) and (imageExtent.height + imageOffset.y) (4) must be >= zero or <= image subresource height (1). The Vulkan spec states: For each element of pRegions, imageOffset.y and (imageExtent.height + imageOffset.y) must both be greater than or equal to 0 and less than or equal to the height of the specified imageSubresource of dstImage (https://vulkan.lunarg.com/doc/view/1.3.283.0/linux/1.3-extensions/vkspec.html#VUID-vkCmdCopyBufferToImage-imageSubresource-07972)

Scroll a lot to the right, and you'll see the complaint regarding the extent of the image. So bimg does produce mipmaps that respect this minimum block size, but Vulkan is not having it, as the mipmap itself is determined to be smaller than that.

Same goes for BC3:

../../../src/renderer_vk.cpp (683): BGFX ---E-       CommandBuffer, Validation, 0: Validation Error: [ VUID-vkCmdCopyBufferToImage-imageSubresource-07971 ] Object 0: handle = 0x7fffe8442b50, type = VK_OBJECT_TYPE_COMMAND_BUFFER; Object 1: handle = 0x10ac5000000007e, name = TH 15: textures/texture_compression_bc3.ktx, type = VK_OBJECT_TYPE_IMAGE; | MessageID = 0x9a6a1fcf | vkCmdCopyBufferToImage(): pRegions[0] imageOffset.x (0) and (imageExtent.width + imageOffset.x) (4) must be >= zero or <= image subresource width (2). The Vulkan spec states: For each element of pRegions, imageOffset.x and (imageExtent.width + imageOffset.x) must both be greater than or equal to 0 and less than or equal to the width of the specified imageSubresource of dstImage (https://vulkan.lunarg.com/doc/view/1.3.283.0/linux/1.3-extensions/vkspec.html#VUID-vkCmdCopyBufferToImage-imageSubresource-07971)
../../../src/renderer_vk.cpp (683): BGFX ---E-       CommandBuffer, Validation, 0: Validation Error: [ VUID-vkCmdCopyBufferToImage-imageSubresource-07972 ] Object 0: handle = 0x7fffe8442b50, type = VK_OBJECT_TYPE_COMMAND_BUFFER; Object 1: handle = 0x10ac5000000007e, name = TH 15: textures/texture_compression_bc3.ktx, type = VK_OBJECT_TYPE_IMAGE; | MessageID = 0x12f1c9cc | vkCmdCopyBufferToImage(): pRegions[0] imageOffset.y (0) and (imageExtent.height + imageOffset.y) (4) must be >= zero or <= image subresource height (2). The Vulkan spec states: For each element of pRegions, imageOffset.y and (imageExtent.height + imageOffset.y) must both be greater than or equal to 0 and less than or equal to the height of the specified imageSubresource of dstImage (https://vulkan.lunarg.com/doc/view/1.3.283.0/linux/1.3-extensions/vkspec.html#VUID-vkCmdCopyBufferToImage-imageSubresource-07972)
../../../src/renderer_vk.cpp (683): BGFX ---E-       CommandBuffer, Validation, 0: Validation Error: [ VUID-vkCmdCopyBufferToImage-imageSubresource-07971 ] Object 0: handle = 0x7fffe8442b50, type = VK_OBJECT_TYPE_COMMAND_BUFFER; Object 1: handle = 0x10ac5000000007e, name = TH 15: textures/texture_compression_bc3.ktx, type = VK_OBJECT_TYPE_IMAGE; | MessageID = 0x9a6a1fcf | vkCmdCopyBufferToImage(): pRegions[0] imageOffset.x (0) and (imageExtent.width + imageOffset.x) (4) must be >= zero or <= image subresource width (1). The Vulkan spec states: For each element of pRegions, imageOffset.x and (imageExtent.width + imageOffset.x) must both be greater than or equal to 0 and less than or equal to the width of the specified imageSubresource of dstImage (https://vulkan.lunarg.com/doc/view/1.3.283.0/linux/1.3-extensions/vkspec.html#VUID-vkCmdCopyBufferToImage-imageSubresource-07971)
../../../src/renderer_vk.cpp (683): BGFX ---E-       CommandBuffer, Validation, 0: Validation Error: [ VUID-vkCmdCopyBufferToImage-imageSubresource-07972 ] Object 0: handle = 0x7fffe8442b50, type = VK_OBJECT_TYPE_COMMAND_BUFFER; Object 1: handle = 0x10ac5000000007e, name = TH 15: textures/texture_compression_bc3.ktx, type = VK_OBJECT_TYPE_IMAGE; | MessageID = 0x12f1c9cc | vkCmdCopyBufferToImage(): pRegions[0] imageOffset.y (0) and (imageExtent.height + imageOffset.y) (4) must be >= zero or <= image subresource height (1). The Vulkan spec states: For each element of pRegions, imageOffset.y and (imageExtent.height + imageOffset.y) must both be greater than or equal to 0 and less than or equal to the height of the specified imageSubresource of dstImage (https://vulkan.lunarg.com/doc/view/1.3.283.0/linux/1.3-extensions/vkspec.html#VUID-vkCmdCopyBufferToImage-imageSubresource-07972)

BC1:

../../../src/renderer_vk.cpp (683): BGFX ---E-       CommandBuffer, Validation, 0: Validation Error: [ VUID-vkCmdCopyBufferToImage-imageSubresource-07971 ] Object 0: handle = 0x7fffe8442b50, type = VK_OBJECT_TYPE_COMMAND_BUFFER; Object 1: handle = 0xb3ffc1000000007a, name = TH 13: textures/texture_compression_bc1.ktx, type = VK_OBJECT_TYPE_IMAGE; | MessageID = 0x9a6a1fcf | vkCmdCopyBufferToImage(): pRegions[0] imageOffset.x (0) and (imageExtent.width + imageOffset.x) (4) must be >= zero or <= image subresource width (2). The Vulkan spec states: For each element of pRegions, imageOffset.x and (imageExtent.width + imageOffset.x) must both be greater than or equal to 0 and less than or equal to the width of the specified imageSubresource of dstImage (https://vulkan.lunarg.com/doc/view/1.3.283.0/linux/1.3-extensions/vkspec.html#VUID-vkCmdCopyBufferToImage-imageSubresource-07971)
../../../src/renderer_vk.cpp (683): BGFX ---E-       CommandBuffer, Validation, 0: Validation Error: [ VUID-vkCmdCopyBufferToImage-imageSubresource-07972 ] Object 0: handle = 0x7fffe8442b50, type = VK_OBJECT_TYPE_COMMAND_BUFFER; Object 1: handle = 0xb3ffc1000000007a, name = TH 13: textures/texture_compression_bc1.ktx, type = VK_OBJECT_TYPE_IMAGE; | MessageID = 0x12f1c9cc | vkCmdCopyBufferToImage(): pRegions[0] imageOffset.y (0) and (imageExtent.height + imageOffset.y) (4) must be >= zero or <= image subresource height (2). The Vulkan spec states: For each element of pRegions, imageOffset.y and (imageExtent.height + imageOffset.y) must both be greater than or equal to 0 and less than or equal to the height of the specified imageSubresource of dstImage (https://vulkan.lunarg.com/doc/view/1.3.283.0/linux/1.3-extensions/vkspec.html#VUID-vkCmdCopyBufferToImage-imageSubresource-07972)
../../../src/renderer_vk.cpp (683): BGFX ---E-       CommandBuffer, Validation, 0: Validation Error: [ VUID-vkCmdCopyBufferToImage-imageSubresource-07971 ] Object 0: handle = 0x7fffe8442b50, type = VK_OBJECT_TYPE_COMMAND_BUFFER; Object 1: handle = 0xb3ffc1000000007a, name = TH 13: textures/texture_compression_bc1.ktx, type = VK_OBJECT_TYPE_IMAGE; | MessageID = 0x9a6a1fcf | vkCmdCopyBufferToImage(): pRegions[0] imageOffset.x (0) and (imageExtent.width + imageOffset.x) (4) must be >= zero or <= image subresource width (1). The Vulkan spec states: For each element of pRegions, imageOffset.x and (imageExtent.width + imageOffset.x) must both be greater than or equal to 0 and less than or equal to the width of the specified imageSubresource of dstImage (https://vulkan.lunarg.com/doc/view/1.3.283.0/linux/1.3-extensions/vkspec.html#VUID-vkCmdCopyBufferToImage-imageSubresource-07971)
../../../src/renderer_vk.cpp (683): BGFX ---E-       CommandBuffer, Validation, 0: Validation Error: [ VUID-vkCmdCopyBufferToImage-imageSubresource-07972 ] Object 0: handle = 0x7fffe8442b50, type = VK_OBJECT_TYPE_COMMAND_BUFFER; Object 1: handle = 0xb3ffc1000000007a, name = TH 13: textures/texture_compression_bc1.ktx, type = VK_OBJECT_TYPE_IMAGE; | MessageID = 0x12f1c9cc | vkCmdCopyBufferToImage(): pRegions[0] imageOffset.y (0) and (imageExtent.height + imageOffset.y) (4) must be >= zero or <= image subresource height (1). The Vulkan spec states: For each element of pRegions, imageOffset.y and (imageExtent.height + imageOffset.y) must both be greater than or equal to 0 and less than or equal to the height of the specified imageSubresource of dstImage (https://vulkan.lunarg.com/doc/view/1.3.283.0/linux/1.3-extensions/vkspec.html#VUID-vkCmdCopyBufferToImage-imageSubresource-07972)

Expected behavior

To quote Nicol Bolas from here:

The limitation with these formats is that the size of the block compressed texture needs to be a multiple of 4.

No such limitation exists in Vulkan. If you use a block-based image format, and the size is not a multiple of the block size, that is fine. Vulkan will ensure that pixels from blocks that are only partially within the image size will not be accessed (by most image accessing processes, at least).

However, this does mean that, for a given image subresource, there is a distinction between the actual size and the size of the data (based on the block count). Again, this is fine in Vulkan, and the API makes it clear when which one matters.

So, if I understand correctly, you should be able to generate the mipmaps all the way down, but we just need to adjust the size of the Region indicated by the VkCopyBufferToImage struct? We'd have to actually lie refer to the target of the image as if it's cropped to the sub-texel-block-size region.

After some investigation, it seems that the example itself is doing this (pay attention to the first two lines, with bx::max()):

https://github.com/bkaradzic/bgfx/blob/9547e798675330df5dacbed8a72feec012ff3ed5/examples/08-update/update.cpp#L183-L205

Deleting those fixes the issue for me on Vulkan. OpenGL is not complaining much in either case. Can't test D3D.

mcourteaux commented 4 months ago

This is the patch that I described in above:

diff --git a/examples/08-update/update.cpp b/examples/08-update/update.cpp
index 109efb0ff..200c92e20 100644
--- a/examples/08-update/update.cpp
+++ b/examples/08-update/update.cpp
@@ -173,18 +173,11 @@ bgfx::TextureHandle loadTextureWithUpdate(const char* _filePath, uint64_t _flags
                                        , NULL
                                        );

-                               const bimg::ImageBlockInfo& blockInfo = getBlockInfo(imageContainer->m_format);
-                               const uint32_t blockWidth  = blockInfo.blockWidth;
-                               const uint32_t blockHeight = blockInfo.blockHeight;
-
                                uint32_t width  = imageContainer->m_width;
                                uint32_t height = imageContainer->m_height;

                                for (uint8_t lod = 0, num = imageContainer->m_numMips; lod < num; ++lod)
                                {
-                                       width  = bx::max(blockWidth,  width);
-                                       height = bx::max(blockHeight, height);
-
                                        bimg::ImageMip mip;

                                        if (bimg::imageGetRawData(*imageContainer, 0, lod, imageContainer->m_data, imageContainer->m_size, mip))
bkaradzic commented 4 months ago

Patch is nonsensical for block compressed textures. Requirements is full mip chain but since 1x1 can't exist with block compression, it's to keep 4x4 (or minimum block size) until you reach 1x1.

Also you should test changes like this with D3D, and other renders.