[Impeller] Adds FragmentProgram.fromAsset #34649
Conversation
lib/ui/painting/fragment_program.cc
Outdated
| FOR_EACH_BINDING(DART_REGISTER_NATIVE)}); | ||
| } | ||
|
|
||
| void FragmentProgram::initFromAsset(std::string asset_name, |
There was a problem hiding this comment.
@jonahwilliams What are the changes to support hot reload? Guessing:
FragmentProgramretainsasset_name- On a hot reload, reload the shader data from the asset manager
- Make a new
SkRuntimeEffectand overwriteruntime_effect_ - Also re-setup the fields of the Dart object.
- Other stuff?
The thing I'm not sure how to do is how to hook into a notification of a hot reload.
There was a problem hiding this comment.
For step 2, rather than signal on a hot reload (because the reload may not have changed that particular asset)- wait for a specifc vm service notification that contains an invalidation for a particular asset. This would need to be dispatched from the tool, but it could be hooked up alongside the existing vm service listeners in service_protocol.cc .
There was a problem hiding this comment.
I'd imagine we'd need to create some sort of shared debug mode only shader manager that could correlate the shader instance with the vm service signal
There was a problem hiding this comment.
This seemed easier to handle on the Dart side. I added a static Map from asset keys to FragmentPrograms to FragmentProgram, and a service extension that takes the asset key, uses it to look up the FragmentProgram and reinitializes it. wdyt?
There was a problem hiding this comment.
The Map should probably hold WeakReferences to the FragmentPrograms, but I'll come back to this after the bugfixes to impellerc land.
There was a problem hiding this comment.
That seems reasonable, also all for doing it in Dart as well!
|
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. |
zanderso
force-pushed
the
fp-init-from-asset
branch
from
c9e1601 to
af8fd60
Compare
chinmaygarde
changed the title
flutter-dashboard
bot
added
the
platform-web
zanderso
force-pushed
the
fp-init-from-asset
branch
2 times, most recently
from
2b87fe0 to
d18df63
Compare
zanderso
force-pushed
the
fp-init-from-asset
branch
from
d18df63 to
a3d21be
Compare
zanderso
changed the title
| impeller::RuntimeUniformType::kSampledImage) { | ||
| sampled_image_count++; | ||
| } else { | ||
| size_t size = uniform_description.dimensions.rows * |
There was a problem hiding this comment.
@chinmaygarde Does this make sense? This is attempting to compute how many bytes are needed for all of the uniforms by adding up (rows * cols * bit_width) / 8.
lib/ui/painting.dart
Outdated
| final WeakReference<FragmentProgram>? programRef = _shaderRegistry[assetKey]; | ||
| FragmentProgram? program; | ||
| if (programRef != null) { | ||
| program = programRef.target; | ||
| } |
There was a problem hiding this comment.
nit:
FragmentProgram? program = _shaderRegistry[assetKey]?.target;
jonahwilliams
left a comment
jonahwilliams
left a comment
There was a problem hiding this comment.
Dart side LGTM
(I wrote up flutter/flutter#107963 which when combined with some new impellerc flags should get us support in the tool)
zanderso
force-pushed
the
fp-init-from-asset
branch
from
a3d21be to
8149ca9
Compare
| impeller::RuntimeUniformType::kSampledImage) { | ||
| sampled_image_count++; | ||
| } else { | ||
| size_t size = uniform_description.dimensions.rows * |
3 participants
This PR adds the API
FragmentProgram.fromAsset, which loads a shader in.iplrformat as output byimpellercdirectly from a bundled asset into aFragmentProgramobject. Sinceimpellerchas already compiled the shader into the correct target-specific format, there is no need for the shader to transit the Dart heap or to be transpiled by the Engine's SPIR-V -> SkSL transpiler.