Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] kick off registration and initial PSO compilation of runtime effect earlier. #52381

Merged
merged 6 commits into from
Apr 25, 2024

Conversation

jonahwilliams
Copy link
Member

@jonahwilliams jonahwilliams commented Apr 25, 2024

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.

image

Fixes flutter/flutter#113719
Fixes flutter/flutter#141222

@jonahwilliams jonahwilliams marked this pull request as ready for review April 25, 2024 03:33
@jonahwilliams jonahwilliams requested a review from bdero April 25, 2024 03:33
Copy link
Member

@chinmaygarde chinmaygarde left a 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,
Copy link
Member

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?

Copy link
Member Author

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.

@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 25, 2024
@auto-submit auto-submit bot merged commit fd437e7 into flutter:main Apr 25, 2024
@jonahwilliams jonahwilliams deleted the register_early branch April 25, 2024 20:26
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 25, 2024
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.
Labels
autosubmit Merge PR when tree becomes green via auto submit App e: impeller
Projects
None yet
2 participants