[Impeller] Share a single ContextGLES among all PlaygroundImplGLES by flar · Pull Request #188080 · flutter/flutter

flar · 2026-06-16T20:52:10Z

Currently running all of the impeller unit tests can take 25 minutes most all of which is caused by recreating an OpenGL context. While #187690 attempts to share that context among all playground implementations to amortize that cost, it is running into issues with interactions between the reused context and the ImGui code. In order to further focus the sharing, this PR attempts to share just the ContextGLES and the ReactorWorker between instantiations of the PlaygroundImplGES.

Fixes #188649

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on [Discord].

If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

…ache content_context_

b-luk · 2026-06-17T19:58:55Z

I looked at this and investigated along with Gemini to try to get it working.

I believe there were three things that were causing issues:

To share GLES context across windows, we need to pass a shared window to the share parameter of glfwCreateWindow. To fix this, I added a shared window to ShareableContext, which is used when glfwCreateWindow is called to create a window when instantiating PlaygroundImplGLES.
Some state in GPUTracerGLES becomes invalid across windows. To fix this, I exposed a method to reset its state and call this after creating the window for PlaygroundImplGLES.
Sharing the GLES context across tests caused a crash related to unexecuted commands in command buffers leaking across tests and getting flushed during teardown after the context is already destroyed. To fix this, I cache ContentContext in EntityPlayground to keep it alive until teardown.

gemini-code-assist

Code Review

This pull request implements OpenGL ES context sharing across Impeller playground tests by caching a shared context statically. The review feedback identifies several critical issues: C++17 compilation failures due to using std::make_shared on an aggregate struct, potential crashes from incorrect member destruction order, rendering issues from sharing a single ContextGLES instance across contexts that do not share VAOs or FBOs, and static teardown crashes if the shared context is destroyed after GLFW is terminated.

gemini-code-assist · 2026-06-17T22:23:41Z

+  if (shared_context->get() == nullptr) {
+    *shared_context = MakeShareableContext(switches_);
+    if (shared_context->get() == nullptr) {
+      FML_LOG(ERROR) << "Could not create GLES context.";
+      return;
+    }
+  }


Using !*shared_context is more idiomatic and readable than shared_context->get() == nullptr when working with pointers to smart pointers.

Additionally, sharing a single ContextGLES C++ instance across multiple OpenGL contexts (even within a share group) introduces a significant correctness risk. In OpenGL, Vertex Array Objects (VAOs) and Framebuffer Objects (FBOs) are not shared between contexts in a share group. If ContextGLES or its pipelines cache any VAOs or FBOs, those cached handles will be invalid in subsequent tests (which run on a new window's context), leading to undefined behavior or rendering artifacts. Consider sharing the entire context (including the window) or ensuring that no non-shareable resources are cached across test boundaries.

if (!*shared_context) { *shared_context = MakeShareableContext(switches_); if (!*shared_context) { FML_LOG(ERROR) << "Could not create GLES context."; return; } }

Changed shared_context to be a ref rather than a pointer. Changed the ->get() == nullptr checks to just use !.

For the concern about "if ContextGLES or its pipelines cache any VAOs or FBOs", gemini is telling me that ContextGLES does not cache FBOs or VAOs, so this concern is not applicable. I don't know GLES well enough to know if this is true, but if this is an issue I expect that it would be apparent in our tests.

gemini-code-assist · 2026-06-17T22:23:41Z

+      static std::shared_ptr<PlaygroundImplGLES::ShareableContext>
+          shared_context;


Using a static shared context means ShareableContext (and its UniqueHandle window) will be destroyed during static teardown after main exits. If GLFW is terminated (e.g., via glfwTerminate()) before static destructors run, the destructor of UniqueHandle will call glfwDestroyWindow on a terminated GLFW instance, leading to a crash or undefined behavior. Consider managing the lifetime of the shared context within a global test environment or explicitly resetting it before GLFW termination.

Done. Changed this to manage the lifetime of the shared context within a global test environment

…ers by flushing in EntityPlayground::TearDown()

… catch them.

Canvas lazily encodes its commands in its destructor, so it needs to be destroyed before we flush the command buffers. Otherwise the buffer retains some commands if the canvas is destroyed after flushing.

flar · 2026-06-18T18:26:56Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces GLES context sharing across playground tests in Impeller by implementing a ShareableContext structure and managing its lifecycle within a global test environment. It also adds a Reset method to GPUTracerGLES to clear query tracking when reusing contexts, and introduces manual command buffer flushing in several tests and teardown paths. Feedback suggests refactoring the manual flushing and canvas resets into the base class PlaygroundTest::TearDown() to reduce boilerplate, and verifying compatibility of PlaygroundSwitches before reusing a cached ShareableContext to prevent silent configuration mismatches.

gemini-code-assist · 2026-06-18T18:29:31Z

+void EntityPlayground::TearDown() {
+  if (content_context_) {
+    (void)content_context_->GetContext()->FlushCommandBuffers();
+    content_context_.reset();
+  }
+  PlaygroundTest::TearDown();
+}


Instead of manually flushing command buffers in EntityPlayground::TearDown() and adding boilerplate canvas.reset() and FlushCommandBuffers() calls to individual tests (e.g., in canvas_unittests.cc and renderer_unittests.cc), we should centrally flush the command buffers in the base class PlaygroundTest::TearDown().\n\nSince PlaygroundTest is the base class for all playground tests, flushing there ensures that all pending GPU commands are automatically flushed at the end of every test (after local variables like canvas go out of scope and are destroyed). This prevents resource leaks and cross-test pollution across all tests when sharing the GLES context, without requiring manual test-by-test updates.

This does not work for cases like the calls in canvas_unittests.cc. The ContentContext object owns the host buffers, so FlushCommandBuffers() must be called before ContentContext goes out of scope at the end of the test method. If the flush happens in a TearDown() method, it will be called after the host buffers no longer exist.

We can add ownership to the ContentContext in the PlaygroundTest then to make sure they are alive at PlaygroundTest::TearDown().

Having to manually remember to add these seems like a footgun.

Ah, now I see what you are talking about below. Let me investigate that so that the tests don't have to know about this.

…ructor

flar

A minor nit, doesn't fix anything.

…ate comment

…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) ...

[Impeller] Share a single ContextGLES among all PlaygroundImplGLES

7ef2da2

flutter-dashboard Bot added the CICD Run CI/CD label Jun 16, 2026

github-actions Bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Jun 16, 2026

Fixes - Add shared window to ShareableContext, reset GPUTracerGLES, c…

de1025f

…ache content_context_

github-actions Bot removed the CICD Run CI/CD label Jun 17, 2026

cleanup on test exit

fb3f32a

flutter-dashboard Bot added the CICD Run CI/CD label Jun 17, 2026

flar marked this pull request as ready for review June 17, 2026 22:21

gemini-code-assist Bot reviewed Jun 17, 2026

View reviewed changes

Address comments. Also fix some crashes due to unflushed command buff…

160aa48

…ers by flushing in EntityPlayground::TearDown()

github-actions Bot removed the CICD Run CI/CD label Jun 17, 2026

b-luk added the CICD Run CI/CD label Jun 17, 2026

Add more flush commands. These don't run on mac so I didn't initially…

cb65446

… catch them.

github-actions Bot removed the CICD Run CI/CD label Jun 18, 2026

b-luk added the CICD Run CI/CD label Jun 18, 2026

Destroy canvas before flushing command buffers.

0382169

Canvas lazily encodes its commands in its destructor, so it needs to be destroyed before we flush the command buffers. Otherwise the buffer retains some commands if the canvas is destroyed after flushing.

github-actions Bot removed the CICD Run CI/CD label Jun 18, 2026

b-luk added the CICD Run CI/CD label Jun 18, 2026

gemini-code-assist Bot reviewed Jun 18, 2026

View reviewed changes

Check compatibility of the shared context in PlaygroundImplGLES const…

e4e769e

…ructor

github-actions Bot removed the CICD Run CI/CD label Jun 18, 2026

b-luk added the CICD Run CI/CD label Jun 18, 2026

flar requested review from gaaclarke, jason-simmons and walley892 June 18, 2026 21:45

flar commented Jun 18, 2026

View reviewed changes

Comment thread engine/src/flutter/impeller/playground/backend/gles/playground_impl_gles.cc Outdated

Don't check enable_wide_gamut for the switch-compatibility check, upd…

935a05d

…ate comment

This was referenced Jul 1, 2026

Roll Flutter from 0c80830e465b to 00085810489f (143 revisions) flutter/packages#12090

Closed

Manual roll Flutter from 0c80830e465b to 8e3ac6c2122e (146 revisions) flutter/packages#12091

Closed

This was referenced Jul 1, 2026

[Impeller] Share OpenGL context between playground unittests #187690

Closed

[Impeller] Share a pipeline library among GLES contexts #188039

Closed

		static std::shared_ptr<PlaygroundImplGLES::ShareableContext>
		shared_context;

Uh oh!

Conversation

flar commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

b-luk commented Jun 17, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

gemini-code-assist Bot Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

b-luk Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

b-luk Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

flar commented Jun 18, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

b-luk Jun 18, 2026 • 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 Jun 26, 2026

Choose a reason for hiding this comment

Uh oh!

flar Jun 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

flar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

flar commented Jun 16, 2026 •

edited

Loading

b-luk Jun 18, 2026 •

edited

Loading