Vulkan-ValidationLayers icon indicating copy to clipboard operation
Vulkan-ValidationLayers copied to clipboard

`CmdCopyImage` erroneously allows copies between incompatible compressed formats

Open Rua opened this issue 3 years ago • 4 comments

The code here is meant to check whether the formats of the source and destination image in vkCmdCopyImage are compatible: https://github.com/KhronosGroup/Vulkan-ValidationLayers/blob/70835ff23b3f513d86fcb918ab8352a8f9ee8c1f/layers/buffer_validation.cpp#L3178-L3184

If neither format is a depth/stencil format, then it performs this check based on the texel block sizes of each format. But this is not correct. In the description section of vkCmdCopyImage the spec states two things:

The formats of srcImage and dstImage must be compatible. Formats are compatible if they share the same class, as shown in the Compatible Formats table. Depth/stencil formats must match exactly.

vkCmdCopyImage allows copying between size-compatible compressed and uncompressed internal formats. Formats are size-compatible if the texel block size of the uncompressed format is equal to the texel block size of the compressed format.

From this I gather that if both formats are compressed/blocked, the size compatibility rule does not hold, and compatibility is determined by the Compatible Formats table.

Rua avatar Mar 28 '22 17:03 Rua

I think doing something a vkCmdCopyImage between a VK_FORMAT_BC3_UNORM_BLOCK and VK_FORMAT_BC5_UNORM_BLOCK is legal... checking on this

So I guess might be safe to assume this is a valid issue and FormatElementSize is only used if both are not FormatIsCompressed (unit tests as always help)

sfricke-samsung avatar Mar 29 '22 03:03 sfricke-samsung

If I'm wrong, then GetAdjustedDestImageExtent may need changing as it currently doesn't account for blocked-to-blocked copies if the block sizes are different.

Rua avatar Mar 29 '22 08:03 Rua

Also may be important: https://github.com/KhronosGroup/Vulkan-Docs/issues/1807

Rua avatar Mar 29 '22 08:03 Rua

So I think the right course of action is to make a issue against Vulkan Docs to clarify it, because if it allowed would want to make sure there is CTS coverage.

I guess also not sure the exact use case of wanting to do on-the-fly recompressing of a texture in a first place. Most people will have spent time offline deciding which BC or ASTC variation they want to my knowledge

sfricke-samsung avatar Mar 29 '22 11:03 sfricke-samsung