Repository navigation
[Flutter GPU] Load a ShaderLibrary from shader bundle bytes - #188596
auto-submit[bot] merged 6 commits into
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 stuartmorgan@google.com flutter/flutter@ca9f874...6995038 2026-07-03 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from sx_eN0J_f2BV6jqjW... to Jr08vyeibMSv3Oxst... (flutter/flutter#188946) 2026-07-03 engine-flutter-autoroll@skia.org Roll Packages from 420e135 to 2fbe873 (1 revision) (flutter/flutter#188945) 2026-07-03 engine-flutter-autoroll@skia.org Roll Dart SDK from 786212a2ce0d to 1f9a08ce0638 (4 revisions) (flutter/flutter#188944) 2026-07-03 engine-flutter-autoroll@skia.org Roll Skia from 5358ab75b840 to 919956953af6 (7 revisions) (flutter/flutter#188943) 2026-07-02 j.ihlas@betclicgroup.com [Windows] Keep regular windows in place when another is activated (flutter/flutter#188016) 2026-07-02 okorohelijah@google.com [flutter_tools] Provide guided message when iOS/macOS build fails due to low minimum version (flutter/flutter#188812) 2026-07-02 bdero@google.com [Flutter GPU] Load a ShaderLibrary from shader bundle bytes (flutter/flutter#188596) 2026-07-02 dacoharkes@google.com [tool] Enable record_use experiment by default on all channels (flutter/flutter#188887) 2026-07-02 engine-flutter-autoroll@skia.org Roll Packages from e742106 to 420e135 (13 revisions) (flutter/flutter#188916) 2026-07-02 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from I2h2eXk06RrA3pIG2... to sx_eN0J_f2BV6jqjW... (flutter/flutter#188915) 2026-07-02 256906086+mvincentong@users.noreply.github.com Clarify layout callback debug flag docs (flutter/flutter#186879) 2026-07-02 engine-flutter-autoroll@skia.org Roll Skia from 0c4faca350cc to 5358ab75b840 (2 revisions) (flutter/flutter#188899) 2026-07-02 engine-flutter-autoroll@skia.org Roll Dart SDK from e47361c7fe9a to 786212a2ce0d (2 revisions) (flutter/flutter#188898) 2026-07-01 engine-flutter-autoroll@skia.org Roll Skia from 0fc8ba72e802 to 0c4faca350cc (2 revisions) (flutter/flutter#188886) 2026-07-01 robert.ancell@canonical.com Hide draft PRs from the triage list (flutter/flutter#188885) 2026-07-01 47866232+chunhtai@users.noreply.github.com Stricten isSemantics and matchesSemantics children mismatch check (flutter/flutter#188827) 2026-07-01 jesswon@google.com [AGP 9] Support Enabling Built-in Kotlin (flutter/flutter#188543) 2026-07-01 engine-flutter-autoroll@skia.org Roll Skia from d19e557ac317 to 0fc8ba72e802 (4 revisions) (flutter/flutter#188879) 2026-07-01 bkonyi@google.com [ci] Increase test timeout for Mac_x64 build_tests shards (flutter/flutter#188804) 2026-07-01 engine-flutter-autoroll@skia.org Roll Skia from bd4ae38ca3bb to d19e557ac317 (1 revision) (flutter/flutter#188865) 2026-07-01 engine-flutter-autoroll@skia.org Roll Dart SDK from 26d723eb89af to e47361c7fe9a (5 revisions) (flutter/flutter#188864) 2026-07-01 katelovett@google.com Update triage links for material_ui and cupertino_ui --> Design triage (flutter/flutter#188567) 2026-07-01 flar@google.com [Impeller] Share a single ContextGLES among all PlaygroundImplGLES (flutter/flutter#188080) 2026-07-01 homobonus-luca@hotmail.it Use null-aware elements in dev/devicelab/lib/integration_tests.dart (flutter/flutter#187852) 2026-07-01 engine-flutter-autoroll@skia.org Roll Packages from 274ed3e to e742106 (18 revisions) (flutter/flutter#188863) 2026-07-01 116356835+AbdeMohlbi@users.noreply.github.com Add android 17 to embedding (flutter/flutter#187965) 2026-07-01 47866232+chunhtai@users.noreply.github.com Adds semantics role check to isSemantics and matchesSemantics (flutter/flutter#188825) 2026-07-01 rmacnak@google.com Roll Dart SDK from e1bdb9ce3327 to 26d723eb89af (3 revisions) (flutter/flutter#188795) 2026-07-01 zhongliu88889@gmail.com [web] Apply autocapitalize to text editing elements (flutter/flutter#188351) 2026-07-01 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from RymJjIj7dd5vQ3Cnh... to I2h2eXk06RrA3pIG2... (flutter/flutter#188852) 2026-07-01 bdero@google.com [Impeller] Compute dispatch takes 3D workgroup counts and honors the shader local_size (flutter/flutter#188601) 2026-07-01 codedoctor@linwood.dev Improve stylus support on linux (flutter/flutter#186831) 2026-07-01 matt.boetger@gmail.com Resolve issue Catch am start failures with 'Error type' and prevent hang (flutter/flutter#187196) 2026-07-01 engine-flutter-autoroll@skia.org Roll Skia from ef178c9898af to bd4ae38ca3bb (3 revisions) (flutter/flutter#188834) 2026-07-01 engine-flutter-autoroll@skia.org Roll Skia from 3ac99be47229 to ef178c9898af (3 revisions) (flutter/flutter#188831) 2026-07-01 31859944+LongCatIsLooong@users.noreply.github.com Add a macosArm64Only feature flag (flutter/flutter#188598) 2026-07-01 engine-flutter-autoroll@skia.org Roll Skia from 15302f1625b2 to 3ac99be47229 (1 revision) (flutter/flutter#188819) 2026-07-01 bkonyi@google.com [flutter_tools] Track asset transformer dependencies for hot reload (Reland #187947) (flutter/flutter#188808) 2026-06-30 737941+loic-sharma@users.noreply.github.com Add TapRegion samples (flutter/flutter#188685) 2026-06-30 31859944+LongCatIsLooong@users.noreply.github.com Print a warning in `flutter doctor` when running on Intel Macs (flutter/flutter#188760) 2026-06-30 zhongliu88889@gmail.com [framework] Keep scrollable semantics role stable (flutter/flutter#187963) 2026-06-30 1961493+harryterkelsen@users.noreply.github.com feat(skills): Add shepherd-prs skill for managing approved external contributor PRs (flutter/flutter#188534) 2026-06-30 engine-flutter-autoroll@skia.org Roll Skia from 71947c4110b0 to 15302f1625b2 (17 revisions) (flutter/flutter#188815) 2026-06-30 bkonyi@google.com [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 stuartmorgan@google.com flutter/flutter@ca9f874...6995038 2026-07-03 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from sx_eN0J_f2BV6jqjW... to Jr08vyeibMSv3Oxst... (flutter/flutter#188946) 2026-07-03 engine-flutter-autoroll@skia.org Roll Packages from 420e135 to 2fbe873 (1 revision) (flutter/flutter#188945) 2026-07-03 engine-flutter-autoroll@skia.org Roll Dart SDK from 786212a2ce0d to 1f9a08ce0638 (4 revisions) (flutter/flutter#188944) 2026-07-03 engine-flutter-autoroll@skia.org Roll Skia from 5358ab75b840 to 919956953af6 (7 revisions) (flutter/flutter#188943) 2026-07-02 j.ihlas@betclicgroup.com [Windows] Keep regular windows in place when another is activated (flutter/flutter#188016) 2026-07-02 okorohelijah@google.com [flutter_tools] Provide guided message when iOS/macOS build fails due to low minimum version (flutter/flutter#188812) 2026-07-02 bdero@google.com [Flutter GPU] Load a ShaderLibrary from shader bundle bytes (flutter/flutter#188596) 2026-07-02 dacoharkes@google.com [tool] Enable record_use experiment by default on all channels (flutter/flutter#188887) 2026-07-02 engine-flutter-autoroll@skia.org Roll Packages from e742106 to 420e135 (13 revisions) (flutter/flutter#188916) 2026-07-02 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from I2h2eXk06RrA3pIG2... to sx_eN0J_f2BV6jqjW... (flutter/flutter#188915) 2026-07-02 256906086+mvincentong@users.noreply.github.com Clarify layout callback debug flag docs (flutter/flutter#186879) 2026-07-02 engine-flutter-autoroll@skia.org Roll Skia from 0c4faca350cc to 5358ab75b840 (2 revisions) (flutter/flutter#188899) 2026-07-02 engine-flutter-autoroll@skia.org Roll Dart SDK from e47361c7fe9a to 786212a2ce0d (2 revisions) (flutter/flutter#188898) 2026-07-01 engine-flutter-autoroll@skia.org Roll Skia from 0fc8ba72e802 to 0c4faca350cc (2 revisions) (flutter/flutter#188886) 2026-07-01 robert.ancell@canonical.com Hide draft PRs from the triage list (flutter/flutter#188885) 2026-07-01 47866232+chunhtai@users.noreply.github.com Stricten isSemantics and matchesSemantics children mismatch check (flutter/flutter#188827) 2026-07-01 jesswon@google.com [AGP 9] Support Enabling Built-in Kotlin (flutter/flutter#188543) 2026-07-01 engine-flutter-autoroll@skia.org Roll Skia from d19e557ac317 to 0fc8ba72e802 (4 revisions) (flutter/flutter#188879) 2026-07-01 bkonyi@google.com [ci] Increase test timeout for Mac_x64 build_tests shards (flutter/flutter#188804) 2026-07-01 engine-flutter-autoroll@skia.org Roll Skia from bd4ae38ca3bb to d19e557ac317 (1 revision) (flutter/flutter#188865) 2026-07-01 engine-flutter-autoroll@skia.org Roll Dart SDK from 26d723eb89af to e47361c7fe9a (5 revisions) (flutter/flutter#188864) 2026-07-01 katelovett@google.com Update triage links for material_ui and cupertino_ui --> Design triage (flutter/flutter#188567) 2026-07-01 flar@google.com [Impeller] Share a single ContextGLES among all PlaygroundImplGLES (flutter/flutter#188080) 2026-07-01 homobonus-luca@hotmail.it Use null-aware elements in dev/devicelab/lib/integration_tests.dart (flutter/flutter#187852) 2026-07-01 engine-flutter-autoroll@skia.org Roll Packages from 274ed3e to e742106 (18 revisions) (flutter/flutter#188863) 2026-07-01 116356835+AbdeMohlbi@users.noreply.github.com Add android 17 to embedding (flutter/flutter#187965) 2026-07-01 47866232+chunhtai@users.noreply.github.com Adds semantics role check to isSemantics and matchesSemantics (flutter/flutter#188825) 2026-07-01 rmacnak@google.com Roll Dart SDK from e1bdb9ce3327 to 26d723eb89af (3 revisions) (flutter/flutter#188795) 2026-07-01 zhongliu88889@gmail.com [web] Apply autocapitalize to text editing elements (flutter/flutter#188351) 2026-07-01 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from RymJjIj7dd5vQ3Cnh... to I2h2eXk06RrA3pIG2... (flutter/flutter#188852) 2026-07-01 bdero@google.com [Impeller] Compute dispatch takes 3D workgroup counts and honors the shader local_size (flutter/flutter#188601) 2026-07-01 codedoctor@linwood.dev Improve stylus support on linux (flutter/flutter#186831) 2026-07-01 matt.boetger@gmail.com Resolve issue Catch am start failures with 'Error type' and prevent hang (flutter/flutter#187196) 2026-07-01 engine-flutter-autoroll@skia.org Roll Skia from ef178c9898af to bd4ae38ca3bb (3 revisions) (flutter/flutter#188834) 2026-07-01 engine-flutter-autoroll@skia.org Roll Skia from 3ac99be47229 to ef178c9898af (3 revisions) (flutter/flutter#188831) 2026-07-01 31859944+LongCatIsLooong@users.noreply.github.com Add a macosArm64Only feature flag (flutter/flutter#188598) 2026-07-01 engine-flutter-autoroll@skia.org Roll Skia from 15302f1625b2 to 3ac99be47229 (1 revision) (flutter/flutter#188819) 2026-07-01 bkonyi@google.com [flutter_tools] Track asset transformer dependencies for hot reload (Reland #187947) (flutter/flutter#188808) 2026-06-30 737941+loic-sharma@users.noreply.github.com Add TapRegion samples (flutter/flutter#188685) 2026-06-30 31859944+LongCatIsLooong@users.noreply.github.com Print a warning in `flutter doctor` when running on Intel Macs (flutter/flutter#188760) 2026-06-30 zhongliu88889@gmail.com [framework] Keep scrollable semantics role stable (flutter/flutter#187963) 2026-06-30 1961493+harryterkelsen@users.noreply.github.com feat(skills): Add shepherd-prs skill for managing approved external contributor PRs (flutter/flutter#188534) 2026-06-30 engine-flutter-autoroll@skia.org Roll Skia from 71947c4110b0 to 15302f1625b2 (17 revisions) (flutter/flutter#188815) 2026-06-30 bkonyi@google.com [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
///).