layers: Don't pre-query surface attributes during creation

layers/core_validation.cpp

@@ -13636,20 +13636,21 @@ bool CoreChecks::ValidateCreateSwapchain(const char *func_name, VkSwapchainCreat
     // All physical devices and queue families are required to be able to present to any native window on Android; require the
     // application to have established support on any other platform.
     if (!instance_extensions.vk_khr_android_surface) {
-        auto support_predicate = [this](decltype(surface_state->gpu_queue_support)::value_type qs) -> bool {
-            // TODO: should restrict search only to queue families of VkDeviceQueueCreateInfos, not whole phys. device
-            return (qs.first.gpu == physical_device) && qs.second;
-        };
-        const auto &support = surface_state->gpu_queue_support;
-        bool is_supported = std::any_of(support.begin(), support.end(), support_predicate);
+        // restrict search only to queue families of VkDeviceQueueCreateInfos, not the whole physical device
+        bool is_supported = false;
+        for (const auto &queue_entry : queueMap) {
+            auto qfi = queue_entry.second->queueFamilyIndex;
+            if (surface_state->GetQueueSupport(physical_device, qfi)) {
+                is_supported = true;
+                break;
+            }
+        }
 
         if (!is_supported) {
-            if (LogError(
-                    device, "VUID-VkSwapchainCreateInfoKHR-surface-01270",
-                    "%s: pCreateInfo->surface is not known at this time to be supported for presentation by this device. The "
-                    "vkGetPhysicalDeviceSurfaceSupportKHR() must be called beforehand, and it must return VK_TRUE support with "
-                    "this surface for at least one queue family of this device.",
-                    func_name)) {
+            LogObjectList objs(device);
+            objs.add(surface_state->Handle());
+            if (LogError(objs, "VUID-VkSwapchainCreateInfoKHR-surface-01270",
+                    "%s: pCreateInfo->surface is not supported for presentation by this device.", func_name)) {
                 return true;
             }
         }
@@ -13823,17 +13824,13 @@ bool CoreChecks::ValidateCreateSwapchain(const char *func_name, VkSwapchainCreat
     }
 
     // Validate pCreateInfo values with the results of vkGetPhysicalDeviceSurfaceFormatsKHR():
-    std::vector<VkSurfaceFormatKHR> surface_formats;
-    uint32_t surface_format_count = 0;
-    DispatchGetPhysicalDeviceSurfaceFormatsKHR(physical_device, pCreateInfo->surface, &surface_format_count, nullptr);
-    surface_formats.resize(surface_format_count);
-    DispatchGetPhysicalDeviceSurfaceFormatsKHR(physical_device, pCreateInfo->surface, &surface_format_count, &surface_formats[0]);
     {
         // Validate pCreateInfo->imageFormat against VkSurfaceFormatKHR::format:
         bool found_format = false;
         bool found_color_space = false;
         bool found_match = false;
-        for (const auto &format : surface_formats) {
+        const auto formats = surface_state->GetFormats(physical_device);
+        for (const auto &format : formats) {
             if (pCreateInfo->imageFormat == format.format) {
                 // Validate pCreateInfo->imageColorSpace against VkSurfaceFormatKHR::colorSpace:
                 found_format = true;
@@ -13865,16 +13862,8 @@ bool CoreChecks::ValidateCreateSwapchain(const char *func_name, VkSwapchainCreat
         }
     }
 
-    // Validate pCreateInfo values with the results of vkGetPhysicalDeviceSurfacePresentModesKHR():
-    std::vector<VkPresentModeKHR> present_modes;
-    uint32_t present_mode_count = 0;
-    DispatchGetPhysicalDeviceSurfacePresentModesKHR(physical_device_state->PhysDev(), pCreateInfo->surface, &present_mode_count,
-                                                    nullptr);
-    present_modes.resize(present_mode_count);
-    DispatchGetPhysicalDeviceSurfacePresentModesKHR(physical_device_state->PhysDev(), pCreateInfo->surface, &present_mode_count,
-                                                    &present_modes[0]);
-
     // Validate pCreateInfo->presentMode against vkGetPhysicalDeviceSurfacePresentModesKHR():
+    auto present_modes = surface_state->GetPresentModes(physical_device);
     bool found_match = std::find(present_modes.begin(), present_modes.end(), present_mode) != present_modes.end();
     if (!found_match) {
         if (LogError(device, "VUID-VkSwapchainCreateInfoKHR-presentMode-01281",
@@ -14165,18 +14154,10 @@ bool CoreChecks::PreCallValidateQueuePresentKHR(VkQueue queue, const VkPresentIn
                 }
             }
 
-            // All physical devices and queue families are required to be able to present to any native window on Android; require
-            // the application to have established support on any other platform.
+            // All physical devices and queue families are required to be able to present to any native window on Android
             if (!instance_extensions.vk_khr_android_surface) {
                 const auto surface_state = GetSurfaceState(swapchain_data->createInfo.surface);
-                auto support_it = surface_state->gpu_queue_support.find({physical_device, queue_state->queueFamilyIndex});
-
-                if (support_it == surface_state->gpu_queue_support.end()) {
-                    skip |= LogError(
-                        pPresentInfo->pSwapchains[i], kVUID_Core_DrawState_SwapchainUnsupportedQueue,
-                        "vkQueuePresentKHR: Presenting pSwapchains[%u] image without calling vkGetPhysicalDeviceSurfaceSupportKHR",
-                        i);
-                } else if (!support_it->second) {
+                if (!surface_state->GetQueueSupport(physical_device, queue_state->queueFamilyIndex)) {
                     skip |= LogError(
                         pPresentInfo->pSwapchains[i], "VUID-vkQueuePresentKHR-pSwapchains-01292",
                         "vkQueuePresentKHR: Presenting pSwapchains[%u] image on queue that cannot present to this surface.", i);
@@ -14335,7 +14316,8 @@ bool CoreChecks::ValidateAcquireNextImage(VkDevice device, const AcquireVersion
 
         const uint32_t acquired_images = swapchain_data->acquired_images;
         const uint32_t swapchain_image_count = static_cast<uint32_t>(swapchain_data->images.size());
-        const auto min_image_count = swapchain_data->surface_capabilities.minImageCount;
+        auto caps = swapchain_data->surface->GetCapabilities(physical_device);
+        const auto min_image_count = caps.minImageCount;
         const bool too_many_already_acquired = acquired_images > swapchain_image_count - min_image_count;
         if (timeout == UINT64_MAX && too_many_already_acquired) {
             const char *vuid = version == ACQUIRE_VERSION_2 ? "VUID-vkAcquireNextImage2KHR-swapchain-01803"
@@ -16435,26 +16417,16 @@ bool CoreChecks::ValidatePhysicalDeviceSurfaceSupport(VkPhysicalDevice physicalD
     bool skip = false;
 
     const auto *pd_state = GetPhysicalDeviceState(physicalDevice);
-    if (pd_state) {
-        const auto *surface_state = GetSurfaceState(surface);
-        VkBool32 supported = VK_FALSE;
-        for (uint32_t i = 0; i < pd_state->queue_family_known_count; ++i) {
-            bool checked = false;
-            if (surface_state) {
-                const auto support_it = surface_state->gpu_queue_support.find({physicalDevice, i});
-                if (support_it != surface_state->gpu_queue_support.end()) {
-                    supported = support_it->second;
-                    checked = true;
-                }
-            }
-            if (!checked) {
-                DispatchGetPhysicalDeviceSurfaceSupportKHR(physicalDevice, i, surface, &supported);
-            }
-            if (supported) {
+    const auto *surface_state = GetSurfaceState(surface);
+    if (pd_state && surface_state) {
+        bool is_supported = false;
+        for (uint32_t i = 0; i < pd_state->queue_family_properties.size(); i++) {
+            if (surface_state->GetQueueSupport(physicalDevice, i)) {
+                is_supported = true;
                 break;
             }
         }
-        if (!supported) {
+        if (!is_supported) {
             skip |= LogError(physicalDevice, vuid, "%s(): surface is not supported by the physicalDevice.", func_name);
         }
     }

layers/image_state.cpp

@@ -499,21 +499,14 @@ static safe_VkImageCreateInfo GetImageCreateInfo(const VkSwapchainCreateInfoKHR
     return safe_VkImageCreateInfo(&image_ci);
 }
 
-static VkSurfaceCapabilitiesKHR GetSurfaceCaps(VkPhysicalDevice physical_device, VkSurfaceKHR surface) {
-    VkSurfaceCapabilitiesKHR result{};
-    DispatchGetPhysicalDeviceSurfaceCapabilitiesKHR(physical_device, surface, &result);
-    return result;
-}
-
 SWAPCHAIN_NODE::SWAPCHAIN_NODE(ValidationStateTracker *dev_data_, const VkSwapchainCreateInfoKHR *pCreateInfo,
                                VkSwapchainKHR swapchain)
     : BASE_NODE(swapchain, kVulkanObjectTypeSwapchainKHR),
       createInfo(pCreateInfo),
       shared_presentable(VK_PRESENT_MODE_SHARED_DEMAND_REFRESH_KHR == pCreateInfo->presentMode ||
                          VK_PRESENT_MODE_SHARED_CONTINUOUS_REFRESH_KHR == pCreateInfo->presentMode),
       image_create_info(GetImageCreateInfo(pCreateInfo)),
-      dev_data(dev_data_),
-      surface_capabilities(GetSurfaceCaps(dev_data->physical_device, pCreateInfo->surface)) {}
+      dev_data(dev_data_) {}
 
 void SWAPCHAIN_NODE::PresentImage(uint32_t image_index) {
     if (image_index >= images.size()) return;
@@ -545,9 +538,9 @@ void SWAPCHAIN_NODE::AcquireImage(uint32_t image_index) {
 void SWAPCHAIN_NODE::Destroy() {
     for (auto &swapchain_image : images) {
         if (swapchain_image.image_state) {
+            RemoveParent(swapchain_image.image_state);
             swapchain_image.image_state->Destroy();
             dev_data->imageMap.erase(swapchain_image.image_state->image());
-            swapchain_image.image_state = nullptr;
         }
         // NOTE: We don't have access to dev_data->fake_memory.Free() here, but it is currently a no-op
     }
@@ -579,3 +572,78 @@ void SURFACE_STATE::RemoveParent(BASE_NODE *parent_node) {
     }
     BASE_NODE::RemoveParent(parent_node);
 }
+
+void SURFACE_STATE::SetQueueSupport(VkPhysicalDevice phys_dev, uint32_t qfi, bool supported) {
+    assert(phys_dev);
+    GpuQueue key{phys_dev, qfi};
+    gpu_queue_support_[key] = supported;
+}
+
+bool SURFACE_STATE::GetQueueSupport(VkPhysicalDevice phys_dev, uint32_t qfi) const {
+    assert(phys_dev);
+    GpuQueue key{phys_dev, qfi};
+    auto iter = gpu_queue_support_.find(key);
+    if (iter != gpu_queue_support_.end()) {
+        return iter->second;
+    }
+    VkBool32 supported = VK_FALSE;
+    DispatchGetPhysicalDeviceSurfaceSupportKHR(phys_dev, qfi, surface(), &supported);
+    gpu_queue_support_[key] = (supported == VK_TRUE);
+    return supported == VK_TRUE;
+}
+
+void SURFACE_STATE::SetPresentModes(VkPhysicalDevice phys_dev, std::vector<VkPresentModeKHR> &&modes) {
+    assert(phys_dev);
+    present_modes_[phys_dev] = std::move(modes);
+}
+
+std::vector<VkPresentModeKHR> SURFACE_STATE::GetPresentModes(VkPhysicalDevice phys_dev) const {
+    assert(phys_dev);
+    auto iter = present_modes_.find(phys_dev);
+    if (iter != present_modes_.end()) {
+        return iter->second;
+    }
+    std::vector<VkPresentModeKHR> result;
+    uint32_t count = 0;
+    DispatchGetPhysicalDeviceSurfacePresentModesKHR(phys_dev, surface(), &count, nullptr);
+    result.resize(count);
+    DispatchGetPhysicalDeviceSurfacePresentModesKHR(phys_dev, surface(), &count, result.data());
+    return result;
+}
+
+void SURFACE_STATE::SetFormats(VkPhysicalDevice phys_dev, std::vector<VkSurfaceFormatKHR> &&fmts) {
+    assert(phys_dev);
+    formats_[phys_dev] = std::move(fmts);
+}
+
+std::vector<VkSurfaceFormatKHR> SURFACE_STATE::GetFormats(VkPhysicalDevice phys_dev) const {
+    assert(phys_dev);
+    auto iter = formats_.find(phys_dev);
+    if (iter != formats_.end()) {
+        return iter->second;
+    }
+    std::vector<VkSurfaceFormatKHR> result;
+    uint32_t count = 0;
+    DispatchGetPhysicalDeviceSurfaceFormatsKHR(phys_dev, surface(), &count, nullptr);
+    result.resize(count);
+    DispatchGetPhysicalDeviceSurfaceFormatsKHR(phys_dev, surface(), &count, result.data());
+    formats_[phys_dev] = result;
+    return result;
+}
+
+void SURFACE_STATE::SetCapabilities(VkPhysicalDevice phys_dev, const VkSurfaceCapabilitiesKHR &caps) {
+    assert(phys_dev);
+    capabilities_[phys_dev] = caps;
+}
+
+VkSurfaceCapabilitiesKHR SURFACE_STATE::GetCapabilities(VkPhysicalDevice phys_dev) const {
+    assert(phys_dev);
+    auto iter = capabilities_.find(phys_dev);
+    if (iter != capabilities_.end()) {
+        return iter->second;
+    }
+    VkSurfaceCapabilitiesKHR result{};
+    DispatchGetPhysicalDeviceSurfaceCapabilitiesKHR(phys_dev, surface(), &result);
+    capabilities_[phys_dev] = result;
+    return result;
+}

layers/image_state.h

@@ -250,7 +250,6 @@ class SWAPCHAIN_NODE : public BASE_NODE {
     const safe_VkImageCreateInfo image_create_info;
     std::shared_ptr<SURFACE_STATE> surface;
     ValidationStateTracker *dev_data;
-    const VkSurfaceCapabilitiesKHR surface_capabilities;
     uint32_t acquired_images = 0;
 
     SWAPCHAIN_NODE(ValidationStateTracker *dev_data, const VkSwapchainCreateInfoKHR *pCreateInfo, VkSwapchainKHR swapchain);
@@ -298,10 +297,7 @@ struct hash<GpuQueue> {
 //    SURFACE_STATE -> nothing
 class SURFACE_STATE : public BASE_NODE {
   public:
-    SWAPCHAIN_NODE *swapchain;
-    layer_data::unordered_map<GpuQueue, bool> gpu_queue_support;
-
-    SURFACE_STATE(VkSurfaceKHR s) : BASE_NODE(s, kVulkanObjectTypeSurfaceKHR), swapchain(nullptr), gpu_queue_support() {}
+    SURFACE_STATE(VkSurfaceKHR s) : BASE_NODE(s, kVulkanObjectTypeSurfaceKHR) {}
 
     ~SURFACE_STATE() {
         if (!Destroyed()) {
@@ -316,4 +312,24 @@ class SURFACE_STATE : public BASE_NODE {
     VkImageCreateInfo GetImageCreateInfo() const;
 
     void RemoveParent(BASE_NODE *parent_node) override;
+
+    void SetQueueSupport(VkPhysicalDevice phys_dev, uint32_t qfi, bool supported);
+    bool GetQueueSupport(VkPhysicalDevice phys_dev, uint32_t qfi) const;
+
+    void SetPresentModes(VkPhysicalDevice phys_dev, std::vector<VkPresentModeKHR> &&modes);
+    std::vector<VkPresentModeKHR> GetPresentModes(VkPhysicalDevice phys_dev) const;
+
+    void SetFormats(VkPhysicalDevice phys_dev, std::vector<VkSurfaceFormatKHR> &&fmts);
+    std::vector<VkSurfaceFormatKHR> GetFormats(VkPhysicalDevice phys_dev) const;
+
+    void SetCapabilities(VkPhysicalDevice phys_dev, const VkSurfaceCapabilitiesKHR &caps);
+    VkSurfaceCapabilitiesKHR GetCapabilities(VkPhysicalDevice phys_dev) const;
+
+    SWAPCHAIN_NODE *swapchain{nullptr};
+
+  private:
+    mutable layer_data::unordered_map<GpuQueue, bool> gpu_queue_support_;
+    mutable layer_data::unordered_map<VkPhysicalDevice, std::vector<VkPresentModeKHR>> present_modes_;
+    mutable layer_data::unordered_map<VkPhysicalDevice, std::vector<VkSurfaceFormatKHR>> formats_;
+    mutable layer_data::unordered_map<VkPhysicalDevice, VkSurfaceCapabilitiesKHR> capabilities_;
 };