From 7622be0544b05a9bb31b1e2664d8444ac518cc57 Mon Sep 17 00:00:00 2001
From: Bryan Bernhart <bryan.bernhart@intel.com>
Date: Tue, 7 Jul 2020 23:32:52 +0000
Subject: [PATCH] D3D12: Avoid re-binding sampler tables.

Skip re-binding samplers since sampler heap
allocations are cached and the pipeline would
redundantly get set every draw. This is
particularly important for draw heavy scenarios.

After the change:
+3% in RendererPerfTest/1.* via SkiaDawn
+100% for Forest Demo FPS (55ms vs 28ms)

BUG=dawn:479

Change-Id: I93cbca4e365d6ff89ec86fc15eaccf70b49ea916
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/24161
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Bryan Bernhart <bryan.bernhart@intel.com>
---
 src/dawn_native/d3d12/CommandBufferD3D12.cpp | 30 +++++++++++++++++---
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/src/dawn_native/d3d12/CommandBufferD3D12.cpp b/src/dawn_native/d3d12/CommandBufferD3D12.cpp
index a9c0249a82..0b8b124788 100644
--- a/src/dawn_native/d3d12/CommandBufferD3D12.cpp
+++ b/src/dawn_native/d3d12/CommandBufferD3D12.cpp
@@ -83,6 +83,8 @@ namespace dawn_native { namespace d3d12 {
     }  // anonymous namespace
 
     class BindGroupStateTracker : public BindGroupAndStorageBarrierTrackerBase<false, uint64_t> {
+        using Base = BindGroupAndStorageBarrierTrackerBase;
+
       public:
         BindGroupStateTracker(Device* device)
             : BindGroupAndStorageBarrierTrackerBase(),
@@ -95,6 +97,17 @@ namespace dawn_native { namespace d3d12 {
             mInCompute = inCompute_;
         }
 
+        void OnSetPipeline(PipelineBase* pipeline) {
+            // Invalidate the root sampler tables previously set in the root signature.
+            // This is because changing the pipeline layout also changes the root signature.
+            const PipelineLayout* pipelineLayout = ToBackend(pipeline->GetLayout());
+            if (mLastAppliedPipelineLayout != pipelineLayout) {
+                mBoundRootSamplerTables = {};
+            }
+
+            Base::OnSetPipeline(pipeline);
+        }
+
         MaybeError Apply(CommandRecordingContext* commandContext) {
             // Bindgroups are allocated in shader-visible descriptor heaps which are managed by a
             // ringbuffer. There can be a single shader-visible descriptor heap of each type bound
@@ -309,10 +322,16 @@ namespace dawn_native { namespace d3d12 {
                 uint32_t parameterIndex = pipelineLayout->GetSamplerRootParameterIndex(index);
                 const D3D12_GPU_DESCRIPTOR_HANDLE baseDescriptor =
                     group->GetBaseSamplerDescriptor();
-                if (mInCompute) {
-                    commandList->SetComputeRootDescriptorTable(parameterIndex, baseDescriptor);
-                } else {
-                    commandList->SetGraphicsRootDescriptorTable(parameterIndex, baseDescriptor);
+                // Check if the group requires its sampler table to be set in the pipeline.
+                // This because sampler heap allocations could be cached and use the same table.
+                if (mBoundRootSamplerTables[index].ptr != baseDescriptor.ptr) {
+                    if (mInCompute) {
+                        commandList->SetComputeRootDescriptorTable(parameterIndex, baseDescriptor);
+                    } else {
+                        commandList->SetGraphicsRootDescriptorTable(parameterIndex, baseDescriptor);
+                    }
+
+                    mBoundRootSamplerTables[index] = baseDescriptor;
                 }
             }
         }
@@ -321,6 +340,9 @@ namespace dawn_native { namespace d3d12 {
 
         bool mInCompute = false;
 
+        ityp::array<BindGroupIndex, D3D12_GPU_DESCRIPTOR_HANDLE, kMaxBindGroups>
+            mBoundRootSamplerTables = {};
+
         ShaderVisibleDescriptorAllocator* mViewAllocator;
         ShaderVisibleDescriptorAllocator* mSamplerAllocator;
     };