gfx/metal uniform data binding and memory leak fix. (#4644)

Co-authored-by: Yong He <yhe@nvidia.com>

6 files changed, 15 insertions, 27 deletions

diff --git a/tools/gfx/d3d11/d3d11-shader-object.cpp b/tools/gfx/d3d11/d3d11-shader-object.cpp
index 40d671843..a83b115fa 100644
--- a/tools/gfx/d3d11/d3d11-shader-object.cpp
+++ b/tools/gfx/d3d11/d3d11-shader-object.cpp
@@ -318,12 +318,6 @@ Result ShaderObjectImpl::bindAsConstantBuffer(
     // buffer for any "ordinary" data in `X`, and then bind the remaining
     // resources and sub-objects.
     //
-    // The one important detail to keep track of its that *if* we bind
-    // a constant buffer for ordinary data we will need to account for
-    // it in the offset we use for binding the remaining data. That
-    // detail is dealt with here by the way that `_bindOrdinaryDataBufferIfNeeded`
-    // will modify the `offset` parameter if it binds anything.
-    //
     BindingOffset offset = inOffset;
     SLANG_RETURN_ON_FAIL(_bindOrdinaryDataBufferIfNeeded(context, /*inout*/ offset, specializedLayout));
 
@@ -331,8 +325,10 @@ Result ShaderObjectImpl::bindAsConstantBuffer(
     // the rest of the state, which can use logic shared with the case
     // for interface-type sub-object ranges.
     //
-    // Note that this call will use the `offset` value that might have
-    // been modified during `_bindOrindaryDataBufferIfNeeded`.
+    // Note that this call will use the `inOffset` value instead of the offset
+    // modified by `_bindOrindaryDataBufferIfNeeded', because the indexOffset in
+    // the binding range should already take care of the offset due to the default
+    // cbuffer.
     //
     SLANG_RETURN_ON_FAIL(bindAsValue(context, inOffset, specializedLayout));
 
diff --git a/tools/gfx/metal/metal-command-buffer.h b/tools/gfx/metal/metal-command-buffer.h
index 6c000b5df..f0b36898d 100644
--- a/tools/gfx/metal/metal-command-buffer.h
+++ b/tools/gfx/metal/metal-command-buffer.h
@@ -28,10 +28,10 @@ public:
     RootShaderObjectImpl m_rootObject;
     // RefPtr<MutableRootShaderObjectImpl> m_mutableRootShaderObject;
 
-    ResourceCommandEncoder* m_resourceCommandEncoder = nullptr;
-    ComputeCommandEncoder* m_computeCommandEncoder = nullptr;
-    RenderCommandEncoder* m_renderCommandEncoder = nullptr;
-    RayTracingCommandEncoder* m_rayTracingCommandEncoder = nullptr;
+    RefPtr<ResourceCommandEncoder> m_resourceCommandEncoder = nullptr;
+    RefPtr<ComputeCommandEncoder> m_computeCommandEncoder = nullptr;
+    RefPtr<RenderCommandEncoder> m_renderCommandEncoder = nullptr;
+    RefPtr<RayTracingCommandEncoder> m_rayTracingCommandEncoder = nullptr;
 
     NS::SharedPtr<MTL::RenderCommandEncoder> m_metalRenderCommandEncoder;
     NS::SharedPtr<MTL::ComputeCommandEncoder> m_metalComputeCommandEncoder;
diff --git a/tools/gfx/metal/metal-device.h b/tools/gfx/metal/metal-device.h
index 6c3ba68f8..4f08b346e 100644
--- a/tools/gfx/metal/metal-device.h
+++ b/tools/gfx/metal/metal-device.h
@@ -135,8 +135,6 @@ public:
     NS::SharedPtr<MTL::Device> m_device;
     NS::SharedPtr<MTL::CommandQueue> m_commandQueue;
 
-    //DescriptorSetAllocator descriptorSetAllocator;
-
     uint32_t m_queueAllocCount;
 
     // A list to hold objects that may have a strong back reference to the device
@@ -150,8 +148,6 @@ public:
     // worrying the `ShaderProgramImpl` object getting destroyed after the completion of
     // `DeviceImpl::~DeviceImpl()'.
     ChunkedList<RefPtr<RefObject>, 1024> m_deviceObjectsWithPotentialBackReferences;
-
-    //RefPtr<FramebufferImpl> m_emptyFramebuffer;
 };
 
 } // namespace metal
diff --git a/tools/gfx/metal/metal-pipeline-state.h b/tools/gfx/metal/metal-pipeline-state.h
index 2a4f431b4..71897925a 100644
--- a/tools/gfx/metal/metal-pipeline-state.h
+++ b/tools/gfx/metal/metal-pipeline-state.h
@@ -14,7 +14,7 @@ namespace metal
 class PipelineStateImpl : public PipelineStateBase
 {
 public:
-    RefPtr<DeviceImpl> m_device;
+    DeviceImpl* m_device;
     NS::SharedPtr<MTL::RenderPipelineState> m_renderPipelineState;
     NS::SharedPtr<MTL::DepthStencilState> m_depthStencilState;
     NS::SharedPtr<MTL::ComputePipelineState> m_computePipelineState;
diff --git a/tools/gfx/metal/metal-shader-object.cpp b/tools/gfx/metal/metal-shader-object.cpp
index 33e19bc6f..865196c5c 100644
--- a/tools/gfx/metal/metal-shader-object.cpp
+++ b/tools/gfx/metal/metal-shader-object.cpp
@@ -334,12 +334,6 @@ Result ShaderObjectImpl::bindAsConstantBuffer(
     // buffer for any "ordinary" data in `X`, and then bind the remaining
     // resources and sub-objects.
     //
-    // The one important detail to keep track of its that *if* we bind
-    // a constant buffer for ordinary data we will need to account for
-    // it in the offset we use for binding the remaining data. That
-    // detail is dealt with here by the way that `_bindOrdinaryDataBufferIfNeeded`
-    // will modify the `offset` parameter if it binds anything.
-    //
     BindingOffset offset = inOffset;
     SLANG_RETURN_ON_FAIL(_bindOrdinaryDataBufferIfNeeded(context, /*inout*/ offset, layout));
 
@@ -347,10 +341,12 @@ Result ShaderObjectImpl::bindAsConstantBuffer(
     // the rest of the state, which can use logic shared with the case
     // for interface-type sub-object ranges.
     //
-    // Note that this call will use the `offset` value that might have
-    // been modified during `_bindOrindaryDataBufferIfNeeded`.
+    // Note that this call will use the `inOffset` value instead of the offset
+    // modified by `_bindOrindaryDataBufferIfNeeded', because the indexOffset in
+    // the binding range should already take care of the offset due to the default
+    // cbuffer.
     //
-    SLANG_RETURN_ON_FAIL(bindAsValue(context, offset, layout));
+    SLANG_RETURN_ON_FAIL(bindAsValue(context, inOffset, layout));
 
     return SLANG_OK;
 }
diff --git a/tools/gfx/metal/metal-shader-program.h b/tools/gfx/metal/metal-shader-program.h
index ed2f231ec..d6deb6574 100644
--- a/tools/gfx/metal/metal-shader-program.h
+++ b/tools/gfx/metal/metal-shader-program.h
@@ -15,7 +15,7 @@ namespace metal
 class ShaderProgramImpl : public ShaderProgramBase
 {
 public:
-    BreakableReference<DeviceImpl> m_device;
+    DeviceImpl* m_device;
     RefPtr<RootShaderObjectLayoutImpl> m_rootObjectLayout;
 
     struct Module