[Flutter GPU] Add explicit draw counts#186639
Conversation
github-actions
Bot
added
engine
|
An existing Git SHA, To re-trigger presubmits after closing or re-opeing a PR, or pushing a HEAD commit (i.e. with |
bdero
force-pushed
the
bdero/flutter-gpu-draw-api
branch
from
32015b6 to
ed965e1
Compare
bdero
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Flutter GPU API by moving vertex and index count parameters from the binding methods, such as bindVertexBuffer and bindIndexBuffer, to the draw and drawIndexed methods. These changes are reflected across the Dart API, the C++ implementation, and the native bindings. Feedback suggests strengthening input validation by replacing asserts with RangeError.checkNotNegative in the Dart public API and adding explicit non-negative checks in the C++ layer to prevent potential integer overflow issues when casting signed integers to size_t.
bdero
force-pushed
the
bdero/flutter-gpu-draw-api
branch
from
ed965e1 to
2705bf3
Compare
Reworks the Flutter GPU draw API so the count of vertices or indices is passed to the draw call rather than smuggled through the buffer-binding calls. bindVertexBuffer and bindIndexBuffer no longer take a count. The single argument-less draw() is replaced by draw(vertexCount) and drawIndexed(indexCount). The count is now explicit, and whether a draw is non-indexed or indexed is chosen by the method called rather than by whether an index buffer happens to be bound. Updates the flutter_gpu tests and the impeller dart_tests fixture, and adds coverage for drawIndexed.
bdero
force-pushed
the
bdero/flutter-gpu-draw-api
branch
from
2705bf3 to
6db9ef3
Compare
3 participants
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,
bindVertexBufferandbindIndexBuffereach took a count, and a single argument-lessdraw()guessed vertex versus index based on whether an index buffer happened to be bound.Sooooo many reasons this was the wrong design:
draw()told you nothing; you had to trace every bind to know what it drew.instanceCountorbaseVertex.draw()time, far from the mistaken line.Now:
bindVertexBufferandbindIndexBufferonly bind buffers.draw(vertexCount)does a non-indexed draw.drawIndexed(indexCount)does an indexed draw.Instanced draws can be landed later via
drawanddrawIndexedgaining aninstanceCountparameter later on, assuming the Impeller-side instancing support in #186653 lands.Pre-launch Checklist
///).