[Impeller] Do not delete the GL object in a HandleGLES if the handle has a cleanup callback by jason-simmons · Pull Request #183561 · flutter/flutter

jason-simmons · 2026-03-12T01:23:32Z

This allows embedders to control the lifetime of textures wrapped by HandleGLES. If the embedder calls RegisterCleanupCallback on the handle, then ReactorGLES will not delete the texture when the handle is collected.

Fixes #183058

gemini-code-assist

Code Review

This pull request modifies Impeller's OpenGL backend to prevent it from deleting GL objects for which the embedder manages the lifetime. This is achieved by checking for a cleanup callback on HandleGLES; if one is present, ReactorGLES will not delete the texture. The changes are logical and well-implemented, including updates to EmbedderExternalTextureGL to always register a callback (even a no-op) to signify embedder ownership, and improving error handling with ScopedCleanupClosure. The accompanying unit test updates correctly validate the new behavior. I have a couple of minor suggestions to improve code readability and maintainability.

gaaclarke

I looked through the tests and I don't see one that seems to assert the texture destruction callback is called in the normal case. Maybe I'm wrong, can you please validate that and possibly add a test for that.

Otherwise, code looks great.

gaaclarke · 2026-03-13T16:54:31Z

+  static GLuint gl_texture = 0;
  auto rendered_scene = context.GetNextSceneImage();
  context.GetRendererConfig().open_gl.gl_external_texture_frame_callback =
      [](void* user_data, int64_t texture_id, size_t width, size_t height,
         FlutterOpenGLTexture* texture) -> bool {


no action required: I think this would be more clear if we used c syntax for setting the callback instead because they are c function pointers. I had that comment the other day were I suggested capturing a local.

Assigning a no-capture lambda to a function pointer is a common idiom. It has the advantage that the lambda can access static variables declared within the enclosing function's scope.

gaaclarke · 2026-03-13T17:14:34Z

  image->SetCoordinateSystem(
      impeller::TextureCoordinateSystem::kUploadFromHost);

+  scoped_cleanup.Release();


Can we add a test that will fail if this isn't present? Looking over the tests, I'm not sure if that is possible. If someone accidentally removed this i guess it would probably be a warning but since it's far away from the comment that explains it, it would be nice to make sure it doesn't get goofed accidentally.

Added a test that checks invocation of the texture destruction callback.

The callback in the test will delete the GL texture. If the callback is called prematurely, then this will cause a mismatch against the expected image.

gaaclarke

thank you, sir, lgtm

…has a cleanup callback This allows embedders to control the lifetime of textures wrapped by HandleGLES. If the embedder calls RegisterCleanupCallback on the handle, then ReactorGLES will not delete the texture when the handle is collected. Fixes flutter#183058

…he handle has a cleanup callback (flutter/flutter#183561)

flutter/flutter@d117642...dd64978 2026-03-18 engine-flutter-autoroll@skia.org Roll Skia from 2fb5fa71eb12 to f0a13e5efbad (2 revisions) (flutter/flutter#183830) 2026-03-18 engine-flutter-autoroll@skia.org Roll Skia from ae3d36cb9e29 to 2fb5fa71eb12 (3 revisions) (flutter/flutter#183823) 2026-03-18 94012149+richardexfo@users.noreply.github.com Linux reuse sibling (flutter/flutter#183653) 2026-03-18 engine-flutter-autoroll@skia.org Roll Skia from 84a180a1fa80 to ae3d36cb9e29 (4 revisions) (flutter/flutter#183812) 2026-03-18 1961493+harryterkelsen@users.noreply.github.com fix(web): handle asynchronously disposed platform views (flutter/flutter#183666) 2026-03-17 87018443+mayanksharma9@users.noreply.github.com (Test cross-imports) Remove legacy Material import from sliver_constraints_test (flutter/flutter#183351) 2026-03-17 74057391+jhonathanqz@users.noreply.github.com Fix Android Studio pluginsPath when version is unknown (do not use 0.0) (flutter/flutter#182681) 2026-03-17 50643541+Mairramer@users.noreply.github.com Fixes animation glitch into bottom sheet (flutter/flutter#183303) 2026-03-17 ahmedsameha1@gmail.com Handle#6537 second grouped test (flutter/flutter#182529) 2026-03-17 ahmedsameha1@gmail.com Add a Clarification for the docs of suggestionsBuilder of SearchAnchor (flutter/flutter#183106) 2026-03-17 nate.w5687@gmail.com Remove obsolete null checks from style guide (flutter/flutter#181703) 2026-03-17 jason-simmons@users.noreply.github.com [Impeller] Do not delete the GL object in a HandleGLES if the handle has a cleanup callback (flutter/flutter#183561) 2026-03-17 jason-simmons@users.noreply.github.com Encode source file patches as UTF-8 in the code formatter script (flutter/flutter#183761) 2026-03-17 82673815+gktirkha@users.noreply.github.com Fix widget inspector control layout and add safe area regression test (flutter/flutter#180789) 2026-03-17 43054281+camsim99@users.noreply.github.com Reland "[Android] Add mechanism for setting Android engine flags via Android manifest (take 2)" (flutter/flutter#182522) 2026-03-17 1961493+harryterkelsen@users.noreply.github.com fix(web): fix crash in Skwasm when transferring non-transferable texture sources (flutter/flutter#183799) 2026-03-17 engine-flutter-autoroll@skia.org Roll Skia from dba893a44d7a to 84a180a1fa80 (7 revisions) (flutter/flutter#183803) 2026-03-17 brunocorona.alcantar@gmail.com Framework: Improve DropdownButton selectedItemBuilder assertion (flutter/flutter#183732) 2026-03-17 brunocorona.alcantar@gmail.com Add mainAxisAlignment to NavigationRail (flutter/flutter#183514) 2026-03-17 34871572+gmackall@users.noreply.github.com Update android triage process to not look at unassigned p1s every week (flutter/flutter#183805) 2026-03-17 30870216+gaaclarke@users.noreply.github.com Adds macos impeller new gallery transition perf test. (flutter/flutter#183802) 2026-03-17 1961493+harryterkelsen@users.noreply.github.com fix(web_ui): move prepareToDraw after raster to improve concurrency and stability (flutter/flutter#183791) 2026-03-17 rmacnak@google.com [build] Generate debug info for assembly. (flutter/flutter#183425) 2026-03-17 mdebbar@google.com [web] Fix occasional failure to find Chrome tab (flutter/flutter#183737) 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

…r#11281) flutter/flutter@d117642...dd64978 2026-03-18 engine-flutter-autoroll@skia.org Roll Skia from 2fb5fa71eb12 to f0a13e5efbad (2 revisions) (flutter/flutter#183830) 2026-03-18 engine-flutter-autoroll@skia.org Roll Skia from ae3d36cb9e29 to 2fb5fa71eb12 (3 revisions) (flutter/flutter#183823) 2026-03-18 94012149+richardexfo@users.noreply.github.com Linux reuse sibling (flutter/flutter#183653) 2026-03-18 engine-flutter-autoroll@skia.org Roll Skia from 84a180a1fa80 to ae3d36cb9e29 (4 revisions) (flutter/flutter#183812) 2026-03-18 1961493+harryterkelsen@users.noreply.github.com fix(web): handle asynchronously disposed platform views (flutter/flutter#183666) 2026-03-17 87018443+mayanksharma9@users.noreply.github.com (Test cross-imports) Remove legacy Material import from sliver_constraints_test (flutter/flutter#183351) 2026-03-17 74057391+jhonathanqz@users.noreply.github.com Fix Android Studio pluginsPath when version is unknown (do not use 0.0) (flutter/flutter#182681) 2026-03-17 50643541+Mairramer@users.noreply.github.com Fixes animation glitch into bottom sheet (flutter/flutter#183303) 2026-03-17 ahmedsameha1@gmail.com Handle#6537 second grouped test (flutter/flutter#182529) 2026-03-17 ahmedsameha1@gmail.com Add a Clarification for the docs of suggestionsBuilder of SearchAnchor (flutter/flutter#183106) 2026-03-17 nate.w5687@gmail.com Remove obsolete null checks from style guide (flutter/flutter#181703) 2026-03-17 jason-simmons@users.noreply.github.com [Impeller] Do not delete the GL object in a HandleGLES if the handle has a cleanup callback (flutter/flutter#183561) 2026-03-17 jason-simmons@users.noreply.github.com Encode source file patches as UTF-8 in the code formatter script (flutter/flutter#183761) 2026-03-17 82673815+gktirkha@users.noreply.github.com Fix widget inspector control layout and add safe area regression test (flutter/flutter#180789) 2026-03-17 43054281+camsim99@users.noreply.github.com Reland "[Android] Add mechanism for setting Android engine flags via Android manifest (take 2)" (flutter/flutter#182522) 2026-03-17 1961493+harryterkelsen@users.noreply.github.com fix(web): fix crash in Skwasm when transferring non-transferable texture sources (flutter/flutter#183799) 2026-03-17 engine-flutter-autoroll@skia.org Roll Skia from dba893a44d7a to 84a180a1fa80 (7 revisions) (flutter/flutter#183803) 2026-03-17 brunocorona.alcantar@gmail.com Framework: Improve DropdownButton selectedItemBuilder assertion (flutter/flutter#183732) 2026-03-17 brunocorona.alcantar@gmail.com Add mainAxisAlignment to NavigationRail (flutter/flutter#183514) 2026-03-17 34871572+gmackall@users.noreply.github.com Update android triage process to not look at unassigned p1s every week (flutter/flutter#183805) 2026-03-17 30870216+gaaclarke@users.noreply.github.com Adds macos impeller new gallery transition perf test. (flutter/flutter#183802) 2026-03-17 1961493+harryterkelsen@users.noreply.github.com fix(web_ui): move prepareToDraw after raster to improve concurrency and stability (flutter/flutter#183791) 2026-03-17 rmacnak@google.com [build] Generate debug info for assembly. (flutter/flutter#183425) 2026-03-17 mdebbar@google.com [web] Fix occasional failure to find Chrome tab (flutter/flutter#183737) 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

…has a cleanup callback (flutter#183561) This allows embedders to control the lifetime of textures wrapped by HandleGLES. If the embedder calls RegisterCleanupCallback on the handle, then ReactorGLES will not delete the texture when the handle is collected. Fixes flutter#183058 --------- Co-authored-by: Xiaowei Guan <xiaowei.guan@samsung.com>

github-actions Bot added engine flutter/engine related. See also e: labels. platform-linux Building on or for Linux specifically a: desktop Running on desktop e: impeller Impeller rendering backend issues and features requests team-linux Owned by the Linux platform team labels Mar 12, 2026

jason-simmons mentioned this pull request Mar 12, 2026

Fix black screen issue when rendering pixel buffer on Linux #183065

Closed

9 tasks

github-actions Bot removed platform-linux Building on or for Linux specifically a: desktop Running on desktop team-linux Owned by the Linux platform team labels Mar 13, 2026

jason-simmons marked this pull request as ready for review March 13, 2026 03:14

jason-simmons requested a review from gaaclarke March 13, 2026 03:14

jason-simmons added the CICD Run CI/CD label Mar 13, 2026

gemini-code-assist Bot reviewed Mar 13, 2026

View reviewed changes

Comment thread engine/src/flutter/shell/platform/embedder/embedder_external_texture_gl.cc Outdated

Comment thread engine/src/flutter/shell/platform/embedder/tests/embedder_gl_unittests.cc Outdated

gaaclarke reviewed Mar 13, 2026

View reviewed changes

gaaclarke approved these changes Mar 13, 2026

View reviewed changes

jason-simmons and others added 5 commits March 17, 2026 08:13

test

163e695

always set a cleanup callback for Impeller in EmbedderExternalTextureGL

238e37b

feedback

1d04957

test of the destruction callback

d45b366

jason-simmons force-pushed the bug_183065_cleanup_callback branch from 6e0d8d6 to d45b366 Compare March 17, 2026 15:13

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

jason-simmons added CICD Run CI/CD autosubmit Merge PR when tree becomes green via auto submit App labels Mar 17, 2026

auto-submit Bot added this pull request to the merge queue Mar 17, 2026

Merged via the queue into flutter:master with commit 221f7eb Mar 18, 2026
190 checks passed

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

xiaowei-guan mentioned this pull request Mar 18, 2026

RTCVideoView runs normally on macOS, but it reports errors when running on Windows, seemingly related to impeller flutter-webrtc/flutter-webrtc#1870

Open

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

Roll Flutter from d117642c18e0 to dd64978dfae1 (24 revisions) flutter/packages#11281

Merged

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

221f7eb26 [Impeller] Do not delete the GL object in a HandleGLES if t…

f2b9f2c

…he handle has a cleanup callback (flutter/flutter#183561)

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

221f7eb26 [Impeller] Do not delete the GL object in a HandleGLES if t…

6dcb67f

…he handle has a cleanup callback (flutter/flutter#183561)

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

Roll Flutter from d117642c18e0 to e6ed6e9e1678 (25 revisions) flutter/packages#11282

Closed

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

221f7eb26 [Impeller] Do not delete the GL object in a HandleGLES if t…

39c2cad

…he handle has a cleanup callback (flutter/flutter#183561)

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

Roll Flutter from d117642c18e0 to 7189eb4d128c (26 revisions) flutter/packages#11284

Closed

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

221f7eb26 [Impeller] Do not delete the GL object in a HandleGLES if t…

cb1392a

…he handle has a cleanup callback (flutter/flutter#183561)

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

Roll Flutter from d117642c18e0 to 6df82a9ec022 (27 revisions) flutter/packages#11286

Closed

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

221f7eb26 [Impeller] Do not delete the GL object in a HandleGLES if t…

31c07f9

…he handle has a cleanup callback (flutter/flutter#183561)

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

Roll Flutter from d117642c18e0 to 73d6bccaf322 (30 revisions) flutter/packages#11288

Closed

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

221f7eb26 [Impeller] Do not delete the GL object in a HandleGLES if t…

b0b2018

…he handle has a cleanup callback (flutter/flutter#183561)

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

Roll Flutter from d117642c18e0 to c0aee3b4ca7d (31 revisions) flutter/packages#11289

Closed

xiaowei-guan mentioned this pull request May 26, 2026

Migrate to Flutter 3.44 flutter-tizen/flutter-tizen#771

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Impeller] Do not delete the GL object in a HandleGLES if the handle has a cleanup callback#183561

[Impeller] Do not delete the GL object in a HandleGLES if the handle has a cleanup callback#183561
auto-submit[bot] merged 5 commits into
flutter:masterfrom
jason-simmons:bug_183065_cleanup_callback

jason-simmons commented Mar 12, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

gaaclarke left a comment

Uh oh!

gaaclarke Mar 13, 2026

Uh oh!

jason-simmons Mar 13, 2026

Uh oh!

gaaclarke Mar 13, 2026

Uh oh!

jason-simmons Mar 13, 2026

Uh oh!

gaaclarke left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

jason-simmons commented Mar 12, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

gaaclarke Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

jason-simmons Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

gaaclarke Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

jason-simmons Mar 13, 2026

Choose a reason for hiding this comment

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants