Reverts "[Flutter GPU] Add explicit draw counts (flutter#186639)" (flutter#187170)

auto-submit[bot] · auto-submit[bot] · web-flow · commit c8f2f166323e · 2026-05-27T13:33:42.000Z
Reverts: flutter#186639   Initiated by: cbracken   Reason for reverting: doesn't compile on mac, or Linux. Mac: ``` ../../flutter/testing/dart/gpu_shader_reload_test.dart:64:36: Error: Too many positional arguments: 1 allowed, but 2 found. Try removing the extra positional arguments. state.renderPass.bindVertexBuffer( ^ ../../flutter/testing/dart/gpu_shader_reload_test.dart:72:24: Error: Too few positional arguments: 1 requ   Original PR Author: bdero   Reviewed By: {walley892}   This change reverts the following previous change: 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.  [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  Co-authored-by: auto-submit[bot] <flutter-engprod-team@google.com>
diff --git a/engine/src/flutter/impeller/fixtures/dart_tests.dart b/engine/src/flutter/impeller/fixtures/dart_tests.dart
@@ -101,11 +101,11 @@ void canCreateRenderPassAndSubmit(int width, int height) {
       0, 1, 0, 1, // color
     ]),
   );
-  encoder.bindVertexBuffer(vertices);
+  encoder.bindVertexBuffer(vertices, 3);
 
   final gpu.UniformSlot vertInfo = pipeline.vertexShader.getUniformSlot('VertInfo');
   encoder.bindUniform(vertInfo, vertInfoData);
-  encoder.draw(3);
+  encoder.draw();
 
   commandBuffer.submit();
 
diff --git a/engine/src/flutter/lib/gpu/lib/src/buffer.dart b/engine/src/flutter/lib/gpu/lib/src/buffer.dart
@@ -58,12 +58,14 @@ base class DeviceBuffer extends NativeFieldWrapperClass1 {
     RenderPass renderPass,
     int offsetInBytes,
     int lengthInBytes,
+    int vertexCount,
     int slot,
   ) {
     renderPass._bindVertexBufferDevice(
       this,
       offsetInBytes,
       lengthInBytes,
+      vertexCount,
       slot,
     );
   }
@@ -73,12 +75,14 @@ base class DeviceBuffer extends NativeFieldWrapperClass1 {
     int offsetInBytes,
     int lengthInBytes,
     IndexType indexType,
+    int indexCount,
   ) {
     renderPass._bindIndexBufferDevice(
       this,
       offsetInBytes,
       lengthInBytes,
       indexType.index,
+      indexCount,
     );
   }
 
diff --git a/engine/src/flutter/lib/gpu/lib/src/render_pass.dart b/engine/src/flutter/lib/gpu/lib/src/render_pass.dart
@@ -302,22 +302,26 @@ base class RenderPass extends NativeFieldWrapperClass1 {
   /// pipeline's `VertexLayout`. The default of 0 matches the default
   /// layout declared by a shader bundle, which is what every single-buffer
   /// call site expects. To bind multiple structure-of-arrays vertex
-  /// buffers, call this method once per slot.
-  ///
-  /// The number of vertices to draw is passed separately, to [draw].
+  /// buffers, call this method once per slot. The [vertexCount] takes
+  /// effect only when no index buffer is bound, and is read from the
+  /// binding at slot 0, so values supplied at higher slots are ignored.
   ///
   /// Sparse bindings are not supported. Every slot in `[0, highestBound]`
-  /// must have been bound before [draw] or [drawIndexed] is called,
-  /// otherwise they throw a [StateError] naming the unbound slots. Slots
-  /// can be bound in any order.
+  /// must have been bound before [draw] is called, otherwise [draw] throws
+  /// a [StateError] naming the unbound slots. Slots can be bound in any
+  /// order.
   ///
   /// [slot] must be in `[0, 16)` (Impeller's HAL caps vertex buffer
   /// bindings at 16). On the OpenGL ES backend, the per-pipeline limit on
   /// the *total attribute count* across all bound buffers is whatever the
   /// device reports for `GL_MAX_VERTEX_ATTRIBS` (minimum 8 on GL ES 2.0,
   /// minimum 16 on GL ES 3.0+), and is enforced by the driver rather than
   /// by this method.
-  void bindVertexBuffer(BufferView bufferView, {int slot = 0}) {
+  void bindVertexBuffer(
+    BufferView bufferView,
+    int vertexCount, {
+    int slot = 0,
+  }) {
     if (slot < 0 || slot >= _kMaxVertexBufferSlots) {
       throw RangeError.range(
         slot,
@@ -335,20 +339,22 @@ base class RenderPass extends NativeFieldWrapperClass1 {
       this,
       bufferView.offsetInBytes,
       bufferView.lengthInBytes,
+      vertexCount,
       slot,
     );
   }
 
-  /// Binds [bufferView] as the index buffer.
-  ///
-  /// [indexType] is the width of each index. The number of indices to draw
-  /// is passed separately, to [drawIndexed].
-  void bindIndexBuffer(BufferView bufferView, IndexType indexType) {
+  void bindIndexBuffer(
+    BufferView bufferView,
+    IndexType indexType,
+    int indexCount,
+  ) {
     bufferView.buffer._bindAsIndexBuffer(
       this,
       bufferView.offsetInBytes,
       bufferView.lengthInBytes,
       indexType,
+      indexCount,
     );
   }
 
@@ -499,29 +505,7 @@ base class RenderPass extends NativeFieldWrapperClass1 {
     _setWindingOrder(windingOrder.index);
   }
 
-  /// Appends a non-indexed draw of [vertexCount] vertices, read from the
-  /// bound vertex buffers.
-  void draw(int vertexCount) {
-    RangeError.checkNotNegative(vertexCount, 'vertexCount');
-    _validateVertexBindings();
-    if (!_draw(vertexCount)) {
-      throw Exception("Failed to append draw");
-    }
-  }
-
-  /// Appends an indexed draw of [indexCount] indices, read from the index
-  /// buffer bound with [bindIndexBuffer].
-  void drawIndexed(int indexCount) {
-    RangeError.checkNotNegative(indexCount, 'indexCount');
-    _validateVertexBindings();
-    if (!_drawIndexed(indexCount)) {
-      throw Exception("Failed to append drawIndexed");
-    }
-  }
-
-  /// Throws a [StateError] when the bound vertex buffer slots are sparse,
-  /// naming the slots in `[0, highestBound]` that were left unbound.
-  void _validateVertexBindings() {
+  void draw() {
     if (_maxBoundVertexSlot >= 0) {
       final int expectedMask = (1 << (_maxBoundVertexSlot + 1)) - 1;
       if (_boundVertexSlotsMask != expectedMask) {
@@ -530,12 +514,15 @@ base class RenderPass extends NativeFieldWrapperClass1 {
             if ((_boundVertexSlotsMask & (1 << i)) == 0) i,
         ];
         throw StateError(
-          'draw called with sparse vertex buffer bindings: slot(s) '
+          'draw() called with sparse vertex buffer bindings: slot(s) '
           '${missing.join(', ')} were not bound but slot $_maxBoundVertexSlot '
           'was. Bind every slot in [0, $_maxBoundVertexSlot] before drawing.',
         );
       }
     }
+    if (!_draw()) {
+      throw Exception("Failed to append draw");
+    }
   }
 
   /// Wrap with native counterpart.
@@ -604,24 +591,26 @@ base class RenderPass extends NativeFieldWrapperClass1 {
   )
   external void _bindPipeline(RenderPipeline pipeline);
 
-  @Native<Void Function(Pointer<Void>, Pointer<Void>, Int, Int, Int)>(
+  @Native<Void Function(Pointer<Void>, Pointer<Void>, Int, Int, Int, Int)>(
     symbol: 'InternalFlutterGpu_RenderPass_BindVertexBufferDevice',
   )
   external void _bindVertexBufferDevice(
     DeviceBuffer buffer,
     int offsetInBytes,
     int lengthInBytes,
+    int vertexCount,
     int slot,
   );
 
-  @Native<Void Function(Pointer<Void>, Pointer<Void>, Int, Int, Int)>(
+  @Native<Void Function(Pointer<Void>, Pointer<Void>, Int, Int, Int, Int)>(
     symbol: 'InternalFlutterGpu_RenderPass_BindIndexBufferDevice',
   )
   external void _bindIndexBufferDevice(
     DeviceBuffer buffer,
     int offsetInBytes,
     int lengthInBytes,
     int indexType,
+    int indexCount,
   );
 
   @Native<
@@ -747,13 +736,8 @@ base class RenderPass extends NativeFieldWrapperClass1 {
   )
   external void _setPolygonMode(int polygonMode);
 
-  @Native<Bool Function(Pointer<Void>, Int)>(
+  @Native<Bool Function(Pointer<Void>)>(
     symbol: 'InternalFlutterGpu_RenderPass_Draw',
   )
-  external bool _draw(int vertexCount);
-
-  @Native<Bool Function(Pointer<Void>, Int)>(
-    symbol: 'InternalFlutterGpu_RenderPass_DrawIndexed',
-  )
-  external bool _drawIndexed(int indexCount);
+  external bool _draw();
 }
diff --git a/engine/src/flutter/lib/gpu/render_pass.cc b/engine/src/flutter/lib/gpu/render_pass.cc
@@ -100,6 +100,7 @@ void RenderPass::ClearBindings() {
   vertex_buffer_count = 0;
   index_buffer = {};
   index_buffer_type = impeller::IndexType::kNone;
+  element_count = 0;
 }
 
 std::shared_ptr<impeller::Pipeline<impeller::PipelineDescriptor>>
@@ -181,12 +182,7 @@ RenderPass::GetOrCreatePipeline() {
   return pipeline;
 }
 
-bool RenderPass::Draw(size_t element_count, bool indexed) {
-  if (indexed && index_buffer_type == impeller::IndexType::kNone) {
-    // drawIndexed was called without an index buffer bound.
-    return false;
-  }
-
+bool RenderPass::Draw() {
   render_pass_->SetPipeline(impeller::PipelineRef(GetOrCreatePipeline()));
 
   for (const auto& [_, buffer] : vertex_uniform_bindings) {
@@ -221,12 +217,7 @@ bool RenderPass::Draw(size_t element_count, bool indexed) {
   }
 
   render_pass_->SetVertexBuffer(vertex_buffers.data(), vertex_buffer_count);
-  if (indexed) {
-    render_pass_->SetIndexBuffer(index_buffer, index_buffer_type);
-  } else {
-    render_pass_->SetIndexBuffer(impeller::BufferView{},
-                                 impeller::IndexType::kNone);
-  }
+  render_pass_->SetIndexBuffer(index_buffer, index_buffer_type);
   render_pass_->SetElementCount(element_count);
 
   render_pass_->SetStencilReference(stencil_reference);
@@ -343,6 +334,7 @@ static void BindVertexBuffer(
     const std::shared_ptr<const impeller::DeviceBuffer>& buffer,
     int offset_in_bytes,
     int length_in_bytes,
+    int vertex_count,
     int slot) {
   if (slot < 0 || static_cast<size_t>(slot) >=
                       flutter::gpu::RenderPass::kMaxVertexBufferSlots) {
@@ -353,37 +345,62 @@ static void BindVertexBuffer(
   if (static_cast<size_t>(slot) >= wrapper->vertex_buffer_count) {
     wrapper->vertex_buffer_count = static_cast<size_t>(slot) + 1;
   }
+
+  // `vertex_count` is only meaningful for slot 0: the Dart-side docstring
+  // for `bindVertexBuffer` states that for slots > 0 the value is ignored
+  // (the slot-0 binding determines the draw range). When the index type is
+  // set, `vertex_count` would become the index count and is already set by
+  // the index-buffer binding path, so we only assign it here if no index
+  // buffer has been bound.
+  // TODO(bdero): Consider just doing a more traditional API with
+  //              draw(vertexCount) and drawIndexed(indexCount). This is fine,
+  //              but overall it would be a bit more explicit and we wouldn't
+  //              have to document this behavior where the presence of the index
+  //              buffer always takes precedent.
+  if (slot == 0 && !wrapper->has_index_buffer) {
+    wrapper->element_count = vertex_count;
+  }
 }
 
 void InternalFlutterGpu_RenderPass_BindVertexBufferDevice(
     flutter::gpu::RenderPass* wrapper,
     flutter::gpu::DeviceBuffer* device_buffer,
     int offset_in_bytes,
     int length_in_bytes,
+    int vertex_count,
     int slot) {
   BindVertexBuffer(wrapper, device_buffer->GetBuffer(), offset_in_bytes,
-                   length_in_bytes, slot);
+                   length_in_bytes, vertex_count, slot);
 }
 
 static void BindIndexBuffer(
     flutter::gpu::RenderPass* wrapper,
     const std::shared_ptr<const impeller::DeviceBuffer>& buffer,
     int offset_in_bytes,
     int length_in_bytes,
-    int index_type) {
+    int index_type,
+    int index_count) {
+  impeller::IndexType type = flutter::gpu::ToImpellerIndexType(index_type);
   wrapper->index_buffer = impeller::BufferView(
       buffer, impeller::Range(offset_in_bytes, length_in_bytes));
-  wrapper->index_buffer_type = flutter::gpu::ToImpellerIndexType(index_type);
+  wrapper->index_buffer_type = type;
+
+  bool setting_index_buffer = type != impeller::IndexType::kNone;
+  if (setting_index_buffer) {
+    wrapper->element_count = index_count;
+  }
+  wrapper->has_index_buffer = setting_index_buffer;
 }
 
 void InternalFlutterGpu_RenderPass_BindIndexBufferDevice(
     flutter::gpu::RenderPass* wrapper,
     flutter::gpu::DeviceBuffer* device_buffer,
     int offset_in_bytes,
     int length_in_bytes,
-    int index_type) {
+    int index_type,
+    int index_count) {
   BindIndexBuffer(wrapper, device_buffer->GetBuffer(), offset_in_bytes,
-                  length_in_bytes, index_type);
+                  length_in_bytes, index_type, index_count);
 }
 
 static bool BindUniform(
@@ -648,15 +665,6 @@ void InternalFlutterGpu_RenderPass_SetPolygonMode(
       flutter::gpu::ToImpellerPolygonMode(polygon_mode));
 }
 
-bool InternalFlutterGpu_RenderPass_Draw(flutter::gpu::RenderPass* wrapper,
-                                        int vertex_count) {
-  // Guard the cast to the size_t element count; a negative value would wrap.
-  return vertex_count >= 0 && wrapper->Draw(vertex_count, /*indexed=*/false);
-}
-
-bool InternalFlutterGpu_RenderPass_DrawIndexed(
-    flutter::gpu::RenderPass* wrapper,
-    int index_count) {
-  // Guard the cast to the size_t element count; a negative value would wrap.
-  return index_count >= 0 && wrapper->Draw(index_count, /*indexed=*/true);
+bool InternalFlutterGpu_RenderPass_Draw(flutter::gpu::RenderPass* wrapper) {
+  return wrapper->Draw();
 }
diff --git a/engine/src/flutter/lib/gpu/render_pass.h b/engine/src/flutter/lib/gpu/render_pass.h
@@ -56,10 +56,7 @@ class RenderPass : public RefCountedDartWrappable<RenderPass> {
 
   void ClearBindings();
 
-  /// Append a draw to the underlying render pass. [element_count] is the
-  /// vertex count for a non-indexed draw, or the index count when
-  /// [indexed] is true.
-  bool Draw(size_t element_count, bool indexed);
+  bool Draw();
 
   struct BufferAndUniformSlot {
     impeller::ShaderUniformSlot slot;
@@ -91,11 +88,16 @@ class RenderPass : public RefCountedDartWrappable<RenderPass> {
   size_t vertex_buffer_count = 0;
   impeller::BufferView index_buffer;
   impeller::IndexType index_buffer_type = impeller::IndexType::kNone;
+  size_t element_count = 0;
 
   uint32_t stencil_reference = 0;
   std::optional<impeller::IRect32> scissor;
   std::optional<impeller::Viewport> viewport;
 
+  // Helper flag to determine whether the vertex_count should override the
+  // element count. The index count takes precedent.
+  bool has_index_buffer = false;
+
  private:
   /// Lookup an Impeller pipeline by building a descriptor based on the current
   /// command state.
@@ -172,6 +174,7 @@ extern void InternalFlutterGpu_RenderPass_BindVertexBufferDevice(
     flutter::gpu::DeviceBuffer* device_buffer,
     int offset_in_bytes,
     int length_in_bytes,
+    int vertex_count,
     int slot);
 
 FLUTTER_GPU_EXPORT
@@ -180,7 +183,8 @@ extern void InternalFlutterGpu_RenderPass_BindIndexBufferDevice(
     flutter::gpu::DeviceBuffer* device_buffer,
     int offset_in_bytes,
     int length_in_bytes,
-    int index_type);
+    int index_type,
+    int index_count);
 
 FLUTTER_GPU_EXPORT
 extern bool InternalFlutterGpu_RenderPass_BindUniformDevice(
@@ -290,13 +294,7 @@ extern void InternalFlutterGpu_RenderPass_SetPolygonMode(
 
 FLUTTER_GPU_EXPORT
 extern bool InternalFlutterGpu_RenderPass_Draw(
-    flutter::gpu::RenderPass* wrapper,
-    int vertex_count);
-
-FLUTTER_GPU_EXPORT
-extern bool InternalFlutterGpu_RenderPass_DrawIndexed(
-    flutter::gpu::RenderPass* wrapper,
-    int index_count);
+    flutter::gpu::RenderPass* wrapper);
 
 }  // extern "C"
 
diff --git a/engine/src/flutter/testing/dart/gpu_test.dart b/engine/src/flutter/testing/dart/gpu_test.dart
