Skip to content

[Impeller] Add a flat VertexAttributeFormat for vertex inputs#188684

Merged
auto-submit[bot] merged 3 commits into
flutter:masterfrom
bdero:bdero/impeller-vertex-attribute-format
Jun 30, 2026
Merged

[Impeller] Add a flat VertexAttributeFormat for vertex inputs#188684
auto-submit[bot] merged 3 commits into
flutter:masterfrom
bdero:bdero/impeller-vertex-attribute-format

Conversation

@bdero

@bdero bdero commented Jun 27, 2026

Copy link
Copy Markdown
Member

Related to #186309.

Adds a VertexAttributeFormat enum and derives it from a reflected ShaderStageIOSlot, then routes the Metal, Vulkan, and GLES backends through it when building their native vertex input format. This replaces three independent switches over the slot's scalar type, bit width, and component count with one shared vocabulary, which is the foundation for the vertex format expansion tracked in the issue.

This is a behavior preserving refactor. Only the formats reflection produces today are reachable, and each backend keeps its existing support set, so GLES still rejects half-float and 32-bit integer attributes while Metal and Vulkan accept them. The one intentional cleanup is that boolean vertex inputs, which are not expressible in a vertex stage, now map to invalid on every backend instead of the previous inconsistent mappings.

New tests cover the format derivation across scalar kinds, component counts, bit widths, and matrix inputs, plus the GLES mapping including the half-float and 32-bit integer formats it rejects.

Pre-launch Checklist

@bdero bdero added the e: impeller Impeller rendering backend issues and features requests label Jun 27, 2026
@github-actions github-actions Bot added the engine flutter/engine related. See also e: labels. label Jun 27, 2026
Introduce a VertexAttributeFormat enum derived from a reflected ShaderStageIOSlot, and route the Metal, Vulkan, and GLES backends through it instead of three independent switches over the slot's scalar type, bit width, and component count.

This is behavior preserving. Only the formats reflection produces today are reachable, and each backend keeps its current support set, so GLES still rejects half-float and 32-bit integer attributes. Boolean vertex inputs, which are not expressible in a vertex stage, now map to invalid on every backend.

Related to flutter#186309.
@bdero bdero force-pushed the bdero/impeller-vertex-attribute-format branch from 92f6155 to ef4c1b4 Compare June 27, 2026 20:37
@bdero bdero added the CICD Run CI/CD label Jun 27, 2026
@bdero bdero marked this pull request as ready for review June 27, 2026 21:08

@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 introduces a unified VertexAttributeFormat enum and a GetVertexAttributeFormat helper on ShaderStageIOSlot to centralize vertex attribute format derivation. The GLES, Metal, and Vulkan backends are updated to use this new format, simplifying their vertex format mapping logic, and corresponding unit tests are added. Feedback suggests capturing vec_size by value instead of capturing this in the lambda within GetVertexAttributeFormat to avoid potential compilation issues on older compilers in a constexpr context.

Comment thread engine/src/flutter/impeller/core/shader_types.h Outdated
Avoids capturing this in the constexpr lambda, which can fail
compile-time evaluation on some compilers.
@bdero bdero added CICD Run CI/CD and removed CICD Run CI/CD labels Jun 27, 2026
@bdero bdero requested a review from gaaclarke June 29, 2026 07:38

@gaaclarke gaaclarke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@gaaclarke gaaclarke requested a review from walley892 June 29, 2026 23:24
@gaaclarke

Copy link
Copy Markdown
Member

@walley892 I'm confused, I can't remember if bdero needs 2 reviews or not. If so can you please provide a second review. LGTM.

@bdero

bdero commented Jun 30, 2026

Copy link
Copy Markdown
Member Author

I'm confused, I can't remember if bdero needs 2 reviews or not. If so can you please provide a second review. LGTM.

I only need one. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD e: impeller Impeller rendering backend issues and features requests engine flutter/engine related. See also e: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants