Skip to content

[Flutter GPU] Add explicit draw counts (reland)#187192

Merged
auto-submit[bot] merged 1 commit into
flutter:masterfrom
bdero:bdero/flutter-gpu-draw-api-reland
May 27, 2026
Merged

[Flutter GPU] Add explicit draw counts (reland)#187192
auto-submit[bot] merged 1 commit into
flutter:masterfrom
bdero:bdero/flutter-gpu-draw-api-reland

Conversation

@bdero

@bdero bdero commented May 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Relands #186639, which was reverted in #187170.

The original PR landed clean, but broke the macOS and Linux builds because engine/src/flutter/testing/dart/gpu_shader_reload_test.dart (added concurrently by #186793, which was in the merge queue at the same time) still called the old bindVertexBuffer(view, count) / draw() API. This reland is identical to #186639 plus a one-spot migration of that test to bindVertexBuffer(view) + draw(3).

Moves the vertex and index count off the buffer-binding calls and onto the draw call.

A simple (breaking) change that fixes a very silly design flaw that I built into the API near the very beginning. Moves Flutter GPU draws towards a design that matches virtually all modern graphics APIs, including today's Impeller! Now is the right time to make changes like this to Flutter GPU, since the API is not yet considered stable.

Before, bindVertexBuffer and bindIndexBuffer each took a count, and a single argument-less draw() guessed vertex versus index based on whether an index buffer happened to be bound.

Sooooo many reasons this was the wrong design:

  • Wrong object. A draw count belongs on the draw, not on a buffer binding.
  • Implicit kind. Indexed vs. non-indexed was inferred from what happened to be bound, not stated.
  • Colliding counts. Vertex and index counts shared one field; the vertex count was dropped when an index buffer was bound.
  • Slot-0-only. The count came from vertex slot 0; values on other slots were silently ignored.
  • Opaque draws. draw() told you nothing; you had to trace every bind to know what it drew.
  • Sticky state. The count leaked across draws in a pass.
  • Redundant re-binds. Changing only the count forced re-binding the buffer.
  • No room to grow. Nowhere natural for per-draw params like instanceCount or baseVertex.
  • Unconventional. WebGPU, Vulkan, Metal, and D3D all put counts on the draw.
  • Late errors. Bad counts failed at draw() time, far from the mistaken line.

Now:

  • bindVertexBuffer and bindIndexBuffer only bind buffers.
  • draw(vertexCount) does a non-indexed draw.
  • drawIndexed(indexCount) does an indexed draw.

Instanced draws can be landed later via draw and drawIndexed gaining an instanceCount parameter later on, assuming the Impeller-side instancing support in #186653 lands.

Pre-launch Checklist

@bdero
[Flutter GPU] Add explicit draw counts (reland)
8ddfaa4 
Relands flutter#186639, which was reverted in flutter#187170.

Moves the vertex and index count off the buffer-binding calls and onto
the draw call:
- `bindVertexBuffer` and `bindIndexBuffer` only bind buffers.
- `draw(vertexCount)` does a non-indexed draw.
- `drawIndexed(indexCount)` does an indexed draw.

The original landed clean but broke the build because
gpu_shader_reload_test.dart (added concurrently by flutter#186793) still used
the old `bindVertexBuffer(view, count)` / `draw()` API. This reland
migrates that test to `bindVertexBuffer(view)` + `draw(3)`.
@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label May 27, 2026
@github-actions github-actions Bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests flutter-gpu team-fluttergpu Owned by Flutter GPU team labels May 27, 2026
@github-project-automation github-project-automation Bot moved this to 🤔 Needs Triage in Flutter GPU May 27, 2026
gemini-code-assist[bot]
gemini-code-assist Bot reviewed May 27, 2026
View reviewed changes

@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 the Flutter GPU RenderPass API by decoupling vertex and index counts from the buffer binding methods (bindVertexBuffer and bindIndexBuffer) and instead passing them directly to the draw calls (draw and drawIndexed). This simplifies the binding process and introduces explicit draw(vertexCount) and drawIndexed(indexCount) methods. The feedback suggests improving error handling when drawIndexed is called without an index buffer bound by tracking the index buffer binding state and throwing a descriptive StateError instead of a generic Exception, along with updating the corresponding tests.

Comment thread engine/src/flutter/lib/gpu/lib/src/render_pass.dart
Comment thread engine/src/flutter/lib/gpu/lib/src/render_pass.dart
Comment thread engine/src/flutter/testing/dart/gpu_test.dart
@bdero bdero requested review from cbracken, gaaclarke and walley892 May 27, 2026 19:41
@bdero bdero added the autosubmit Merge PR when tree becomes green via auto submit App label May 27, 2026
@auto-submit auto-submit Bot added this pull request to the merge queue May 27, 2026
Merged via the queue into flutter:master with commit d0d06b2 May 27, 2026
206 checks passed
@github-project-automation github-project-automation Bot moved this from 🤔 Needs Triage to ✅ Done in Flutter GPU May 27, 2026
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 27, 2026
@bdero bdero deleted the bdero/flutter-gpu-draw-api-reland branch May 28, 2026 01:12
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 28, 2026
Merged
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request May 28, 2026
@engine-flutter-autoroll
Roll Flutter from c8f2f166323e to e70534db5a44 (18 revisions) (#11799)
744bbf0 
flutter/flutter@c8f2f16...e70534d

2026-05-28 30870216+gaaclarke@users.noreply.github.com Linux glyph gamma correction (flutter/flutter#187122)
2026-05-28 chris@bracken.jp [iOS] Eliminate strong retain cycle from VSyncClient (flutter/flutter#187164)
2026-05-28 katelovett@google.com Revert "[pubspec] Bump Dart SDK constraint to ^3.13.0 (#186957)" (flutter/flutter#187209)
2026-05-28 engine-flutter-autoroll@skia.org Roll Skia from f1b8ba877c07 to 32acea791248 (3 revisions) (flutter/flutter#187220)
2026-05-27 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from k9EizfEGJO4WwQN9-... to SBpmmPxqx3lAvGojE... (flutter/flutter#187211)
2026-05-27 bdero@google.com [Impeller] Add block-compressed texture format support (BC, ETC2, ASTC) (flutter/flutter#187077)
2026-05-27 katelovett@google.com Disable analyzer (flutter/flutter#187205)
2026-05-27 bdero@google.com [Flutter GPU] Add explicit draw counts (reland) (flutter/flutter#187192)
2026-05-27 bdero@google.com [Flutter GPU] Inject per-backend defines into shader bundle compilation (flutter/flutter#187081)
2026-05-27 engine-flutter-autoroll@skia.org Roll Skia from fa944af10f91 to f1b8ba877c07 (13 revisions) (flutter/flutter#187194)
2026-05-27 47866232+chunhtai@users.noreply.github.com Fixes bug when changing accessibilityFocusBlockType doesn't update ch… (flutter/flutter#186596)
2026-05-27 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#187191)
2026-05-27 kevmoo@users.noreply.github.com [web, tool] Support recompiling shaders and unify asset processing (2nd try) (flutter/flutter#186902)
2026-05-27 robert.ancell@canonical.com Fix crash if FlView is destroyed during a draw. (flutter/flutter#186848)
2026-05-27 engine-flutter-autoroll@skia.org Roll Packages from fc795e5 to 4b424d7 (4 revisions) (flutter/flutter#187174)
2026-05-27 15619084+vashworth@users.noreply.github.com Stop prefetching Swift packages in pub get (flutter/flutter#187113)
2026-05-27 jesswon@google.com Update CI with newer android sdk package (flutter/flutter#187143)
2026-05-27 jason-simmons@users.noreply.github.com Add a tag to the Linux platform properties in .ci.yaml that specifies x64 CPUs (flutter/flutter#187124)

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
@engine-flutter-autoroll
Roll Flutter from c8f2f166323e to e70534db5a44 (18 revisions) (flutte…
8597a81 
…r#11799)

flutter/flutter@c8f2f16...e70534d

2026-05-28 30870216+gaaclarke@users.noreply.github.com Linux glyph gamma correction (flutter/flutter#187122)
2026-05-28 chris@bracken.jp [iOS] Eliminate strong retain cycle from VSyncClient (flutter/flutter#187164)
2026-05-28 katelovett@google.com Revert "[pubspec] Bump Dart SDK constraint to ^3.13.0 (#186957)" (flutter/flutter#187209)
2026-05-28 engine-flutter-autoroll@skia.org Roll Skia from f1b8ba877c07 to 32acea791248 (3 revisions) (flutter/flutter#187220)
2026-05-27 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from k9EizfEGJO4WwQN9-... to SBpmmPxqx3lAvGojE... (flutter/flutter#187211)
2026-05-27 bdero@google.com [Impeller] Add block-compressed texture format support (BC, ETC2, ASTC) (flutter/flutter#187077)
2026-05-27 katelovett@google.com Disable analyzer (flutter/flutter#187205)
2026-05-27 bdero@google.com [Flutter GPU] Add explicit draw counts (reland) (flutter/flutter#187192)
2026-05-27 bdero@google.com [Flutter GPU] Inject per-backend defines into shader bundle compilation (flutter/flutter#187081)
2026-05-27 engine-flutter-autoroll@skia.org Roll Skia from fa944af10f91 to f1b8ba877c07 (13 revisions) (flutter/flutter#187194)
2026-05-27 47866232+chunhtai@users.noreply.github.com Fixes bug when changing accessibilityFocusBlockType doesn't update ch… (flutter/flutter#186596)
2026-05-27 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#187191)
2026-05-27 kevmoo@users.noreply.github.com [web, tool] Support recompiling shaders and unify asset processing (2nd try) (flutter/flutter#186902)
2026-05-27 robert.ancell@canonical.com Fix crash if FlView is destroyed during a draw. (flutter/flutter#186848)
2026-05-27 engine-flutter-autoroll@skia.org Roll Packages from fc795e5 to 4b424d7 (4 revisions) (flutter/flutter#187174)
2026-05-27 15619084+vashworth@users.noreply.github.com Stop prefetching Swift packages in pub get (flutter/flutter#187113)
2026-05-27 jesswon@google.com Update CI with newer android sdk package (flutter/flutter#187143)
2026-05-27 jason-simmons@users.noreply.github.com Add a tag to the Linux platform properties in .ci.yaml that specifies x64 CPUs (flutter/flutter#187124)

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
@bdero @via-guy
[Flutter GPU] Add explicit draw counts (reland) (flutter#187192)
b1bf03e 
Relands flutter#186639, which was reverted in flutter#187170.

The original PR landed clean, but broke the macOS and Linux builds
because `engine/src/flutter/testing/dart/gpu_shader_reload_test.dart`
(added concurrently by flutter#186793, which was in the merge queue at the same
time) still called the old `bindVertexBuffer(view, count)` / `draw()`
API. This reland is identical to flutter#186639 plus a one-spot migration of
that test to `bindVertexBuffer(view)` + `draw(3)`.

---

Moves the vertex and index count off the buffer-binding calls and onto
the draw call.

A simple (breaking) change that fixes a very silly design flaw that I
built into the API near the very beginning. Moves Flutter GPU draws
towards a design that matches virtually all modern graphics APIs,
including today's Impeller! Now is the right time to make changes like
this to Flutter GPU, since the API is not yet considered stable.

Before, `bindVertexBuffer` and `bindIndexBuffer` each took a count, and
a single argument-less `draw()` guessed vertex versus index based on
whether an index buffer happened to be bound.

Sooooo many reasons this was the wrong design:

- **Wrong object.** A draw count belongs on the draw, not on a buffer
binding.
- **Implicit kind.** Indexed vs. non-indexed was inferred from what
happened to be bound, not stated.
- **Colliding counts.** Vertex and index counts shared one field; the
vertex count was dropped when an index buffer was bound.
- **Slot-0-only.** The count came from vertex slot 0; values on other
slots were silently ignored.
- **Opaque draws.** `draw()` told you nothing; you had to trace every
bind to know what it drew.
- **Sticky state.** The count leaked across draws in a pass.
- **Redundant re-binds.** Changing only the count forced re-binding the
buffer.
- **No room to grow.** Nowhere natural for per-draw params like
`instanceCount` or `baseVertex`.
- **Unconventional.** WebGPU, Vulkan, Metal, and D3D all put counts on
the draw.
- **Late errors.** Bad counts failed at `draw()` time, far from the
mistaken line.

Now:
- `bindVertexBuffer` and `bindIndexBuffer` only bind buffers.
- `draw(vertexCount)` does a non-indexed draw.
- `drawIndexed(indexCount)` does an indexed draw.

Instanced draws can be landed later via `draw` and `drawIndexed` gaining
an `instanceCount` parameter later on, assuming the Impeller-side
instancing support in flutter#186653 lands.

## 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.
- [ ] All existing and new tests are passing.

<!-- 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

Reviewers

@walley892 walley892 walley892 approved these changes
@cbracken cbracken Awaiting requested review from cbracken
@gaaclarke gaaclarke Awaiting requested review from gaaclarke
+1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CICD Run CI/CD e: impeller Impeller rendering backend issues and features requests engine flutter/engine related. See also e: labels. flutter-gpu team-fluttergpu Owned by Flutter GPU team

Projects

Flutter GPU
Status: ✅ Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bdero @walley892