KhronosGroup / Vulkan-Docs

The Vulkan API Specification and related tools
Other
2.78k stars 466 forks source link

Texture-to-texture copy issue with compressed formats #1005

Open Jiawei-Shao opened 5 years ago

Jiawei-Shao commented 5 years ago

It seems there is an issue in Vulkan SPEC about the texture-to-texture copies (vkCmdCopyImage) with compressed formats with the parameter "extent" fitting in one subresource but not fitting in another.

For example, there are two textures with same BC format whose sizes are both 60x60, when I want to copy a 16x16 region from the location (0, 0) of level 0 of one texture into the location (0, 0) of the level 2 of another texture, I find I fail to specify a valid "extent" in vkCmdCopyImage():

So in this case we cannot choose a valid "extent" that is not against Vulkan SPEC. Is it a bug in the SPEC or intended to disallow such usage in Vulkan?

Note that in D3D we can do such kind of copy because D3D always uses the physical size instead of the virtual size of a texture in BC formats in the copies, so such copies are supported in the hardware which is compatible to D3D.

mark-lunarg commented 5 years ago

@daveh-lunarg, can you give this a quick once-over, please?

daveh-lunarg commented 5 years ago

Without actually looking at VL code:

First case looks right to me, MIP lvl 2 is 15x15 so can't specify 16x16 (even though it's stored in a 16x16 compressed block).

Second one looks like a validation error. It should compare the pRegion to the mip 2 extent (15x15), but the error message implies it compared to the mip 0 extent (60).

Jiawei-Shao commented 5 years ago

The SPEC only says "When copying between compressed and uncompressed formats the extent members represent the texel dimensions of the source image and not the destination. ", and there is no such special statement over the copies between compressed formats, so I don't think it a bug in Vulkan Validation layer but an issue in Vulkan SPEC.

daveh-lunarg commented 5 years ago

When copying between like formats, the dimensions are always in texels. When copying compressed->uncompressed or uncompressed->compressed it's potentially ambiguous because the number of texels differs between source & dest - so the spec has a statement just for that case to clarify that the copy extent is in source texels. In the case above the texel size (i.e. bits-per-texel) is the same for source and dest, so no disambiguation is needed. Possibly the spec could use some more discussion on the subject, but it doesn't appear to be incorrect as-is, to me.

From the description above (Both images compressed, 60x60, with mips, copying 15x15 texels from first image mip-0 to second image mip-2) I would expect the extent (15, 15, 1) case to work. The mip-2 dimensions are 15x15x1, so the error message that says must equal the image subresource width (60) implies to me the VL code is comparing against the full mip-0 width of 60, when it should be looking at the mip-2 width of 15.

Jiawei-Shao commented 5 years ago

The reason for the error message in case (2) is (15, 15, 1) is neither "a multiple of a compressed block width" nor " (extent.width + srcOffset.x) must equal the source image subresource width" (it seems you miss the first statement).

The VL code follows the SPEC as it first tests if (15, 15, 1) is "a multiple of a compressed block width" then tests "(extent.width + srcOffset.x) must equal the source image subresource width". You say "it should be looking at the mip-2 width of 15", I think VL does this check for the destination texture, but Vulkan SPEC also requires the Extent follow this rule for the source texture, that is the true root cause of this issue, which is in Vulkan SPEC, not in VL code.

daveh-lunarg commented 5 years ago

Sorry, I didn't read the VL error text closely enough - I assumed both error conditions noted were complaints about the dest image dimensions.

As I understand it, the only non-block-dimension regions accessible for copying in Vulkan compressed images are regions that include the final partial block (in any combination of X, Y, and Z) and this has the effect of disallowing the copies between mip levels you describe above. (Unrelated to the mip levels, actually - but that's where it's most likely to come up.)

So, I'm now convinced that this isn't a VL issue. To add the capability to copy from non-edge partial-block regions in a compressed source image would require a spec change. IMO this isn't a bug in the spec, but a design decision - but in any case the right place to raise the issue is in the -Docs repo.

(@oddhack) Transferring this back to Vulkan-Docs.

jeffbolznv commented 5 years ago

Added the "Resolving Inside Khronos" tag. We'll probably try to address this by expressing the VUs in terms of texel blocks rather than texels, but it may take some time to get it done.

manas-kulkarni commented 3 years ago

Any update on this? Looks like it complains either way (cannot be multiple of 4 and should be a multiple of 4).

Kangz commented 3 years ago

Any updates on this? At the moment we work around the issue in Dawn with an extra copy through a buffer, but would like to disable the workaround after the spec is fixed (for drivers passing new CTS tests for that behavior).

sfricke-samsung commented 3 years ago

This issue has been just lacking a champion to drive the changes... I have recently raised a few related other formats discussion internally and plan to try to address this issue as well