Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Jiawei-Shao opened this issue Jul 10, 2019 · 10 comments
Open

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

Jiawei-Shao opened this issue Jul 10, 2019 · 10 comments

Comments

@Jiawei-Shao
Copy link

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():

  • if extent == (16, 16, 1), then I get the following error message "Dest pRegion[0] with mipLevel [ 2 ], offset [ 0, 0, 0 ], extent [ 16, 16, 1 ] exceeds the destination image dimensions. The spec valid usage text states 'The destination region specified by each element of pRegions must be a region that is contained within dstImage'"
  • if extent == (15, 15, 1), then I get the following error message "pRegion[0] extent width (15) must be a multiple of the compressed texture block width (4), or when added to srcOffset.x (0) must equal the image subresource width (60). The spec valid usage text states 'If the calling command's srcImage is a compressed image, or a single-plane, "_422" image format, extent.width must be a multiple of the compressed texel block width or (extent.width + srcOffset.x) must equal the source image subresource width'"

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.

@oddhack oddhack transferred this issue from KhronosGroup/Vulkan-Docs Jul 15, 2019
@mark-lunarg
Copy link

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

@daveh-lunarg
Copy link

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
Copy link
Author

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
Copy link

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
Copy link
Author

Jiawei-Shao commented Jul 19, 2019

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
Copy link

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.

@daveh-lunarg daveh-lunarg transferred this issue from KhronosGroup/Vulkan-ValidationLayers Jul 19, 2019
@jeffbolznv
Copy link
Contributor

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
Copy link

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

@Kangz
Copy link

Kangz commented Jun 7, 2021

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
Copy link

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

@sfricke-samsung sfricke-samsung self-assigned this Sep 24, 2021
@uazo uazo mentioned this issue Oct 19, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants