vkCmdCopyBufferToImage with the BC3_SRGB_BLOCK format when imageExtent + imageOffset equals the subresource dimensions, but is less than the compressed texel block dimensions, causes Metal assertion failure about moltenvk HOT 5 CLOSED

@hideyhole

I'm not clear on what your scenario is. Are you attempting to copy less than a single compressed block, and MoltenVK is incorrectly rounding down to zero extent?

Can you clarify what you expect the correct behaviour to be?

Or better yet...since it looks like you have delved into the code already...if you have a good idea of what the behaviour should be...please feel free to submit a Pull Request with a fix.

from moltenvk.

Are you attempting to copy less than a single compressed block, and MoltenVK is incorrectly rounding down to zero extent?

Yes. Certain texture exporters that export to the DDS format export DDS files with a mip chain that fully extends all the way down to 1x1. When we try to load these textures in by first uploading to a linear staging buffer and then vkCmdCopyBufferToImage-ing to an optimal-tiled image, MoltenVK crashes in the BlitCommandEncoder's copyFromBuffer call because the sourceBytesPerRow parameter, as calculated by the call to mvkMTLPixelFormatBytesPerRow, is 0, since MoltenVK incorrectly rounds down the number of blocks to be copied to 0 (because of the integer division). This is a violation of the spec, as far as I can tell.

Given Microsoft's documentation on this, I think it might be safe to assume that the texture tools that do this already adds the padding bytes (the DDS should be invalid, otherwise), and hence, the source buffer should also have them. In that case, the fix would be straightforward - simply clamp the number of blocks to be copied to 1.

However, as far as the Vulkan spec is concerned, there's no such requirement (AFAIK). Which means that the assumption that the source buffer should already have padding is not safe, and simply clamping the number of blocks to 1 could result in a wrong texture and/or a segfault.

Therefore, in this situation, the safe way to handle this would be to copy the individual texels to an internal MTLBuffer, add the padding bytes, and then use this internal buffer and the clamped number of blocks in the BlitCommandEncoder's copyFromBuffer call.

from moltenvk.

@hideyhole

The end of Section 18.4 includes a discussion about how to interpret VkBufferImageCopy when copying compressed formats.

It provides pseudo code that includes an integer division (rounding down) of the imageExtent components.

It also describes:

• "the addressing math operates on whole compressed texel blocks"
• "Copying to or from block-compressed images is typically done in multiples of the compressed texel block size.".
• "the imageExtent must be a multiple of the compressed texel block dimension"

It does provide one exception to the last requirement...when imageOffset + imageExtent is equal to the image sub resource size.

AFAICT...MoltenVK is following this design. However...in copying small mip layers...we may be encountering that exception rule...and MoltenVK may not be handling that properly. Perhaps a rounding up on the division, with a clamp to subresource size would be more accurate.

Can you have a look at that section of the spec and give my your thoughts on design implementation, please?

from moltenvk.

@billhollings, Thinking further through this, I believe that it can be safely assumed that the source buffer holds enough bytes for the dimensions rounded up to a full block, since the source data has to be the raw bytes of a BC compressed image, which contain entire blocks and not texels. In the event that one of the mipmap levels has sub-block dimensions, the image file should still contain the bytes for the whole block, with the padding texels/bytes added as described above, and therefore, so should the source buffer.

In that case, simply clamping the number of blocks to 1 should fix this particular issue.

In your comments, you identified a further issue with the copy operation, which is that texels near the edge of the image subresource could be left uninitialized/populated with garbage if the subresource dimensions are not exactly divisible by the block dimensions. Rounding the division up should fix that issue. In fact, it should fix both issues.

from moltenvk.

Fixed in PR #122.

from moltenvk.