[impeller] Use the GLES3 shaders in the embedder if supported by planetmarshall · Pull Request #180072 · flutter/flutter

planetmarshall · 2025-12-18T16:20:56Z

Adds the GLES3 shaders to the embedder (used by Linux and custom embedders) if they are supported.

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 ///).
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-12-18T16:21:00Z

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 updates the embedder to use GLES3 shaders when supported by the driver. The logic for creating shader mappings has been refactored into a new helper function GetShaderMappings. My review identified a critical issue in this new function where the logic for selecting between GLES2 and GLES3 shaders is inverted, which would lead to loading the incorrect shader set. I've provided a code suggestion to correct this logic.

engine/src/flutter/shell/platform/embedder/embedder_surface_gl_impeller.cc

gaaclarke

What about test coverage for this?

planetmarshall · 2026-01-27T09:01:07Z

What about test coverage for this?

Sure, I'll take a look and see if I can add something.

…ted with the v3 shaders present

…3-shaders # Conflicts: # engine/src/flutter/impeller/playground/backend/gles/playground_impl_gles.cc

planetmarshall · 2026-02-08T11:18:20Z

Addressed an issue in the playground tests where the Runtime Stage and Backend are not 1:1 - When the backend is OpenGLES, the Runtime Stage can be either OpenGLES or OpenGLES3.
Added a note in the docs on using the OpenGLES software rasterizer locally to run tests. I've found this useful to reproduce test results more reliably.

planetmarshall · 2026-02-08T22:34:22Z

CI failing due to an error in ANGLE. Will ask on Discord.

[ RUN      ] Play/AiksTest.DrawAtlasNoColor/OpenGLES
../../../flutter/third_party/libcxx/src/new.cpp:63: assertion !std::__is_function_overridden(static_cast<void* (*)(std::size_t)>(&operator new)) failed: libc++ was configured with exceptions disabled and `operator new(size_t)` has been overridden, but `operator new(size_t, nothrow_t)` has not been overridden. This is problematic because `operator new(size_t, nothrow_t)` must call `operator new(size_t)`, which will terminate in case it fails to allocate, making it impossible for `operator new(size_t, nothrow_t)` to fulfill its contract (since it should return nullptr upon failure). Please make sure you override `operator new(size_t, nothrow_t)` as well.
[ERROR:flutter/fml/backtrace.cc(108)] Caught signal SIGABRT during program execution.
Frame 0: 0x19c6149e4 abort
Frame 1: 0x109c7f478 egl::AttributeMap::AttributeMap()
Frame 2: 0x109c5ce64 operator new()
Frame 3: 0x10a844e20 std::__fl::__allocate_unique_temporary_buffer[abi:nn210000]<>()
Frame 4: 0x10a844ba4 std::__fl::__stable_sort_impl[abi:nn210000]<>()
Frame 5: 0x10a83b948 std::__fl::stable_sort[abi:nn210000]<>()
Frame 6: 0x10a83ad08 sh::TIntermTraverser::updateTree()
Frame 7: 0x10a82f788 sh::SimplifyLoopConditions()
Frame 8: 0x10a713498 sh::TCompiler::checkAndSimplifyAST()
Frame 9: 0x10a711498 sh::TCompiler::compileTreeImpl()
Frame 10: 0x10a715298 sh::TCompiler::compile()
Frame 11: 0x10a7315a0 sh::Compile()
Frame 12: 0x10a123a04 rx::ShaderTranslateTask::translate()
Frame 13: 0x109f8e564 gl::(anonymous namespace)::CompileTask::compileImpl()
Frame 14: 0x109f8e3c4 gl::(anonymous namespace)::CompileTask::operator()()
Frame 15: 0x10a604374 angle::AsyncWorkerPool::threadLoop()

planetmarshall · 2026-02-08T23:46:15Z

ANGLE needs a patch for operator new - similar to the existing one for operator new[]. Have requested assistance on Discord as I am (currently) unable to get Gerrit credentials for the ANGLE SCM.

planetmarshall · 2026-02-10T13:02:32Z

Upstream PR added for ANGLE by @jason-simmons

https://flutter-review.googlesource.com/c/third_party/angle/+/75120

…3-shaders # Conflicts: # engine/src/flutter/impeller/fixtures/BUILD.gn

gaaclarke

Approach looks good to me. Testing looks good. I just have some stylistic feedback and one code quality request.

gaaclarke · 2026-02-27T22:41:03Z

engine/src/flutter/impeller/playground/backend/gles/playground_impl_gles.h

  std::shared_ptr<ReactorWorker> worker_;
  const bool use_angle_;
  void* angle_glesv2_;
+  mutable bool is_gles3_ = false;


Please remove this mutable ivar. You should be able to query the worker_ instead. Somewhere we can query this instead of caching it.

I've made it a property of the local worker class. I'm not sure that this is all that much better but I'm happy to take advice as it wasn't all that obvious how to do it.

My last review was in reference to this. Sorry, if it wasn't clear. Everything else looks good =)

engine/src/flutter/shell/platform/embedder/tests/embedder_test_surface_gl_impeller.cc

gaaclarke

I'm not a fan of this approach because that aspect doesn't really belong to the ReactorWorker, it reflects no meaningful state for its behavior.

I took a look and there are 2 clean ways to address this:

Stop caching the result to a bool and just call gl->GetDescription()->GetGlVersion().IsAtLeast(Version(3)) when you need it.
Set is_gles3_ in the PlaygroundImplGLES constructor.

Setting it in the constructor will probably mean a bit of refactoring that you would want to avoid. We'd probably have to make the constructor private and create a factory function that returns an absl::StatusOr<PlaygroundImplGLES> for the case the proc table fails to get created.

If you think this is a performance bottleneck that needs to be cached, just throw a TODO into the code and we can refactor it later. This is testing code so performance isn't super critical.

gaaclarke · 2026-03-09T16:28:57Z

engine/src/flutter/impeller/playground/backend/gles/playground_impl_gles.cc

    reactions_allowed_[std::this_thread::get_id()] = allowed;
  }

+  bool IsGLES3() { return is_gles3_; }


Suggested change

bool IsGLES3() { return is_gles3_; }

bool IsGLES3() const { return is_gles3_; }

planetmarshall · 2026-03-09T17:49:01Z

If you think this is a performance bottleneck that needs to be cached, just throw a TODO into the code and we can refactor it later. This is testing code so performance isn't super critical.

No it's not a performance concern. Just lack of familiarity with the code and as you say I don't want to do any heavyweight refactoring.

I've had another attempt :)

gaaclarke

Thanks, I know this may be slower but it's better to avoid surprising side effects for the sake of performance, especially in non-production code. We can make it faster in the future with some refactoring if we need to.

auto-submit · 2026-03-09T18:18:06Z

autosubmit label was removed for flutter/flutter/180072, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR.

auto-submit · 2026-03-09T18:18:07Z

autosubmit label was removed for flutter/flutter/180072, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

jason-simmons · 2026-03-09T20:51:31Z

engine/src/flutter/impeller/playground/backend/gles/playground_impl_gles.cc

 private:
  mutable RWMutex mutex_;
  std::map<std::thread::id, bool> reactions_allowed_ IPLR_GUARDED_BY(mutex_);
+  bool is_gles3_;


Remove this - it's no longer used after the latest changes

auto-submit · 2026-03-10T23:26:23Z

autosubmit label was removed for flutter/flutter/180072, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR.

…ed (flutter/flutter#180072)

flutter/flutter@195ae7b...3f400d7 2026-03-11 [email protected] Roll Clang to 80743bd43fd5b38fedc503308e7a652e23d3ec93 (flutter/flutter#182919) 2026-03-11 [email protected] Roll Fuchsia Linux SDK from 8C_qfgWgoNhkV0_Mn... to QD887D4OanteB7UKM... (flutter/flutter#183492) 2026-03-11 [email protected] Roll Dart SDK from fecf806be5d0 to 8531f7c2bdae (2 revisions) (flutter/flutter#183489) 2026-03-11 [email protected] [impeller] Use the GLES3 shaders in the embedder if supported (flutter/flutter#180072) 2026-03-11 [email protected] Roll Dart SDK from ae2be9700800 to fecf806be5d0 (1 revision) (flutter/flutter#183482) 2026-03-11 [email protected] Add GLES support for the same pixel formats for copying texture -> buffer as when copying buffer -> texture (flutter/flutter#183428) 2026-03-11 [email protected] Roll Skia from 257d04225d0c to 0cab3e4ee34b (8 revisions) (flutter/flutter#183476) 2026-03-10 [email protected] Fix GitHub workflows to use the `flutteractionsbot` mirror for PR branches. (flutter/flutter#183470) 2026-03-10 [email protected] [material/menu_anchor.dart] Ensure positioned menus always begin animating at the target position (flutter/flutter#182932) 2026-03-10 [email protected] Roll pub packages (flutter/flutter#183467) 2026-03-10 [email protected] Roll Dart SDK from ebef6c849489 to ae2be9700800 (4 revisions) (flutter/flutter#183460) 2026-03-10 [email protected] Roll Skia from 4b35832cc7ea to 257d04225d0c (5 revisions) (flutter/flutter#183457) 2026-03-10 [email protected] dev: Use a super-parameter in several missed cases (flutter/flutter#182251) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: 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

github-actions bot added the engine flutter/engine related. See also e: labels. label Dec 18, 2025

gemini-code-assist bot reviewed Dec 18, 2025

View reviewed changes

engine/src/flutter/shell/platform/embedder/embedder_surface_gl_impeller.cc Show resolved Hide resolved

use the GLES3 shaders in the embedder if supported

db49ef4

planetmarshall force-pushed the embedder-add-gles3-shaders branch from 753394a to db49ef4 Compare December 18, 2025 16:24

gaaclarke reviewed Jan 27, 2026

View reviewed changes

planetmarshall added 2 commits February 6, 2026 21:26

Add GLESv3 shaders to the impeller playground tests

075856c

Add unit tests to verify that the embedder surface for GLESv3 is crea…

0b1fea8

…ted with the v3 shaders present

github-actions bot added the e: impeller Impeller rendering backend issues and features requests label Feb 6, 2026

planetmarshall added 3 commits February 6, 2026 21:33

Merge remote-tracking branch 'upstream/master' into embedder-add-gles…

973910c

…3-shaders # Conflicts: # engine/src/flutter/impeller/playground/backend/gles/playground_impl_gles.cc

move helper into anonymous namespace

c4c55f4

use GLES3 shaders for the runtime shader stage where available

c99e87e

github-actions bot added team-engine Owned by Engine team d: docs/ flutter/flutter/docs, for contributors labels Feb 8, 2026

add implementation for impeller golden test playground

3cec016

planetmarshall force-pushed the embedder-add-gles3-shaders branch from f96a6a1 to 3cec016 Compare February 8, 2026 13:27

initialize member var in declaration

47c4fcf

planetmarshall added 2 commits February 27, 2026 14:49

Merge remote-tracking branch 'upstream/master' into embedder-add-gles…

f5b3edb

…3-shaders # Conflicts: # engine/src/flutter/impeller/fixtures/BUILD.gn

update runtime effect test after merge

f5ac60f

planetmarshall requested a review from gaaclarke February 27, 2026 22:27

gaaclarke requested changes Feb 27, 2026

View reviewed changes

planetmarshall added 3 commits March 9, 2026 14:41

make GLESv3 support a property of the playground worker

4001fa3

remove non-obvious type deductions and use positive assert

ced1899

arg comments

5f203a6

planetmarshall requested a review from gaaclarke March 9, 2026 16:48

gaaclarke requested changes Mar 9, 2026

View reviewed changes

cleanup gles3 determination in playground test

97adeab

gaaclarke requested changes Mar 9, 2026

View reviewed changes

gaaclarke approved these changes Mar 9, 2026

View reviewed changes

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 9, 2026

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 9, 2026

gaaclarke requested a review from jason-simmons March 9, 2026 20:31

jason-simmons approved these changes Mar 9, 2026

View reviewed changes

remove unused is_gles3_ member in PlaygroundImplGLES::ReactorWorker

de89466

jason-simmons added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 10, 2026

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 10, 2026

Merge branch 'master' into embedder-add-gles3-shaders

19d28c0

jason-simmons added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 11, 2026

auto-submit bot added this pull request to the merge queue Mar 11, 2026

Merged via the queue into flutter:master with commit 0b3147c Mar 11, 2026
185 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 11, 2026

engine-flutter-autoroll mentioned this pull request Mar 11, 2026

Roll Flutter from 195ae7b3a122 to 3f400d7c69e1 (13 revisions) flutter/packages#11228

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 11, 2026

0b3147cd3 [impeller] Use the GLES3 shaders in the embedder if support…

bf49a18

…ed (flutter/flutter#180072)

Piinks mentioned this pull request Mar 13, 2026

Flutter Notable Commits - 2026 #181433

Open

	bool IsGLES3() { return is_gles3_; }
	bool IsGLES3() const { return is_gles3_; }

Conversation

planetmarshall commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

flutter-dashboard bot commented Dec 18, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

planetmarshall commented Jan 27, 2026

Uh oh!

planetmarshall commented Feb 8, 2026

Uh oh!

planetmarshall commented Feb 8, 2026

Uh oh!

planetmarshall commented Feb 8, 2026

Uh oh!

planetmarshall commented Feb 10, 2026

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

gaaclarke Feb 27, 2026

Choose a reason for hiding this comment

Uh oh!

planetmarshall Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

gaaclarke Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gaaclarke left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gaaclarke Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

planetmarshall commented Mar 9, 2026

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Mar 9, 2026

Uh oh!

auto-submit bot commented Mar 9, 2026

Uh oh!

jason-simmons Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Mar 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

planetmarshall commented Dec 18, 2025 •

edited

Loading

gaaclarke left a comment •

edited

Loading