[impeller] Support partitioned host buffer #174463

eyebrowsoffire · 2025-08-26T20:10:39Z

This change adds support for host buffer that keeps index buffers separate from other general-purpose data. This is a requirement for WebGL: https://registry.khronos.org/webgl/specs/latest/2.0/#BUFFER_OBJECT_BINDING

No current backends use the partitioned implementation, but once I add WebGL support it will use it to avoid #149603

gaaclarke

Introducing polymorphism in HostBuffer adds complication, overhead, and hurts locality. It's not necessary for what we are trying to do.

All you need to do is introduce a new datatype:

struct HostBuffers {
  HostBuffer data;
  std::optional<HostBuffer> indices;
};

engine/src/flutter/impeller/core/partitioned_host_buffer.h

engine/src/flutter/impeller/entity/contents/host_buffer_unittests.cc

eyebrowsoffire · 2025-08-27T17:59:08Z

Introducing polymorphism in HostBuffer adds complication, overhead, and hurts locality. It's not necessary for what we are trying to do.

All you need to do is introduce a new datatype:
struct HostBuffers {

  HostBuffer data;

  std::optional<HostBuffer> indices;

};

The idea here is that most backends still should use a unified host buffer under the hood. Separation of indices into another buffer is pretty much specific to WebGL, so for other backends the specification of the buffer category is basically a no-op. But the backend-agnostic code needs to specify whether it is going to use the buffer for indexes or for other data, so that if the backend needs to separate it it can. Hence the polymorphism, we need different behavior for different backends.

However, it doesn't necessarily have to be dynamic polymorphism, I'm happy to refactor this a different way. But I'm not sure your suggestion on its own solves the problem. We can jump on a call to discuss the details and get aligned on a solution.

gaaclarke

LGTM, thanks, this is a much better approach I think. It might be worth telling the gemini bot to double check it since there is a lot of small refactoring which it's good at checking. I reviewed it too and looked all good.

gaaclarke · 2025-08-28T15:21:59Z

engine/src/flutter/impeller/renderer/renderer_unittests.cc

+  auto data_host_buffer = HostBuffer::Create(
+      GetContext()->GetResourceAllocator(), GetContext()->GetIdleWaiter(),
+      GetContext()->GetCapabilities()->GetMinimumUniformAlignment());
+  auto indexes_host_buffer =
+      GetContext()->GetCapabilities()->NeedsPartitionedHostBuffer()
+          ? HostBuffer::Create(
+                GetContext()->GetResourceAllocator(),
+                GetContext()->GetIdleWaiter(),
+                GetContext()->GetCapabilities()->GetMinimumUniformAlignment())
+          : data_host_buffer;


nit: We can probably pull this into a function since it happens a fair bit.

gaaclarke · 2025-08-28T15:23:59Z

engine/src/flutter/impeller/entity/contents/content_context.h

+  /// @brief Retrieve the current host buffer for transient storage of indexes
+  ///        used for indexed draws.


It's worth noting that GetTransientsIndexesBuffer may return the same value as GetTransientsDataBuffer in certain circumstances.

gaaclarke · 2025-08-28T15:26:44Z

engine/src/flutter/impeller/entity/contents/content_context.cc

+      indexes_host_buffer_(
+          context_->GetCapabilities()->NeedsPartitionedHostBuffer()
+              ? HostBuffer::Create(
+                    context_->GetResourceAllocator(),
+                    context_->GetIdleWaiter(),
+                    context_->GetCapabilities()->GetMinimumUniformAlignment())
+              : data_host_buffer_),


Please move this into the constructor. The order in which initializers are executed is controlled by the order in the header. I think we have a lint that forces them to be in the same order but I think it's safer just to avoid order issues in initializers.

gaaclarke · 2025-08-28T15:29:10Z

fyi the test failure looks apropos, ResetHostBufferBasedOnFrameBoundary

gemini-code-assist

Code Review

This pull request introduces support for partitioned host buffers, a necessary change for upcoming WebGL support. The primary modification involves splitting GetTransientsBuffer() into GetTransientsDataBuffer() and GetTransientsIndexesBuffer(), and updating numerous call sites to use the appropriate buffer for vertex, index, and uniform data. The implementation correctly handles backends that don't require partitioning by using a shared buffer. The changes are extensive but have been applied consistently across the codebase. I've identified one pre-existing issue in a test file that is now surfaced by these changes.

engine/src/flutter/impeller/entity/entity_unittests.cc

auto-submit · 2025-08-28T18:46:33Z

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

The status or check suite Linux linux_clang_tidy has failed. Please fix the issues identified (or deflake) before re-applying this label.

auto-submit · 2025-08-28T22:05:02Z

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

…#174463)

Roll Flutter from da5523afc3c1 to 6b18740d5a23 (49 revisions) flutter/flutter@da5523a...6b18740 2025-08-29 [email protected] Roll Dart SDK from 11f6cd99f6b3 to 72cda0f3dc42 (2 revisions) (flutter/flutter#174697) 2025-08-29 [email protected] Fix empty adaptive text selection toolbars building. (flutter/flutter#174656) 2025-08-29 [email protected] [flutter_test] update the _isImportantForAccessibility method in SemanticsController to include tooltip (flutter/flutter#174476) 2025-08-29 [email protected] Roll Skia from 89794f0b5384 to 43e79dc80ca8 (1 revision) (flutter/flutter#174678) 2025-08-29 [email protected] Roll Skia from f3c8b4c677f5 to 89794f0b5384 (6 revisions) (flutter/flutter#174675) 2025-08-29 [email protected] Implement Overlay.of with inherited widget (flutter/flutter#174315) 2025-08-29 [email protected] [impeller] Support partitioned host buffer (flutter/flutter#174463) 2025-08-29 [email protected] Adds semantics for disabled buttons in date picker (flutter/flutter#174064) 2025-08-29 [email protected] Roll Fuchsia Linux SDK from bHYRvLv2Dg56RWZF2... to 00VSr-5B7hq0G2eZx... (flutter/flutter#174667) 2025-08-29 [email protected] Check GTK calls are done on the same thread. (#174488) (flutter/flutter#174624) 2025-08-29 [email protected] [ Tool ] Only listen for DebugConnectionInfo if the service protocol is supported (flutter/flutter#174664) 2025-08-29 [email protected] Roll Dart SDK from 89023922f96d to 11f6cd99f6b3 (9 revisions) (flutter/flutter#174669) 2025-08-28 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[web] Refactor renderers to use the same frontend code (#174588)" (flutter/flutter#174672) 2025-08-28 [email protected] [a11y] [test] containsSemantics can ignore SemanticsValidationResult (flutter/flutter#174608) 2025-08-28 [email protected] Fix some issues in engine-tool README. (flutter/flutter#174512) 2025-08-28 [email protected] Marks Linux_pixel_7pro new_gallery__transition_perf to be flaky (flutter/flutter#174106) 2025-08-28 [email protected] Make sure that an AlertDialog doesn't crash in 0x0 environment (flutter/flutter#174091) 2025-08-28 [email protected] Marks Linux_pixel_7pro hello_world_impeller to be flaky (flutter/flutter#173699) 2025-08-28 [email protected] Marks Linux_pixel_7pro drive_perf_debug_warning to be flaky (flutter/flutter#174112) 2025-08-28 [email protected] Marks Linux_android_emu android_display_cutout to be flaky (flutter/flutter#174501) 2025-08-28 [email protected] Marks Linux_pixel_7pro service_extensions_test to be flaky (flutter/flutter#174114) 2025-08-28 [email protected] Marks Windows plugin_test to be flaky (flutter/flutter#174117) 2025-08-28 [email protected] Marks Windows_mokey basic_material_app_win__compile to be flaky (flutter/flutter#173702) 2025-08-28 [email protected] Marks Mac_mokey microbenchmarks to be flaky (flutter/flutter#174102) 2025-08-28 [email protected] Marks Linux_mokey complex_layout__start_up to be flaky (flutter/flutter#173692) 2025-08-28 [email protected] Marks Linux build_android_host_app_with_module_aar to be flaky (flutter/flutter#172631) 2025-08-28 [email protected] Marks Linux_pixel_7pro new_gallery_opengles_impeller__transition_perf to be flaky (flutter/flutter#173338) 2025-08-28 [email protected] Marks Linux_pixel_7pro platform_views_scroll_perf_impeller__timeline_summary to be flaky (flutter/flutter#172211) 2025-08-28 [email protected] Remove the option to disable the merged platform/UI thread on Android and iOS (flutter/flutter#174408) 2025-08-28 [email protected] Don't fail when hot restarting `web-server` and there are no connected clients (flutter/flutter#174600) 2025-08-28 [email protected] Roll Skia from 7c2fe2629d4a to f3c8b4c677f5 (7 revisions) (flutter/flutter#174654) 2025-08-28 [email protected] [WebParagraph] More plumbing towards making it usable in Flutter apps (flutter/flutter#174587) 2025-08-28 [email protected] Roll Packages from 86fbeec to 141d8e3 (6 revisions) (flutter/flutter#174645) 2025-08-28 [email protected] [web] Refactor renderers to use the same frontend code (flutter/flutter#174588) 2025-08-28 [email protected] Roll Skia from eb000b138a9d to 7c2fe2629d4a (3 revisions) (flutter/flutter#174637) 2025-08-28 [email protected] [ Tool ] Roll package:dwds 25.0.4 (flutter/flutter#174601) 2025-08-28 [email protected] Roll Skia from 9b1642f2cfea to eb000b138a9d (2 revisions) (flutter/flutter#174627) 2025-08-28 [email protected] Roll Skia from 430d60054d66 to 9b1642f2cfea (7 revisions) (flutter/flutter#174625) 2025-08-28 [email protected] Refactored Canvas to disallow null inline contexts. (flutter/flutter#174530) 2025-08-28 [email protected] Revert "Check GTK calls are done on the same thread." (flutter/flutter#174604) 2025-08-27 [email protected] Roll Skia from 2a12b57fbbf0 to 430d60054d66 (3 revisions) (flutter/flutter#174590) 2025-08-27 [email protected] Retry "Implements the Android native stretch effect as a fragment shader (Impeller-only)." (flutter/flutter#173885) 2025-08-27 [email protected] Use raw `--removal-label "cp: ..."` when removing labels for unmerged PRs (flutter/flutter#174596) 2025-08-27 [email protected] Flutter driver deserialization (flutter/flutter#172927) 2025-08-27 [email protected] Check GTK calls are done on the same thread. (flutter/flutter#174488) 2025-08-27 [email protected] Fix broken reference to `PULL_REQUEST_CP_TEMPLATE.md` after refactor (flutter/flutter#174595) ...

This change adds support for host buffer that keeps index buffers separate from other general-purpose data. This is a requirement for WebGL: https://registry.khronos.org/webgl/specs/latest/2.0/#BUFFER_OBJECT_BINDING No current backends use the partitioned implementation, but once I add WebGL support it will use it to avoid flutter#149603

…#174463)

This change adds support for host buffer that keeps index buffers separate from other general-purpose data. This is a requirement for WebGL: https://registry.khronos.org/webgl/specs/latest/2.0/#BUFFER_OBJECT_BINDING No current backends use the partitioned implementation, but once I add WebGL support it will use it to avoid flutter#149603

Adjust host buffer interface.

007e11f

github-actions bot added a: text input Entering text in a text field or keyboard related problems engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Aug 26, 2025

eyebrowsoffire added 2 commits August 26, 2025 14:31

Added partitioned host buffer.

4dd3be0

Add unit tests for simple and partitioned host buffer.

e4f9e2a

eyebrowsoffire marked this pull request as ready for review August 26, 2025 22:23

eyebrowsoffire changed the title ~~Support partitioned host buffer~~ [impeller] Support partitioned host buffer Aug 26, 2025

Add some more doc comments and fix clang-tidy.

97b7994

gaaclarke requested changes Aug 27, 2025

View reviewed changes

engine/src/flutter/impeller/core/partitioned_host_buffer.h Outdated Show resolved Hide resolved

engine/src/flutter/impeller/core/partitioned_host_buffer.h Outdated Show resolved Hide resolved

engine/src/flutter/impeller/entity/contents/host_buffer_unittests.cc Outdated Show resolved Hide resolved

eyebrowsoffire added 2 commits August 27, 2025 18:43

Remove polymorphism and split the host buffers at the ContentContext.

50299af

Add unit test for new capability field.

5c4f7f6

gaaclarke approved these changes Aug 28, 2025

View reviewed changes

Addressed Aaron's comments and fix unit test.

09dbd9a

gemini-code-assist bot reviewed Aug 28, 2025

View reviewed changes

engine/src/flutter/impeller/entity/entity_unittests.cc Outdated Show resolved Hide resolved

Fix pre-existing issue pointed out by Gemini.

e763600

eyebrowsoffire added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 28, 2025

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

Fix clang-tidy issue.

cbe7e36

eyebrowsoffire added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 28, 2025

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

eyebrowsoffire added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 28, 2025

auto-submit bot added this pull request to the merge queue Aug 29, 2025

Merged via the queue into flutter:master with commit c98cc29 Aug 29, 2025
189 checks passed

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

engine-flutter-autoroll mentioned this pull request Aug 31, 2025

Roll Flutter from da5523afc3c1 to c24e1b783819 (71 revisions) flutter/packages#9934

Closed

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

c98cc294b [impeller] Support partitioned host buffer (flutter/flutter…

98a69f6

…#174463)

engine-flutter-autoroll mentioned this pull request Sep 1, 2025

Roll Flutter from da5523afc3c1 to 1bbbf060f14f (74 revisions) flutter/packages#9936

Closed

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

c98cc294b [impeller] Support partitioned host buffer (flutter/flutter…

cebe889

…#174463)

engine-flutter-autoroll mentioned this pull request Sep 1, 2025

Roll Flutter from da5523afc3c1 to 5ab400daa744 (75 revisions) flutter/packages#9937

Closed

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

c98cc294b [impeller] Support partitioned host buffer (flutter/flutter…

76e390e

…#174463)

engine-flutter-autoroll mentioned this pull request Sep 1, 2025

Roll Flutter from da5523afc3c1 to d7f80a59fe9b (76 revisions) flutter/packages#9939

Closed

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

c98cc294b [impeller] Support partitioned host buffer (flutter/flutter…

d4f38bb

…#174463)

engine-flutter-autoroll mentioned this pull request Sep 2, 2025

Roll Flutter from da5523afc3c1 to fd4e3a0a3786 (82 revisions) flutter/packages#9940

Closed

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

c98cc294b [impeller] Support partitioned host buffer (flutter/flutter…

7d13e43

…#174463)

engine-flutter-autoroll mentioned this pull request Sep 2, 2025

Roll Flutter from da5523afc3c1 to 7a13e1be3a98 (83 revisions) flutter/packages#9941

Closed

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

c98cc294b [impeller] Support partitioned host buffer (flutter/flutter…

85a5af5

…#174463)

eyebrowsoffire mentioned this pull request Sep 2, 2025

[Impeller] OpenGLES backend tries to reuse buffers across different buffer targets, which fails on WebGL. #149603

Closed

engine-flutter-autoroll mentioned this pull request Sep 2, 2025

Roll Flutter from da5523afc3c1 to a889fae0c20b (84 revisions) flutter/packages#9942

Closed

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

c98cc294b [impeller] Support partitioned host buffer (flutter/flutter…

b06322f

…#174463)

engine-flutter-autoroll mentioned this pull request Sep 2, 2025

Roll Flutter from da5523afc3c1 to 1d19f9783377 (85 revisions) flutter/packages#9943

Closed

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

c98cc294b [impeller] Support partitioned host buffer (flutter/flutter…

b4f6508

…#174463)

engine-flutter-autoroll mentioned this pull request Sep 2, 2025

Roll Flutter from da5523afc3c1 to 432cfa62aca5 (86 revisions) flutter/packages#9946

Closed

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

c98cc294b [impeller] Support partitioned host buffer (flutter/flutter…

ec41b55

…#174463)

engine-flutter-autoroll mentioned this pull request Sep 2, 2025

Roll Flutter from da5523afc3c1 to 3cdc9070cc4f (87 revisions) flutter/packages#9947

Closed

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

c98cc294b [impeller] Support partitioned host buffer (flutter/flutter…

f0c503b

…#174463)

engine-flutter-autoroll mentioned this pull request Sep 2, 2025

Roll Flutter from da5523afc3c1 to fb754dc30c10 (88 revisions) flutter/packages#9948

Closed

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

c98cc294b [impeller] Support partitioned host buffer (flutter/flutter…

b6ecb82

…#174463)

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

c98cc294b [impeller] Support partitioned host buffer (flutter/flutter…

37427c8

…#174463)

eyebrowsoffire deleted the partitioned_host_buffer branch December 12, 2025 21:51

		/// @brief Retrieve the current host buffer for transient storage of indexes
		/// used for indexed draws.

[impeller] Support partitioned host buffer #174463

[impeller] Support partitioned host buffer #174463

Uh oh!

Conversation

eyebrowsoffire commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eyebrowsoffire commented Aug 27, 2025

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

gaaclarke Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

gaaclarke Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

gaaclarke Aug 28, 2025

Choose a reason for hiding this comment

Uh oh!

gaaclarke commented Aug 28, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

auto-submit bot commented Aug 28, 2025

Uh oh!

auto-submit bot commented Aug 28, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

eyebrowsoffire commented Aug 26, 2025 •

edited

Loading