This repository was archived by the owner on Feb 25, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 6k
[Impeller] Add support for specialization constants redux. #47678
Merged
auto-submit
merged 23 commits into
flutter:main
from
jonahwilliams:add_specialization_support
Nov 7, 2023
Merged
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
f0b499d
[Impeller] add support for specialization constants.
0bfb741
++
3c32f10
++
2baf3f6
Merge branch 'main' into add_specialization_support
432700d
self code review.
bdab80d
++
ea7ce18
Merge branch 'add_specialization_support' of github.com:jonahwilliams…
e10ebb2
++
ea91c9f
GLES fixes
bd3e5dd
well I tried.
43e2309
++
a1ab22f
update markdown doc.
2bea539
++
153072a
bdero review.
967dfff
++
e94b0a5
Merge branch 'main' of github.com:flutter/engine into add_specializat…
4aca527
claw back performance.
8f84e16
++
a336bc6
++
94f6548
fixup.
b500bc4
Update pipeline_library_mtl.mm
af74dae
Update pipeline_library_mtl.mm
ef7761b
Update pipeline_library_mtl.mm
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
fixup.
- Loading branch information
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,7 +31,7 @@ static void GetMTLRenderPipelineDescriptor(const PipelineDescriptor& desc, | |
| auto descriptor = [[MTLRenderPipelineDescriptor alloc] init]; | ||
| descriptor.label = @(desc.GetLabel().c_str()); | ||
| descriptor.rasterSampleCount = static_cast<NSUInteger>(desc.GetSampleCount()); | ||
| bool async = false; | ||
| bool created_specialized_function = false; | ||
|
|
||
| if (const auto& vertex_descriptor = desc.GetVertexDescriptor()) { | ||
| VertexDescriptorMTL vertex_descriptor_mtl; | ||
|
|
@@ -55,7 +55,7 @@ static void GetMTLRenderPipelineDescriptor(const PipelineDescriptor& desc, | |
|
|
||
| // This latch is used to ensure that GetMTLFunctionSpecialized does not finish | ||
| // before the descriptor is completely set up. | ||
| fml::CountDownLatch latch(1u); | ||
| auto latch = std::make_shared<fml::CountDownLatch>(1u); | ||
| const auto& constants = desc.GetSpecializationConstants(); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is still a bit iffy, but we improve performance by not blocking on the specialization, it all goes into the same pipeline future which is only blocking once we actually trry to use it. Since the compilation is out of process anyway, blocking on it is pointless and only serves to park the thread.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Feels like we're writing javascript here with this chain of nested completion callbacks. ;) Looks safe to me, though. |
||
| for (const auto& entry : desc.GetStageEntrypoints()) { | ||
| if (entry.first == ShaderStage::kVertex) { | ||
|
|
@@ -67,21 +67,22 @@ static void GetMTLRenderPipelineDescriptor(const PipelineDescriptor& desc, | |
| descriptor.fragmentFunction = | ||
| ShaderFunctionMTL::Cast(*entry.second).GetMTLFunction(); | ||
| } else { | ||
| async = true; | ||
| // This code only expects a single specialized function per pipeline. | ||
| FML_CHECK(!created_specialized_function); | ||
| created_specialized_function = true; | ||
| ShaderFunctionMTL::Cast(*entry.second) | ||
| .GetMTLFunctionSpecialized( | ||
| constants, | ||
| [callback, descriptor, &latch](id<MTLFunction> function) { | ||
| [callback, descriptor, latch](id<MTLFunction> function) { | ||
| descriptor.fragmentFunction = function; | ||
| latch.Wait(); | ||
| callback(descriptor); | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| latch.CountDown(); | ||
| if (!async) { | ||
| latch->CountDown(); | ||
| if (!created_specialized_function) { | ||
| callback(descriptor); | ||
| } | ||
| } | ||
|
|
||
This file contains hidden or 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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nothing appears to be waiting on this latch, and I'm not sure why we'd need one. It looks like the descriptor is already done being set up by the time we call
GetMTLFunctionSpecialized.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed. This was left over from an earlier iteration.