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.

Sign up for GitHub

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

Clean up for "ExtensionNotEnabled" with prerequisite framework fixes. #320

Merged
merged 4 commits into from
Sep 12, 2018
Merged

Clean up for "ExtensionNotEnabled" with prerequisite framework fixes. #320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d46951c
tests: Add ability for test to request API version
jzulauf-lunarg Sep 7, 2018
3ecd90d
tests: Modify 1.1 aware tests to request 1.1. inst
jzulauf-lunarg Sep 7, 2018
482a8b5
layers: Init extension tables w/ requested API
jzulauf-lunarg Sep 5, 2018
42f7372
tests: Cleanup unexpected in ExtensionNotEnabled
jzulauf-lunarg Sep 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions layers/core_validation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2310,8 +2310,10 @@ VKAPI_ATTR VkResult VKAPI_CALL CreateDevice(VkPhysicalDevice gpu, const VkDevice
// Get physical device limits for this device
instance_data->dispatch_table.GetPhysicalDeviceProperties(gpu, &(device_data->phys_dev_properties.properties));

device_data->api_version = device_data->extensions.InitFromDeviceCreateInfo(
&instance_data->extensions, device_data->phys_dev_properties.properties.apiVersion, pCreateInfo);
// Setup the validation tables based on the application API version from the instance and the capabilities of the device driver.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the driver version can be more or less than the instance version. For purposes of validation, if the application version is less than the driver version, we'll validate as if the driver was the lesser version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the instance supplied version is the lesser of the instances enumerated version and the apiVersion supplied to CreateInstance. (if the apiVersion is higher than the enumerated version, CreateInstance fails)

uint32_t effective_api_version = std::min(device_data->phys_dev_properties.properties.apiVersion, instance_data->api_version);
device_data->api_version =
device_data->extensions.InitFromDeviceCreateInfo(&instance_data->extensions, effective_api_version, pCreateInfo);

uint32_t count;
instance_data->dispatch_table.GetPhysicalDeviceQueueFamilyProperties(gpu, &count, nullptr);
Expand Down
10 changes: 5 additions & 5 deletions layers/parameter_validation_utils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -324,11 +324,11 @@ VKAPI_ATTR VkResult VKAPI_CALL vkCreateInstance(const VkInstanceCreateInfo *pCre
? pCreateInfo->pApplicationInfo->apiVersion
: VK_API_VERSION_1_0;

uint32_t effective_api_version = my_instance_data->extensions.InitFromInstanceCreateInfo(api_version, pCreateInfo);
my_instance_data->api_version = my_instance_data->extensions.InitFromInstanceCreateInfo(api_version, pCreateInfo);

// Ordinarily we'd check these before calling down the chain, but none of the layer support is in place until now, if we
// survive we can report the issue now.
validate_api_version(my_instance_data, api_version, effective_api_version);
validate_api_version(my_instance_data, api_version, my_instance_data->api_version);
validate_instance_extensions(my_instance_data, pCreateInfo);

parameter_validation_vkCreateInstance(*pInstance, pCreateInfo, pAllocator, pInstance);
Expand Down Expand Up @@ -588,10 +588,10 @@ VKAPI_ATTR VkResult VKAPI_CALL vkCreateDevice(VkPhysicalDevice physicalDevice, c
VkPhysicalDeviceProperties device_properties = {};
my_instance_data->dispatch_table.GetPhysicalDeviceProperties(physicalDevice, &device_properties);

// Set up the extension structure also for validation.
// Setup the validation tables based on the application API version from the instance and the capabilities of the device driver.
uint32_t effective_api_version = std::min(device_properties.apiVersion, my_instance_data->api_version);
DeviceExtensions extensions;
uint32_t api_version =
extensions.InitFromDeviceCreateInfo(&my_instance_data->extensions, device_properties.apiVersion, pCreateInfo);
uint32_t api_version = extensions.InitFromDeviceCreateInfo(&my_instance_data->extensions, effective_api_version, pCreateInfo);

std::unique_lock<std::mutex> lock(global_lock);

Expand Down
61 changes: 50 additions & 11 deletions tests/layer_validation_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,8 @@ class VkLayerTest : public VkRenderFramework {

protected:
ErrorMonitor *m_errorMonitor;
uint32_t m_instance_api_version = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "instance" version is what enumerates... (or by failing to enumerate informs us of being 1.0). The "target version" is what the framework will try to create.

uint32_t m_target_api_version = 0;

public:
ErrorMonitor *Monitor() { return m_errorMonitor; }
Expand Down Expand Up @@ -498,6 +500,31 @@ class VkLayerTest : public VkRenderFramework {
this->app_info.apiVersion = VK_API_VERSION_1_0;

m_errorMonitor = new ErrorMonitor;

// Find out what version the instance supports and record the default target instance
auto enumerateInstanceVersion =
(PFN_vkEnumerateInstanceVersion)vkGetInstanceProcAddr(nullptr, "vkEnumerateInstanceVersion");
if (enumerateInstanceVersion) {
enumerateInstanceVersion(&m_instance_api_version);
} else {
m_instance_api_version = VK_API_VERSION_1_0;
}
m_target_api_version = app_info.apiVersion;
}

uint32_t SetTargetApiVersion(uint32_t target_api_version) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not binding on the framework, the return is what CreateInstance will request.

if (target_api_version == 0) target_api_version = VK_API_VERSION_1_0;
if (target_api_version <= m_instance_api_version) {
m_target_api_version = target_api_version;
app_info.apiVersion = m_target_api_version;
}
return m_target_api_version;
}
uint32_t DeviceValidationVersion() {
// The validation layers, assume the version we are validating to is the apiVersion unless the device apiVersion is lower
VkPhysicalDeviceProperties props;
GetPhysicalDeviceProperties(&props);
return std::min(m_target_api_version, props.apiVersion);
}

bool LoadDeviceProfileLayer(
Expand Down Expand Up @@ -22053,7 +22080,7 @@ TEST_F(VkLayerTest, CopyImageTypeExtentMismatchMaintenance1) {

TEST_F(VkLayerTest, CopyImageCompressedBlockAlignment) {
// Image copy tests on compressed images with block alignment errors

SetTargetApiVersion(VK_API_VERSION_1_1);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test really is 1.1 aware and thus should request 1.1. We don't care about what is request, as we test below.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also the case for the other tests with this call added.

ASSERT_NO_FATAL_FAILURE(Init());

// Select a compressed format and verify support
Expand Down Expand Up @@ -22124,7 +22151,7 @@ TEST_F(VkLayerTest, CopyImageCompressedBlockAlignment) {

std::string vuid;
bool ycbcr = (DeviceExtensionEnabled(VK_KHR_SAMPLER_YCBCR_CONVERSION_EXTENSION_NAME) ||
(m_device->props.apiVersion >= VK_API_VERSION_1_1));
(DeviceValidationVersion() >= VK_API_VERSION_1_1));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The call hides the logic matching the logic the validation uses for "how we will validate"


// Src, Dest offsets must be multiples of compressed block sizes {4, 4, 1}
// Image transfer granularity gets set to compressed block size, so an ITG error is also (unavoidably) triggered.
Expand Down Expand Up @@ -22849,6 +22876,7 @@ TEST_F(VkLayerTest, CopyImageSampleCountMismatch) {

TEST_F(VkLayerTest, CopyImageAspectMismatch) {
TEST_DESCRIPTION("Image copies with aspect mask errors");
SetTargetApiVersion(VK_API_VERSION_1_1);
ASSERT_NO_FATAL_FAILURE(Init());
auto ds_format = FindSupportedDepthStencilFormat(gpu());
if (!ds_format) {
Expand Down Expand Up @@ -22897,7 +22925,7 @@ TEST_F(VkLayerTest, CopyImageAspectMismatch) {
// Src and dest aspect masks don't match
copyRegion.dstSubresource.aspectMask = VK_IMAGE_ASPECT_STENCIL_BIT;
bool ycbcr = (DeviceExtensionEnabled(VK_KHR_SAMPLER_YCBCR_CONVERSION_EXTENSION_NAME) ||
(m_device->props.apiVersion >= VK_API_VERSION_1_1));
(DeviceValidationVersion() >= VK_API_VERSION_1_1));
std::string vuid = (ycbcr ? "VUID-VkImageCopy-srcImage-01551" : "VUID-VkImageCopy-aspectMask-00137");
m_errorMonitor->SetDesiredFailureMsg(VK_DEBUG_REPORT_ERROR_BIT_EXT, vuid);
vkCmdCopyImage(m_commandBuffer->handle(), ds_image.handle(), VK_IMAGE_LAYOUT_GENERAL, ds_image.handle(),
Expand Down Expand Up @@ -23795,16 +23823,26 @@ TEST_F(VkLayerTest, ExecuteSecondaryCBWithLayoutMismatch) {
TEST_F(VkLayerTest, ExtensionNotEnabled) {
TEST_DESCRIPTION("Validate that using an API from an unenabled extension returns an error");

ASSERT_NO_FATAL_FAILURE(InitFramework(myDbgFunc, m_errorMonitor));

// Do NOT enable prerequesite extensions for YCBCR...
// Do NOT enable VK_KHR_maintenance1
if (DeviceExtensionSupported(gpu(), nullptr, VK_KHR_SAMPLER_YCBCR_CONVERSION_EXTENSION_NAME)) {
m_device_extension_names.push_back(VK_KHR_SAMPLER_YCBCR_CONVERSION_EXTENSION_NAME);
if (InstanceExtensionSupported(VK_KHR_GET_PHYSICAL_DEVICE_PROPERTIES_2_EXTENSION_NAME)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests is changed is to ensure that we get the same single error we expect on 1.0 and 1.1 drivers.

m_instance_extension_names.push_back(VK_KHR_GET_PHYSICAL_DEVICE_PROPERTIES_2_EXTENSION_NAME);
} else {
printf("%s test requires KHR YCBR conversion extension, not available. Skipping.\n", kSkipPrefix);
printf("%s Did not find required instance extension %s; skipped.\n", kSkipPrefix,
VK_KHR_GET_PHYSICAL_DEVICE_PROPERTIES_2_EXTENSION_NAME);
return;
}
ASSERT_NO_FATAL_FAILURE(InitFramework(myDbgFunc, m_errorMonitor));

// Required extensions except VK_KHR_GET_MEMORY_REQUIREMENTS_2 -- to create the needed error
std::vector<const char *> required_device_extensions = {VK_KHR_MAINTENANCE1_EXTENSION_NAME, VK_KHR_BIND_MEMORY_2_EXTENSION_NAME,
VK_KHR_SAMPLER_YCBCR_CONVERSION_EXTENSION_NAME};
for (auto dev_ext : required_device_extensions) {
if (DeviceExtensionSupported(gpu(), nullptr, dev_ext)) {
m_device_extension_names.push_back(dev_ext);
} else {
printf("%s Did not find required device extension %s; skipped.\n", kSkipPrefix, dev_ext);
break;
}
}

// Need to ignore this error to get to the one we're testing
m_errorMonitor->SetUnexpectedError("VUID-vkCreateDevice-ppEnabledExtensionNames-01387");
Expand Down Expand Up @@ -23932,6 +23970,7 @@ TEST_F(VkLayerTest, InvalidCreateBufferSize) {
TEST_F(VkLayerTest, SetDynViewportParamTests) {
TEST_DESCRIPTION("Test parameters of vkCmdSetViewport without multiViewport feature");

SetTargetApiVersion(VK_API_VERSION_1_1);
VkPhysicalDeviceFeatures features{};
ASSERT_NO_FATAL_FAILURE(Init(&features));

Expand Down Expand Up @@ -24006,7 +24045,7 @@ TEST_F(VkLayerTest, SetDynViewportParamTests) {
{{0.0, 0.0, 64.0, 64.0, 0.0, NAN}, "VUID-VkViewport-maxDepth-01235"},
};

if (m_device->props.apiVersion < VK_API_VERSION_1_1) {
if (DeviceValidationVersion() < VK_API_VERSION_1_1) {
test_cases.push_back({{0.0, 0.0, 64.0, 0.0, 0.0, 1.0}, "VUID-VkViewport-height-01772"});
test_cases.push_back({{0.0, 0.0, 64.0, NAN, 0.0, 1.0}, "VUID-VkViewport-height-01772"});
} else {
Expand Down
4 changes: 4 additions & 0 deletions tests/vkrenderframework.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,10 @@ void VkRenderFramework::GetPhysicalDeviceFeatures(VkPhysicalDeviceFeatures *feat
}
}

void VkRenderFramework::GetPhysicalDeviceProperties(VkPhysicalDeviceProperties *props) {
*props = vk_testing::PhysicalDevice(gpu()).properties();
}

void VkRenderFramework::InitState(VkPhysicalDeviceFeatures *features, VkPhysicalDeviceFeatures2 *features2,
const VkCommandPoolCreateFlags flags) {
// Remove any unsupported device extension names from list
Expand Down
1 change: 1 addition & 0 deletions tests/vkrenderframework.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ class VkRenderFramework : public VkTestFramework {

void ShutdownFramework();
void GetPhysicalDeviceFeatures(VkPhysicalDeviceFeatures *features);
void GetPhysicalDeviceProperties(VkPhysicalDeviceProperties *props);
void InitState(VkPhysicalDeviceFeatures *features = nullptr, VkPhysicalDeviceFeatures2 *features2 = nullptr,
const VkCommandPoolCreateFlags flags = 0);

Expand Down