Refactor pNext-util, enforce usage (#1700)

Refactor pNext-util, enforce usage

- pNext-chains: replace a commonly repeated loop-pattern with an existing utility-function
- resolve sType with existing GetSType-util
- rename, doc and move to dedicated header
- add a static_assert to ensure only vulkan-structs are passed
- applied in locations matching grep-pattern '== VK_STRUCTURE_TYPE'

android/framework/graphics/CMakeLists.txt

@@ -13,6 +13,7 @@ target_sources(gfxrecon_graphics
                     ${GFXRECON_SOURCE_DIR}/framework/graphics/vulkan_util.h
                     ${GFXRECON_SOURCE_DIR}/framework/graphics/vulkan_util.cpp
                     ${GFXRECON_SOURCE_DIR}/framework/graphics/vulkan_struct_deep_copy.h
+                    ${GFXRECON_SOURCE_DIR}/framework/graphics/vulkan_struct_get_pnext.h
                     ${GFXRECON_SOURCE_DIR}/framework/generated/generated_vulkan_struct_deep_copy.cpp
                     ${GFXRECON_SOURCE_DIR}/framework/generated/generated_vulkan_struct_deep_copy_stype.cpp
                     ${GFXRECON_SOURCE_DIR}/framework/graphics/vulkan_struct_extract_handles.h

framework/decode/vulkan_replay_consumer_base.cpp

@@ -41,6 +41,7 @@
 #include "graphics/vulkan_check_buffer_references.h"
 #include "graphics/vulkan_device_util.h"
 #include "graphics/vulkan_util.h"
+#include "graphics/vulkan_struct_get_pnext.h"
 #include "graphics/vulkan_struct_deep_copy.h"
 #include "graphics/vulkan_struct_extract_handles.h"
 #include "util/file_path.h"
@@ -1297,33 +1298,16 @@ void VulkanReplayConsumerBase::SetPhysicalDeviceProperties(PhysicalDeviceInfo*
 {
     SetPhysicalDeviceProperties(physical_device_info, &capture_properties->properties, &replay_properties->properties);
 
-    auto get_ray_properties =
-        [](const VkPhysicalDeviceProperties2* device_props) -> const VkPhysicalDeviceRayTracingPipelinePropertiesKHR* {
-        if (device_props != nullptr)
-        {
-            const void* pNext = device_props->pNext;
-
-            while (pNext != nullptr)
-            {
-                auto base = reinterpret_cast<const VkBaseInStructure*>(pNext);
-                if (base->sType == VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_RAY_TRACING_PIPELINE_PROPERTIES_KHR)
-                {
-                    return reinterpret_cast<const VkPhysicalDeviceRayTracingPipelinePropertiesKHR*>(base);
-                }
-                pNext = base->pNext;
-            }
-        }
-        return nullptr;
-    };
-
-    if (auto ray_capture_props = get_ray_properties(capture_properties))
+    if (auto ray_capture_props =
+            graphics::vulkan_struct_get_pnext<VkPhysicalDeviceRayTracingPipelinePropertiesKHR>(capture_properties))
     {
         physical_device_info->shaderGroupBaseAlignment   = ray_capture_props->shaderGroupBaseAlignment;
         physical_device_info->shaderGroupHandleAlignment = ray_capture_props->shaderGroupHandleAlignment;
         physical_device_info->shaderGroupHandleSize      = ray_capture_props->shaderGroupHandleSize;
     }
 
-    if (auto ray_replay_props = get_ray_properties(replay_properties))
+    if (auto ray_replay_props =
+            graphics::vulkan_struct_get_pnext<VkPhysicalDeviceRayTracingPipelinePropertiesKHR>(replay_properties))
     {
         physical_device_info->replay_device_info->raytracing_properties        = *ray_replay_props;
         physical_device_info->replay_device_info->raytracing_properties->pNext = nullptr;
@@ -1949,25 +1933,16 @@ void VulkanReplayConsumerBase::ProcessCreateInstanceDebugCallbackInfo(const Deco
 {
     assert(instance_info != nullptr);
 
-    if (instance_info->pNext != nullptr)
+    if (auto debug_report_info =
+            graphics::vulkan_struct_get_pnext<VkDebugReportCallbackCreateInfoEXT>(instance_info->decoded_value))
     {
-        // 'Out' struct for non-const pNext pointers.
-        auto pnext = reinterpret_cast<VkBaseOutStructure*>(instance_info->pNext->GetPointer());
-        while (pnext != nullptr)
-        {
-            if (pnext->sType == VK_STRUCTURE_TYPE_DEBUG_REPORT_CALLBACK_CREATE_INFO_EXT)
-            {
-                auto debug_report_info         = reinterpret_cast<VkDebugReportCallbackCreateInfoEXT*>(pnext);
-                debug_report_info->pfnCallback = DebugReportCallback;
-            }
-            else if (pnext->sType == VK_STRUCTURE_TYPE_DEBUG_UTILS_MESSENGER_CREATE_INFO_EXT)
-            {
-                auto debug_utils_info             = reinterpret_cast<VkDebugUtilsMessengerCreateInfoEXT*>(pnext);
-                debug_utils_info->pfnUserCallback = DebugUtilsCallback;
-            }
+        debug_report_info->pfnCallback = DebugReportCallback;
+    }
 
-            pnext = pnext->pNext;
-        }
+    if (auto debug_utils_info =
+            graphics::vulkan_struct_get_pnext<VkDebugUtilsMessengerCreateInfoEXT>(instance_info->decoded_value))
+    {
+        debug_utils_info->pfnUserCallback = DebugUtilsCallback;
     }
 }
 
@@ -1976,49 +1951,38 @@ void VulkanReplayConsumerBase::ProcessSwapchainFullScreenExclusiveInfo(
 {
     assert(swapchain_info != nullptr);
 
-    if (swapchain_info->pNext != nullptr)
+    if (auto full_screen_info =
+            graphics::vulkan_struct_get_pnext<VkSurfaceFullScreenExclusiveWin32InfoEXT>(swapchain_info->decoded_value))
     {
-        // 'Out' struct for non-const pNext pointers.
-        auto pnext = reinterpret_cast<VkBaseOutStructure*>(swapchain_info->pNext->GetPointer());
-        while (pnext != nullptr)
-        {
-            if (pnext->sType == VK_STRUCTURE_TYPE_SURFACE_FULL_SCREEN_EXCLUSIVE_WIN32_INFO_EXT)
-            {
 #if defined(VK_USE_PLATFORM_WIN32_KHR)
-                // Get the surface info from the Decoded_VkSwapchainCreateInfoKHR handle id.
-                HMONITOR   hmonitor     = nullptr;
-                const auto surface_info = object_info_table_.GetSurfaceKHRInfo(swapchain_info->surface);
-
-                if ((surface_info != nullptr) && (surface_info->window != nullptr))
-                {
-                    // Try to retrieve an HWND value from the window.
-                    HWND hwnd = nullptr;
-                    if (surface_info->window->GetNativeHandle(Window::kWin32HWnd, reinterpret_cast<void**>(&hwnd)))
-                    {
-                        hmonitor = MonitorFromWindow(hwnd, MONITOR_DEFAULTTONEAREST);
-                    }
-                }
+        // Get the surface info from the Decoded_VkSwapchainCreateInfoKHR handle id.
+        HMONITOR   hmonitor     = nullptr;
+        const auto surface_info = object_info_table_.GetSurfaceKHRInfo(swapchain_info->surface);
 
-                if (hmonitor != nullptr)
-                {
-                    auto full_screen_info      = reinterpret_cast<VkSurfaceFullScreenExclusiveWin32InfoEXT*>(pnext);
-                    full_screen_info->hmonitor = hmonitor;
-                }
-                else
-                {
-                    GFXRECON_LOG_WARNING(
-                        "Failed to obtain a valid HMONITOR handle for the VkSurfaceFullScreenExclusiveWin32InfoEXT "
-                        "extension structure provided to vkCreateSwapchainKHR")
-                }
-#else
-                GFXRECON_LOG_WARNING("vkCreateSwapchainKHR called with the VkSurfaceFullScreenExclusiveWin32InfoEXT "
-                                     "extension structure, which is not supported by this platform")
-#endif
-                break;
+        if ((surface_info != nullptr) && (surface_info->window != nullptr))
+        {
+            // Try to retrieve an HWND value from the window.
+            HWND hwnd = nullptr;
+            if (surface_info->window->GetNativeHandle(Window::kWin32HWnd, reinterpret_cast<void**>(&hwnd)))
+            {
+                hmonitor = MonitorFromWindow(hwnd, MONITOR_DEFAULTTONEAREST);
             }
+        }
 
-            pnext = pnext->pNext;
+        if (hmonitor != nullptr)
+        {
+            full_screen_info->hmonitor = hmonitor;
         }
+        else
+        {
+            GFXRECON_LOG_WARNING(
+                "Failed to obtain a valid HMONITOR handle for the VkSurfaceFullScreenExclusiveWin32InfoEXT "
+                "extension structure provided to vkCreateSwapchainKHR")
+        }
+#else
+        GFXRECON_LOG_WARNING("vkCreateSwapchainKHR called with the VkSurfaceFullScreenExclusiveWin32InfoEXT "
+                             "extension structure, which is not supported by this platform")
+#endif
     }
 }
 

framework/encode/vulkan_capture_manager.cpp

@@ -33,6 +33,7 @@
 #include "generated/generated_vulkan_struct_handle_wrappers.h"
 #include "graphics/vulkan_check_buffer_references.h"
 #include "graphics/vulkan_device_util.h"
+#include "graphics/vulkan_struct_get_pnext.h"
 #include "graphics/vulkan_util.h"
 #include "util/compressor.h"
 #include "util/logging.h"
@@ -1001,33 +1002,22 @@ VkResult VulkanCaptureManager::OverrideAllocateMemory(VkDevice
     VkMemoryAllocateInfo* pAllocateInfo_unwrapped =
         const_cast<VkMemoryAllocateInfo*>(vulkan_wrappers::UnwrapStructPtrHandles(pAllocateInfo, handle_unwrap_memory));
 
-#if defined(VK_USE_PLATFORM_ANDROID_KHR)
-    const VkImportAndroidHardwareBufferInfoANDROID* import_ahb_info =
-        FindAllocateMemoryExtensions(pAllocateInfo_unwrapped);
-#endif
-
     bool                   uses_address         = false;
     VkMemoryAllocateFlags* modified_alloc_flags = nullptr;
     VkMemoryAllocateFlags  incoming_alloc_flags;
     if (device_wrapper->property_feature_info.feature_bufferDeviceAddressCaptureReplay)
     {
-        VkBaseOutStructure* current_struct = reinterpret_cast<VkBaseOutStructure*>(pAllocateInfo_unwrapped)->pNext;
-        while (current_struct != nullptr)
+        if (auto alloc_flags_info =
+                graphics::vulkan_struct_get_pnext<VkMemoryAllocateFlagsInfo>(pAllocateInfo_unwrapped))
         {
-            if (current_struct->sType == VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_FLAGS_INFO)
+            if ((alloc_flags_info->flags & VK_MEMORY_ALLOCATE_DEVICE_ADDRESS_BIT) ==
+                VK_MEMORY_ALLOCATE_DEVICE_ADDRESS_BIT)
             {
-                auto alloc_flags_info = reinterpret_cast<VkMemoryAllocateFlagsInfo*>(current_struct);
-                if ((alloc_flags_info->flags & VK_MEMORY_ALLOCATE_DEVICE_ADDRESS_BIT) ==
-                    VK_MEMORY_ALLOCATE_DEVICE_ADDRESS_BIT)
-                {
-                    uses_address         = true;
-                    incoming_alloc_flags = alloc_flags_info->flags;
-                    alloc_flags_info->flags |= VK_MEMORY_ALLOCATE_DEVICE_ADDRESS_CAPTURE_REPLAY_BIT;
-                    modified_alloc_flags = &(alloc_flags_info->flags);
-                }
-                break;
+                uses_address         = true;
+                incoming_alloc_flags = alloc_flags_info->flags;
+                alloc_flags_info->flags |= VK_MEMORY_ALLOCATE_DEVICE_ADDRESS_CAPTURE_REPLAY_BIT;
+                modified_alloc_flags = &(alloc_flags_info->flags);
             }
-            current_struct = current_struct->pNext;
         }
     }
 
@@ -1127,9 +1117,13 @@ VkResult VulkanCaptureManager::OverrideAllocateMemory(VkDevice
         }
 
 #if defined(VK_USE_PLATFORM_ANDROID_KHR)
-        if ((import_ahb_info != nullptr) && (import_ahb_info->buffer != nullptr))
+        if (auto import_ahb_info =
+                graphics::vulkan_struct_get_pnext<VkImportAndroidHardwareBufferInfoANDROID>(pAllocateInfo_unwrapped))
         {
-            ProcessImportAndroidHardwareBuffer(device, *pMemory, import_ahb_info->buffer);
+            if (import_ahb_info->buffer != nullptr)
+            {
+                ProcessImportAndroidHardwareBuffer(device, *pMemory, import_ahb_info->buffer);
+            }
         }
 #endif
     }
@@ -1671,32 +1665,6 @@ VkMemoryPropertyFlags VulkanCaptureManager::GetMemoryProperties(vulkan_wrappers:
     return memory_properties->memoryTypes[memory_type_index].propertyFlags;
 }
 
-const VkImportAndroidHardwareBufferInfoANDROID*
-VulkanCaptureManager::FindAllocateMemoryExtensions(const VkMemoryAllocateInfo* allocate_info)
-{
-    const VkImportAndroidHardwareBufferInfoANDROID* import_ahb_info = nullptr;
-
-#if defined(VK_USE_PLATFORM_ANDROID_KHR)
-    assert(allocate_info != nullptr);
-
-    const VkBaseInStructure* pnext = reinterpret_cast<const VkBaseInStructure*>(allocate_info->pNext);
-    while (pnext != nullptr)
-    {
-        if (pnext->sType == VK_STRUCTURE_TYPE_IMPORT_ANDROID_HARDWARE_BUFFER_INFO_ANDROID)
-        {
-            import_ahb_info = reinterpret_cast<const VkImportAndroidHardwareBufferInfoANDROID*>(pnext);
-            break;
-        }
-
-        pnext = pnext->pNext;
-    }
-#else
-    GFXRECON_UNREFERENCED_PARAMETER(allocate_info);
-#endif
-
-    return import_ahb_info;
-}
-
 void VulkanCaptureManager::ProcessReferenceToAndroidHardwareBuffer(VkDevice device, AHardwareBuffer* hardware_buffer)
 {
 #if defined(VK_USE_PLATFORM_ANDROID_KHR)
@@ -2748,27 +2716,14 @@ bool VulkanCaptureManager::CheckCommandBufferWrapperForFrameBoundary(
 
 bool VulkanCaptureManager::CheckPNextChainForFrameBoundary(const VkBaseInStructure* current)
 {
-    if (current == nullptr)
+    if (auto frame_boundary = graphics::vulkan_struct_get_pnext<VkFrameBoundaryEXT>(current))
     {
-        return false;
-    }
-
-    while (current->sType != VK_STRUCTURE_TYPE_FRAME_BOUNDARY_EXT && current->pNext != nullptr)
-    {
-        current = current->pNext;
-    }
-
-    if (current->sType == VK_STRUCTURE_TYPE_FRAME_BOUNDARY_EXT)
-    {
-        const VkFrameBoundaryEXT* frame_boundary = reinterpret_cast<const VkFrameBoundaryEXT*>(current);
-
         if (frame_boundary->flags & VK_FRAME_BOUNDARY_FRAME_END_BIT_EXT)
         {
             EndFrame();
             return true;
         }
     }
-
     return false;
 }
 

framework/encode/vulkan_capture_manager.h

@@ -1333,9 +1333,6 @@ class VulkanCaptureManager : public ApiCaptureManager
     VkMemoryPropertyFlags GetMemoryProperties(vulkan_wrappers::DeviceWrapper* device_wrapper,
                                               uint32_t                        memory_type_index);
 
-    const VkImportAndroidHardwareBufferInfoANDROID*
-    FindAllocateMemoryExtensions(const VkMemoryAllocateInfo* allocate_info);
-
     void ProcessReferenceToAndroidHardwareBuffer(VkDevice device, AHardwareBuffer* hardware_buffer);
     void ProcessImportAndroidHardwareBuffer(VkDevice device, VkDeviceMemory memory, AHardwareBuffer* hardware_buffer);
     void ReleaseAndroidHardwareBuffer(AHardwareBuffer* hardware_buffer);

framework/encode/vulkan_state_tracker.cpp

@@ -26,7 +26,7 @@
 #include "encode/custom_vulkan_struct_handle_wrappers.h"
 #include "encode/vulkan_handle_wrapper_util.h"
 #include "encode/vulkan_track_struct.h"
-#include "graphics/vulkan_util.h"
+#include "graphics/vulkan_struct_get_pnext.h"
 
 #include <algorithm>
 
@@ -138,33 +138,26 @@ void VulkanStateTracker::TrackPhysicalDeviceQueueFamilyProperties2(format::ApiCa
     // Copy pNext structure and update pNext pointers.
     for (uint32_t i = 0; i < property_count; ++i)
     {
-        if (properties[i].pNext != nullptr)
+        if (auto queue_family_properties_nv =
+                graphics::vulkan_struct_get_pnext<VkQueueFamilyCheckpointPropertiesNV>(properties + i))
         {
-            const VkBaseOutStructure* next = reinterpret_cast<const VkBaseOutStructure*>(properties[i].pNext);
-            if (next->sType == VK_STRUCTURE_TYPE_QUEUE_FAMILY_CHECKPOINT_PROPERTIES_NV)
-            {
-                const VkQueueFamilyCheckpointPropertiesNV* original =
-                    reinterpret_cast<const VkQueueFamilyCheckpointPropertiesNV*>(next);
-
-                std::unique_ptr<VkQueueFamilyCheckpointPropertiesNV> copy =
-                    std::make_unique<VkQueueFamilyCheckpointPropertiesNV>(*original);
-
-                if (copy->pNext != nullptr)
-                {
-                    // At time of implementation, only VkQueueFamilyCheckpointPropertiesNV was allowed.
-                    copy->pNext = nullptr;
-                    GFXRECON_LOG_WARNING("Omitting unrecognized pNext structure from queue family properties tracking");
-                }
+            std::unique_ptr<VkQueueFamilyCheckpointPropertiesNV> copy =
+                std::make_unique<VkQueueFamilyCheckpointPropertiesNV>(*queue_family_properties_nv);
 
-                wrapper->queue_family_properties2[i].pNext = copy.get();
-                wrapper->queue_family_checkpoint_properties.push_back(std::move(copy));
-            }
-            else
+            if (copy->pNext != nullptr)
             {
                 // At time of implementation, only VkQueueFamilyCheckpointPropertiesNV was allowed.
-                wrapper->queue_family_properties2[i].pNext = nullptr;
+                copy->pNext = nullptr;
                 GFXRECON_LOG_WARNING("Omitting unrecognized pNext structure from queue family properties tracking");
             }
+            wrapper->queue_family_properties2[i].pNext = copy.get();
+            wrapper->queue_family_checkpoint_properties.push_back(std::move(copy));
+        }
+        else
+        {
+            // At time of implementation, only VkQueueFamilyCheckpointPropertiesNV was allowed.
+            wrapper->queue_family_properties2[i].pNext = nullptr;
+            GFXRECON_LOG_WARNING("Omitting unrecognized pNext structure from queue family properties tracking");
         }
     }
 }
@@ -631,9 +624,9 @@ void VulkanStateTracker::TrackUpdateDescriptorSets(uint32_t                    w
     // exists at state write time by checking for the ID in the active state table.
     if (writes != nullptr)
     {
-        for (uint32_t i = 0; i < write_count; ++i)
+        for (uint32_t wi = 0; wi < write_count; ++wi)
         {
-            const VkWriteDescriptorSet* write = &writes[i];
+            const VkWriteDescriptorSet* write = &writes[wi];
             auto wrapper = vulkan_wrappers::GetWrapper<vulkan_wrappers::DescriptorSetWrapper>(write->dstSet);
             assert(wrapper != nullptr);
 
@@ -747,9 +740,8 @@ void VulkanStateTracker::TrackUpdateDescriptorSets(uint32_t                    w
                     }
                     case VK_DESCRIPTOR_TYPE_INLINE_UNIFORM_BLOCK:
                     {
-                        VkWriteDescriptorSetInlineUniformBlock* write_inline_uniform_struct =
-                            graphics::GetPNextStruct<VkWriteDescriptorSetInlineUniformBlock>(
-                                write, VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET_INLINE_UNIFORM_BLOCK);
+                        auto write_inline_uniform_struct =
+                            graphics::vulkan_struct_get_pnext<VkWriteDescriptorSetInlineUniformBlock>(write);
 
                         if (write_inline_uniform_struct != nullptr)
                         {
@@ -765,9 +757,8 @@ void VulkanStateTracker::TrackUpdateDescriptorSets(uint32_t                    w
                         break;
                     case VK_DESCRIPTOR_TYPE_ACCELERATION_STRUCTURE_KHR:
                     {
-                        VkWriteDescriptorSetAccelerationStructureKHR* write_accel_struct =
-                            graphics::GetPNextStruct<VkWriteDescriptorSetAccelerationStructureKHR>(
-                                write, VK_STRUCTURE_TYPE_WRITE_DESCRIPTOR_SET_ACCELERATION_STRUCTURE_KHR);
+                        auto write_accel_struct =
+                            graphics::vulkan_struct_get_pnext<VkWriteDescriptorSetAccelerationStructureKHR>(write);
 
                         if (write_accel_struct != nullptr)
                         {