[Impeller] Skip binding dead-code-eliminated resources on Metal by bdero · Pull Request #190040 · flutter/flutter

bdero · 2026-07-26T05:41:35Z

Fixes #190034.

When the shader compiler dead-code-eliminates a fragment sampler or uniform block, reflection still lists it but stamps its Metal binding index with the out-of-range sentinel uint32_t(-1). The Metal backend forwarded that index straight to setFragmentTexture:atIndex:, which has no bounds check, so it crashed in the AGX driver. The GLES backend already skips optimized-out bindings and Vulkan binds by the stable SPIR-V decoration, so only Metal was affected.

This drops the optimized-out binding at flutter_gpu shader-library load so it is never registered, and guards the Metal bind cache so an out-of-range index can never reach the encoder. A shared kOptimizedOutBinding constant names the sentinel.

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 and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

Fixes flutter#190034. A fragment sampler or uniform block the shader compiler dead-code-eliminates is still listed in reflection but stamped with an out-of-range binding sentinel (uint32_t(-1)). The Metal backend forwarded that sentinel straight to setFragmentTexture:atIndex:, which has no bounds check, so it crashed in the AGX driver. GLES already skips optimized-out bindings and Vulkan binds by the stable SPIR-V decoration; this brings Metal to parity. Skip the sentinel at flutter_gpu shader-library load so it is never registered, and guard the Metal bind cache so an out-of-range index can never reach the encoder.

gemini-code-assist

Code Review

This pull request introduces handling for dead-code-eliminated shader resources by defining a sentinel value kOptimizedOutBinding and skipping their registration and binding in the Metal backend and Flutter GPU frontend. Feedback on the changes highlights a critical bug in PassBindingsCacheMTL where returning true instead of false for optimized-out bindings would still trigger a crash. Additionally, the reviewer suggested using uint32_t instead of size_t for the sentinel constant to align with the Google C++ Style Guide, and recommended reusing the shared sentinel constant in the unit tests.

Type the kOptimizedOutBinding sentinel as uint32_t (it is a 32-bit resource binding index, not a size), and reference the shared constant from the load tests instead of redefining the value.

gaaclarke · 2026-07-27T18:18:13Z

@bdero is this ready to review?

bdero · 2026-07-28T21:14:40Z

Yup this one is good to review, thanks

gaaclarke

logic and code looks good to me, i have some questions about testing

gaaclarke · 2026-07-28T22:46:43Z

                                     uint64_t index,
                                     uint64_t offset,
                                     id<MTLBuffer> buffer) {
+  if (index == kOptimizedOutBinding) {


There's no tests that cover this code. Do you think we should add dart integration tests?

yess, added in 8bffb09

Cover the dead-code-eliminated-sampler path end to end: a fragment shader whose declared sampler the compiler drops is loaded, its slot bound, and the pass drawn, asserting the process survives. Before the fix, binding the optimized-out sampler forwarded an out-of-range index to Metal and crashed.

gaaclarke

lgtm! thanks

bdero · 2026-07-29T18:34:35Z

Oops, there was a formatting issue. Not used to the new dashboard situation.

auto-submit · 2026-08-01T11:26:30Z

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

@autoclosure

…12406) Manual roll Flutter from e52f01c920ad to b766512c65d8 (42 revisions) Manual roll requested by stuartmorgan@google.com flutter/flutter@e52f01c...b766512 2026-08-04 engine-flutter-autoroll@skia.org Roll Dart SDK from 2a799a2404e9 to 9859c0a39adb (4 revisions) (flutter/flutter#190521) 2026-08-04 154381524+flutteractionsbot@users.noreply.github.com Revert: iOS: Eliminate use of IOSContextNoop in platform view tests (flutter/flutter#190501) 2026-08-04 125822178+guszxtavo@users.noreply.github.com [Impeller] Enable ETC2/ASTC LDR/BC texture compression features at Vulkan device creation (flutter/flutter#189303) 2026-08-03 30870216+gaaclarke@users.noreply.github.com Remove openglessdf from impeller_unittests. (flutter/flutter#190469) 2026-08-03 1961493+harryterkelsen@users.noreply.github.com [web] Unify image decoding and codecs on CanvasKit and Skwasm (flutter/flutter#188573) 2026-08-03 chris@bracken.jp iOS: Eliminate use of IOSContextNoop in platform view tests (flutter/flutter#190419) 2026-08-03 evanwall@buffalo.edu Add path rendering benchmarks (flutter/flutter#188654) 2026-08-03 97480502+b-luk@users.noreply.github.com Add windows platform support for primitive_shape_test integration test (flutter/flutter#190464) 2026-08-03 engine-flutter-autoroll@skia.org Roll Skia from 958c1c1921a1 to a08d918ebd6a (3 revisions) (flutter/flutter#190467) 2026-08-03 chris@bracken.jp tests: add --ios-runtime param (flutter/flutter#190414) 2026-08-03 chris@bracken.jp iOS: Remove the synchronous first-frame wait (flutter/flutter#190432) 2026-08-03 chris@bracken.jp iOS: Eliminate the Impeller/Skia backend selection params (flutter/flutter#190416) 2026-08-03 chris@bracken.jp iOS,macOS: Use @autoclosure in Logger (flutter/flutter#190417) 2026-08-03 chris@bracken.jp tools: Support FLUTTER_HOST_ARCH in update_dart_sdk scripts (flutter/flutter#190421) 2026-08-03 chris@bracken.jp iOS: Hardcode rendering API to Metal in tests (no-op) (flutter/flutter#190422) 2026-08-03 chris@bracken.jp a11y: Map disabled/read-only semantics to AX node restriction (flutter/flutter#190353) 2026-08-03 kevmoo@users.noreply.github.com [Infra] Replace defunct umbrella template with Wasm issue form (flutter/flutter#190471) 2026-08-03 engine-flutter-autoroll@skia.org Roll Skia from abecb0dc02c1 to 958c1c1921a1 (4 revisions) (flutter/flutter#190459) 2026-08-03 engine-flutter-autoroll@skia.org Roll Dart SDK from 65b163be2485 to 2a799a2404e9 (3 revisions) (flutter/flutter#190454) 2026-08-03 engine-flutter-autoroll@skia.org Roll Skia from 68efb3f2ad16 to abecb0dc02c1 (1 revision) (flutter/flutter#190443) 2026-08-03 engine-flutter-autoroll@skia.org Roll Packages from 5351d8c to ac87e65 (4 revisions) (flutter/flutter#190441) 2026-08-03 engine-flutter-autoroll@skia.org Roll Skia from 5a761eb826c1 to 68efb3f2ad16 (1 revision) (flutter/flutter#190440) 2026-08-03 engine-flutter-autoroll@skia.org Roll Skia from 4c9f8b4805e2 to 5a761eb826c1 (1 revision) (flutter/flutter#190437) 2026-08-03 ellie@edencrew.com [macOS] Resume app lifecycle on becomeActive to avoid frozen UI after occlusion (flutter/flutter#188772) 2026-08-03 engine-flutter-autoroll@skia.org Roll Skia from 39cda9d6d7d2 to 4c9f8b4805e2 (6 revisions) (flutter/flutter#190426) 2026-08-03 engine-flutter-autoroll@skia.org Roll Skia from df13bfb5a54e to 39cda9d6d7d2 (2 revisions) (flutter/flutter#190425) 2026-08-02 chris@bracken.jp iOS: Serialise CADisplayLink access in VSyncClient tests (flutter/flutter#190335) 2026-08-02 engine-flutter-autoroll@skia.org Roll Skia from 32329e5643b5 to df13bfb5a54e (1 revision) (flutter/flutter#190394) 2026-08-02 bdero@google.com [Impeller] Skip binding dead-code-eliminated resources on Metal (flutter/flutter#190040) 2026-08-01 bdero@google.com [Flutter GPU] Raise Dart errors for invalid render pipelines and memoize per-draw pipeline state (flutter/flutter#189899) 2026-08-01 41930132+hellohuanlin@users.noreply.github.com Revert "Improve non rect platform view rendering (#182662)" (flutter/flutter#190003) 2026-08-01 engine-flutter-autoroll@skia.org Roll Skia from ebf50520d720 to 32329e5643b5 (1 revision) (flutter/flutter#190389) 2026-08-01 engine-flutter-autoroll@skia.org Roll Skia from f73c4510d12d to ebf50520d720 (6 revisions) (flutter/flutter#190376) 2026-07-31 97480502+b-luk@users.noreply.github.com Primitive shape integration test (flutter/flutter#190368) 2026-07-31 97480502+b-luk@users.noreply.github.com Eliminate some early returns in uber_sdf.frag to fix broken UberSDF AA on Windows (flutter/flutter#190260) 2026-07-31 codefu@google.com chore: swiftshader mirrored + llvm16 (flutter/flutter#181225) 2026-07-31 1961493+harryterkelsen@users.noreply.github.com [web] Remove in-repo agent documentation (flutter/flutter#190326) 2026-07-31 30870216+gaaclarke@users.noreply.github.com [windows]: Uses offscreen MSAA when implicit msaa isn't available. (flutter/flutter#190256) 2026-07-31 srawlins@google.com flutter_tools: Use new FileSystemExtension from devtools (flutter/flutter#190360) 2026-07-31 engine-flutter-autoroll@skia.org Roll Dart SDK from c3acfc2479f6 to 65b163be2485 (1 revision) (flutter/flutter#190358) 2026-07-31 magder@google.com Use devicectl for screenshots on Xcode 27, remove idevicescreenshot artifact (flutter/flutter#189091) 2026-07-31 engine-flutter-autoroll@skia.org Roll Skia from 7ef86a5b0eb9 to f73c4510d12d (1 revision) (flutter/flutter#190352) If this roll has caused a breakage, revert this CL and stop the roller ...

bdero added e: impeller Impeller rendering backend issues and features requests platform-macos Building on or for macOS specifically labels Jul 26, 2026

github-actions Bot added engine flutter/engine related. See also e: labels. flutter-gpu team-fluttergpu Owned by Flutter GPU team and removed platform-macos Building on or for macOS specifically labels Jul 26, 2026

github-project-automation Bot added this to Flutter GPU Jul 26, 2026

github-project-automation Bot moved this to 🤔 Needs Triage in Flutter GPU Jul 26, 2026

bdero marked this pull request as ready for review July 26, 2026 07:03

gemini-code-assist Bot reviewed Jul 26, 2026

View reviewed changes

Address review: uint32_t sentinel, reuse constant in tests

f8fbf31

Type the kOptimizedOutBinding sentinel as uint32_t (it is a 32-bit resource binding index, not a size), and reference the shared constant from the load tests instead of redefining the value.

bdero requested a review from gaaclarke July 28, 2026 21:14

gaaclarke reviewed Jul 28, 2026

View reviewed changes

bdero added the CICD Run CI/CD label Jul 29, 2026

bdero force-pushed the bdero/impeller-dce-sampler-skip branch from 8bffb09 to a3f84b2 Compare July 29, 2026 01:36

bdero requested a review from gaaclarke July 29, 2026 01:57

Merge branch 'master' into bdero/impeller-dce-sampler-skip

000da75

gaaclarke previously approved these changes Jul 29, 2026

View reviewed changes

Format gpu_test.dart with dart format

7faacae

bdero dismissed gaaclarke’s stale review via 7faacae July 29, 2026 18:31

bdero requested a review from gaaclarke July 29, 2026 18:34

gaaclarke approved these changes Jul 29, 2026

View reviewed changes

bdero added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 30, 2026

Merge branch 'master' into bdero/impeller-dce-sampler-skip

4845ae5

auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 1, 2026

Merge branch 'master' into bdero/impeller-dce-sampler-skip

d1e292d

bdero added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 2, 2026

auto-submit Bot added this pull request to the merge queue Aug 2, 2026

Merged via the queue into flutter:master with commit 6c071c2 Aug 2, 2026
22 checks passed

github-project-automation Bot moved this from 🤔 Needs Triage to ✅ Done in Flutter GPU Aug 2, 2026

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

Piinks mentioned this pull request Aug 8, 2026

Flutter Notable Commits - 2026 #181433

Open

This was referenced Aug 9, 2026

Manual roll Flutter from e52f01c920ad to b766512c65d8 (42 revisions) flutter/packages#12406

Merged

Manual roll Flutter from e52f01c920ad to fd004496f408 (25 revisions) flutter/packages#12416

Closed

Navigation Menu

Repository navigation

[Impeller] Skip binding dead-code-eliminated resources on Metal - #190040

[Impeller] Skip binding dead-code-eliminated resources on Metal#190040
auto-submit[bot] merged 7 commits into
flutter:masterfrom
bdero:bdero/impeller-dce-sampler-skip

bdero commented Jul 26, 2026 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gaaclarke commented Jul 27, 2026

Uh oh!

bdero commented Jul 28, 2026

Uh oh!

gaaclarke left a comment

Uh oh!

gaaclarke Jul 28, 2026

Uh oh!

bdero Jul 29, 2026

Uh oh!

gaaclarke left a comment

Uh oh!

bdero commented Jul 29, 2026

Uh oh!

auto-submit Bot commented Aug 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

bdero commented Jul 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gaaclarke commented Jul 27, 2026

Uh oh!

bdero commented Jul 28, 2026

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

gaaclarke Jul 28, 2026

Choose a reason for hiding this comment

Uh oh!

bdero Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

bdero commented Jul 29, 2026

Uh oh!

auto-submit Bot commented Aug 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bdero commented Jul 26, 2026 •

edited

Loading