add FragmentProgram.fromBytes to c++ and web engines. by timmaffett · Pull Request #175479 · flutter/flutter

timmaffett · 2025-09-17T06:53:17Z

Fixes #169442
Fixes #166944
Accomodates #167404 and some of #166944

This PR adds a FragmentProgram.fromBytes() method to allow loading of fragment shaders from outside the original asset bundle. This greatly extends the capabilities of the current restrictions of only allowing fragment shaders to be bundled in the build time asset bundle. (allowing many new capabilities, including items like the protection of shader source code via encryption, downloads of new shaders without reinstallation of program, runtime shader editing, ad infinitum). The referenced issues only partially touch upon why this feature is desperately needed.

This PR goes hand in hand with #175470 which allows versioning of impellerc outputs and protection for the engine from attempting to load incompatible versions of any of the impellerc output flat buffers. PR #175470 should be considered a required component of this PR to protect the engine from unexpected input.

I will incorporate unit tests for these capabilities which exercise the fromBytes() method - I wanted to initiate the conversation for now. I personally feel that the bang for the buck from this PR is extraordinary.

Other issues that this PR resolves or at least provides capabilities for that allow work-arounds:
resolves - #169442 - Allow for shader loading at runtime
partial - #171284 - Add support for loading assets with non-package paths in dart:ui APIs
(for the FragmentProgram.fromAsset() limitations .fromBytes() can be used to load your assets from
any AssetBundle)

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
I signed the [CLA].
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
[FORTHCOMING] I added new tests to check the change I am making, or this PR is [test-exempt].
I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
All existing and new tests are passing.

flutter-dashboard · 2025-09-17T06:53:20Z

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

gemini-code-assist

Code Review

This pull request introduces FragmentProgram.fromBytes, a valuable addition that allows loading fragment shaders from byte data, extending beyond the asset bundle. The implementation is consistent across the C++ and web engines, and the refactoring in the C++ code to share logic between fromAsset and fromBytes is a nice improvement. My main feedback is to add a warning to the documentation about the potential for crashes if an incompatible shader version is loaded, as you've noted that versioning is handled in a separate PR.

gemini-code-assist · 2025-09-17T06:55:13Z

+  /// Creates a fragment program from the provided byte data.
+  ///
+  /// The byte data must be produced as the output of the `impellerc`
+  /// compiler. The constructed object should then be reused via the
+  /// [fragmentShader] method to create [Shader] objects that can be used by
+  /// [Paint.shader].


Given that loading shaders from bytes can be risky without version checking (as you mentioned in the PR description regarding #175470), it would be beneficial to add a warning to the documentation for fromBytes. This will help prevent developers from accidentally crashing their applications by loading incompatible shader versions.

Here's a suggested addition to the doc comment:

/// Creates a fragment program from the provided byte data. /// /// The byte data must be produced as the output of the `impellerc` /// compiler. The constructed object should then be reused via the /// [fragmentShader] method to create [Shader] objects that can be used by /// [Paint.shader]. /// /// > [!WARNING] /// > The format of the byte data is tied to the version of the Flutter engine. /// > Attempting to load a shader compiled with a different version of `impellerc` /// > than the one used by the engine may result in a crash or unexpected behavior.

I stated that this PR goes hand in hand with #175470 and with such a requirement a doc comment would not be necessary because this PR should not be merged without #175470 being merged. I separated them as #175470 has other general advantages outside of protecting FragmentProgram (ie protecting ShaderLibrary, et al.)

harryterkelsen

LGTM for the web bits. Don't have much familiarity with the native engine but the change looks pretty straightforward and correct to me there, too.

mdebbar · 2026-01-07T16:33:10Z

Does this PR also fix #166944?

This still needs a review from the engine team. Maybe @gaaclarke or @flar?

timmaffett · 2026-01-07T18:08:28Z

@mdebbar Yes, it does address #166944.

The outstanding requirements were adding versioning to the FlatBuffer format for compiled shaders (implemented and merged in #175470) and the ability to load shaders from sources other than compile-time assets, which this PR provides.

gaaclarke · 2026-02-10T16:29:13Z

I'm still interested in this PR. We've had to tweak the shader format already a few times in the past week as we support better runtime reflection. I'd like to feel like we have a good stopping point before supporting this officially.

timmaffett · 2026-02-10T19:54:00Z

@gaaclarke It is exciting to hear that we may be getting closer! 🤞

gaaclarke · 2026-04-13T18:02:15Z

This is still on the backburner.

gaaclarke · 2026-06-01T18:31:32Z

Once we accept this we have a responsibility to maintain support for a file format. I'm still hesitant to finally accept that file format. I'm not sure what criteria I'd even need to comfortably make that designation. We'd probably need some documentation that at least explains the file format formally. I am interested in this however.

harryterkelsen · 2026-06-24T22:49:38Z

What do we want to do for this one? There are merge conflicts now

gaaclarke · 2026-06-24T23:07:58Z

I'm still of the opinion I stated a couple weeks ago here: #175479 (comment)

I wouldn't spend any time addressing conflicts until we give it the green light.

gaaclarke · 2026-06-29T22:03:09Z

Okay, I think we're ready to accept this feature now. We are doing it for flutter-gpu, so why not for flutter. The big use-case there was for creating a scene preview / editor for a Flutter project. The binary needed to be able to load shaders from outside its asset bundle in order to do that correctly.

In 2bd9ecb#diff-54b00397d8da03d8fc5e01b0c4f2c5e2bdb5a91ef78f1ebf75433e5bf8bf79dd we added c++ unit tests. I would like a dart integration test to make sure that we get clear messaging from the dart side about version incompatibilities.

gaaclarke

This PR needs integration tests, check out //engine/src/flutter/testing/dart/fragment_shader_test.dart for an example.

gaaclarke · 2026-06-29T22:11:26Z

-    return std::string("Asset '") + asset_name + std::string("' not found");
-  }
-
+std::string FragmentProgram::Init(const std::string& asset_name,


This should use absl::Status. I know we aren't using it elsewhere but we can make this do the right thing at least.

gaaclarke · 2026-06-29T22:12:39Z

+    const uint8_t* bytes = static_cast<const uint8_t*>(data);
+    NativeType result(bytes, bytes + length);


Do we want to copy the data or do we want to just retain a hold on the dart data?

gaaclarke · 2026-06-29T22:13:52Z

+  /// The byte data must be produced as the output of the `impellerc`
+  /// compiler. The constructed object should then be reused via the
+  /// [fragmentShader] method to create [Shader] objects that can be used by
+  /// [Paint.shader].


The ramifications to hot-reload need to be spelled out in the docstring.

gaaclarke · 2026-06-29T22:14:30Z

+    if (program != null) {
+      return Future<FragmentProgram>.value(program);
+    }
+    return Future<FragmentProgram>.microtask(() {


Comment on why this is a microtask.

add FragmentProgram.fromBytes to c++ and web engines

895ea78

github-actions Bot added engine flutter/engine related. See also e: labels. platform-web Web applications specifically labels Sep 17, 2025

gemini-code-assist Bot reviewed Sep 17, 2025

View reviewed changes

timmaffett added 2 commits September 17, 2025 06:44

dart format

d06a400

formatting

c372979

mdebbar requested a review from harryterkelsen September 17, 2025 18:21

gaaclarke changed the title ~~add FragmentProgram.fromBytes to c++ and web engines. Fixes #169442, Accomodates #167404 and some of #166944~~ add FragmentProgram.fromBytes to c++ and web engines. Oct 1, 2025

timmaffett mentioned this pull request Oct 6, 2025

Adds Impellerc flatbuffer format versioning. #175470

Merged

8 tasks

harryterkelsen approved these changes Nov 19, 2025

View reviewed changes

mdebbar added the triaged-web Triaged by Web platform team label Dec 3, 2025

walley892 mentioned this pull request Jan 26, 2026

Fixes getUniformX for Vulkan #181286

Merged

9 tasks

gaaclarke mentioned this pull request Feb 10, 2026

[Impeller] add run time shaders #168294

Closed

harryterkelsen self-requested a review June 24, 2026 22:49

gaaclarke mentioned this pull request Jun 26, 2026

[Flutter GPU] Load a ShaderLibrary from shader bundle bytes #188596

Merged

10 tasks

gaaclarke requested changes Jun 29, 2026

View reviewed changes

		const uint8_t* bytes = static_cast<const uint8_t*>(data);
		NativeType result(bytes, bytes + length);

Uh oh!

Conversation

timmaffett commented Sep 17, 2025 • edited by mdebbar Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

flutter-dashboard Bot commented Sep 17, 2025

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

timmaffett Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

harryterkelsen left a comment

Choose a reason for hiding this comment

Uh oh!

mdebbar commented Jan 7, 2026

Uh oh!

timmaffett commented Jan 7, 2026

Uh oh!

gaaclarke commented Feb 10, 2026

Uh oh!

timmaffett commented Feb 10, 2026

Uh oh!

gaaclarke commented Apr 13, 2026

Uh oh!

gaaclarke commented Jun 1, 2026

Uh oh!

harryterkelsen commented Jun 24, 2026

Uh oh!

gaaclarke commented Jun 24, 2026

Uh oh!

gaaclarke commented Jun 29, 2026

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

gaaclarke Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

gaaclarke Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

gaaclarke Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

gaaclarke Jun 29, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

timmaffett commented Sep 17, 2025 •

edited by mdebbar

Loading