Skip to content

Commit

Permalink
[rhi] MetalCommandList store unique_ptr to resource set (#8312)
Browse files Browse the repository at this point in the history
Issue: #

### Brief Summary

Directly storing a pointer to the resource set is bad because the
resource set can be destroyed before the command list is done.

### Walkthrough

<!--
copilot:walkthrough
-->
### <samp>🤖 Generated by Copilot at f7a1b72</samp>

* Initialize members to avoid dangling pointers and use
`std::unique_ptr` to manage shader resource set lifetime in
`MetalCommandList` class
([link](https://github.com/taichi-dev/taichi/pull/8312/files?diff=unified&w=0#diff-5b304e188996abd217fad85fd8b2434e729ad9c089eb9ef48a848c0fcc74d614L268-R269),
)
* Copy shader resource set argument instead of modifying it and cast it
to `MetalShaderResourceSet` pointer in `bindShaderResourceSet` method
([link](https://github.com/taichi-dev/taichi/pull/8312/files?diff=unified&w=0#diff-c350a27659df1ebcf2c854e2c21a80f82de87d3b325a80e00f6a6dd6ca53303bL428-R429))

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
AntonioFerreras and pre-commit-ci[bot] authored Aug 2, 2023
1 parent 7a7ea19 commit 398b284
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 3 deletions.
4 changes: 2 additions & 2 deletions taichi/rhi/metal/metal_device.h
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,8 @@ class MetalCommandList final : public CommandList {
MTLCommandBuffer_id cmdbuf_;

// Non-null after `bind*` methods.
const MetalPipeline *current_pipeline_;
const MetalShaderResourceSet *current_shader_resource_set_;
const MetalPipeline *current_pipeline_{nullptr};
std::unique_ptr<MetalShaderResourceSet> current_shader_resource_set_{nullptr};
};

class MetalStream final : public Stream {
Expand Down
4 changes: 3 additions & 1 deletion taichi/rhi/metal/metal_device.mm
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,9 @@ MTLTextureUsage usage2mtl(ImageAllocUsage usage) {
int set_index) noexcept {
RHI_ASSERT(res != nullptr);
RHI_ASSERT(set_index == 0);
current_shader_resource_set_ = (MetalShaderResourceSet *)res;
MetalShaderResourceSet *res_metal = (MetalShaderResourceSet *)res;
current_shader_resource_set_ =
std::make_unique<MetalShaderResourceSet>(*res_metal);
return RhiResult::success;
}

Expand Down

0 comments on commit 398b284

Please sign in to comment.