From: Chris Forbes Date: Fri, 25 Nov 2016 03:37:41 +0000 (+1300) Subject: layers: Move surface format tracking to CV X-Git-Tag: sdk-1.0.37.0~131 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5faa662f6859b01c72d79027abde363d5f10dcd7;p=platform%2Fupstream%2FVulkan-LoaderAndValidationLayers.git layers: Move surface format tracking to CV Signed-off-by: Chris Forbes --- diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index ee6768c..99542cc 100644 --- a/layers/core_validation.cpp +++ b/layers/core_validation.cpp @@ -11756,6 +11756,61 @@ static bool PreCallValidateCreateSwapchainKHR(layer_data *dev_data, VkSwapchainC } // Validate pCreateInfo values with the results of + // vkGetPhysicalDeviceSurfaceFormatsKHR(): + if (physical_device_state->vkGetPhysicalDeviceSurfaceFormatsKHRState != QUERY_DETAILS) { + if (log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT, + reinterpret_cast(dev_data->device), __LINE__, DRAWSTATE_SWAPCHAIN_CREATE_BEFORE_QUERY, "DS", + "vkCreateSwapchainKHR() called before calling vkGetPhysicalDeviceSurfaceFormatsKHR().")) + return true; + } else { + // Validate pCreateInfo->imageFormat against + // VkSurfaceFormatKHR::format: + bool foundFormat = false; + bool foundColorSpace = false; + bool foundMatch = false; + for (auto const &format : physical_device_state->surface_formats) { + if (pCreateInfo->imageFormat == format.format) { + // Validate pCreateInfo->imageColorSpace against + // VkSurfaceFormatKHR::colorSpace: + foundFormat = true; + if (pCreateInfo->imageColorSpace == format.colorSpace) { + foundMatch = true; + break; + } + } else { + if (pCreateInfo->imageColorSpace == format.colorSpace) { + foundColorSpace = true; + } + } + } + if (!foundMatch) { + if (!foundFormat) { + if (!foundColorSpace) { + if (log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT, + reinterpret_cast(dev_data->device), __LINE__, DRAWSTATE_SWAPCHAIN_BAD_FORMAT, "DS", + "vkCreateSwapchainKHR() called with neither a supported pCreateInfo->imageFormat " + "(i.e. %d) nor a supported " + "pCreateInfo->imageColorSpace (i.e. %d).", + pCreateInfo->imageFormat, pCreateInfo->imageColorSpace)) + return true; + } else { + if (log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT, + reinterpret_cast(dev_data->device), __LINE__, DRAWSTATE_SWAPCHAIN_BAD_FORMAT, "DS", + "vkCreateSwapchainKHR() called with a non-supported pCreateInfo->imageFormat (i.e. %d)", + pCreateInfo->imageFormat)) + return true; + } + } else if (!foundColorSpace) { + if (log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT, + reinterpret_cast(dev_data->device), __LINE__, DRAWSTATE_SWAPCHAIN_BAD_FORMAT, "DS", + "vkCreateSwapchainKHR() called with a non-supported pCreateInfo->imageColorSpace (i.e. %d).", + pCreateInfo->imageColorSpace)) + return true; + } + } + } + + // Validate pCreateInfo values with the results of // vkGetPhysicalDeviceSurfacePresentModesKHR(): if (physical_device_state->vkGetPhysicalDeviceSurfacePresentModesKHRState != QUERY_DETAILS) { /* FIFO is required to always be supported */ @@ -12385,13 +12440,75 @@ VKAPI_ATTR VkResult VKAPI_CALL GetPhysicalDeviceSurfacePresentModesKHR(VkPhysica physical_device_state->present_modes[i] = pPresentModes[i]; } } + } + + return result; +} + +VKAPI_ATTR VkResult VKAPI_CALL GetPhysicalDeviceSurfaceFormatsKHR(VkPhysicalDevice physicalDevice, VkSurfaceKHR surface, + uint32_t *pSurfaceFormatCount, + VkSurfaceFormatKHR *pSurfaceFormats) { + bool skip_call = false; + auto instance_data = get_my_data_ptr(get_dispatch_key(physicalDevice), instance_layer_data_map); + std::unique_lock lock(global_lock); + auto physical_device_state = getPhysicalDeviceState(instance_data, physicalDevice); + auto & call_state = physical_device_state->vkGetPhysicalDeviceSurfaceFormatsKHRState; + + if (pSurfaceFormats) { + auto prev_format_count = (uint32_t) physical_device_state->surface_formats.size(); + + switch (call_state) { + case UNCALLED: + // Since we haven't recorded a preliminary value of *pSurfaceFormatCount, that likely means that the application didn't + // previously call this function with a NULL value of pSurfaceFormats: + skip_call |= log_msg( + instance_data->report_data, VK_DEBUG_REPORT_WARNING_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT, + reinterpret_cast(physicalDevice), __LINE__, DEVLIMITS_MUST_QUERY_COUNT, "DL", + "vkGetPhysicalDeviceSurfaceFormatsKHR() called with non-NULL pSurfaceFormatCount; but no prior positive " + "value has been seen for pSurfaceFormats."); + break; + default: + if (prev_format_count != *pSurfaceFormatCount) { + skip_call |= log_msg( + instance_data->report_data, VK_DEBUG_REPORT_WARNING_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT, + reinterpret_cast(physicalDevice), __LINE__, DEVLIMITS_COUNT_MISMATCH, "DL", + "vkGetPhysicalDeviceSurfaceFormatsKHR() called with non-NULL pSurfaceFormatCount, and with pSurfaceFormats set to " + "a value (%u) that is greater than the value (%u) that was returned when pSurfaceFormatCount was NULL.", + *pSurfaceFormatCount, prev_format_count); + } + break; } } + lock.unlock(); + + if (skip_call) + return VK_ERROR_VALIDATION_FAILED_EXT; + // Call down the call chain: + auto result = instance_data->dispatch_table.GetPhysicalDeviceSurfaceFormatsKHR(physicalDevice, surface, pSurfaceFormatCount, + pSurfaceFormats); + + if (result == VK_SUCCESS || result == VK_INCOMPLETE) { + + lock.lock(); + + if (*pSurfaceFormatCount) { + if (call_state < QUERY_COUNT) call_state = QUERY_COUNT; + if (*pSurfaceFormatCount > physical_device_state->surface_formats.size()) + physical_device_state->surface_formats.resize(*pSurfaceFormatCount); + } + if (pSurfaceFormats) { + if (call_state < QUERY_DETAILS) call_state = QUERY_DETAILS; + for (uint32_t i = 0; i < *pSurfaceFormatCount; i++) { + physical_device_state->surface_formats[i] = pSurfaceFormats[i]; + } + } + } return result; } + VKAPI_ATTR VkResult VKAPI_CALL CreateDebugReportCallbackEXT(VkInstance instance, const VkDebugReportCallbackCreateInfoEXT *pCreateInfo, const VkAllocationCallbacks *pAllocator, VkDebugReportCallbackEXT *pMsgCallback) { @@ -12739,6 +12856,8 @@ intercept_khr_surface_command(const char *name, VkInstance instance) { &instance_layer_data::surfaceExtensionEnabled}, {"vkGetPhysicalDeviceSurfacePresentModesKHR", reinterpret_cast(GetPhysicalDeviceSurfacePresentModesKHR), &instance_layer_data::surfaceExtensionEnabled}, + {"vkGetPhysicalDeviceSurfaceFormatsKHR", reinterpret_cast(GetPhysicalDeviceSurfaceFormatsKHR), + &instance_layer_data::surfaceExtensionEnabled}, }; instance_layer_data *instance_data = nullptr; diff --git a/layers/core_validation.h b/layers/core_validation.h index 850fc6c..e09fd7d 100644 --- a/layers/core_validation.h +++ b/layers/core_validation.h @@ -218,11 +218,13 @@ struct PHYSICAL_DEVICE_STATE { CALL_STATE vkGetPhysicalDeviceFeaturesState = UNCALLED; CALL_STATE vkGetPhysicalDeviceSurfaceCapabilitiesKHRState = UNCALLED; CALL_STATE vkGetPhysicalDeviceSurfacePresentModesKHRState = UNCALLED; + CALL_STATE vkGetPhysicalDeviceSurfaceFormatsKHRState = UNCALLED; VkPhysicalDeviceFeatures features = {}; VkPhysicalDevice phys_device = VK_NULL_HANDLE; std::vector queue_family_properties; VkSurfaceCapabilitiesKHR surfaceCapabilities = {}; std::vector present_modes; + std::vector surface_formats; }; struct GpuQueue { diff --git a/layers/core_validation_error_enums.h b/layers/core_validation_error_enums.h index 8076455..2ea083c 100644 --- a/layers/core_validation_error_enums.h +++ b/layers/core_validation_error_enums.h @@ -149,6 +149,7 @@ enum DRAW_STATE_ERROR { DRAWSTATE_SWAPCHAIN_BAD_USAGE_FLAGS, DRAWSTATE_SWAPCHAIN_TOO_MANY_IMAGES, DRAWSTATE_SWAPCHAIN_BAD_PRESENT_MODE, + DRAWSTATE_SWAPCHAIN_BAD_FORMAT, }; // Shader Checker ERROR codes diff --git a/layers/swapchain.cpp b/layers/swapchain.cpp index 9f98be2..9cc65ab 100644 --- a/layers/swapchain.cpp +++ b/layers/swapchain.cpp @@ -191,7 +191,6 @@ VKAPI_ATTR void VKAPI_CALL DestroyInstance(VkInstance instance, const VkAllocati SWAPCHAIN_DEL_OBJECT_BEFORE_CHILDREN, swapchain_layer_name, "VkDestroyInstance() called before all of its associated VkDevices were destroyed."); } - free(pPhysicalDevice->pSurfaceFormats); } // Erase the SwpPhysicalDevice's from the my_data->physicalDeviceMap (which @@ -852,8 +851,6 @@ VKAPI_ATTR VkResult VKAPI_CALL EnumeratePhysicalDevices(VkInstance instance, uin my_data->physicalDeviceMap[pPhysicalDevices[i]].pInstance = pInstance; my_data->physicalDeviceMap[pPhysicalDevices[i]].pDevice = NULL; my_data->physicalDeviceMap[pPhysicalDevices[i]].gotQueueFamilyPropertyCount = false; - my_data->physicalDeviceMap[pPhysicalDevices[i]].surfaceFormatCount = 0; - my_data->physicalDeviceMap[pPhysicalDevices[i]].pSurfaceFormats = NULL; // Point to the associated SwpInstance: if (pInstance) { pInstance->physicalDevices[pPhysicalDevices[i]] = &my_data->physicalDeviceMap[pPhysicalDevices[i]]; @@ -1003,80 +1000,6 @@ VKAPI_ATTR VkResult VKAPI_CALL GetPhysicalDeviceSurfaceSupportKHR(VkPhysicalDevi return VK_ERROR_VALIDATION_FAILED_EXT; } -VKAPI_ATTR VkResult VKAPI_CALL GetPhysicalDeviceSurfaceFormatsKHR(VkPhysicalDevice physicalDevice, VkSurfaceKHR surface, - uint32_t *pSurfaceFormatCount, - VkSurfaceFormatKHR *pSurfaceFormats) { - VkResult result = VK_SUCCESS; - bool skip_call = false; - layer_data *my_data = get_my_data_ptr(get_dispatch_key(physicalDevice), layer_data_map); - std::unique_lock lock(global_lock); - SwpPhysicalDevice *pPhysicalDevice = NULL; - { - auto it = my_data->physicalDeviceMap.find(physicalDevice); - pPhysicalDevice = (it == my_data->physicalDeviceMap.end()) ? NULL : &it->second; - } - - if (pPhysicalDevice && pSurfaceFormats) { - // Compare the preliminary value of *pSurfaceFormatCount with the value this time: - if (pPhysicalDevice->surfaceFormatCount == 0) { - // Since we haven't recorded a preliminary value of *pSurfaceFormatCount, that likely means that the application didn't - // previously call this function with a NULL value of pSurfaceFormats: - skip_call |= log_msg( - my_data->report_data, VK_DEBUG_REPORT_WARNING_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT, - reinterpret_cast(pPhysicalDevice->physicalDevice), __LINE__, SWAPCHAIN_PRIOR_COUNT, swapchain_layer_name, - "vkGetPhysicalDeviceSurfaceFormatsKHR() called with non-NULL pSurfaceFormatCount; but no prior positive " - "value has been seen for pSurfaceFormats."); - } else if (*pSurfaceFormatCount > pPhysicalDevice->surfaceFormatCount) { - skip_call |= log_msg( - my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT, - reinterpret_cast(pPhysicalDevice->physicalDevice), __LINE__, SWAPCHAIN_INVALID_COUNT, - swapchain_layer_name, - "vkGetPhysicalDeviceSurfaceFormatsKHR() called with non-NULL pSurfaceFormatCount, and with pSurfaceFormats set to " - "a value (%d) that is greater than the value (%d) that was returned when pSurfaceFormatCount was NULL.", - *pSurfaceFormatCount, pPhysicalDevice->surfaceFormatCount); - } - } - lock.unlock(); - - if (!skip_call) { - // Call down the call chain: - result = my_data->instance_dispatch_table->GetPhysicalDeviceSurfaceFormatsKHR(physicalDevice, surface, pSurfaceFormatCount, - pSurfaceFormats); - lock.lock(); - - // Obtain this pointer again after locking: - { - auto it = my_data->physicalDeviceMap.find(physicalDevice); - pPhysicalDevice = (it == my_data->physicalDeviceMap.end()) ? NULL : &it->second; - } - if ((result == VK_SUCCESS) && pPhysicalDevice && !pSurfaceFormats && pSurfaceFormatCount) { - // Record the result of this preliminary query: - pPhysicalDevice->surfaceFormatCount = *pSurfaceFormatCount; - } else if (((result == VK_SUCCESS) || (result == VK_INCOMPLETE)) && pPhysicalDevice && pSurfaceFormats && - pSurfaceFormatCount && (*pSurfaceFormatCount > 0)) { - // Record the result of this query: - - // Note: for poorly-written applications (e.g. that don't call this command - // twice, the first time with pSurfaceFormats set to NULL, and the second time - // with a non-NULL pSurfaceFormats and with the same count as returned the - // first time), record again the count when pSurfaceFormats is non-NULL: - pPhysicalDevice->surfaceFormatCount = *pSurfaceFormatCount; - pPhysicalDevice->pSurfaceFormats = (VkSurfaceFormatKHR *)malloc(*pSurfaceFormatCount * sizeof(VkSurfaceFormatKHR)); - if (pPhysicalDevice->pSurfaceFormats) { - for (uint32_t i = 0; i < *pSurfaceFormatCount; i++) { - pPhysicalDevice->pSurfaceFormats[i] = pSurfaceFormats[i]; - } - } else { - pPhysicalDevice->surfaceFormatCount = 0; - } - } - lock.unlock(); - - return result; - } - return VK_ERROR_VALIDATION_FAILED_EXT; -} - // This function does the up-front validation work for vkCreateSwapchainKHR(), // and returns true if a logging callback indicates that the call down the // chain should be skipped: @@ -1119,61 +1042,6 @@ static bool validateCreateSwapchainKHR(VkDevice device, const VkSwapchainCreateI } - // Validate pCreateInfo values with the results of - // vkGetPhysicalDeviceSurfaceFormatsKHR(): - if (!pPhysicalDevice || !pPhysicalDevice->surfaceFormatCount) { - skip_call |= - log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT, - reinterpret_cast(device), __LINE__, SWAPCHAIN_CREATE_SWAP_WITHOUT_QUERY, swapchain_layer_name, - "vkCreateSwapchainKHR() called before calling vkGetPhysicalDeviceSurfaceFormatsKHR()."); - } else if (pCreateInfo) { - // Validate pCreateInfo->imageFormat against - // VkSurfaceFormatKHR::format: - bool foundFormat = false; - bool foundColorSpace = false; - bool foundMatch = false; - for (uint32_t i = 0; i < pPhysicalDevice->surfaceFormatCount; i++) { - if (pCreateInfo->imageFormat == pPhysicalDevice->pSurfaceFormats[i].format) { - // Validate pCreateInfo->imageColorSpace against - // VkSurfaceFormatKHR::colorSpace: - foundFormat = true; - if (pCreateInfo->imageColorSpace == pPhysicalDevice->pSurfaceFormats[i].colorSpace) { - foundMatch = true; - break; - } - } else { - if (pCreateInfo->imageColorSpace == pPhysicalDevice->pSurfaceFormats[i].colorSpace) { - foundColorSpace = true; - } - } - } - if (!foundMatch) { - if (!foundFormat) { - if (!foundColorSpace) { - skip_call |= log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, - VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT, reinterpret_cast(device), __LINE__, - SWAPCHAIN_CREATE_SWAP_BAD_IMG_FMT_CLR_SP, swapchain_layer_name, - "vkCreateSwapchainKHR() called with neither a supported pCreateInfo->imageFormat " - "(i.e. %d) nor a supported " - "pCreateInfo->imageColorSpace (i.e. %d).", - pCreateInfo->imageFormat, pCreateInfo->imageColorSpace); - } else { - skip_call |= log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, - VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT, reinterpret_cast(device), __LINE__, - SWAPCHAIN_CREATE_SWAP_BAD_IMG_FORMAT, swapchain_layer_name, - "vkCreateSwapchainKHR() called with a non-supported pCreateInfo->imageFormat (i.e. %d)", - pCreateInfo->imageFormat); - } - } else if (!foundColorSpace) { - skip_call |= log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT, - reinterpret_cast(device), __LINE__, SWAPCHAIN_CREATE_SWAP_BAD_IMG_COLOR_SPACE, - swapchain_layer_name, - "vkCreateSwapchainKHR() called with a non-supported pCreateInfo->imageColorSpace (i.e. %d).", - pCreateInfo->imageColorSpace); - } - } - } - // Validate pCreateInfo->imageSharingMode and related values: if (pCreateInfo->imageSharingMode == VK_SHARING_MODE_CONCURRENT) { if ((pCreateInfo->queueFamilyIndexCount <= 1) || !pCreateInfo->pQueueFamilyIndices) { @@ -1523,7 +1391,6 @@ static PFN_vkVoidFunction intercept_khr_surface_command(const char *name, VkInst #endif // VK_USE_PLATFORM_XLIB_KHR {"vkDestroySurfaceKHR", reinterpret_cast(DestroySurfaceKHR)}, {"vkGetPhysicalDeviceSurfaceSupportKHR", reinterpret_cast(GetPhysicalDeviceSurfaceSupportKHR)}, - {"vkGetPhysicalDeviceSurfaceFormatsKHR", reinterpret_cast(GetPhysicalDeviceSurfaceFormatsKHR)}, {"vkGetPhysicalDeviceDisplayPlanePropertiesKHR", reinterpret_cast(GetPhysicalDeviceDisplayPlanePropertiesKHR)}, {"vkGetDisplayPlaneSupportedDisplaysKHR", reinterpret_cast(GetDisplayPlaneSupportedDisplaysKHR)}, diff --git a/layers/swapchain.h b/layers/swapchain.h index 34ccdf4..e3c67bd 100644 --- a/layers/swapchain.h +++ b/layers/swapchain.h @@ -146,12 +146,6 @@ struct SwpPhysicalDevice { // called for: unordered_map supportedSurfaces; - // TODO: Record/use this info per-surface, not per-device, once a - // non-dispatchable surface object is added to WSI: - // Count and VkSurfaceFormatKHR's returned by vkGetPhysicalDeviceSurfaceFormatsKHR(): - uint32_t surfaceFormatCount; - VkSurfaceFormatKHR *pSurfaceFormats; - // Count returned by vkGetPhysicalDeviceDisplayPlanePropertiesKHR(): uint32_t displayPlanePropertyCount; bool gotDisplayPlanePropertyCount;