layers: Move surface format tracking to CV

Signed-off-by: Chris Forbes <chrisforbes@google.com>

diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp
index ee6768c..99542cc 100644 (file)
--- 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<uint64_t>(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<uint64_t>(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<uint64_t>(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<uint64_t>(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<std::mutex> 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<uint64_t>(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<uint64_t>(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<PFN_vkVoidFunction>(GetPhysicalDeviceSurfacePresentModesKHR),
             &instance_layer_data::surfaceExtensionEnabled},
+        {"vkGetPhysicalDeviceSurfaceFormatsKHR", reinterpret_cast<PFN_vkVoidFunction>(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 (file)
--- 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<VkQueueFamilyProperties> queue_family_properties;
     VkSurfaceCapabilitiesKHR surfaceCapabilities = {};
     std::vector<VkPresentModeKHR> present_modes;
+    std::vector<VkSurfaceFormatKHR> surface_formats;
 };
 
 struct GpuQueue {

diff --git a/layers/core_validation_error_enums.h b/layers/core_validation_error_enums.h
index 8076455..2ea083c 100644 (file)
--- 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 (file)
--- 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<std::mutex> 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<uint64_t>(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<uint64_t>(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<uint64_t>(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<uint64_t>(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<uint64_t>(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<uint64_t>(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<PFN_vkVoidFunction>(DestroySurfaceKHR)},
         {"vkGetPhysicalDeviceSurfaceSupportKHR", reinterpret_cast<PFN_vkVoidFunction>(GetPhysicalDeviceSurfaceSupportKHR)},
-        {"vkGetPhysicalDeviceSurfaceFormatsKHR", reinterpret_cast<PFN_vkVoidFunction>(GetPhysicalDeviceSurfaceFormatsKHR)},
         {"vkGetPhysicalDeviceDisplayPlanePropertiesKHR",
          reinterpret_cast<PFN_vkVoidFunction>(GetPhysicalDeviceDisplayPlanePropertiesKHR)},
         {"vkGetDisplayPlaneSupportedDisplaysKHR", reinterpret_cast<PFN_vkVoidFunction>(GetDisplayPlaneSupportedDisplaysKHR)},

diff --git a/layers/swapchain.h b/layers/swapchain.h
index 34ccdf4..e3c67bd 100644 (file)
--- a/layers/swapchain.h
+++ b/layers/swapchain.h
@@ -146,12 +146,6 @@ struct SwpPhysicalDevice {
     // called for:
     unordered_map<VkSurfaceKHR, SwpSurface *> 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;