search

KhronosGroup / Vulkan-LoaderAndValidationLayers

**Deprecated repository** for Vulkan loader and validation layers
Apache License 2.0
414 stars 172 forks source link

tests: Quick-fix VkImageCopy aspectMask test on 1.1 #2513

Closed krOoze closed 6 years ago

daveh-lunarg commented 6 years ago

This is also a problem in Vulkan 1.0 when the sampler_YCbCr extension is enabled, not strictly a 1.1 issue. Also the test should remain but expect a different VUID, post-YCbCr.

I have these fixes already in a large-ish sampler_YCbCr PR that will go up later this week, so I recommend abandoning this PR.

krOoze commented 6 years ago

This is also a problem in Vulkan 1.0 when the sampler_YCbCr extension is enabled

If by "problem" you mean that such cases are not tested (nor validated by layers) yet. It is not a problem here, since the test does not enable any extensions.

I went with a guard style that was used previously, but it's bit of a larger problem. We ideally want to test both 1.0 and 1.1 (not either-or).

karl-lunarg commented 6 years ago

I'd be in favor of this temporary quick fix going in now even if it will be replaced soon by a better fix. It is in line with other tests that had to be disabled for 1.1 and it looks like this one just got missed.

My CI machine with 1.1 drivers is flagging this error continuously which is annoying and hiding actual new problems that might show up.

daveh-lunarg commented 6 years ago

Sorry if I was unclear, as I think we're both saying the same thing. The test should be run regardless of API version, but for API >= 1.1 the VUID should be VALIDATION_ERROR_09c00c1e.

But, 09c00c1e isn't implemented on master yet, it's sitting on my dave-ycbcr branch. If time is of the essence I'm not opposed to this PR now.

krOoze commented 6 years ago

Sorry if I was unclear, as I think we're both saying the same thing. The test should be run regardless of API version, but for API >= 1.1 the VUID should be VALIDATION_ERROR_09c00c1e.

Sure, but I was making a larger point about the if(m_device->props.apiVersion <=> VK_API_VERSION_1_1) we started using.

I mean on API >= 1.1 the VUID should ideally be both cases, as 1.1 implementation is capable running both 1.0 and 1.1 based on VkApplicationInfo::apiVersion.

Ideally, I should not add the if and the test should be run with VkApplicationInfo::apiVersion = 1.0 instead.

daveh-lunarg commented 6 years ago

VkApplicationInfo::apiVersion is set according to the installed driver, and is read-only to the test.

Closing this PR in favor of #2531.