KhronosGroup / VK-GL-CTS

Khronos Vulkan, OpenGL, and OpenGL ES Conformance Tests
https://www.khronos.org/
Apache License 2.0
522 stars 290 forks source link

compressed_formats test doesn't appear to match spec #33

Closed gary-sweet closed 7 years ago

gary-sweet commented 7 years ago

When running dEQP-VK.api.info.format_properties.compressed_formats I am getting a failure that I don't believe is correct.

The failure shows:

ERROR: textureCompressionBC = VK_TRUE but BC formats not supported
All BC formats are supported
All ETC2 formats are supported
All ASTC LDR formats are supported
Result: Compressed format support not valid

We are exposing support for subset of the BC texture formats (not all of them), and we set textureCompressionBC in VkPhysicalDeviceFeatures.

The spec states:

textureCompressionBC indicates whether the BC compressed texture formats are supported.
If this feature is not enabled, the following formats must not be used to create images:

VK_FORMAT_BC*
...
vkGetPhysicalDeviceFormatProperties is used to check for the supported properties of individual formats.

As far as I understand, we have to fully support one of ASTC, ETC or BC, but can partially support others as desired.

The CTS should only be testing that one type of compressed format is fully supported. It appears to be testing that if you claim support for a format type that you must support it in its entirety.

dgkoch commented 7 years ago

The spec wasn't really designed with partial support for a set of compressed texture types in mind. IMO, you shouldn't advertise textureCompressionBC unless all of the relevant formats are supported. The problem is the current wording of the spec kind of states the negative of that (since it's trying to be written in terms of "Valid Usage"), so I can see why you'd come to the conclusion you did. I'd envisioned textureCompressionBC to be a short-cut for guaranteeing that all the BC formats are supported. If you really want it to be valid to support only a subset of a type of compressed formats, I think this would require a spec change, and you should file a spec issue.

gary-sweet commented 7 years ago

So, I think the spec fairly clearly allows for partial support with its current wording, whether that was the intent or not. In fact I'd argue that a spec change was required if your view of the situation is the correct one, not the other way around.

dgkoch commented 7 years ago

Well I'm pretty sure I wrote that portion of the spec, and I'd never considered partial support one way or the other, so if that's something you want to support I'd suggest requesting a spec clarification. Clearly, whoever wrote the CTS test was also interpreting it to mean that the feature being advertised implies support for all the formats (which I think is the more useful meaning of it).

(BTW if you are a Khronos member please file an internal spec issue for this.)

gary-sweet commented 7 years ago

Clearly, whoever wrote the CTS test was also interpreting it to mean that

Indeed. After my first reading of the spec I also thought the same, but on closer inspection of the passages I included, I assumed I was mistaken and couldn't then find anything that ruled out partial support.

I'll file an internal spec issue to clarify the situation one way or the other.

Thanks

phaulos commented 7 years ago

Khronos members should use internal issue tracker for reporting CTS issues as well. Thanks!