Revert some VUlkan backend changes to fix regression on Metal devices

2024-02-07 12:16:11 +01:00
parent 9f279d4b5e
commit 17361d6bc1
9 changed files with 49 additions and 93 deletions
--- a/Source/Engine/GraphicsDevice/Vulkan/CmdBufferVulkan.cpp
+++ b/Source/Engine/GraphicsDevice/Vulkan/CmdBufferVulkan.cpp
@@ -211,9 +211,7 @@ CmdBufferPoolVulkan::CmdBufferPoolVulkan(GPUDeviceVulkan* device)
 CmdBufferPoolVulkan::~CmdBufferPoolVulkan()
 {
    for (int32 i = 0; i < _cmdBuffers.Count(); i++)
-    {
-        Delete(_cmdBuffers.Get()[i]);
-    }
+        Delete(_cmdBuffers[i]);
    vkDestroyCommandPool(_device->Device, _handle, nullptr);
 }

@@ -221,7 +219,7 @@ void CmdBufferPoolVulkan::RefreshFenceStatus(const CmdBufferVulkan* skipCmdBuffe
 {
    for (int32 i = 0; i < _cmdBuffers.Count(); i++)
    {
-        const auto cmdBuffer = _cmdBuffers.Get()[i];
+        auto cmdBuffer = _cmdBuffers[i];
        if (cmdBuffer != skipCmdBuffer)
        {
            cmdBuffer->RefreshFenceStatus();
--- a/Source/Engine/GraphicsDevice/Vulkan/CmdBufferVulkan.h
+++ b/Source/Engine/GraphicsDevice/Vulkan/CmdBufferVulkan.h
@@ -56,7 +56,6 @@ private:

 public:
    CmdBufferVulkan(GPUDeviceVulkan* device, CmdBufferPoolVulkan* pool);
-
    ~CmdBufferVulkan();

 public:
@@ -140,7 +139,6 @@ public:
 class CmdBufferPoolVulkan
 {
    friend class CmdBufferManagerVulkan;
-
 private:
    GPUDeviceVulkan* _device;
    VkCommandPool _handle;
@@ -157,7 +155,6 @@ public:
 public:
    inline VkCommandPool GetHandle() const
    {
-        ASSERT(_handle != VK_NULL_HANDLE);
        return _handle;
    }

@@ -206,14 +203,11 @@ public:

 public:
    void SubmitActiveCmdBuffer(SemaphoreVulkan* signalSemaphore = nullptr);
-
    void WaitForCmdBuffer(CmdBufferVulkan* cmdBuffer, float timeInSecondsToWait = 1.0f);
-
    void RefreshFenceStatus(CmdBufferVulkan* skipCmdBuffer = nullptr)
    {
        _pool.RefreshFenceStatus(skipCmdBuffer);
    }
-
    void PrepareForNewActiveCommandBuffer();

    void OnQueryBegin(GPUTimerQueryVulkan* query);
--- a/Source/Engine/GraphicsDevice/Vulkan/Config.h
+++ b/Source/Engine/GraphicsDevice/Vulkan/Config.h
@@ -27,7 +27,7 @@

 #define VULKAN_ENABLE_API_DUMP 0
 #define VULKAN_RESET_QUERY_POOLS 0
-#define VULKAN_HASH_POOLS_WITH_TYPES_USAGE_ID 1
+#define VULKAN_HASH_POOLS_WITH_LAYOUT_TYPES 1
 #define VULKAN_USE_DEBUG_LAYER GPU_ENABLE_DIAGNOSTICS
 #define VULKAN_USE_DEBUG_DATA (GPU_ENABLE_DIAGNOSTICS && COMPILE_WITH_DEV_ENV)

--- a/Source/Engine/GraphicsDevice/Vulkan/DescriptorSetVulkan.cpp
+++ b/Source/Engine/GraphicsDevice/Vulkan/DescriptorSetVulkan.cpp
@@ -30,7 +30,7 @@ void DescriptorSetLayoutInfoVulkan::AddBindingsForStage(VkShaderStageFlagBits st
        binding.descriptorCount = descriptor.Count;

        LayoutTypes[binding.descriptorType]++;
-        descSetLayout.Add(binding);
+        descSetLayout.LayoutBindings.Add(binding);
        Hash = Crc::MemCrc32(&binding, sizeof(binding), Hash);
    }
 }
@@ -39,19 +39,18 @@ bool DescriptorSetLayoutInfoVulkan::operator==(const DescriptorSetLayoutInfoVulk
 {
    if (other.SetLayouts.Count() != SetLayouts.Count())
        return false;
-    if (other.TypesUsageID != TypesUsageID)
+#if VULKAN_HASH_POOLS_WITH_LAYOUT_TYPES
+    if (other.SetLayoutsHash != SetLayoutsHash)
        return false;
-
+#endif
    for (int32 index = 0; index < other.SetLayouts.Count(); index++)
    {
-        auto& setLayout = SetLayouts[index];
-        auto& setLayoutOther = other.SetLayouts[index];
-        if (setLayoutOther.Count() != setLayout.Count())
+        const int32 bindingsCount = SetLayouts[index].LayoutBindings.Count();
+        if (other.SetLayouts[index].LayoutBindings.Count() != bindingsCount)
            return false;
-        if (setLayout.Count() != 0 && Platform::MemoryCompare(setLayoutOther.Get(), setLayout.Get(), setLayout.Count() * sizeof(VkDescriptorSetLayoutBinding)))
+        if (bindingsCount != 0 && Platform::MemoryCompare(other.SetLayouts[index].LayoutBindings.Get(), SetLayouts[index].LayoutBindings.Get(), bindingsCount * sizeof(VkDescriptorSetLayoutBinding)))
            return false;
    }
-
    return true;
 }

@@ -63,16 +62,13 @@ DescriptorSetLayoutVulkan::DescriptorSetLayoutVulkan(GPUDeviceVulkan* device)
 DescriptorSetLayoutVulkan::~DescriptorSetLayoutVulkan()
 {
    for (VkDescriptorSetLayout& handle : Handles)
-    {
        Device->DeferredDeletionQueue.EnqueueResource(DeferredDeletionQueueVulkan::Type::DescriptorSetLayout, handle);
-    }
 }

 void DescriptorSetLayoutVulkan::Compile()
 {
+#if !BUILD_RELEASE
    ASSERT(Handles.IsEmpty());
-
-    // Validate device limits for the engine
    const VkPhysicalDeviceLimits& limits = Device->PhysicalDeviceLimits;
    ASSERT(LayoutTypes[VK_DESCRIPTOR_TYPE_SAMPLER] + LayoutTypes[VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER] < limits.maxDescriptorSetSamplers);
    ASSERT(LayoutTypes[VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER]+ LayoutTypes[VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER_DYNAMIC] < limits.maxDescriptorSetUniformBuffers);
@@ -81,34 +77,23 @@ void DescriptorSetLayoutVulkan::Compile()
    ASSERT(LayoutTypes[VK_DESCRIPTOR_TYPE_STORAGE_BUFFER_DYNAMIC] < limits.maxDescriptorSetStorageBuffersDynamic);
    ASSERT(LayoutTypes[VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER] + LayoutTypes[VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE] + LayoutTypes[VK_DESCRIPTOR_TYPE_UNIFORM_TEXEL_BUFFER] < limits.maxDescriptorSetSampledImages);
    ASSERT(LayoutTypes[VK_DESCRIPTOR_TYPE_STORAGE_IMAGE] + LayoutTypes[VK_DESCRIPTOR_TYPE_STORAGE_TEXEL_BUFFER]< limits.maxDescriptorSetStorageImages);
+#endif

    Handles.Resize(SetLayouts.Count());
    for (int32 i = 0; i < SetLayouts.Count(); i++)
    {
-        auto& layout = SetLayouts[i];
+        auto& layout = SetLayouts.Get()[i];
        VkDescriptorSetLayoutCreateInfo layoutInfo;
        RenderToolsVulkan::ZeroStruct(layoutInfo, VK_STRUCTURE_TYPE_DESCRIPTOR_SET_LAYOUT_CREATE_INFO);
-        layoutInfo.bindingCount = layout.Count();
-        layoutInfo.pBindings = layout.Get();
+        layoutInfo.bindingCount = layout.LayoutBindings.Count();
+        layoutInfo.pBindings = layout.LayoutBindings.Get();
        VALIDATE_VULKAN_RESULT(vkCreateDescriptorSetLayout(Device->Device, &layoutInfo, nullptr, &Handles[i]));
    }

-    if (TypesUsageID == ~0)
-    {
-        // Cache usage ID
-        static uint32 uniqueID = 1;
-        static Dictionary<uint32, uint32> typesUsageHashMap;
-        const uint32 typesUsageHash = Crc::MemCrc32(LayoutTypes, sizeof(LayoutTypes));
-        uint32 id;
-        Device->Locker.Lock();
-        if (!typesUsageHashMap.TryGet(typesUsageHash, id))
-        {
-            id = uniqueID++;
-            typesUsageHashMap.Add(typesUsageHash, id);
-        }
-        Device->Locker.Unlock();
-        TypesUsageID = id;
-    }
+#if VULKAN_HASH_POOLS_WITH_LAYOUT_TYPES
+    if (SetLayoutsHash == 0)
+        SetLayoutsHash = Crc::MemCrc32(LayoutTypes, sizeof(LayoutTypes));
+#endif

    RenderToolsVulkan::ZeroStruct(AllocateInfo, VK_STRUCTURE_TYPE_DESCRIPTOR_SET_ALLOCATE_INFO);
    AllocateInfo.descriptorSetCount = Handles.Count();
@@ -127,7 +112,7 @@ DescriptorPoolVulkan::DescriptorPoolVulkan(GPUDeviceVulkan* device, const Descri

    // The maximum amount of descriptor sets layout allocations to hold
    const uint32 MaxSetsAllocations = 256;
-    _descriptorSetsMax = MaxSetsAllocations * (VULKAN_HASH_POOLS_WITH_TYPES_USAGE_ID ? 1 : _layout.SetLayouts.Count());
+    _descriptorSetsMax = MaxSetsAllocations * (VULKAN_HASH_POOLS_WITH_LAYOUT_TYPES ? 1 : _layout.SetLayouts.Count());
    for (uint32 typeIndex = VULKAN_DESCRIPTOR_TYPE_BEGIN; typeIndex <= VULKAN_DESCRIPTOR_TYPE_END; typeIndex++)
    {
        const VkDescriptorType descriptorType = (VkDescriptorType)typeIndex;
@@ -274,7 +259,7 @@ DescriptorPoolSetContainerVulkan::~DescriptorPoolSetContainerVulkan()

 TypedDescriptorPoolSetVulkan* DescriptorPoolSetContainerVulkan::AcquireTypedPoolSet(const DescriptorSetLayoutVulkan& layout)
 {
-    const uint32 hash = VULKAN_HASH_POOLS_WITH_TYPES_USAGE_ID ? layout.TypesUsageID : layout.Hash;
+    const uint32 hash = VULKAN_HASH_POOLS_WITH_LAYOUT_TYPES ? layout.SetLayoutsHash : GetHash(layout);
    TypedDescriptorPoolSetVulkan* typedPool;
    if (!_typedDescriptorPools.TryGet(hash, typedPool))
    {
@@ -347,26 +332,24 @@ void DescriptorPoolsManagerVulkan::GC()
 }

 PipelineLayoutVulkan::PipelineLayoutVulkan(GPUDeviceVulkan* device, const DescriptorSetLayoutInfoVulkan& layout)
-    : _device(device)
-    , _handle(VK_NULL_HANDLE)
-    , _descriptorSetLayout(device)
+    : Device(device)
+    , Handle(VK_NULL_HANDLE)
+    , DescriptorSetLayout(device)
 {
-    _descriptorSetLayout.CopyFrom(layout);
-    _descriptorSetLayout.Compile();
+    DescriptorSetLayout.CopyFrom(layout);
+    DescriptorSetLayout.Compile();

    VkPipelineLayoutCreateInfo createInfo;
    RenderToolsVulkan::ZeroStruct(createInfo, VK_STRUCTURE_TYPE_PIPELINE_LAYOUT_CREATE_INFO);
-    createInfo.setLayoutCount = _descriptorSetLayout.Handles.Count();
-    createInfo.pSetLayouts = _descriptorSetLayout.Handles.Get();
-    VALIDATE_VULKAN_RESULT(vkCreatePipelineLayout(_device->Device, &createInfo, nullptr, &_handle));
+    createInfo.setLayoutCount = DescriptorSetLayout.Handles.Count();
+    createInfo.pSetLayouts = DescriptorSetLayout.Handles.Get();
+    VALIDATE_VULKAN_RESULT(vkCreatePipelineLayout(device->Device, &createInfo, nullptr, &Handle));
 }

 PipelineLayoutVulkan::~PipelineLayoutVulkan()
 {
-    if (_handle != VK_NULL_HANDLE)
-    {
-        _device->DeferredDeletionQueue.EnqueueResource(DeferredDeletionQueueVulkan::Type::PipelineLayout, _handle);
-    }
+    if (Handle != VK_NULL_HANDLE)
+        Device->DeferredDeletionQueue.EnqueueResource(DeferredDeletionQueueVulkan::Type::PipelineLayout, Handle);
 }

 uint32 DescriptorSetWriterVulkan::SetupDescriptorWrites(const SpirvShaderDescriptorInfo& info, VkWriteDescriptorSet* writeDescriptors, VkDescriptorImageInfo* imageInfo, VkDescriptorBufferInfo* bufferInfo, VkBufferView* texelBufferView, uint8* bindingToDynamicOffset)
--- a/Source/Engine/GraphicsDevice/Vulkan/DescriptorSetVulkan.h
+++ b/Source/Engine/GraphicsDevice/Vulkan/DescriptorSetVulkan.h
@@ -54,14 +54,15 @@ namespace DescriptorSet
 class DescriptorSetLayoutInfoVulkan
 {
 public:
-    typedef Array<VkDescriptorSetLayoutBinding> SetLayout;
+    struct SetLayout
+    {
+        Array<VkDescriptorSetLayoutBinding> LayoutBindings;
+    };

    uint32 Hash = 0;
-    uint32 TypesUsageID = ~0;
-    Array<SetLayout> SetLayouts;
+    uint32 SetLayoutsHash = 0;
    uint32 LayoutTypes[VULKAN_DESCRIPTOR_TYPE_END];
-
-    void CacheTypesUsageID();
+    Array<SetLayout> SetLayouts;

 public:
    DescriptorSetLayoutInfoVulkan()
@@ -75,7 +76,7 @@ public:
    {
        Platform::MemoryCopy(LayoutTypes, info.LayoutTypes, sizeof(LayoutTypes));
        Hash = info.Hash;
-        TypesUsageID = info.TypesUsageID;
+        SetLayoutsHash = info.SetLayoutsHash;
        SetLayouts = info.SetLayouts;
    }

@@ -248,30 +249,13 @@ public:

 class PipelineLayoutVulkan
 {
-private:
-    GPUDeviceVulkan* _device;
-    VkPipelineLayout _handle;
-    DescriptorSetLayoutVulkan _descriptorSetLayout;
-
 public:
+    GPUDeviceVulkan* Device;
+    VkPipelineLayout Handle;
+    DescriptorSetLayoutVulkan DescriptorSetLayout;
+
    PipelineLayoutVulkan(GPUDeviceVulkan* device, const DescriptorSetLayoutInfoVulkan& layout);
    ~PipelineLayoutVulkan();
-
-public:
-    inline VkPipelineLayout GetHandle() const
-    {
-        return _handle;
-    }
-
-    inline const DescriptorSetLayoutVulkan& GetDescriptorSetLayout() const
-    {
-        return _descriptorSetLayout;
-    }
-
-    inline bool HasDescriptors() const
-    {
-        return _descriptorSetLayout.SetLayouts.HasItems();
-    }
 };

 struct DescriptorSetWriteContainerVulkan
--- a/Source/Engine/GraphicsDevice/Vulkan/GPUContextVulkan.cpp
+++ b/Source/Engine/GraphicsDevice/Vulkan/GPUContextVulkan.cpp
@@ -335,13 +335,12 @@ DescriptorPoolVulkan* GPUContextVulkan::AllocateDescriptorSets(const VkDescripto
    VkDescriptorSetAllocateInfo allocateInfo = descriptorSetAllocateInfo;
    DescriptorPoolVulkan* pool = nullptr;

-    const uint32 hash = VULKAN_HASH_POOLS_WITH_TYPES_USAGE_ID ? layout.TypesUsageID : layout.Hash;
+    const uint32 hash = VULKAN_HASH_POOLS_WITH_LAYOUT_TYPES ? layout.SetLayoutsHash : GetHash(layout);
    DescriptorPoolArray* typedDescriptorPools = _descriptorPools.TryGet(hash);

    if (typedDescriptorPools != nullptr)
    {
        pool = typedDescriptorPools->HasItems() ? typedDescriptorPools->Last() : nullptr;
-
        if (pool && pool->CanAllocate(layout))
        {
            allocateInfo.descriptorPool = pool->GetHandle();
@@ -361,7 +360,6 @@ DescriptorPoolVulkan* GPUContextVulkan::AllocateDescriptorSets(const VkDescripto
        }
        else
        {
-            // Spec says any negative value could be due to fragmentation, so create a new Pool. If it fails here then we really are out of memory!
            pool = New<DescriptorPoolVulkan>(_device, layout);
            typedDescriptorPools->Add(pool);
            allocateInfo.descriptorPool = pool->GetHandle();
@@ -689,7 +687,7 @@ void GPUContextVulkan::OnDrawCall()
        vkCmdBindDescriptorSets(
            cmdBuffer->GetHandle(),
            VK_PIPELINE_BIND_POINT_GRAPHICS,
-            pipelineState->GetLayout()->GetHandle(),
+            pipelineState->GetLayout()->Handle,
            0,
            pipelineState->DescriptorSetHandles.Count(),
            pipelineState->DescriptorSetHandles.Get(),
--- a/Source/Engine/GraphicsDevice/Vulkan/GPUPipelineStateVulkan.cpp
+++ b/Source/Engine/GraphicsDevice/Vulkan/GPUPipelineStateVulkan.cpp
@@ -54,7 +54,7 @@ ComputePipelineStateVulkan* GPUShaderProgramCSVulkan::GetOrCreateState()
    VkComputePipelineCreateInfo desc;
    RenderToolsVulkan::ZeroStruct(desc, VK_STRUCTURE_TYPE_COMPUTE_PIPELINE_CREATE_INFO);
    desc.basePipelineIndex = -1;
-    desc.layout = layout->GetHandle();
+    desc.layout = layout->Handle;
    RenderToolsVulkan::ZeroStruct(desc.stage, VK_STRUCTURE_TYPE_PIPELINE_SHADER_STAGE_CREATE_INFO);
    auto& stage = desc.stage;
    RenderToolsVulkan::ZeroStruct(stage, VK_STRUCTURE_TYPE_PIPELINE_SHADER_STAGE_CREATE_INFO);
@@ -72,7 +72,7 @@ ComputePipelineStateVulkan* GPUShaderProgramCSVulkan::GetOrCreateState()
    // Setup the state
    _pipelineState = New<ComputePipelineStateVulkan>(_device, pipeline, layout);
    _pipelineState->DescriptorInfo = &DescriptorInfo;
-    _pipelineState->DescriptorSetsLayout = &layout->GetDescriptorSetLayout();
+    _pipelineState->DescriptorSetsLayout = &layout->DescriptorSetLayout;
    _pipelineState->DescriptorSetHandles.AddZeroed(_pipelineState->DescriptorSetsLayout->Handles.Count());
    uint32 dynamicOffsetsCount = 0;
    if (DescriptorInfo.DescriptorTypesCount != 0)
@@ -146,7 +146,7 @@ PipelineLayoutVulkan* GPUPipelineStateVulkan::GetLayout()

    _layout = _device->GetOrCreateLayout(descriptorSetLayoutInfo);
    ASSERT(_layout);
-    DescriptorSetsLayout = &_layout->GetDescriptorSetLayout();
+    DescriptorSetsLayout = &_layout->DescriptorSetLayout;
    DescriptorSetHandles.AddZeroed(DescriptorSetsLayout->Handles.Count());

    return _layout;
@@ -179,7 +179,7 @@ VkPipeline GPUPipelineStateVulkan::GetState(RenderPassVulkan* renderPass)
    // Check if has missing layout
    if (_desc.layout == VK_NULL_HANDLE)
    {
-        _desc.layout = GetLayout()->GetHandle();
+        _desc.layout = GetLayout()->Handle;
    }

    // Create object
--- a/Source/Engine/GraphicsDevice/Vulkan/GPUPipelineStateVulkan.h
+++ b/Source/Engine/GraphicsDevice/Vulkan/GPUPipelineStateVulkan.h
@@ -63,7 +63,7 @@ public:
        vkCmdBindDescriptorSets(
            cmdBuffer->GetHandle(),
            VK_PIPELINE_BIND_POINT_COMPUTE,
-            GetLayout()->GetHandle(),
+            GetLayout()->Handle,
            0,
            DescriptorSetHandles.Count(),
            DescriptorSetHandles.Get(),
--- a/Source/Engine/GraphicsDevice/Vulkan/QueueVulkan.cpp
+++ b/Source/Engine/GraphicsDevice/Vulkan/QueueVulkan.cpp
@@ -21,7 +21,6 @@ QueueVulkan::QueueVulkan(GPUDeviceVulkan* device, uint32 familyIndex)
 void QueueVulkan::Submit(CmdBufferVulkan* cmdBuffer, uint32 signalSemaphoresCount, const VkSemaphore* signalSemaphores)
 {
    ASSERT(cmdBuffer->HasEnded());
-
    auto fence = cmdBuffer->GetFence();
    ASSERT(!fence->IsSignaled);