[Flutter GPU] Load a ShaderLibrary from shader bundle bytes#188596
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
c69f8c6 to
b4bf206
Compare
b4bf206 to
9ab6ce4
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to load and reinitialize a ShaderLibrary directly from raw bytes (ByteData) in Dart, with corresponding native C++ implementations and unit tests. Feedback on the changes includes adding a defensive null check for the native ShaderLibrary wrapper to prevent potential crashes, changing ShaderLibrary.fromBytes to return a non-nullable Future and removing the unnecessary async keyword, simplifying the associated test, and capturing a pointer by value instead of by reference in a C++ lambda to avoid lifetime issues.
gaaclarke
left a comment
There was a problem hiding this comment.
We've had a similar PR in flight for runtime effects: #175479
I'm hesitant to introduce this API because then we are on the hook for maintaining a file format. Since holding off on that PR we have already changed the file format in order to accommodate setting uniforms by name.
|
That concern makes sense. I don't want to sign us up for a file-format burden either. But the shaderbundle format already leaks to users today and users are already expected to be responsible and treat it like a non-backwards-compatible black-box intermediary.
So To be more specific here: I'm trying to support workflows for advanced tooling, demonstrated by Flutter Scene's editor. Are there any alternative solutions that would make you more comfortable here? Screen.Recording.2026-06-26.at.5.39.20.PM.mov |
One alternative would be to instead of having a compatibility number in the shader bundle, tie the shader bundle to a specific version of the flutter sdk. This would make it so we don't have to test any backwards scenarios. This would also make it infeasible to author your editor binary since you'd have to ship a new binary for every single Flutter SDK. So, that's not worth doing. I'd feel more comfortable if we are sure that we are surfacing these version failures up to dart. 2bd9ecb introduced the version numbers and added c++ tests. We didn't add an integration test that makes sure the errors are surfaced to dart. Another thing that makes me a bit wary too is that these file formats all have binary blobs in them for compiled shaders. We could easily accidently change what those binary blobs mean and forget to increment the compatibility version numbers. We'll have to be careful of that. |
gaaclarke
left a comment
There was a problem hiding this comment.
Okay, I'm on board. Let's do this. I would like to see 2 things though:
- Integration test for the error path when the compatibility version of a shader bundle doesn't match what we support. This probably already works but I want to make sure it keeps working.
- An integration test for reinitializing a shader.
| /// | ||
| /// Returns false if invoking the function failed or if any unhandled | ||
| /// exceptions were thrown. | ||
| bool RunDartFunctionWithShaderBundle(const char* dart_function_name) { |
There was a problem hiding this comment.
Can't we use //engine/src/flutter/testing/dart for these tests instead? I know we have existing tests like this but those are a bit more straightforward.
There was a problem hiding this comment.
Yeah, moved to testing/dart/gpu_shader_reload_test.dart.
| /// | ||
| /// Returns null on success, or an error message if [bytes] could not be | ||
| /// parsed (the live shaders are left unchanged in that case). | ||
| String? reinitializeFromBytes(ByteData bytes) => |
There was a problem hiding this comment.
What test do we have for this?
There was a problem hiding this comment.
None before, just added one. There's now a reinitializeFromBytes test in testing/dart/gpu_shader_reload_test.dart that mirrors the asset reload test, plus a draw-based eviction one.
|
(The dart integration test for the compatibility version number is probably better off as its own PR) |
Add `ShaderLibrary.fromBytes(ByteData)`, which parses a shader bundle from a byte buffer instead of fetching it through the asset manager like `fromAsset`. This loads shader bundles produced or fetched at runtime, which have no entry in the asset manifest, for example a tool that compiles a shader with impellerc and wants to use the result without rebundling the app. The bytes feed into the same `MakeFromFlatbuffer` path `fromAsset` uses after its asset fetch, so the parse, registration, and shader-id namespacing are unchanged. The result is not cached (the bytes carry no stable key), so the asset-path hot reload does not apply; `reinitializeFromBytes` reparses a recompiled bundle into the same library in place, preserving the identity of any `Shader`s already handed out. Tested with a new renderer fixture that hands the `playground.shaderbundle` fixture's bytes to Dart and loads a usable library from them.
…alizeFromBytes Relocate the ShaderLibrary.fromBytes coverage out of the renderer C++ dart fixture into testing/dart/gpu_shader_reload_test.dart, alongside the existing shader reload tests, which is a simpler harness. The tests fetch a fixture bundle's bytes through the asset platform channel and feed them into fromBytes/reinitializeFromBytes, so the renderer fixture plumbing (RunDartFunctionWithShaderBundle, the canCreateShaderLibraryFromBytes entry point, and the dart_byte_data.h include) is no longer needed. Adds tests for loading a usable library from bytes, surfacing a parse failure through the Future, reparsing in place with reinitializeFromBytes (preserving library and shader identities, marking only the changed shader dirty), and re-registering cleanly after an in-place reload.
b9af480 to
9ab011a
Compare
…the identifier ParseShaderBundle called ShaderBundleBufferHasIdentifier, which reads the file identifier at a fixed offset, before the flatbuffers verifier ran. A buffer smaller than the root offset plus the identifier (8 bytes) was read out of bounds, which AddressSanitizer flags as a heap-buffer-overflow. fromBytes lets a caller pass arbitrary bytes, so guard the size before sniffing the identifier and let the existing verifier handle the rest. Surfaced by the new "fromBytes surfaces a parse failure through the Future" test, which feeds a 4-byte buffer.
…12135) Manual roll Flutter from ca9f874f5284 to 6995038d96ef (44 revisions) Manual roll requested by [email protected] flutter/flutter@ca9f874...6995038 2026-07-03 [email protected] Roll Fuchsia Linux SDK from sx_eN0J_f2BV6jqjW... to Jr08vyeibMSv3Oxst... (flutter/flutter#188946) 2026-07-03 [email protected] Roll Packages from 420e135 to 2fbe873 (1 revision) (flutter/flutter#188945) 2026-07-03 [email protected] Roll Dart SDK from 786212a2ce0d to 1f9a08ce0638 (4 revisions) (flutter/flutter#188944) 2026-07-03 [email protected] Roll Skia from 5358ab75b840 to 919956953af6 (7 revisions) (flutter/flutter#188943) 2026-07-02 [email protected] [Windows] Keep regular windows in place when another is activated (flutter/flutter#188016) 2026-07-02 [email protected] [flutter_tools] Provide guided message when iOS/macOS build fails due to low minimum version (flutter/flutter#188812) 2026-07-02 [email protected] [Flutter GPU] Load a ShaderLibrary from shader bundle bytes (flutter/flutter#188596) 2026-07-02 [email protected] [tool] Enable record_use experiment by default on all channels (flutter/flutter#188887) 2026-07-02 [email protected] Roll Packages from e742106 to 420e135 (13 revisions) (flutter/flutter#188916) 2026-07-02 [email protected] Roll Fuchsia Linux SDK from I2h2eXk06RrA3pIG2... to sx_eN0J_f2BV6jqjW... (flutter/flutter#188915) 2026-07-02 [email protected] Clarify layout callback debug flag docs (flutter/flutter#186879) 2026-07-02 [email protected] Roll Skia from 0c4faca350cc to 5358ab75b840 (2 revisions) (flutter/flutter#188899) 2026-07-02 [email protected] Roll Dart SDK from e47361c7fe9a to 786212a2ce0d (2 revisions) (flutter/flutter#188898) 2026-07-01 [email protected] Roll Skia from 0fc8ba72e802 to 0c4faca350cc (2 revisions) (flutter/flutter#188886) 2026-07-01 [email protected] Hide draft PRs from the triage list (flutter/flutter#188885) 2026-07-01 [email protected] Stricten isSemantics and matchesSemantics children mismatch check (flutter/flutter#188827) 2026-07-01 [email protected] [AGP 9] Support Enabling Built-in Kotlin (flutter/flutter#188543) 2026-07-01 [email protected] Roll Skia from d19e557ac317 to 0fc8ba72e802 (4 revisions) (flutter/flutter#188879) 2026-07-01 [email protected] [ci] Increase test timeout for Mac_x64 build_tests shards (flutter/flutter#188804) 2026-07-01 [email protected] Roll Skia from bd4ae38ca3bb to d19e557ac317 (1 revision) (flutter/flutter#188865) 2026-07-01 [email protected] Roll Dart SDK from 26d723eb89af to e47361c7fe9a (5 revisions) (flutter/flutter#188864) 2026-07-01 [email protected] Update triage links for material_ui and cupertino_ui --> Design triage (flutter/flutter#188567) 2026-07-01 [email protected] [Impeller] Share a single ContextGLES among all PlaygroundImplGLES (flutter/flutter#188080) 2026-07-01 [email protected] Use null-aware elements in dev/devicelab/lib/integration_tests.dart (flutter/flutter#187852) 2026-07-01 [email protected] Roll Packages from 274ed3e to e742106 (18 revisions) (flutter/flutter#188863) 2026-07-01 [email protected] Add android 17 to embedding (flutter/flutter#187965) 2026-07-01 [email protected] Adds semantics role check to isSemantics and matchesSemantics (flutter/flutter#188825) 2026-07-01 [email protected] Roll Dart SDK from e1bdb9ce3327 to 26d723eb89af (3 revisions) (flutter/flutter#188795) 2026-07-01 [email protected] [web] Apply autocapitalize to text editing elements (flutter/flutter#188351) 2026-07-01 [email protected] Roll Fuchsia Linux SDK from RymJjIj7dd5vQ3Cnh... to I2h2eXk06RrA3pIG2... (flutter/flutter#188852) 2026-07-01 [email protected] [Impeller] Compute dispatch takes 3D workgroup counts and honors the shader local_size (flutter/flutter#188601) 2026-07-01 [email protected] Improve stylus support on linux (flutter/flutter#186831) 2026-07-01 [email protected] Resolve issue Catch am start failures with 'Error type' and prevent hang (flutter/flutter#187196) 2026-07-01 [email protected] Roll Skia from ef178c9898af to bd4ae38ca3bb (3 revisions) (flutter/flutter#188834) 2026-07-01 [email protected] Roll Skia from 3ac99be47229 to ef178c9898af (3 revisions) (flutter/flutter#188831) 2026-07-01 [email protected] Add a macosArm64Only feature flag (flutter/flutter#188598) 2026-07-01 [email protected] Roll Skia from 15302f1625b2 to 3ac99be47229 (1 revision) (flutter/flutter#188819) 2026-07-01 [email protected] [flutter_tools] Track asset transformer dependencies for hot reload (Reland #187947) (flutter/flutter#188808) 2026-06-30 [email protected] Add TapRegion samples (flutter/flutter#188685) 2026-06-30 [email protected] Print a warning in `flutter doctor` when running on Intel Macs (flutter/flutter#188760) 2026-06-30 [email protected] [framework] Keep scrollable semantics role stable (flutter/flutter#187963) 2026-06-30 [email protected] feat(skills): Add shepherd-prs skill for managing approved external contributor PRs (flutter/flutter#188534) 2026-06-30 [email protected] Roll Skia from 71947c4110b0 to 15302f1625b2 (17 revisions) (flutter/flutter#188815) 2026-06-30 [email protected] [Tool] Run re-entrant upgrade in original CWD (flutter/flutter#188794) ...
…lutter#12135) Manual roll Flutter from ca9f874f5284 to 6995038d96ef (44 revisions) Manual roll requested by [email protected] flutter/flutter@ca9f874...6995038 2026-07-03 [email protected] Roll Fuchsia Linux SDK from sx_eN0J_f2BV6jqjW... to Jr08vyeibMSv3Oxst... (flutter/flutter#188946) 2026-07-03 [email protected] Roll Packages from 420e135 to 2fbe873 (1 revision) (flutter/flutter#188945) 2026-07-03 [email protected] Roll Dart SDK from 786212a2ce0d to 1f9a08ce0638 (4 revisions) (flutter/flutter#188944) 2026-07-03 [email protected] Roll Skia from 5358ab75b840 to 919956953af6 (7 revisions) (flutter/flutter#188943) 2026-07-02 [email protected] [Windows] Keep regular windows in place when another is activated (flutter/flutter#188016) 2026-07-02 [email protected] [flutter_tools] Provide guided message when iOS/macOS build fails due to low minimum version (flutter/flutter#188812) 2026-07-02 [email protected] [Flutter GPU] Load a ShaderLibrary from shader bundle bytes (flutter/flutter#188596) 2026-07-02 [email protected] [tool] Enable record_use experiment by default on all channels (flutter/flutter#188887) 2026-07-02 [email protected] Roll Packages from e742106 to 420e135 (13 revisions) (flutter/flutter#188916) 2026-07-02 [email protected] Roll Fuchsia Linux SDK from I2h2eXk06RrA3pIG2... to sx_eN0J_f2BV6jqjW... (flutter/flutter#188915) 2026-07-02 [email protected] Clarify layout callback debug flag docs (flutter/flutter#186879) 2026-07-02 [email protected] Roll Skia from 0c4faca350cc to 5358ab75b840 (2 revisions) (flutter/flutter#188899) 2026-07-02 [email protected] Roll Dart SDK from e47361c7fe9a to 786212a2ce0d (2 revisions) (flutter/flutter#188898) 2026-07-01 [email protected] Roll Skia from 0fc8ba72e802 to 0c4faca350cc (2 revisions) (flutter/flutter#188886) 2026-07-01 [email protected] Hide draft PRs from the triage list (flutter/flutter#188885) 2026-07-01 [email protected] Stricten isSemantics and matchesSemantics children mismatch check (flutter/flutter#188827) 2026-07-01 [email protected] [AGP 9] Support Enabling Built-in Kotlin (flutter/flutter#188543) 2026-07-01 [email protected] Roll Skia from d19e557ac317 to 0fc8ba72e802 (4 revisions) (flutter/flutter#188879) 2026-07-01 [email protected] [ci] Increase test timeout for Mac_x64 build_tests shards (flutter/flutter#188804) 2026-07-01 [email protected] Roll Skia from bd4ae38ca3bb to d19e557ac317 (1 revision) (flutter/flutter#188865) 2026-07-01 [email protected] Roll Dart SDK from 26d723eb89af to e47361c7fe9a (5 revisions) (flutter/flutter#188864) 2026-07-01 [email protected] Update triage links for material_ui and cupertino_ui --> Design triage (flutter/flutter#188567) 2026-07-01 [email protected] [Impeller] Share a single ContextGLES among all PlaygroundImplGLES (flutter/flutter#188080) 2026-07-01 [email protected] Use null-aware elements in dev/devicelab/lib/integration_tests.dart (flutter/flutter#187852) 2026-07-01 [email protected] Roll Packages from 274ed3e to e742106 (18 revisions) (flutter/flutter#188863) 2026-07-01 [email protected] Add android 17 to embedding (flutter/flutter#187965) 2026-07-01 [email protected] Adds semantics role check to isSemantics and matchesSemantics (flutter/flutter#188825) 2026-07-01 [email protected] Roll Dart SDK from e1bdb9ce3327 to 26d723eb89af (3 revisions) (flutter/flutter#188795) 2026-07-01 [email protected] [web] Apply autocapitalize to text editing elements (flutter/flutter#188351) 2026-07-01 [email protected] Roll Fuchsia Linux SDK from RymJjIj7dd5vQ3Cnh... to I2h2eXk06RrA3pIG2... (flutter/flutter#188852) 2026-07-01 [email protected] [Impeller] Compute dispatch takes 3D workgroup counts and honors the shader local_size (flutter/flutter#188601) 2026-07-01 [email protected] Improve stylus support on linux (flutter/flutter#186831) 2026-07-01 [email protected] Resolve issue Catch am start failures with 'Error type' and prevent hang (flutter/flutter#187196) 2026-07-01 [email protected] Roll Skia from ef178c9898af to bd4ae38ca3bb (3 revisions) (flutter/flutter#188834) 2026-07-01 [email protected] Roll Skia from 3ac99be47229 to ef178c9898af (3 revisions) (flutter/flutter#188831) 2026-07-01 [email protected] Add a macosArm64Only feature flag (flutter/flutter#188598) 2026-07-01 [email protected] Roll Skia from 15302f1625b2 to 3ac99be47229 (1 revision) (flutter/flutter#188819) 2026-07-01 [email protected] [flutter_tools] Track asset transformer dependencies for hot reload (Reland #187947) (flutter/flutter#188808) 2026-06-30 [email protected] Add TapRegion samples (flutter/flutter#188685) 2026-06-30 [email protected] Print a warning in `flutter doctor` when running on Intel Macs (flutter/flutter#188760) 2026-06-30 [email protected] [framework] Keep scrollable semantics role stable (flutter/flutter#187963) 2026-06-30 [email protected] feat(skills): Add shepherd-prs skill for managing approved external contributor PRs (flutter/flutter#188534) 2026-06-30 [email protected] Roll Skia from 71947c4110b0 to 15302f1625b2 (17 revisions) (flutter/flutter#188815) 2026-06-30 [email protected] [Tool] Run re-entrant upgrade in original CWD (flutter/flutter#188794) ...
ShaderLibrary.fromBytes(ByteData)parses a shader bundle from a byte buffer instead of fetching it through the asset manager likefromAsset. This loads shader bundles produced or fetched at runtime, which have no entry in the asset manifest (for example a tool that compiles a shader withimpellercand wants to use the result without rebundling the app).The bytes feed into the same
MakeFromFlatbufferpathfromAssetuses after its asset fetch, so the parse, shader registration, and shader-id namespacing are unchanged. The result is not cached (the bytes carry no stable key), so the asset-path hot reload does not apply;reinitializeFromBytesreparses a recompiled bundle into the same library in place, keeping the identity of anyShaders already handed out.Builds on "[Flutter GPU] Make ShaderLibrary.fromAsset asynchronous" (the commit below this one on the branch);
fromBytesmirrors that async shape so every platform has one consistent loader. Stacked on that change, so please review it after the async one lands.Adds a
CanCreateShaderLibraryFromBytesrenderer dart unit test that hands theplayground.shaderbundlefixture's bytes to Dart and loads a usable library from them.Issue: to be linked (tracking the ability to load a
ShaderLibraryfrom runtime bytes). Draft until the tracking issue is filed and the engine unit tests have a green run.Pre-launch Checklist
///).