layers: Fix Graphics Pipeline pointers not ignored

Some VkGraphicsPipelineCreateInfo pointers must be ignored under some
conditions, but were not in the layers.

Add relevant tests.

Fix tests found broken (using depth or color without attachment in
subpass)

7 files changed, 201 insertions, 22 deletions

diff --git a/layers/core_validation.cpp b/layers/core_validation.cpp
index 01040e4a..42f656e3 100644
--- a/layers/core_validation.cpp
+++ b/layers/core_validation.cpp
@@ -1287,13 +1287,13 @@ static bool ValidatePipelineUnlocked(layer_data *dev_data, std::vector<std::uniq
     // Ensure the subpass index is valid. If not, then validate_and_capture_pipeline_shader_state
     // produces nonsense errors that confuse users. Other layers should already
     // emit errors for renderpass being invalid.
-    auto subpass_desc = &pPipeline->render_pass_ci.pSubpasses[pPipeline->graphicsPipelineCI.subpass];
-    if (pPipeline->graphicsPipelineCI.subpass >= pPipeline->render_pass_ci.subpassCount) {
+    auto subpass_desc = &pPipeline->rp_state->createInfo.pSubpasses[pPipeline->graphicsPipelineCI.subpass];
+    if (pPipeline->graphicsPipelineCI.subpass >= pPipeline->rp_state->createInfo.subpassCount) {
         skip |= log_msg(dev_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_PIPELINE_EXT,
                         HandleToUint64(pPipeline->pipeline), __LINE__, VALIDATION_ERROR_096005ee, "DS",
                         "Invalid Pipeline CreateInfo State: Subpass index %u "
                             "is out of range for this renderpass (0..%u). %s",
-                        pPipeline->graphicsPipelineCI.subpass, pPipeline->render_pass_ci.subpassCount - 1,
+                        pPipeline->graphicsPipelineCI.subpass, pPipeline->rp_state->createInfo.subpassCount - 1,
                         validation_error_map[VALIDATION_ERROR_096005ee]);
         subpass_desc = nullptr;
     }
@@ -1500,11 +1500,11 @@ static bool ValidatePipelineUnlocked(layer_data *dev_data, std::vector<std::uniq
         VkSampleCountFlagBits max_sample_count = static_cast<VkSampleCountFlagBits>(0);
         for (uint32_t i = 0; i < subpass_desc->colorAttachmentCount; ++i) {
             if (subpass_desc->pColorAttachments[i].attachment != VK_ATTACHMENT_UNUSED) {
-                max_sample_count = std::max(max_sample_count, pPipeline->render_pass_ci.pAttachments[subpass_desc->pColorAttachments[i].attachment].samples);
+                max_sample_count = std::max(max_sample_count, pPipeline->rp_state->createInfo.pAttachments[subpass_desc->pColorAttachments[i].attachment].samples);
             }
         }
         if (subpass_desc->pDepthStencilAttachment && subpass_desc->pDepthStencilAttachment->attachment != VK_ATTACHMENT_UNUSED) {
-            max_sample_count = std::max(max_sample_count, pPipeline->render_pass_ci.pAttachments[subpass_desc->pDepthStencilAttachment->attachment].samples);
+            max_sample_count = std::max(max_sample_count, pPipeline->rp_state->createInfo.pAttachments[subpass_desc->pDepthStencilAttachment->attachment].samples);
         }
         if (pPipeline->graphicsPipelineCI.pMultisampleState->rasterizationSamples != max_sample_count) {
             skip |=
@@ -4613,7 +4613,6 @@ VKAPI_ATTR VkResult VKAPI_CALL CreateGraphicsPipelines(VkDevice device, VkPipeli
     for (i = 0; i < count; i++) {
         pipe_state.push_back(std::unique_ptr<PIPELINE_STATE>(new PIPELINE_STATE));
         pipe_state[i]->initGraphicsPipeline(&pCreateInfos[i], GetRenderPassStateSharedPtr(dev_data, pCreateInfos[i].renderPass));
-        pipe_state[i]->render_pass_ci.initialize(GetRenderPassState(dev_data, pCreateInfos[i].renderPass)->createInfo.ptr());
         pipe_state[i]->pipeline_layout = *getPipelineLayout(dev_data, pCreateInfos[i].layout);
     }
 
diff --git a/layers/core_validation_types.h b/layers/core_validation_types.h
index bc5232b9..f661473a 100644
--- a/layers/core_validation_types.h
+++ b/layers/core_validation_types.h
@@ -559,8 +559,6 @@ class PIPELINE_STATE : public BASE_NODE {
     std::vector<VkVertexInputBindingDescription> vertexBindingDescriptions;
     std::vector<VkPipelineColorBlendAttachmentState> attachments;
     bool blendConstantsEnabled;  // Blend constants enabled for any attachments
-    // Store RPCI b/c renderPass may be destroyed after Pipeline creation
-    safe_VkRenderPassCreateInfo render_pass_ci;
     PIPELINE_LAYOUT_NODE pipeline_layout;
 
     // Default constructor
@@ -575,11 +573,26 @@ class PIPELINE_STATE : public BASE_NODE {
           vertexBindingDescriptions(),
           attachments(),
           blendConstantsEnabled(false),
-          render_pass_ci(),
           pipeline_layout() {}
 
     void initGraphicsPipeline(const VkGraphicsPipelineCreateInfo *pCreateInfo, std::shared_ptr<RENDER_PASS_STATE> &&rpstate) {
-        graphicsPipelineCI.initialize(pCreateInfo);
+        bool uses_color_attachment = false;
+        bool uses_depthstencil_attachment = false;
+        if (pCreateInfo->subpass < rpstate->createInfo.subpassCount) {
+            const auto &subpass = rpstate->createInfo.pSubpasses[pCreateInfo->subpass];
+
+            for (uint32_t i = 0; i < subpass.colorAttachmentCount; ++i) {
+                if (subpass.pColorAttachments[i].attachment != VK_ATTACHMENT_UNUSED) {
+                    uses_color_attachment = true;
+                    break;
+                }
+            }
+
+            if (subpass.pDepthStencilAttachment && subpass.pDepthStencilAttachment->attachment != VK_ATTACHMENT_UNUSED) {
+                uses_depthstencil_attachment = true;
+            }
+        }
+        graphicsPipelineCI.initialize(pCreateInfo, uses_color_attachment, uses_depthstencil_attachment);
         // Make sure compute pipeline is null
         VkComputePipelineCreateInfo emptyComputeCI = {};
         computePipelineCI.initialize(&emptyComputeCI);
@@ -588,15 +601,15 @@ class PIPELINE_STATE : public BASE_NODE {
             this->duplicate_shaders |= this->active_shaders & pPSSCI->stage;
             this->active_shaders |= pPSSCI->stage;
         }
-        if (pCreateInfo->pVertexInputState) {
-            const VkPipelineVertexInputStateCreateInfo *pVICI = pCreateInfo->pVertexInputState;
+        if (graphicsPipelineCI.pVertexInputState) {
+            const auto pVICI = graphicsPipelineCI.pVertexInputState;
             if (pVICI->vertexBindingDescriptionCount) {
                 this->vertexBindingDescriptions = std::vector<VkVertexInputBindingDescription>(
                     pVICI->pVertexBindingDescriptions, pVICI->pVertexBindingDescriptions + pVICI->vertexBindingDescriptionCount);
             }
         }
-        if (pCreateInfo->pColorBlendState) {
-            const VkPipelineColorBlendStateCreateInfo *pCBCI = pCreateInfo->pColorBlendState;
+        if (graphicsPipelineCI.pColorBlendState) {
+            const auto pCBCI = graphicsPipelineCI.pColorBlendState;
             if (pCBCI->attachmentCount) {
                 this->attachments = std::vector<VkPipelineColorBlendAttachmentState>(pCBCI->pAttachments,
                                                                                      pCBCI->pAttachments + pCBCI->attachmentCount);
@@ -609,7 +622,7 @@ class PIPELINE_STATE : public BASE_NODE {
         computePipelineCI.initialize(pCreateInfo);
         // Make sure gfx pipeline is null
         VkGraphicsPipelineCreateInfo emptyGraphicsCI = {};
-        graphicsPipelineCI.initialize(&emptyGraphicsCI);
+        graphicsPipelineCI.initialize(&emptyGraphicsCI, false, false);
         switch (computePipelineCI.stage.stage) {
             case VK_SHADER_STAGE_COMPUTE_BIT:
                 this->active_shaders |= VK_SHADER_STAGE_COMPUTE_BIT;
diff --git a/layers/parameter_validation.h b/layers/parameter_validation.h
index 8acd34bd..ced0810c 100644
--- a/layers/parameter_validation.h
+++ b/layers/parameter_validation.h
@@ -27,6 +27,7 @@
 #include <sstream>
 #include <bitset>
 #include <mutex>
+#include <unordered_map>
 #include <unordered_set>
 
 #include "vulkan/vulkan.h"
@@ -81,6 +82,13 @@ struct layer_data {
     VkDevice device = VK_NULL_HANDLE;
     DeviceExtensions extensions;
 
+    struct SubpassesUsageStates {
+        std::unordered_set<uint32_t> subpasses_using_color_attachment;
+        std::unordered_set<uint32_t> subpasses_using_depthstencil_attachment;
+    };
+
+    std::unordered_map<VkRenderPass, SubpassesUsageStates> renderpasses_states;
+
     VkLayerDispatchTable dispatch_table = {};
 };
 
diff --git a/layers/parameter_validation_utils.cpp b/layers/parameter_validation_utils.cpp
index 7617826e..3058cb4c 100644
--- a/layers/parameter_validation_utils.cpp
+++ b/layers/parameter_validation_utils.cpp
@@ -83,6 +83,10 @@ extern bool parameter_validation_vkDestroyDebugReportCallbackEXT(VkInstance inst
                                                                  const VkAllocationCallbacks *pAllocator);
 extern bool parameter_validation_vkCreateCommandPool(VkDevice device, const VkCommandPoolCreateInfo *pCreateInfo,
                                                      const VkAllocationCallbacks *pAllocator, VkCommandPool *pCommandPool);
+extern bool parameter_validation_vkCreateRenderPass(VkDevice device, const VkRenderPassCreateInfo *pCreateInfo,
+                                                    const VkAllocationCallbacks *pAllocator, VkRenderPass *pRenderPass);
+extern bool parameter_validation_vkDestroyRenderPass(VkDevice device, VkRenderPass renderPass,
+                                                     const VkAllocationCallbacks *pAllocator);
 
 // TODO : This can be much smarter, using separate locks for separate global data
 std::mutex global_lock;
@@ -578,6 +582,76 @@ VKAPI_ATTR VkResult VKAPI_CALL vkCreateQueryPool(VkDevice device, const VkQueryP
     return result;
 }
 
+VKAPI_ATTR VkResult VKAPI_CALL vkCreateRenderPass(VkDevice device, const VkRenderPassCreateInfo *pCreateInfo,
+                                                  const VkAllocationCallbacks *pAllocator, VkRenderPass *pRenderPass) {
+    layer_data *device_data = GetLayerDataPtr(get_dispatch_key(device), layer_data_map);
+    bool skip = false;
+    VkResult result = VK_ERROR_VALIDATION_FAILED_EXT;
+
+    {
+        std::unique_lock<std::mutex> lock(global_lock);
+        skip |= parameter_validation_vkCreateRenderPass(device, pCreateInfo, pAllocator, pRenderPass);
+
+        typedef bool (*PFN_manual_vkCreateRenderPass)(VkDevice device, const VkRenderPassCreateInfo* pCreateInfo, const VkAllocationCallbacks* pAllocator, VkRenderPass* pRenderPass);
+        PFN_manual_vkCreateRenderPass custom_func = (PFN_manual_vkCreateRenderPass)custom_functions["vkCreateRenderPass"];
+        if (custom_func != nullptr) {
+            skip |= custom_func(device, pCreateInfo, pAllocator, pRenderPass);
+        }
+    }
+
+    if (!skip) {
+        result = device_data->dispatch_table.CreateRenderPass(device, pCreateInfo, pAllocator, pRenderPass);
+
+        // track the state necessary for checking vkCreateGraphicsPipeline (subpass usage of depth and color attachments)
+        if (result == VK_SUCCESS) {
+            std::unique_lock<std::mutex> lock(global_lock);
+            const auto renderPass = *pRenderPass;
+            auto &renderpass_state = device_data->renderpasses_states[renderPass];
+
+            for (uint32_t subpass = 0; subpass < pCreateInfo->subpassCount; ++subpass) {
+                bool uses_color = false;
+                for (uint32_t i = 0; i < pCreateInfo->pSubpasses[subpass].colorAttachmentCount && !uses_color; ++i)
+                    if (pCreateInfo->pSubpasses[subpass].pColorAttachments[i].attachment != VK_ATTACHMENT_UNUSED) uses_color = true;
+
+                bool uses_depthstencil = false;
+                if (pCreateInfo->pSubpasses[subpass].pDepthStencilAttachment)
+                    if (pCreateInfo->pSubpasses[subpass].pDepthStencilAttachment->attachment != VK_ATTACHMENT_UNUSED)
+                        uses_depthstencil = true;
+
+                if (uses_color) renderpass_state.subpasses_using_color_attachment.insert(subpass);
+                if (uses_depthstencil) renderpass_state.subpasses_using_depthstencil_attachment.insert(subpass);
+            }
+        }
+    }
+    return result;
+}
+
+VKAPI_ATTR void VKAPI_CALL vkDestroyRenderPass(VkDevice device, VkRenderPass renderPass, const VkAllocationCallbacks *pAllocator) {
+    layer_data *device_data = GetLayerDataPtr(get_dispatch_key(device), layer_data_map);
+    bool skip = false;
+
+    {
+        std::unique_lock<std::mutex> lock(global_lock);
+        skip |= parameter_validation_vkDestroyRenderPass(device, renderPass, pAllocator);
+
+        typedef bool (*PFN_manual_vkDestroyRenderPass)(VkDevice device, VkRenderPass renderPass, const VkAllocationCallbacks* pAllocator);
+        PFN_manual_vkDestroyRenderPass custom_func = (PFN_manual_vkDestroyRenderPass)custom_functions["vkDestroyRenderPass"];
+        if (custom_func != nullptr) {
+            skip |= custom_func(device, renderPass, pAllocator);
+        }
+    }
+
+    if (!skip) {
+        device_data->dispatch_table.DestroyRenderPass(device, renderPass, pAllocator);
+
+        // track the state necessary for checking vkCreateGraphicsPipeline (subpass usage of depth and color attachments)
+        {
+            std::unique_lock<std::mutex> lock(global_lock);
+            device_data->renderpasses_states.erase(renderPass);
+        }
+    }
+}
+
 bool pv_vkCreateBuffer(VkDevice device, const VkBufferCreateInfo *pCreateInfo, const VkAllocationCallbacks *pAllocator,
                        VkBuffer *pBuffer) {
     bool skip = false;
@@ -1238,8 +1312,20 @@ bool pv_vkCreateGraphicsPipelines(VkDevice device, VkPipelineCache pipelineCache
                     }
                 }
 
-                // TODO: Conditional NULL check based on subpass depth/stencil attachment
-                if (pCreateInfos[i].pDepthStencilState != nullptr) {
+                bool uses_color_attachment = false;
+                bool uses_depthstencil_attachment = false;
+                {
+                    const auto subpasses_uses_it = device_data->renderpasses_states.find(pCreateInfos[i].renderPass);
+                    if (subpasses_uses_it != device_data->renderpasses_states.end()) {
+                        const auto &subpasses_uses = subpasses_uses_it->second;
+                        if (subpasses_uses.subpasses_using_color_attachment.count(pCreateInfos[i].subpass))
+                            uses_color_attachment = true;
+                        if (subpasses_uses.subpasses_using_depthstencil_attachment.count(pCreateInfos[i].subpass))
+                            uses_depthstencil_attachment = true;
+                    }
+                }
+
+                if (pCreateInfos[i].pDepthStencilState != nullptr && uses_depthstencil_attachment) {
                     skip |= validate_struct_pnext(
                         report_data, "vkCreateGraphicsPipelines",
                         ParameterName("pCreateInfos[%i].pDepthStencilState->pNext", ParameterName::IndexVector{i}), NULL,
@@ -1333,8 +1419,7 @@ bool pv_vkCreateGraphicsPipelines(VkDevice device, VkPipelineCache pipelineCache
                     }
                 }
 
-                // TODO: Conditional NULL check based on subpass color attachment
-                if (pCreateInfos[i].pColorBlendState != nullptr) {
+                if (pCreateInfos[i].pColorBlendState != nullptr && uses_color_attachment) {
                     skip |= validate_struct_pnext(
                         report_data, "vkCreateGraphicsPipelines",
                         ParameterName("pCreateInfos[%i].pColorBlendState->pNext", ParameterName::IndexVector{i}), NULL,
diff --git a/layers/shader_validation.cpp b/layers/shader_validation.cpp
index 7847e3de..0af30fbe 100644
--- a/layers/shader_validation.cpp
+++ b/layers/shader_validation.cpp
@@ -771,7 +771,7 @@ static bool validate_vi_against_vs_inputs(debug_report_data const *report_data,
 static bool validate_fs_outputs_against_render_pass(debug_report_data const *report_data, shader_module const *fs,
                                                     spirv_inst_iter entrypoint, PIPELINE_STATE const *pipeline,
                                                     uint32_t subpass_index) {
-    auto rpci = pipeline->render_pass_ci.ptr();
+    auto rpci = pipeline->rp_state->createInfo.ptr();
 
     std::map<uint32_t, VkFormat> color_attachments;
     auto subpass = rpci->pSubpasses[subpass_index];
@@ -1356,7 +1356,7 @@ static bool validate_pipeline_shader_stage(
     if (pStage->stage == VK_SHADER_STAGE_FRAGMENT_BIT) {
         auto input_attachment_uses = collect_interface_by_input_attachment_index(module, accessible_ids);
 
-        auto rpci = pipeline->render_pass_ci.ptr();
+        auto rpci = pipeline->rp_state->createInfo.ptr();
         auto subpass = pipeline->graphicsPipelineCI.subpass;
 
         for (auto use : input_attachment_uses) {
diff --git a/layers/unique_objects.cpp b/layers/unique_objects.cpp
index 8515a645..0fa94a7f 100644
--- a/layers/unique_objects.cpp
+++ b/layers/unique_objects.cpp
@@ -328,7 +328,22 @@ VKAPI_ATTR VkResult VKAPI_CALL CreateGraphicsPipelines(VkDevice device, VkPipeli
         local_pCreateInfos = new safe_VkGraphicsPipelineCreateInfo[createInfoCount];
         std::lock_guard<std::mutex> lock(global_lock);
         for (uint32_t idx0 = 0; idx0 < createInfoCount; ++idx0) {
-            local_pCreateInfos[idx0].initialize(&pCreateInfos[idx0]);
+            bool uses_color_attachment = false;
+            bool uses_depthstencil_attachment = false;
+            {
+                const auto subpasses_uses_it =
+                    device_data->renderpasses_states.find(Unwrap(device_data, pCreateInfos[idx0].renderPass));
+                if (subpasses_uses_it != device_data->renderpasses_states.end()) {
+                    const auto &subpasses_uses = subpasses_uses_it->second;
+                    if (subpasses_uses.subpasses_using_color_attachment.count(pCreateInfos[idx0].subpass))
+                        uses_color_attachment = true;
+                    if (subpasses_uses.subpasses_using_depthstencil_attachment.count(pCreateInfos[idx0].subpass))
+                        uses_depthstencil_attachment = true;
+                }
+            }
+
+            local_pCreateInfos[idx0].initialize(&pCreateInfos[idx0], uses_color_attachment, uses_depthstencil_attachment);
+
             if (pCreateInfos[idx0].basePipelineHandle) {
                 local_pCreateInfos[idx0].basePipelineHandle = Unwrap(device_data, pCreateInfos[idx0].basePipelineHandle);
             }
@@ -366,6 +381,55 @@ VKAPI_ATTR VkResult VKAPI_CALL CreateGraphicsPipelines(VkDevice device, VkPipeli
     return result;
 }
 
+static void PostCallCreateRenderPass(layer_data *dev_data, const VkRenderPassCreateInfo *pCreateInfo, VkRenderPass renderPass) {
+    auto &renderpass_state = dev_data->renderpasses_states[renderPass];
+
+    for (uint32_t subpass = 0; subpass < pCreateInfo->subpassCount; ++subpass) {
+        bool uses_color = false;
+        for (uint32_t i = 0; i < pCreateInfo->pSubpasses[subpass].colorAttachmentCount && !uses_color; ++i)
+            if (pCreateInfo->pSubpasses[subpass].pColorAttachments[i].attachment != VK_ATTACHMENT_UNUSED) uses_color = true;
+
+        bool uses_depthstencil = false;
+        if (pCreateInfo->pSubpasses[subpass].pDepthStencilAttachment)
+            if (pCreateInfo->pSubpasses[subpass].pDepthStencilAttachment->attachment != VK_ATTACHMENT_UNUSED)
+                uses_depthstencil = true;
+
+        if (uses_color) renderpass_state.subpasses_using_color_attachment.insert(subpass);
+        if (uses_depthstencil) renderpass_state.subpasses_using_depthstencil_attachment.insert(subpass);
+    }
+}
+
+VKAPI_ATTR VkResult VKAPI_CALL CreateRenderPass(VkDevice device, const VkRenderPassCreateInfo *pCreateInfo,
+                                                const VkAllocationCallbacks *pAllocator, VkRenderPass *pRenderPass) {
+    layer_data *dev_data = GetLayerDataPtr(get_dispatch_key(device), layer_data_map);
+    VkResult result = dev_data->dispatch_table.CreateRenderPass(device, pCreateInfo, pAllocator, pRenderPass);
+    if (VK_SUCCESS == result) {
+        std::lock_guard<std::mutex> lock(global_lock);
+
+        PostCallCreateRenderPass(dev_data, pCreateInfo, *pRenderPass);
+
+        *pRenderPass = WrapNew(dev_data, *pRenderPass);
+    }
+    return result;
+}
+
+static void PostCallDestroyRenderPass(layer_data *dev_data, VkRenderPass renderPass) {
+    dev_data->renderpasses_states.erase(renderPass);
+}
+
+VKAPI_ATTR void VKAPI_CALL DestroyRenderPass(VkDevice device, VkRenderPass renderPass, const VkAllocationCallbacks *pAllocator) {
+    layer_data *dev_data = GetLayerDataPtr(get_dispatch_key(device), layer_data_map);
+    std::unique_lock<std::mutex> lock(global_lock);
+    uint64_t renderPass_id = reinterpret_cast<uint64_t &>(renderPass);
+    renderPass = (VkRenderPass)dev_data->unique_id_mapping[renderPass_id];
+    dev_data->unique_id_mapping.erase(renderPass_id);
+    lock.unlock();
+    dev_data->dispatch_table.DestroyRenderPass(device, renderPass, pAllocator);
+
+    lock.lock();
+    PostCallDestroyRenderPass(dev_data, renderPass);
+}
+
 VKAPI_ATTR VkResult VKAPI_CALL CreateSwapchainKHR(VkDevice device, const VkSwapchainCreateInfoKHR *pCreateInfo,
                                                   const VkAllocationCallbacks *pAllocator, VkSwapchainKHR *pSwapchain) {
     layer_data *my_map_data = GetLayerDataPtr(get_dispatch_key(device), layer_data_map);
diff --git a/layers/unique_objects.h b/layers/unique_objects.h
index 57b9dbf3..e604e915 100644
--- a/layers/unique_objects.h
+++ b/layers/unique_objects.h
@@ -21,6 +21,9 @@
 
 #include "vulkan/vulkan.h"
 
+#include <unordered_map>
+#include <unordered_set>
+
 #include "vk_layer_data.h"
 #include "vk_safe_struct.h"
 #include "vk_layer_utils.h"
@@ -69,6 +72,13 @@ struct layer_data {
     std::unordered_map<uint64_t, uint64_t> unique_id_mapping;  // Map uniqueID to actual object handle
     VkPhysicalDevice gpu;
 
+    struct SubpassesUsageStates {
+        std::unordered_set<uint32_t> subpasses_using_color_attachment;
+        std::unordered_set<uint32_t> subpasses_using_depthstencil_attachment;
+    };
+    // uses unwrapped handles
+    std::unordered_map<VkRenderPass, SubpassesUsageStates> renderpasses_states;
+
     layer_data() : wsi_enabled(false), gpu(VK_NULL_HANDLE){};
 };