From 6bcf92d28d919eba7607c93c8581966875d2add6 Mon Sep 17 00:00:00 2001 From: Yong He Date: Mon, 15 Jul 2024 22:23:10 -0700 Subject: gfx/metal uniform data binding and memory leak fix. (#4644) Co-authored-by: Yong He --- tools/gfx/d3d11/d3d11-shader-object.cpp | 12 ++++-------- tools/gfx/metal/metal-command-buffer.h | 8 ++++---- tools/gfx/metal/metal-device.h | 4 ---- tools/gfx/metal/metal-pipeline-state.h | 2 +- tools/gfx/metal/metal-shader-object.cpp | 14 +++++--------- tools/gfx/metal/metal-shader-program.h | 2 +- 6 files changed, 15 insertions(+), 27 deletions(-) (limited to 'tools') 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 m_mutableRootShaderObject; - ResourceCommandEncoder* m_resourceCommandEncoder = nullptr; - ComputeCommandEncoder* m_computeCommandEncoder = nullptr; - RenderCommandEncoder* m_renderCommandEncoder = nullptr; - RayTracingCommandEncoder* m_rayTracingCommandEncoder = nullptr; + RefPtr m_resourceCommandEncoder = nullptr; + RefPtr m_computeCommandEncoder = nullptr; + RefPtr m_renderCommandEncoder = nullptr; + RefPtr m_rayTracingCommandEncoder = nullptr; NS::SharedPtr m_metalRenderCommandEncoder; NS::SharedPtr 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 m_device; NS::SharedPtr 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, 1024> m_deviceObjectsWithPotentialBackReferences; - - //RefPtr 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 m_device; + DeviceImpl* m_device; NS::SharedPtr m_renderPipelineState; NS::SharedPtr m_depthStencilState; NS::SharedPtr 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 m_device; + DeviceImpl* m_device; RefPtr m_rootObjectLayout; struct Module -- cgit v1.2.3