Skip to content

Shares opengles golden context#187243

Merged
auto-submit[bot] merged 3 commits into
flutter:masterfrom
gaaclarke:share-opengles-golden-context
May 28, 2026
Merged

Shares opengles golden context#187243
auto-submit[bot] merged 3 commits into
flutter:masterfrom
gaaclarke:share-opengles-golden-context

Conversation

@gaaclarke

Copy link
Copy Markdown
Member

This results in a ~10x speedup of opengles golden tests.

issue: #187233

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.

@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label May 28, 2026
@github-actions github-actions Bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels May 28, 2026
@gaaclarke gaaclarke marked this pull request as ready for review May 28, 2026 17:21

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors OpenGL ES playground creation and context management in Impeller's golden tests. It introduces a shared OpenGL ES playground instance to be reused across tests, caches the GLES context in PlaygroundImplGLES instead of recreating it on every call to GetContext, and updates context creation logic for different backends. Feedback recommends adding null checks to prevent potential null pointer dereferences and crashes during test setup and cleanup when accessing the shared playground or its context.

Comment on lines +79 to +80
static fml::ScopedCleanupClosure context_cleanup(
[&] { (*opengl_playground)->GetContext()->Shutdown(); });

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If MakeOpenGLESPlayground() fails to initialize the playground or if the context creation fails, opengl_playground or its context can be nullptr. Dereferencing them without a null check during teardown will cause a segmentation fault. Adding a null check prevents potential crashes during test cleanup.

  static fml::ScopedCleanupClosure context_cleanup(
      [&] {
        const auto& playground = *opengl_playground;
        if (playground && playground->GetContext()) {
          playground->GetContext()->Shutdown();
        }
      });

Comment on lines +226 to +229
const std::unique_ptr<PlaygroundImpl>& playground =
GetSharedOpenGLESPlayground();
::glfwMakeContextCurrent(
reinterpret_cast<GLFWwindow*>(playground->GetWindowHandle()));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

If GetSharedOpenGLESPlayground() fails to initialize and returns nullptr, calling playground->GetWindowHandle() will result in a null pointer dereference and crash the test. Adding an explicit FML_CHECK ensures a clear error message and fail-fast behavior if initialization fails.

Suggested change
const std::unique_ptr<PlaygroundImpl>& playground =
GetSharedOpenGLESPlayground();
::glfwMakeContextCurrent(
reinterpret_cast<GLFWwindow*>(playground->GetWindowHandle()));
const std::unique_ptr<PlaygroundImpl>& playground =
GetSharedOpenGLESPlayground();
FML_CHECK(playground) << "Failed to get shared OpenGL ES playground.";
::glfwMakeContextCurrent(
reinterpret_cast<GLFWwindow*>(playground->GetWindowHandle()));

@gaaclarke gaaclarke requested a review from walley892 May 28, 2026 19:26

@walley892 walley892 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if CI is happy

@gaaclarke gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label May 28, 2026
@auto-submit auto-submit Bot added this pull request to the merge queue May 28, 2026
Merged via the queue into flutter:master with commit 91d6ef6 May 28, 2026
207 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 28, 2026
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request May 29, 2026
flutter/flutter@e70534d...b05a9d7

2026-05-29 engine-flutter-autoroll@skia.org Roll Skia from 47155534833e to d9d6b440c4e7 (1 revision) (flutter/flutter#187301)
2026-05-29 engine-flutter-autoroll@skia.org Roll Skia from f93ed13d77fb to 47155534833e (4 revisions) (flutter/flutter#187291)
2026-05-29 kevmoo@users.noreply.github.com [web_ui] Optimize skwasm text layout and path decoding to eliminate dynamic boxing churn under Wasm (flutter/flutter#186978)
2026-05-29 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from SBpmmPxqx3lAvGojE... to jMR_VXQi07kAk8vbR... (flutter/flutter#187279)
2026-05-29 burak.karahan@mail.ru Remove Material import from sliver tree rendering test (flutter/flutter#187000)
2026-05-29 bdero@google.com [Impeller] Remove the y_coord_scale Y-flip plumbing (flutter/flutter#187224)
2026-05-29 31859944+LongCatIsLooong@users.noreply.github.com Add/remove overlay child RenderObject from the tree in `attach`/`detach` (flutter/flutter#186564)
2026-05-29 engine-flutter-autoroll@skia.org Roll Skia from fcfe5975c945 to f93ed13d77fb (4 revisions) (flutter/flutter#187273)
2026-05-28 evanwall@buffalo.edu Handle complex RSE rendering in the uber SDF pipeline (flutter/flutter#186434)
2026-05-28 engine-flutter-autoroll@skia.org Roll Dart SDK from 082191101fcc to 683322426411 (2 revisions) (flutter/flutter#187270)
2026-05-28 mvincentong@gmail.com Clarify route transition animations (flutter/flutter#186552)
2026-05-28 116356835+AbdeMohlbi@users.noreply.github.com document that the default Key is null and explain proper usage in list diffing (flutter/flutter#185197)
2026-05-28 srawlins@google.com [flutter_tools] Use super parameters in missed spots (flutter/flutter#186197)
2026-05-28 mr_nadeem_iqbal@yahoo.com docs: Document MediaQueryData.alwaysUse24HourFormat on macOS, Windows, Linux, web (#160664) (flutter/flutter#186642)
2026-05-28 engine-flutter-autoroll@skia.org Roll Skia from 5493e4c144cd to fcfe5975c945 (3 revisions) (flutter/flutter#187256)
2026-05-28 30870216+gaaclarke@users.noreply.github.com Shares opengles golden context (flutter/flutter#187243)
2026-05-28 jason-simmons@users.noreply.github.com Update the Curl CIPD package in .ci.yaml to version 8.20.0 (flutter/flutter#187133)
2026-05-28 737941+loic-sharma@users.noreply.github.com Improve SizedBox's docs (flutter/flutter#187208)
2026-05-28 bdero@google.com [Impeller] Support instanced rendering across all backends (flutter/flutter#186653)
2026-05-28 43054281+camsim99@users.noreply.github.com [Android] Reset system UI visibility flags when setting edge-to-edge mode (flutter/flutter#187207)
2026-05-28 engine-flutter-autoroll@skia.org Roll Skia from a38708fb7926 to 5493e4c144cd (7 revisions) (flutter/flutter#187241)
2026-05-28 30870216+gaaclarke@users.noreply.github.com Turned on impeller by default on macos (flutter/flutter#186546)
2026-05-28 mvincentong@gmail.com Clarify lazy scroll extent docs (flutter/flutter#186864)
2026-05-28 mdebbar@google.com [web] Fix WebParagraph locales test (flutter/flutter#186813)
2026-05-28 engine-flutter-autoroll@skia.org Roll Packages from 4b424d7 to 10cbdc5 (3 revisions) (flutter/flutter#187238)
2026-05-28 engine-flutter-autoroll@skia.org Roll Dart SDK from f3db7b7d9801 to 082191101fcc (8 revisions) (flutter/flutter#187235)
2026-05-28 engine-flutter-autoroll@skia.org Roll Skia from 32acea791248 to a38708fb7926 (1 revision) (flutter/flutter#187221)
2026-05-28 bdero@google.com [Flutter GPU] Add r32Float and remove Apple-only XR pixel formats (flutter/flutter#187069)

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 boetger@google.com,stuartmorgan@google.com 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
creatorpiyush pushed a commit to creatorpiyush/packages that referenced this pull request Jun 10, 2026
…r#11803)

flutter/flutter@e70534d...b05a9d7

2026-05-29 engine-flutter-autoroll@skia.org Roll Skia from 47155534833e to d9d6b440c4e7 (1 revision) (flutter/flutter#187301)
2026-05-29 engine-flutter-autoroll@skia.org Roll Skia from f93ed13d77fb to 47155534833e (4 revisions) (flutter/flutter#187291)
2026-05-29 kevmoo@users.noreply.github.com [web_ui] Optimize skwasm text layout and path decoding to eliminate dynamic boxing churn under Wasm (flutter/flutter#186978)
2026-05-29 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from SBpmmPxqx3lAvGojE... to jMR_VXQi07kAk8vbR... (flutter/flutter#187279)
2026-05-29 burak.karahan@mail.ru Remove Material import from sliver tree rendering test (flutter/flutter#187000)
2026-05-29 bdero@google.com [Impeller] Remove the y_coord_scale Y-flip plumbing (flutter/flutter#187224)
2026-05-29 31859944+LongCatIsLooong@users.noreply.github.com Add/remove overlay child RenderObject from the tree in `attach`/`detach` (flutter/flutter#186564)
2026-05-29 engine-flutter-autoroll@skia.org Roll Skia from fcfe5975c945 to f93ed13d77fb (4 revisions) (flutter/flutter#187273)
2026-05-28 evanwall@buffalo.edu Handle complex RSE rendering in the uber SDF pipeline (flutter/flutter#186434)
2026-05-28 engine-flutter-autoroll@skia.org Roll Dart SDK from 082191101fcc to 683322426411 (2 revisions) (flutter/flutter#187270)
2026-05-28 mvincentong@gmail.com Clarify route transition animations (flutter/flutter#186552)
2026-05-28 116356835+AbdeMohlbi@users.noreply.github.com document that the default Key is null and explain proper usage in list diffing (flutter/flutter#185197)
2026-05-28 srawlins@google.com [flutter_tools] Use super parameters in missed spots (flutter/flutter#186197)
2026-05-28 mr_nadeem_iqbal@yahoo.com docs: Document MediaQueryData.alwaysUse24HourFormat on macOS, Windows, Linux, web (#160664) (flutter/flutter#186642)
2026-05-28 engine-flutter-autoroll@skia.org Roll Skia from 5493e4c144cd to fcfe5975c945 (3 revisions) (flutter/flutter#187256)
2026-05-28 30870216+gaaclarke@users.noreply.github.com Shares opengles golden context (flutter/flutter#187243)
2026-05-28 jason-simmons@users.noreply.github.com Update the Curl CIPD package in .ci.yaml to version 8.20.0 (flutter/flutter#187133)
2026-05-28 737941+loic-sharma@users.noreply.github.com Improve SizedBox's docs (flutter/flutter#187208)
2026-05-28 bdero@google.com [Impeller] Support instanced rendering across all backends (flutter/flutter#186653)
2026-05-28 43054281+camsim99@users.noreply.github.com [Android] Reset system UI visibility flags when setting edge-to-edge mode (flutter/flutter#187207)
2026-05-28 engine-flutter-autoroll@skia.org Roll Skia from a38708fb7926 to 5493e4c144cd (7 revisions) (flutter/flutter#187241)
2026-05-28 30870216+gaaclarke@users.noreply.github.com Turned on impeller by default on macos (flutter/flutter#186546)
2026-05-28 mvincentong@gmail.com Clarify lazy scroll extent docs (flutter/flutter#186864)
2026-05-28 mdebbar@google.com [web] Fix WebParagraph locales test (flutter/flutter#186813)
2026-05-28 engine-flutter-autoroll@skia.org Roll Packages from 4b424d7 to 10cbdc5 (3 revisions) (flutter/flutter#187238)
2026-05-28 engine-flutter-autoroll@skia.org Roll Dart SDK from f3db7b7d9801 to 082191101fcc (8 revisions) (flutter/flutter#187235)
2026-05-28 engine-flutter-autoroll@skia.org Roll Skia from 32acea791248 to a38708fb7926 (1 revision) (flutter/flutter#187221)
2026-05-28 bdero@google.com [Flutter GPU] Add r32Float and remove Apple-only XR pixel formats (flutter/flutter#187069)

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 boetger@google.com,stuartmorgan@google.com 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
via-guy pushed a commit to via-guy/flutter that referenced this pull request Jun 26, 2026
This results in a ~10x speedup of opengles golden tests.

issue: flutter#187233

## Pre-launch Checklist

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

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](https://developers.google.com/gemini-code-assist/docs/review-github-code).
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.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[AI contribution guidelines]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD e: impeller Impeller rendering backend issues and features requests engine flutter/engine related. See also e: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants