From 0e4a632417625d1dceb28a2d62d14a0aa8d277e9 Mon Sep 17 00:00:00 2001 From: moudgils Date: Thu, 3 Jun 2021 22:45:50 -0700 Subject: [PATCH] Add support for re-binding SRG entries if the drawItem has a new pso which changes how a srg is used in the shader. Also optimized api usage to use single call for UseResource and for setting stream buffers. --- .../Shaders/MorphTargets/MorphTargetSRG.azsli | 2 +- .../Metal/Code/Source/RHI/ArgumentBuffer.cpp | 133 +++++++++--------- .../Metal/Code/Source/RHI/ArgumentBuffer.h | 15 +- .../RHI/Metal/Code/Source/RHI/CommandList.cpp | 56 ++++++-- .../RHI/Metal/Code/Source/RHI/CommandList.h | 1 + .../Metal/Code/Source/RHI/PipelineLayout.cpp | 7 + .../Metal/Code/Source/RHI/PipelineLayout.h | 6 + 7 files changed, 134 insertions(+), 86 deletions(-) diff --git a/Gems/Atom/Feature/Common/Assets/Shaders/MorphTargets/MorphTargetSRG.azsli b/Gems/Atom/Feature/Common/Assets/Shaders/MorphTargets/MorphTargetSRG.azsli index 171e803c2c..83193c8559 100644 --- a/Gems/Atom/Feature/Common/Assets/Shaders/MorphTargets/MorphTargetSRG.azsli +++ b/Gems/Atom/Feature/Common/Assets/Shaders/MorphTargets/MorphTargetSRG.azsli @@ -16,7 +16,7 @@ ShaderResourceGroup MorphTargetPassSrg : SRG_PerPass { - RWStruturedBuffer m_accumulatedDeltas; + RWStructuredBuffer m_accumulatedDeltas; } // This class represents the data that is passed to the morph target compute shader of an individual delta diff --git a/Gems/Atom/RHI/Metal/Code/Source/RHI/ArgumentBuffer.cpp b/Gems/Atom/RHI/Metal/Code/Source/RHI/ArgumentBuffer.cpp index 6ffd15e0bc..1e2edc6520 100644 --- a/Gems/Atom/RHI/Metal/Code/Source/RHI/ArgumentBuffer.cpp +++ b/Gems/Atom/RHI/Metal/Code/Source/RHI/ArgumentBuffer.cpp @@ -386,6 +386,11 @@ namespace AZ void ArgumentBuffer::AddUntrackedResourcesToEncoder(id commandEncoder, const ShaderResourceGroupVisibility& srgResourcesVisInfo) const { + + ComputeResourcesToMakeResidentMap resourcesToMakeResidentCompute; + GraphicsResourcesToMakeResidentMap resourcesToMakeResidentGraphics; + + //Cache the constant buffer associated with a srg if (m_constantBufferSize) { uint8_t numBitsSet = RHI::CountBitsSet(static_cast(srgResourcesVisInfo.m_constantDataStageMask)); @@ -393,28 +398,19 @@ namespace AZ { if(RHI::CheckBitsAny(srgResourcesVisInfo.m_constantDataStageMask, RHI::ShaderStageMask::Compute)) { - [static_cast>(commandEncoder) useResource:m_constantBuffer.GetGpuAddress>() usage:MTLResourceUsageRead]; + resourcesToMakeResidentCompute[MTLResourceUsageRead].m_resourceArray[resourcesToMakeResidentCompute[MTLResourceUsageRead].m_resourceArrayLen++] = m_constantBuffer.GetGpuAddress>(); } else { MTLRenderStages mtlRenderStages = GetRenderStages(srgResourcesVisInfo.m_constantDataStageMask); - [static_cast>(commandEncoder) useResource:m_constantBuffer.GetGpuAddress>() - usage:MTLResourceUsageRead - stages:mtlRenderStages]; + AZStd::pair key = AZStd::make_pair(MTLResourceUsageRead, mtlRenderStages); + resourcesToMakeResidentGraphics[key].m_resourceArray[resourcesToMakeResidentGraphics[key].m_resourceArrayLen++] = m_constantBuffer.GetGpuAddress>(); } - } } - ApplyUseResource(commandEncoder, m_resourceBindings, srgResourcesVisInfo); - } - - void ArgumentBuffer::ApplyUseResource(id encoder, - const ResourceBindingsMap& resourceMap, - const ShaderResourceGroupVisibility& srgResourcesVisInfo) const - { - - CommandEncoderType encodeType = CommandEncoderType::Invalid; - for (const auto& it : resourceMap) + + //Cach all the resources within a srg that are used by the shader based on the visibility information + for (const auto& it : m_resourceBindings) { //Extract the visibility mask for the give resource auto visMaskIt = srgResourcesVisInfo.m_resourcesStageMask.find(it.first); @@ -426,75 +422,50 @@ namespace AZ { if(RHI::CheckBitsAny(visMaskIt->second, RHI::ShaderStageMask::Compute)) { - //Call UseResource on all resources for Compute stage - ApplyUseResourceToCompute(encoder, it.second); - encodeType = CommandEncoderType::Compute; + ApplyUseResourceToCompute(commandEncoder, it.second, resourcesToMakeResidentCompute); } else { - //Call UseResource on all resources for Vertex and Fragment stages AZ_Assert(RHI::CheckBitsAny(visMaskIt->second, RHI::ShaderStageMask::Vertex) || RHI::CheckBitsAny(visMaskIt->second, RHI::ShaderStageMask::Fragment), "The visibility mask %i is not set for Vertex or fragment stage", visMaskIt->second); - ApplyUseResourceToGraphic(encoder, visMaskIt->second, it.second); - encodeType = CommandEncoderType::Render; + ApplyUseResourceToGraphic(commandEncoder, visMaskIt->second, it.second, resourcesToMakeResidentGraphics); } } } - } - - void ArgumentBuffer::ApplyUseResourceToCompute(id encoder, const ResourceBindingsSet& resourceBindingDataSet) const - { - for (const auto& resourceBindingData : resourceBindingDataSet) - { - ResourceType rescType = resourceBindingData.m_resourcPtr->GetResourceType(); - switch(rescType) - { - case ResourceType::MtlTextureType: - { - MTLResourceUsage resourceUsage = GetImageResourceUsage(resourceBindingData.m_imageAccess); - [static_cast>(encoder) useResource:resourceBindingData.m_resourcPtr->GetGpuAddress>() usage:resourceUsage]; - - break; - } - case ResourceType::MtlBufferType: - { - MTLResourceUsage resourceUsage = GetBufferResourceUsage(resourceBindingData.m_bufferAccess); - [static_cast>(encoder) useResource:resourceBindingData.m_resourcPtr->GetGpuAddress>() usage:resourceUsage]; - - break; - } - default: - { - AZ_Assert(false, "Undefined Resource type"); - } - } - } - } - - void ArgumentBuffer::ApplyUseResourceToGraphic(id encoder, RHI::ShaderStageMask visShaderMask, const ResourceBindingsSet& resourceBindingDataSet) const - { - MTLRenderStages mtlRenderStages = GetRenderStages(visShaderMask); + //Call UseResource on all resources for Compute stage + for (const auto& key : resourcesToMakeResidentCompute) + { + [static_cast>(commandEncoder) useResources: key.second.m_resourceArray.data() + count: key.second.m_resourceArrayLen + usage: key.first]; + } + + //Call UseResource on all resources for Vertex and Fragment stages + for (const auto& key : resourcesToMakeResidentGraphics) + { + [static_cast>(commandEncoder) useResources: key.second.m_resourceArray.data() + count: key.second.m_resourceArrayLen + usage: key.first.first + stages: key.first.second]; + } + } + + void ArgumentBuffer::ApplyUseResourceToCompute(id encoder, const ResourceBindingsSet& resourceBindingDataSet, ComputeResourcesToMakeResidentMap& resourcesToMakeResidentMap) const + { for (const auto& resourceBindingData : resourceBindingDataSet) { ResourceType rescType = resourceBindingData.m_resourcPtr->GetResourceType(); + MTLResourceUsage resourceUsage = MTLResourceUsageRead; switch(rescType) { case ResourceType::MtlTextureType: { - MTLResourceUsage resourceUsage = GetImageResourceUsage(resourceBindingData.m_imageAccess); - [static_cast>(encoder) useResource:resourceBindingData.m_resourcPtr->GetGpuAddress>() - usage:resourceUsage - stages:mtlRenderStages]; - + resourceUsage |= GetImageResourceUsage(resourceBindingData.m_imageAccess); break; } case ResourceType::MtlBufferType: { - MTLResourceUsage resourceUsage = GetBufferResourceUsage(resourceBindingData.m_bufferAccess); - [static_cast>(encoder) useResource:resourceBindingData.m_resourcPtr->GetGpuAddress>() - usage:resourceUsage - stages:mtlRenderStages]; - + resourceUsage |= GetBufferResourceUsage(resourceBindingData.m_bufferAccess); break; } default: @@ -502,8 +473,38 @@ namespace AZ AZ_Assert(false, "Undefined Resource type"); } } + resourcesToMakeResidentMap[resourceUsage].m_resourceArray[resourcesToMakeResidentMap[resourceUsage].m_resourceArrayLen++] = resourceBindingData.m_resourcPtr->GetGpuAddress>(); + } + } + + void ArgumentBuffer::ApplyUseResourceToGraphic(id encoder, RHI::ShaderStageMask visShaderMask, const ResourceBindingsSet& resourceBindingDataSet, GraphicsResourcesToMakeResidentMap& resourcesToMakeResidentMap) const + { + + MTLRenderStages mtlRenderStages = GetRenderStages(visShaderMask); + MTLResourceUsage resourceUsage = MTLResourceUsageRead; + for (const auto& resourceBindingData : resourceBindingDataSet) + { + ResourceType rescType = resourceBindingData.m_resourcPtr->GetResourceType(); + switch(rescType) + { + case ResourceType::MtlTextureType: + { + resourceUsage |= GetImageResourceUsage(resourceBindingData.m_imageAccess); + break; + } + case ResourceType::MtlBufferType: + { + resourceUsage |= GetBufferResourceUsage(resourceBindingData.m_bufferAccess); + break; + } + default: + { + AZ_Assert(false, "Undefined Resource type"); + } + } + AZStd::pair key = AZStd::make_pair(resourceUsage, mtlRenderStages); + resourcesToMakeResidentMap[key].m_resourceArray[resourcesToMakeResidentMap[key].m_resourceArrayLen++] = resourceBindingData.m_resourcPtr->GetGpuAddress>(); } } - } } diff --git a/Gems/Atom/RHI/Metal/Code/Source/RHI/ArgumentBuffer.h b/Gems/Atom/RHI/Metal/Code/Source/RHI/ArgumentBuffer.h index a5a8e00e69..2434b8312d 100644 --- a/Gems/Atom/RHI/Metal/Code/Source/RHI/ArgumentBuffer.h +++ b/Gems/Atom/RHI/Metal/Code/Source/RHI/ArgumentBuffer.h @@ -119,8 +119,17 @@ namespace AZ using ResourceBindingsMap = AZStd::unordered_map; ResourceBindingsMap m_resourceBindings; - void ApplyUseResourceToCompute(id encoder, const ResourceBindingsSet& resourceBindingData) const; - void ApplyUseResourceToGraphic(id encoder, RHI::ShaderStageMask visShaderMask, const ResourceBindingsSet& resourceBindingDataSet) const; + static const int MaxEntriesInArgTable = 31; + struct MetalResourceArray + { + AZStd::array, MaxEntriesInArgTable> m_resourceArray; + int m_resourceArrayLen = 0; + }; + using ComputeResourcesToMakeResidentMap = AZStd::unordered_map; + using GraphicsResourcesToMakeResidentMap = AZStd::unordered_map, MetalResourceArray>; + + void ApplyUseResourceToCompute(id encoder, const ResourceBindingsSet& resourceBindingData, ComputeResourcesToMakeResidentMap& resourcesToMakeResidentMap) const; + void ApplyUseResourceToGraphic(id encoder, RHI::ShaderStageMask visShaderMask, const ResourceBindingsSet& resourceBindingDataSet, GraphicsResourcesToMakeResidentMap& resourcesToMakeResidentMap) const; //! Use visibility information to call UseResource on all resources for this Argument Buffer void ApplyUseResource(id encoder, const ResourceBindingsMap& resourceMap, @@ -144,8 +153,6 @@ namespace AZ #endif ShaderResourceGroupPool* m_srgPool = nullptr; - - static const int MaxEntriesInArgTable = 31; NSCache* m_samplerCache; }; } diff --git a/Gems/Atom/RHI/Metal/Code/Source/RHI/CommandList.cpp b/Gems/Atom/RHI/Metal/Code/Source/RHI/CommandList.cpp index 3eca8dabd8..10fbe372b1 100644 --- a/Gems/Atom/RHI/Metal/Code/Source/RHI/CommandList.cpp +++ b/Gems/Atom/RHI/Metal/Code/Source/RHI/CommandList.cpp @@ -258,24 +258,19 @@ namespace AZ continue; } + uint32_t srgVisIndex = pipelineLayout.GetSlotByIndex(shaderResourceGroup->GetBindingSlot()); + const RHI::ShaderStageMask& srgVisInfo = pipelineLayout.GetSrgVisibility(srgVisIndex); + if (bindings.m_srgsByIndex[srgIndex] != shaderResourceGroup) { bindings.m_srgsByIndex[srgIndex] = shaderResourceGroup; auto& compiledArgBuffer = shaderResourceGroup->GetCompiledArgumentBuffer(); - id argBuffer = compiledArgBuffer.GetArgEncoderBuffer(); size_t argBufferOffset = compiledArgBuffer.GetOffset(); - - uint32_t srgVisIndex = pipelineLayout.GetSlotByIndex(shaderResourceGroup->GetBindingSlot()); - const RHI::ShaderStageMask& srgVisInfo = pipelineLayout.GetSrgVisibility(srgVisIndex); - + if(srgVisInfo != RHI::ShaderStageMask::None) { - const ShaderResourceGroupVisibility& srgResourcesVisInfo = pipelineLayout.GetSrgResourcesVisibility(srgVisIndex); - - //For graphics and compute encoder bind the argument buffer and - //make the resource resident for the duration of the work associated with the current scope - //and ensure that it's in a format compatible with the appropriate metal function. + //For graphics and compute encoder bind the argument buffer if(m_commandEncoderType == CommandEncoderType::Render) { id renderEncoder = GetEncoder>(); @@ -293,7 +288,6 @@ namespace AZ offset:argBufferOffset atIndex:slotIndex]; } - shaderResourceGroup->AddUntrackedResourcesToEncoder(m_encoder, srgResourcesVisInfo); } else if(m_commandEncoderType == CommandEncoderType::Compute) { @@ -301,6 +295,28 @@ namespace AZ [computeEncoder setBuffer:argBuffer offset:argBufferOffset atIndex:pipelineLayout.GetSlotByIndex(srgIndex)]; + } + } + } + + //Check againgst the srg resources visibility hash as it is possible for draw items to have different PSO in the same pass. + const AZ::HashValue64 srgResourcesVisHash = pipelineLayout.GetSrgResourcesVisibilityHash(srgVisIndex); + if(bindings.m_srgVisHashByIndex[srgIndex] != srgResourcesVisHash) + { + bindings.m_srgVisHashByIndex[srgIndex] = srgResourcesVisHash; + if(srgVisInfo != RHI::ShaderStageMask::None) + { + const ShaderResourceGroupVisibility& srgResourcesVisInfo = pipelineLayout.GetSrgResourcesVisibility(srgVisIndex); + + //For graphics and compute encoder bind the argument buffer and + //make the resource resident for the duration of the work associated with the current scope + //and ensure that it's in a format compatible with the appropriate metal function. + if(m_commandEncoderType == CommandEncoderType::Render) + { + shaderResourceGroup->AddUntrackedResourcesToEncoder(m_encoder, srgResourcesVisInfo); + } + else if(m_commandEncoderType == CommandEncoderType::Compute) + { shaderResourceGroup->AddUntrackedResourcesToEncoder(m_encoder, srgResourcesVisInfo); } } @@ -447,6 +463,7 @@ namespace AZ for (size_t i = 0; i < bindings.m_srgsByIndex.size(); ++i) { bindings.m_srgsByIndex[i] = nullptr; + bindings.m_srgVisHashByIndex[i] = AZ::HashValue64{0}; } const PipelineLayout& pipelineLayout = pipelineState->GetPipelineLayout(); @@ -469,6 +486,10 @@ namespace AZ void CommandList::SetStreamBuffers(const RHI::StreamBufferView* streams, uint32_t count) { + int bufferArrayLen = 0; + AZStd::array, METAL_MAX_ENTRIES_BUFFER_ARG_TABLE> mtlStreamBuffers; + AZStd::array mtlStreamBufferOffsets; + AZ::HashValue64 streamsHash = AZ::HashValue64{0}; for (uint32_t i = 0; i < count; ++i) { @@ -479,18 +500,23 @@ namespace AZ { m_state.m_streamsHash = streamsHash; AZ_Assert(count <= METAL_MAX_ENTRIES_BUFFER_ARG_TABLE , "Slots needed cannot exceed METAL_MAX_ENTRIES_BUFFER_ARG_TABLE"); - for (uint32_t i = 0; i < count; ++i) + + NSRange range = {METAL_MAX_ENTRIES_BUFFER_ARG_TABLE - count, count}; + //For metal the stream buffers are populated from bottom to top as the top slots are taken by argument buffers + for (int i = count-1; i >= 0; --i) { if (streams[i].GetBuffer()) { const Buffer * buff = static_cast(streams[i].GetBuffer()); id mtlBuff = buff->GetMemoryView().GetGpuAddress>(); - uint32_t VBIndex = (METAL_MAX_ENTRIES_BUFFER_ARG_TABLE - 1) - i; uint32_t offset = streams[i].GetByteOffset() + buff->GetMemoryView().GetOffset(); - id renderEncoder = GetEncoder>(); - [renderEncoder setVertexBuffer: mtlBuff offset: offset atIndex: VBIndex]; + mtlStreamBuffers[bufferArrayLen] = mtlBuff; + mtlStreamBufferOffsets[bufferArrayLen] = offset; + bufferArrayLen++; } } + id renderEncoder = GetEncoder>(); + [renderEncoder setVertexBuffers: mtlStreamBuffers.data() offsets: mtlStreamBufferOffsets.data() withRange: range]; } } diff --git a/Gems/Atom/RHI/Metal/Code/Source/RHI/CommandList.h b/Gems/Atom/RHI/Metal/Code/Source/RHI/CommandList.h index 6660beb2c4..dd4e382471 100644 --- a/Gems/Atom/RHI/Metal/Code/Source/RHI/CommandList.h +++ b/Gems/Atom/RHI/Metal/Code/Source/RHI/CommandList.h @@ -99,6 +99,7 @@ namespace AZ { AZStd::array m_srgsByIndex; AZStd::array m_srgsBySlot; + AZStd::array m_srgVisHashByIndex; }; ShaderResourceBindings& GetShaderResourceBindingsByPipelineType(RHI::PipelineStateType pipelineType); diff --git a/Gems/Atom/RHI/Metal/Code/Source/RHI/PipelineLayout.cpp b/Gems/Atom/RHI/Metal/Code/Source/RHI/PipelineLayout.cpp index f237884aab..70d36f8d6d 100644 --- a/Gems/Atom/RHI/Metal/Code/Source/RHI/PipelineLayout.cpp +++ b/Gems/Atom/RHI/Metal/Code/Source/RHI/PipelineLayout.cpp @@ -70,6 +70,7 @@ namespace AZ m_srgVisibilities.resize(RHI::Limits::Pipeline::ShaderResourceGroupCountMax); m_srgResourcesVisibility.resize(RHI::Limits::Pipeline::ShaderResourceGroupCountMax); + m_srgResourcesVisibilityHash.resize(RHI::Limits::Pipeline::ShaderResourceGroupCountMax); for (uint32_t srgLayoutIdx = 0; srgLayoutIdx < groupLayoutCount; ++srgLayoutIdx) { const RHI::ShaderResourceGroupLayout& srgLayout = *descriptor.GetShaderResourceGroupLayout(srgLayoutIdx); @@ -111,6 +112,7 @@ namespace AZ m_srgVisibilities[srgIndex] = mask; m_srgResourcesVisibility[srgIndex] = srgVis; + m_srgResourcesVisibilityHash[srgIndex] = srgVis.GetHash(); } // Cache the inline constant size and slot index @@ -141,6 +143,11 @@ namespace AZ return m_srgResourcesVisibility[index]; } + const AZ::HashValue64 PipelineLayout::GetSrgResourcesVisibilityHash(uint32_t index) const + { + return m_srgResourcesVisibilityHash[index]; + } + uint32_t PipelineLayout::GetRootConstantsSize() const { return m_rootConstantsSize; diff --git a/Gems/Atom/RHI/Metal/Code/Source/RHI/PipelineLayout.h b/Gems/Atom/RHI/Metal/Code/Source/RHI/PipelineLayout.h index e8cd249393..6fd06ea29b 100644 --- a/Gems/Atom/RHI/Metal/Code/Source/RHI/PipelineLayout.h +++ b/Gems/Atom/RHI/Metal/Code/Source/RHI/PipelineLayout.h @@ -57,6 +57,9 @@ namespace AZ /// Returns srgVisibility data const ShaderResourceGroupVisibility& GetSrgResourcesVisibility(uint32_t index) const; + /// Returns srgVisibility hash + const AZ::HashValue64 GetSrgResourcesVisibilityHash(uint32_t index) const; + /// Returns the root constant specific layout information uint32_t GetRootConstantsSize() const; uint32_t GetRootConstantsSlotIndex() const; @@ -84,6 +87,9 @@ namespace AZ /// Cache Visibility across all the resources within the SRG AZStd::fixed_vector m_srgResourcesVisibility; + /// Cache Visibility hash across all the resources within the SRG + AZStd::fixed_vector m_srgResourcesVisibilityHash; + uint32_t m_rootConstantSlotIndex = (uint32_t)-1; uint32_t m_rootConstantsSize = 0; };