Vulkan CTS test crash because of invalid dimensions for PVRTC texture #1760
Comments
|
|
Hmmm. I guess we should at least review what MoltenVK is reporting as supported. We may then have to add a flag to the Vulkan portability extension to indicate whether square PVRTC dimensions is a requirement. |
Hmm, now that I google that seems like it is true, though I'm finding conflicting information online about this. The only properly credible sources that say that PVRTC1 is power-of-two are Skia and the PVRTC2 announcement by the fact that PVRTC2 does not require power-of-two sizes. Should this maybe be raised as an issue to the Vulkan-Docs repository so that the Vulkan spec might add this requirement? Then I'd also make the change to the CTS so that it never uses a non power of two size for PVRTC1. |
Yeah. It's hard to find. There is surprisingly little documentation on that fact, considering it's such a definitive user restriction. The PVRTC1 section of the Khronos Data Format Specification includes the line:
|
|
This restriction is mentioned in the original OpenGL ES extension and is also present in the KDFS (as quoted above), which is normatively referenced by Vulkan. For completeness, the Vulkan extension should mention that as well. |
|
Another Metal restriction that applies to all PVRTC1 formats:
|
This part is fixed in #1780. |
billhollings
added
the
Metal improvement required
|
With 1.3.279 the power-of-two requirement has been added as a VUID to the Vulkan specification. Would you prefer to catch the invalid usage to avoid Metal throwing an assertion, or do we just keep that as an error for anyone who uses it? I think this issue can now be closed. I have sent a PR to the conformance tests to fix the validation. |
billhollings
added
the
Completed
Thanks for the intel. With Vulkan, implementations are deliberately not tasked with enforcing validation. That's the job of the Validation Layer. This approach allows implementations to avoid the extra effort and time to check everything. So, we can ignore it, and let Metal validation take care of spanking the app for not following the validation rules.
Great. Doing so now!
Thanks for taking care of that! |
|
@billhollings I suggest keeping this issue open a bit longer because:
|
billhollings
added
Completed
I'm not too bothered about the VUID implementation, because either way we have nothing to track or do for MoltenVK. Good point about portability. We don't include So I think we're okay to leave this closed for now. |
|
@billhollings I had an idea about the requirement of square issues imposed by the Metal driver. What if MoltenVK would just extend the image size internally to be square to make the Metal driver happy? Or would that be too large a memory overhead to justify? |
|
@spnda There are multiple major issues with implicit padding:
|
@spnda Good "out of the box" thinking, nevertheless. |
This happens with MoltenVK from master and VK-GL-CTS from master on macOS 13.1 with a M1. Looks like MoltenVK reports a lot of image formats as supported in the
vkGetPhysicalDeviceImageFormatPropertiescall even though they're not supported. Though the reason for the crash, namely Metal asserting because PVRTC_RGBA_4BPP did not have a compatible size.From the CTS source it seems like the test generates a random image width and height, ranging somewhere between 2 and 18. It does not look like the Vulkan spec makes a requirement for the size of a PVRTC image dimensions needing to be a power of 2 or be a perfect square. The Metal docs also don't note any requirement either, though it seems like Apple GPUs have these implicit requirements in their PVRTC implementation.
The text was updated successfully, but these errors were encountered: