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] kick off registration and initial PSO compilation of runtime effect earlier. #52381
Merged
Conversation
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
jonahwilliams
commented
Apr 25, 2024
chinmaygarde
approved these changes
Apr 25, 2024
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.
Good idea!
@@ -410,6 +412,7 @@ std::shared_ptr<RuntimeStageData::Shader> Reflector::GenerateRuntimeStageData() | |||
.name = ubo.name, | |||
.location = 64, // Magic constant that must match the descriptor set | |||
// location for fragment programs. | |||
.binding = 64, |
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.
How did we get away without needing this earlier?
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.
location and binding are the same for uniform floats but different for samplers. We were guessing the sampler binding based off of the last buffer binding but that seems ... fragile. There may still be bugs here.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 25, 2024
…n of runtime effect earlier. (flutter/engine#52381)
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Apr 26, 2024
…147391) flutter/engine@a09295f...e99fc6e 2024-04-25 jonahwilliams@google.com [Impeller] while we still have benchmarks, see if we're efficient enough for this to be faster. (flutter/engine#52398) 2024-04-25 jonahwilliams@google.com [Impeller] Vulkan validation off by default. (flutter/engine#52397) 2024-04-25 skia-flutter-autoroll@skia.org Roll Skia from cb32ad619678 to 52083c205016 (1 revision) (flutter/engine#52396) 2024-04-25 jonahwilliams@google.com [Impeller] kick off registration and initial PSO compilation of runtime effect earlier. (flutter/engine#52381) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC jimgraham@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
I thought about changing this API so that it blocks on compilation, but I think that isn't necesasary as the workload can happen while the UI thread is building. Plus, even blocking on shader completion does not guarantee that we won't need to compile a variant anyway.
While I was at it I moved the descriptor sets computation to the runtime effect constructor.
With this change the pipleine variant used during the frame is produced much faster than the initial 12ms compilation.
Fixes flutter/flutter#113719
Fixes flutter/flutter#141222