From 8a630566844b7b76bd5c74555088064e4593ed2f Mon Sep 17 00:00:00 2001 From: Chris Forbes Date: Fri, 25 Nov 2016 13:17:36 +1300 Subject: [PATCH] layers: Move present modes tracking to CV Signed-off-by: Chris Forbes --- layers/core_validation.cpp | 93 ++++++++++++++++++++++++++++++ layers/core_validation.h | 2 + layers/core_validation_error_enums.h | 1 + layers/swapchain.cpp | 107 ----------------------------------- layers/swapchain.h | 6 -- 5 files changed, 96 insertions(+), 113 deletions(-) diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp index 36dd1d1..7db1142 100644 --- a/layers/core_validation.cpp +++ b/layers/core_validation.cpp @@ -11754,6 +11754,33 @@ static bool PreCallValidateCreateSwapchainKHR(layer_data *dev_data, VkSwapchainC } } + // Validate pCreateInfo values with the results of + // vkGetPhysicalDeviceSurfacePresentModesKHR(): + if (physical_device_state->vkGetPhysicalDeviceSurfacePresentModesKHRState != QUERY_DETAILS) { + /* FIFO is required to always be supported */ + if (pCreateInfo->presentMode != VK_PRESENT_MODE_FIFO_KHR) { + 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 " + "vkGetPhysicalDeviceSurfacePresentModesKHR().")) + return true; + } + } else { + // Validate pCreateInfo->presentMode against + // vkGetPhysicalDeviceSurfacePresentModesKHR(): + bool foundMatch = std::find(physical_device_state->present_modes.begin(), + physical_device_state->present_modes.end(), + pCreateInfo->presentMode) != physical_device_state->present_modes.end(); + if (!foundMatch) { + 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_PRESENT_MODE, "DS", + "vkCreateSwapchainKHR() called with a non-supported pCreateInfo->presentMode (i.e. %s).", + string_VkPresentModeKHR(pCreateInfo->presentMode))) + return true; + } + } + + return false; } @@ -12300,6 +12327,70 @@ VKAPI_ATTR VkResult VKAPI_CALL GetPhysicalDeviceSurfaceSupportKHR(VkPhysicalDevi return result; } + +VKAPI_ATTR VkResult VKAPI_CALL GetPhysicalDeviceSurfacePresentModesKHR(VkPhysicalDevice physicalDevice, VkSurfaceKHR surface, + uint32_t *pPresentModeCount, + VkPresentModeKHR *pPresentModes) { + 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); + // TODO: this isn't quite right. available modes may differ by surface AND physical device. + auto physical_device_state = getPhysicalDeviceState(instance_data, physicalDevice); + auto & call_state = physical_device_state->vkGetPhysicalDeviceSurfacePresentModesKHRState; + + if (pPresentModes) { + // Compare the preliminary value of *pPresentModeCount with the value this time: + auto prev_mode_count = (uint32_t) physical_device_state->present_modes.size(); + switch (call_state) { + case UNCALLED: + 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", + "vkGetPhysicalDeviceSurfacePresentModesKHR() called with non-NULL pPresentModeCount; but no prior positive " + "value has been seen for pPresentModeCount."); + break; + default: + // both query count and query details + if (*pPresentModeCount != prev_mode_count) { + 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", + "vkGetPhysicalDeviceSurfacePresentModesKHR() called with *pPresentModeCount (%u) that differs from the value " + "(%u) that was returned when pPresentModes was NULL.", + *pPresentModeCount, prev_mode_count); + } + break; + } + } + lock.unlock(); + + if (skip_call) + return VK_ERROR_VALIDATION_FAILED_EXT; + + auto result = instance_data->dispatch_table.GetPhysicalDeviceSurfacePresentModesKHR(physicalDevice, surface, pPresentModeCount, pPresentModes); + + if (result == VK_SUCCESS || result == VK_INCOMPLETE) { + + lock.lock(); + + if (*pPresentModeCount) { + if (call_state < QUERY_COUNT) call_state = QUERY_COUNT; + if (*pPresentModeCount > physical_device_state->present_modes.size()) + physical_device_state->present_modes.resize(*pPresentModeCount); + } + if (pPresentModes) { + if (call_state < QUERY_DETAILS) call_state = QUERY_DETAILS; + for (uint32_t i = 0; i < *pPresentModeCount; i++) { + physical_device_state->present_modes[i] = pPresentModes[i]; + } + } + + } + } + + return result; +} + VKAPI_ATTR VkResult VKAPI_CALL CreateDebugReportCallbackEXT(VkInstance instance, const VkDebugReportCallbackCreateInfoEXT *pCreateInfo, const VkAllocationCallbacks *pAllocator, VkDebugReportCallbackEXT *pMsgCallback) { @@ -12645,6 +12736,8 @@ intercept_khr_surface_command(const char *name, VkInstance instance) { &instance_layer_data::surfaceExtensionEnabled}, {"vkGetPhysicalDeviceSurfaceSupportKHR", reinterpret_cast(GetPhysicalDeviceSurfaceSupportKHR), &instance_layer_data::surfaceExtensionEnabled}, + {"vkGetPhysicalDeviceSurfacePresentModesKHR", reinterpret_cast(GetPhysicalDeviceSurfacePresentModesKHR), + &instance_layer_data::surfaceExtensionEnabled}, }; instance_layer_data *instance_data = nullptr; diff --git a/layers/core_validation.h b/layers/core_validation.h index ac43138..850fc6c 100644 --- a/layers/core_validation.h +++ b/layers/core_validation.h @@ -217,10 +217,12 @@ struct PHYSICAL_DEVICE_STATE { CALL_STATE vkGetPhysicalDeviceExtensionPropertiesState = UNCALLED; CALL_STATE vkGetPhysicalDeviceFeaturesState = UNCALLED; CALL_STATE vkGetPhysicalDeviceSurfaceCapabilitiesKHRState = UNCALLED; + CALL_STATE vkGetPhysicalDeviceSurfacePresentModesKHRState = UNCALLED; VkPhysicalDeviceFeatures features = {}; VkPhysicalDevice phys_device = VK_NULL_HANDLE; std::vector queue_family_properties; VkSurfaceCapabilitiesKHR surfaceCapabilities = {}; + std::vector present_modes; }; struct GpuQueue { diff --git a/layers/core_validation_error_enums.h b/layers/core_validation_error_enums.h index 4554f72..8076455 100644 --- a/layers/core_validation_error_enums.h +++ b/layers/core_validation_error_enums.h @@ -148,6 +148,7 @@ enum DRAW_STATE_ERROR { DRAWSTATE_SWAPCHAIN_BAD_LAYER_COUNT, DRAWSTATE_SWAPCHAIN_BAD_USAGE_FLAGS, DRAWSTATE_SWAPCHAIN_TOO_MANY_IMAGES, + DRAWSTATE_SWAPCHAIN_BAD_PRESENT_MODE, }; // Shader Checker ERROR codes diff --git a/layers/swapchain.cpp b/layers/swapchain.cpp index 6ae123a..9f98be2 100644 --- a/layers/swapchain.cpp +++ b/layers/swapchain.cpp @@ -192,7 +192,6 @@ VKAPI_ATTR void VKAPI_CALL DestroyInstance(VkInstance instance, const VkAllocati "VkDestroyInstance() called before all of its associated VkDevices were destroyed."); } free(pPhysicalDevice->pSurfaceFormats); - free(pPhysicalDevice->pPresentModes); } // Erase the SwpPhysicalDevice's from the my_data->physicalDeviceMap (which @@ -855,8 +854,6 @@ VKAPI_ATTR VkResult VKAPI_CALL EnumeratePhysicalDevices(VkInstance instance, uin my_data->physicalDeviceMap[pPhysicalDevices[i]].gotQueueFamilyPropertyCount = false; my_data->physicalDeviceMap[pPhysicalDevices[i]].surfaceFormatCount = 0; my_data->physicalDeviceMap[pPhysicalDevices[i]].pSurfaceFormats = NULL; - my_data->physicalDeviceMap[pPhysicalDevices[i]].presentModeCount = 0; - my_data->physicalDeviceMap[pPhysicalDevices[i]].pPresentModes = NULL; // Point to the associated SwpInstance: if (pInstance) { pInstance->physicalDevices[pPhysicalDevices[i]] = &my_data->physicalDeviceMap[pPhysicalDevices[i]]; @@ -1080,80 +1077,6 @@ VKAPI_ATTR VkResult VKAPI_CALL GetPhysicalDeviceSurfaceFormatsKHR(VkPhysicalDevi return VK_ERROR_VALIDATION_FAILED_EXT; } -VKAPI_ATTR VkResult VKAPI_CALL GetPhysicalDeviceSurfacePresentModesKHR(VkPhysicalDevice physicalDevice, VkSurfaceKHR surface, - uint32_t *pPresentModeCount, - VkPresentModeKHR *pPresentModes) { - 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 && pPresentModes) { - // Compare the preliminary value of *pPresentModeCount with the value this time: - if (pPhysicalDevice->presentModeCount == 0) { - // Since we haven't recorded a preliminary value of *pPresentModeCount, that likely means that the application didn't - // previously call this function with a NULL value of pPresentModes: - 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, - "vkGetPhysicalDeviceSurfacePresentModesKHR() called with non-NULL pPresentModeCount; but no prior positive " - "value has been seen for pPresentModes."); - } else if (*pPresentModeCount > pPhysicalDevice->presentModeCount) { - 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, - "vkGetPhysicalDeviceSurfacePresentModesKHR() called with non-NULL pPresentModeCount, and with pPresentModes set to " - "a value (%d) that is greater than the value (%d) that was returned when pPresentModeCount was NULL.", - *pPresentModeCount, pPhysicalDevice->presentModeCount); - } - } - lock.unlock(); - - if (!skip_call) { - // Call down the call chain: - result = my_data->instance_dispatch_table->GetPhysicalDeviceSurfacePresentModesKHR(physicalDevice, surface, - pPresentModeCount, pPresentModes); - 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 && !pPresentModes && pPresentModeCount) { - // Record the result of this preliminary query: - pPhysicalDevice->presentModeCount = *pPresentModeCount; - } else if (((result == VK_SUCCESS) || (result == VK_INCOMPLETE)) && pPhysicalDevice && - pPresentModes && pPresentModeCount && (*pPresentModeCount > 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 pPresentModes set to NULL, and the second time - // with a non-NULL pPresentModes and with the same count as returned the - // first time), record again the count when pPresentModes is non-NULL: - pPhysicalDevice->presentModeCount = *pPresentModeCount; - pPhysicalDevice->pPresentModes = (VkPresentModeKHR *)malloc(*pPresentModeCount * sizeof(VkPresentModeKHR)); - if (pPhysicalDevice->pPresentModes) { - for (uint32_t i = 0; i < *pPresentModeCount; i++) { - pPhysicalDevice->pPresentModes[i] = pPresentModes[i]; - } - } else { - pPhysicalDevice->presentModeCount = 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: @@ -1251,34 +1174,6 @@ static bool validateCreateSwapchainKHR(VkDevice device, const VkSwapchainCreateI } } - // Validate pCreateInfo values with the results of - // vkGetPhysicalDeviceSurfacePresentModesKHR(): - if (!pPhysicalDevice || !pPhysicalDevice->presentModeCount) { - if (!pCreateInfo || (pCreateInfo->presentMode != VK_PRESENT_MODE_FIFO_KHR)) { - 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 " - "vkGetPhysicalDeviceSurfacePresentModesKHR()."); - } - } else if (pCreateInfo) { - // Validate pCreateInfo->presentMode against - // vkGetPhysicalDeviceSurfacePresentModesKHR(): - bool foundMatch = false; - for (uint32_t i = 0; i < pPhysicalDevice->presentModeCount; i++) { - if (pPhysicalDevice->pPresentModes[i] == pCreateInfo->presentMode) { - foundMatch = true; - break; - } - } - if (!foundMatch) { - 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_PRESENT_MODE, swapchain_layer_name, - "vkCreateSwapchainKHR() called with a non-supported pCreateInfo->presentMode (i.e. %s).", - presentModeStr(pCreateInfo->presentMode)); - } - } - // Validate pCreateInfo->imageSharingMode and related values: if (pCreateInfo->imageSharingMode == VK_SHARING_MODE_CONCURRENT) { if ((pCreateInfo->queueFamilyIndexCount <= 1) || !pCreateInfo->pQueueFamilyIndices) { @@ -1629,8 +1524,6 @@ static PFN_vkVoidFunction intercept_khr_surface_command(const char *name, VkInst {"vkDestroySurfaceKHR", reinterpret_cast(DestroySurfaceKHR)}, {"vkGetPhysicalDeviceSurfaceSupportKHR", reinterpret_cast(GetPhysicalDeviceSurfaceSupportKHR)}, {"vkGetPhysicalDeviceSurfaceFormatsKHR", reinterpret_cast(GetPhysicalDeviceSurfaceFormatsKHR)}, - {"vkGetPhysicalDeviceSurfacePresentModesKHR", - reinterpret_cast(GetPhysicalDeviceSurfacePresentModesKHR)}, {"vkGetPhysicalDeviceDisplayPlanePropertiesKHR", reinterpret_cast(GetPhysicalDeviceDisplayPlanePropertiesKHR)}, {"vkGetDisplayPlaneSupportedDisplaysKHR", reinterpret_cast(GetDisplayPlaneSupportedDisplaysKHR)}, diff --git a/layers/swapchain.h b/layers/swapchain.h index a826534..34ccdf4 100644 --- a/layers/swapchain.h +++ b/layers/swapchain.h @@ -152,12 +152,6 @@ struct SwpPhysicalDevice { uint32_t surfaceFormatCount; VkSurfaceFormatKHR *pSurfaceFormats; - // TODO: Record/use this info per-surface, not per-device, once a - // non-dispatchable surface object is added to WSI: - // Count and VkPresentModeKHR's returned by vkGetPhysicalDeviceSurfacePresentModesKHR(): - uint32_t presentModeCount; - VkPresentModeKHR *pPresentModes; - // Count returned by vkGetPhysicalDeviceDisplayPlanePropertiesKHR(): uint32_t displayPlanePropertyCount; bool gotDisplayPlanePropertyCount; -- 2.7.4