Metal: Performance improvements and bug fixes #98212
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stuartcarnie
commented
Oct 15, 2024
| @@ -58,7 +58,7 @@ | |||
|
|
|||
| void MDCommandBuffer::begin() { | |||
| DEV_ASSERT(commandBuffer == nil); | |||
| commandBuffer = queue.commandBuffer; | |||
| commandBuffer = queue.commandBufferWithUnretainedReferences; | |||
There was a problem hiding this comment.
The Metal driver maintains references to the Metal resources, so there is no need for the command buffer to perform add ref and release calls.
Comment on lines
+2063
to
+2066
| primary.arrayLength = 1; | ||
| for (uint32_t const &a : a_type.array) { | ||
| primary.arrayLength *= a; | ||
| } |
There was a problem hiding this comment.
Fix to support arrays of samplers
5 tasks
clayjohn
approved these changes
Oct 18, 2024
AThousandShips
approved these changes
Oct 18, 2024
drivers/metal/metal_objects.h
Outdated
| @@ -271,8 +288,14 @@ class API_AVAILABLE(macos(11.0), ios(14.0)) MDCommandBuffer { | |||
| blend_constants.reset(); | |||
| vertex_buffers.clear(); | |||
| vertex_offsets.clear(); | |||
| // keep the keys, as they are likely to be used again | |||
There was a problem hiding this comment.
Suggested change
| // keep the keys, as they are likely to be used again | |
| // Keep the keys, as they are likely to be used again. |
drivers/metal/metal_objects.h
Outdated
| _FORCE_INLINE_ void reset() { | ||
| pipeline = nil; | ||
| encoder = nil; | ||
| // keep the keys, as they are likely to be used again |
There was a problem hiding this comment.
Suggested change
| // keep the keys, as they are likely to be used again | |
| // Keep the keys, as they are likely to be used again. |
drivers/metal/metal_objects.mm
Outdated
| if (resources == nullptr) { | ||
| resources = &p_dst.insert(keyval.key, ResourceVector())->value; | ||
| } | ||
| // reserve space for the new resources, assuming they are all added |
There was a problem hiding this comment.
Suggested change
| // reserve space for the new resources, assuming they are all added | |
| // Reserve space for the new resources, assuming they are all added. |
drivers/metal/metal_objects.mm
Outdated
| uint32_t i = 0, j = 0; | ||
| __unsafe_unretained id<MTLResource> *resources_ptr = resources->ptr(); | ||
| const __unsafe_unretained id<MTLResource> *keyval_ptr = keyval.value.ptr(); | ||
| // 2-way merge |
There was a problem hiding this comment.
Suggested change
| // 2-way merge | |
| // 2-way merge. |
drivers/metal/metal_objects.mm
Outdated
| return; | ||
| } | ||
|
|
||
| // bind all resources |
There was a problem hiding this comment.
Suggested change
| // bind all resources | |
| // Bind all resources. |
drivers/metal/metal_objects.mm
Outdated
| return; | ||
| } | ||
|
|
||
| // bind all resources |
There was a problem hiding this comment.
Suggested change
| // bind all resources | |
| // Bind all resources. |
stuartcarnie
force-pushed
the
sgc/metal_improvements
branch
from
10b9041 to
83ac274
Compare
|
@AThousandShips thanks for the feedback – all those changes are in |
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR includes general improvements to the Metal rendering driver.
MTLRenderCommandEncoderincludes a large number ofid<MTLResources>by removing redundantuseResource:calls when binding the same resources over multiple draw callsdrawcalltestproject that alternates between 2D rect and line canvas items