fix(impeller): fix params to glDiscardFrameBufferEXT #175589

planetmarshall · 2025-09-18T12:55:01Z

Fix parameters to glDiscardFramebufferEXT when an application-defined framebuffer is bound. The is_default_fbo parameter is set to true if color_gles.IsWrapped() is true, however according to the GLES spec, a non-default framebuffer is bound if glBindFramebuffer(GL_FRAMEBUFFER, framebuffer) is called with framebuffer != 0:

The namespace for framebuffer objects is the unsigned integers, with zero re-
served by OpenGL ES to refer to the default framebuffer. A framebuffer object
is created by binding an unused name to the target FRAMEBUFFER.

To reproduce this issue, run a flutter application with --enable-impeller on a platform where glDiscardFramebufferEXT is available. The following error is reported:

[FATAL:flutter/impeller/renderer/backend/gles/proc_table_gles.h(43)] Fatal GL Error GL_INVALID_ENUM(1280) encountered on call to glDiscardFramebufferEXT

List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.

closes #175588

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

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.

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

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 · 2025-09-18T12:55:05Z

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 aims to fix incorrect parameters passed to glDiscardFramebufferEXT for application-defined framebuffers. However, the current implementation introduces a new bug for offscreen framebuffers by inverting the logic for selecting discard attachments. Additionally, while it fixes the issue for application-defined onscreen framebuffers, it breaks functionality for the default onscreen framebuffer. I've added a critical comment explaining the issues.

engine/src/flutter/impeller/renderer/backend/gles/render_pass_gles.cc

chinmaygarde

Other than the nit about changing the variable name, lgtm.

chinmaygarde · 2025-09-29T21:15:04Z

engine/src/flutter/impeller/renderer/backend/gles/render_pass_gles.cc

    // TODO(130048): discarding stencil or depth on the default fbo causes Angle
    // to discard the entire render target. Until we know the reason, default to
    // storing.
    bool angle_safe = gl.GetCapabilities()->IsANGLE() ? !is_default_fbo : true;


is_default_fbo is just poorly named. Instead, I suggest renaming it to is_wrapped_fbo or something similar.

This rest of the change is accurate. Whether the FBO is wrapped of not, the type of enum to used to discard will depend on whether it is the default framebuffer object (and not if its wrapped).

auto-submit · 2025-10-01T15:53:46Z

autosubmit label was removed for flutter/flutter/175589, 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.

planetmarshall · 2025-10-07T14:02:45Z

Added unit test and applied suggestions from @chinmaygarde

auto-submit · 2025-10-13T19:02:28Z

autosubmit label was removed for flutter/flutter/175589, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

flutter-dashboard · 2025-11-02T19:22:49Z

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

…er/flutter#175589)

Roll Flutter from 027f2e410241 to e5d5c01850f2 (73 revisions) flutter/flutter@027f2e4...e5d5c01 2025-11-04 [email protected] [web] Upgrade Chrome to 141 (for engine tests) (flutter/flutter#177743) 2025-11-04 [email protected] Replace deprecated withOpacity in `chip_animation_style.0.dart‎` example (flutter/flutter#177834) 2025-11-04 [email protected] Roll Skia from 03612114a36d to c89b6118266b (1 revision) (flutter/flutter#177991) 2025-11-04 [email protected] [web] Delete unused futurize util (flutter/flutter#177861) 2025-11-04 [email protected] Roll Skia from 1a179581af75 to 03612114a36d (1 revision) (flutter/flutter#177984) 2025-11-04 [email protected] Roll Skia from 150f844e83fb to 1a179581af75 (3 revisions) (flutter/flutter#177979) 2025-11-04 [email protected] Roll Dart SDK from 6f1bd8d05b1d to 156bf665dba4 (2 revisions) (flutter/flutter#177976) 2025-11-04 [email protected] Roll Skia from 3990fc40acdf to 150f844e83fb (1 revision) (flutter/flutter#177964) 2025-11-04 [email protected] Roll Skia from 85adfd3b8e75 to 3990fc40acdf (1 revision) (flutter/flutter#177962) 2025-11-04 [email protected] Fix `isHeif` crash (flutter/flutter#177944) 2025-11-04 [email protected] Roll Dart SDK from ac065fcd782e to 6f1bd8d05b1d (1 revision) (flutter/flutter#177955) 2025-11-04 [email protected] Roll Skia from 001516d132f0 to 85adfd3b8e75 (4 revisions) (flutter/flutter#177958) 2025-11-04 [email protected] [ Widget Preview ] Add analytic event that's reported when the previewer is opened (flutter/flutter#177949) 2025-11-04 [email protected] Marks Linux_pixel_7pro dynamic_path_stroke_tessellation_perf__timeline_summary to be unflaky (flutter/flutter#171144) 2025-11-03 [email protected] Roll Skia from da9b8fb01101 to 001516d132f0 (6 revisions) (flutter/flutter#177951) 2025-11-03 [email protected] Add FAQ entry about our attempts to remove Skia image codecs. (flutter/flutter#177947) 2025-11-03 [email protected] Add blockAccessibilityFocus flag (flutter/flutter#175551) 2025-11-03 [email protected] Make sure that a SnackBarAction doesn't crash in 0x0 environment (flutter/flutter#177899) 2025-11-03 [email protected] Make sure that a SelectionArea doesn't crash in 0x0 environment (flutter/flutter#177876) 2025-11-03 [email protected] Speculative fix for android_verified_input_test (flutter/flutter#177937) 2025-11-03 [email protected] [skia] Explicitly disable XPS backend (flutter/flutter#177050) 2025-11-03 [email protected] Roll Dart SDK from 2d3aa1d5cb9f to ac065fcd782e (2 revisions) (flutter/flutter#177927) 2025-11-03 [email protected] Improve Impeller's docs in the top-level docs folder (flutter/flutter#177848) 2025-11-03 [email protected] Make sure that GridTile doesn't crash in 0x0 environment (flutter/flutter#175321) 2025-11-03 [email protected] Make sure that a Slider doesn't crash in 0x0 environment (flutter/flutter#177882) 2025-11-03 [email protected] fix(impeller): fix params to glDiscardFrameBufferEXT (flutter/flutter#175589) 2025-11-03 [email protected] Roll Skia from 56a10bf56ee0 to da9b8fb01101 (5 revisions) (flutter/flutter#177928) 2025-11-03 [email protected] wires up set application locale to web engine (flutter/flutter#177284) 2025-11-03 [email protected] [Android 16] Update `linux_android_emu_unstable` to use avd rev 7 (flutter/flutter#177854) 2025-11-03 [email protected] Make sure that a CheckedPopupMenuItem doesn't crash in 0x0 environment (flutter/flutter#177545) 2025-11-03 [email protected] Roll Fuchsia Linux SDK from Ynp3qiXocK8WLTjlb... to vxK5obzfr1X9P2kSh... (flutter/flutter#177917) 2025-11-03 [email protected] Update pubspec.yaml.tmpl build version tracker on line 5 (flutter/flutter#173600) 2025-11-03 [email protected] [Impeller] Fix the source rectangle used when rendering an AtlasContents with the strict mode for nine-patch images (flutter/flutter#177860) 2025-11-03 [email protected] docs: Default Splash / Loading screen for web app in index.html (flutter/flutter#177084) 2025-11-03 [email protected] Roll Skia from 25e1e4e96980 to 56a10bf56ee0 (6 revisions) (flutter/flutter#177910) 2025-11-03 [email protected] Make sure that a PopupMenuItem doesn't crash in 0x0 environment (flutter/flutter#177533) 2025-11-03 [email protected] Make sure that a RefreshProgressIndicator doesn't crash in 0x0 enviro… (flutter/flutter#177556) 2025-11-03 [email protected] Make sure that a PopupMenuButton doesn't crash in 0x0 environment (flutter/flutter#177493) 2025-11-03 [email protected] Make sure that a CircularProgressIndicator doesn't crash in 0x0 envir… (flutter/flutter#177555) 2025-11-02 [email protected] Make sure that a Radio doesn't crash in 0x0 environment (flutter/flutter#177580) 2025-11-02 [email protected] Make sure that a RadioListTile doesn't crash in 0x0 environment (flutter/flutter#177576) 2025-11-02 [email protected] Make sure that an ExpandIcon doesn't crash in 0x0 envrionment (flutter/flutter#175042) 2025-11-02 [email protected] Make sure that a RangeSlider doesn't crash in 0x0 environment (flutter/flutter#177642) 2025-11-02 [email protected] Make sure that a GridTileBar doesn't crash in 0x0 environment (flutter/flutter#177546) 2025-11-02 [email protected] Make sure that a LinearProgressIndicator doesn't crash in 0x0 environ… (flutter/flutter#177553) 2025-11-02 [email protected] Make sure that a PopupMenuDivider doesn't crash in 0x0 environment (flutter/flutter#177445) ...

Fix parameters to [glDiscardFramebufferEXT](https://registry.khronos.org/OpenGL/extensions/EXT/EXT_discard_framebuffer.txt) when an application-defined framebuffer is bound. The `is_default_fbo` parameter is set to `true` if `color_gles.IsWrapped()` is true, however according to the GLES spec, a non-default framebuffer is bound if `glBindFramebuffer(GL_FRAMEBUFFER, framebuffer)` is called with `framebuffer != 0`: > _The namespace for framebuffer objects is the unsigned integers, with zero re- served by OpenGL ES to refer to the default framebuffer. A framebuffer object is created by binding an unused name to the target FRAMEBUFFER._ To reproduce this issue, run a flutter application with `--enable-impeller` on a platform where `glDiscardFramebufferEXT` is available. The following error is reported: ``` [FATAL:flutter/impeller/renderer/backend/gles/proc_table_gles.h(43)] Fatal GL Error GL_INVALID_ENUM(1280) encountered on call to glDiscardFramebufferEXT ``` *List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.* closes flutter#175588 *If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].* ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [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]. **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.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [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

github-actions bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Sep 18, 2025

gemini-code-assist bot reviewed Sep 18, 2025

View reviewed changes

engine/src/flutter/impeller/renderer/backend/gles/render_pass_gles.cc Outdated Show resolved Hide resolved

chinmaygarde approved these changes Sep 29, 2025

View reviewed changes

chinmaygarde force-pushed the fix-impeller-discard-framebufer branch from 7f58ed2 to 457dd22 Compare September 29, 2025 22:40

jtmcdole added customer: blue autosubmit Merge PR when tree becomes green via auto submit App labels Oct 1, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 1, 2025

jason-simmons approved these changes Oct 6, 2025

View reviewed changes

planetmarshall force-pushed the fix-impeller-discard-framebufer branch 2 times, most recently from 8146189 to b3dd6cb Compare October 7, 2025 14:01

chinmaygarde approved these changes Oct 13, 2025

View reviewed changes

chinmaygarde added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 13, 2025

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 13, 2025

jason-simmons force-pushed the fix-impeller-discard-framebufer branch from c6dfb14 to 843148e Compare October 13, 2025 19:20

planetmarshall added 5 commits November 3, 2025 07:33

fix(impeller): fix params to glDiscardFrameBufferEXT

35f8cf7

fix default framebuffer logic

b78c3a6

add unit test for correct params to glDiscardFrameBufferExt call

2cdfe50

refactor variable naming

d619fde

add copyright header

b6b0c1d

jason-simmons force-pushed the fix-impeller-discard-framebufer branch from 843148e to b6b0c1d Compare November 3, 2025 15:33

jason-simmons added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 3, 2025

auto-submit bot added this pull request to the merge queue Nov 3, 2025

Merged via the queue into flutter:master with commit a642edd Nov 3, 2025
187 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 3, 2025

engine-flutter-autoroll mentioned this pull request Nov 4, 2025

Roll Flutter from 027f2e410241 to afbfaf93566f (69 revisions) flutter/packages#10359

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 4, 2025

a642eddae fix(impeller): fix params to glDiscardFrameBufferEXT (flutt…

bc9fac6

…er/flutter#175589)

engine-flutter-autoroll mentioned this pull request Nov 4, 2025

Roll Flutter from 027f2e410241 to 0f9853333f2f (70 revisions) flutter/packages#10360

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 4, 2025

a642eddae fix(impeller): fix params to glDiscardFrameBufferEXT (flutt…

9d02c17

…er/flutter#175589)

engine-flutter-autoroll mentioned this pull request Nov 4, 2025

Roll Flutter from 027f2e410241 to a913d39131af (72 revisions) flutter/packages#10361

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 4, 2025

a642eddae fix(impeller): fix params to glDiscardFrameBufferEXT (flutt…

f196cb3

…er/flutter#175589)

engine-flutter-autoroll mentioned this pull request Nov 4, 2025

Roll Flutter from 027f2e410241 to e5d5c01850f2 (73 revisions) flutter/packages#10362

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 4, 2025

a642eddae fix(impeller): fix params to glDiscardFrameBufferEXT (flutt…

e135521

…er/flutter#175589)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(impeller): fix params to glDiscardFrameBufferEXT #175589

fix(impeller): fix params to glDiscardFrameBufferEXT #175589

Uh oh!

planetmarshall commented Sep 18, 2025 •

edited

Loading

Uh oh!

flutter-dashboard bot commented Sep 18, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

chinmaygarde left a comment

Uh oh!

chinmaygarde Sep 29, 2025

Uh oh!

auto-submit bot commented Oct 1, 2025

Uh oh!

planetmarshall commented Oct 7, 2025

Uh oh!

auto-submit bot commented Oct 13, 2025

Uh oh!

flutter-dashboard bot commented Nov 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fix(impeller): fix params to glDiscardFrameBufferEXT #175589

fix(impeller): fix params to glDiscardFrameBufferEXT #175589

Uh oh!

Conversation

planetmarshall commented Sep 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 Sep 18, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

chinmaygarde left a comment

Choose a reason for hiding this comment

Uh oh!

chinmaygarde Sep 29, 2025

Choose a reason for hiding this comment

Uh oh!

auto-submit bot commented Oct 1, 2025

Uh oh!

planetmarshall commented Oct 7, 2025

Uh oh!

auto-submit bot commented Oct 13, 2025

Uh oh!

flutter-dashboard bot commented Nov 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

planetmarshall commented Sep 18, 2025 •

edited

Loading