Skip to content

Fixes metal vec3 uniform padding#181340

Merged
auto-submit[bot] merged 16 commits into
flutter:masterfrom
gaaclarke:metal-vec3-padding
Jan 26, 2026
Merged

Fixes metal vec3 uniform padding#181340
auto-submit[bot] merged 16 commits into
flutter:masterfrom
gaaclarke:metal-vec3-padding

Conversation

@gaaclarke

@gaaclarke gaaclarke commented Jan 22, 2026

Copy link
Copy Markdown
Member

fixes #180873

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

@github-actions github-actions Bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Jan 22, 2026
@fluttergithubbot

Copy link
Copy Markdown
Contributor

An existing Git SHA, 89cef184de21c39d5957838e6bd79f50c248f416, was detected, and no actions were taken.

To re-trigger presubmits after closing or re-opeing a PR, or pushing a HEAD commit (i.e. with --force) that already was pushed before, push a blank commit (git commit --allow-empty -m "Trigger Build") or rebase to continue.

entry_point_name);

Reflector::Options reflector_options;
reflector_options.target_platform = GetParam();

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is just a residual change from when gemini was fiddling with the compiler tests. I kept it since it's a good change.

@gaaclarke gaaclarke marked this pull request as ready for review January 23, 2026 02:13
@gaaclarke gaaclarke requested review from b-luk and walley892 January 23, 2026 02:13

@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 addresses an issue with vec3 uniform padding on the Metal backend. The core of the fix involves updating the shader reflector to correctly account for Metal's specific alignment rules for vec3 and mat3 types. Additionally, the code is refactored to use a more descriptively named padding_layout instead of struct_layout, and uniform emplacement is optimized to avoid creating an intermediate buffer. My review identifies a critical bug in the uniform size calculation that would lead to incorrect buffer allocations, and a minor code cleanup opportunity to remove unused variables.

Comment on lines +382 to +391
size_t floats_per_element = 0;
size_t padding_floats = 0;
if (spir_type.vecsize == 3 && spir_type.columns == 1) {
// float3
floats_per_element = 3;
padding_floats = 1;
} else if (spir_type.vecsize == 3 && spir_type.columns == 3) {
// float3x3
floats_per_element = 9;
}

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.

medium

The variables floats_per_element and padding_floats are declared and assigned values but are never used later in the function. They should be removed to improve code clarity and avoid confusion.

@b-luk b-luk 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.

I'm not very familiar with this code, so I just have a bunch of nits for you and don't have any higher level thoughts on the overall approach. Maybe walley892 can probably be helpful for that.

Comment thread engine/src/flutter/impeller/entity/contents/runtime_effect_contents.cc Outdated
Comment thread engine/src/flutter/impeller/entity/contents/runtime_effect_contents.cc Outdated
Comment thread engine/src/flutter/impeller/compiler/reflector.cc
Comment thread engine/src/flutter/impeller/compiler/types.h Outdated
Comment thread engine/src/flutter/impeller/compiler/types.h Outdated
@gaaclarke gaaclarke requested a review from b-luk January 23, 2026 19:52
Comment thread engine/src/flutter/impeller/compiler/reflector.cc

@walley892 walley892 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.

I'm not sure of the overall approach in the long term, but this looks good for now. Could of comments

Comment thread engine/src/flutter/impeller/compiler/types.h
Comment on lines +16 to +18
if (type == kStruct) {
size += sizeof(float) * padding_layout.size();
}

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.

Can you document that GetSize has two different meanings?

myStruct::GetSize is the total physical size of the thing. I.e, what we send to the GPU including padding

myArrayUniform::GetSize is the logical size of the thing, not what we actually send over to the GPU.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yea, I'm not a huge fan of this either. I'll spend some time trying to detangle this. Unfortunately the docstring is kind of ambiguous. This is what I'd expect the implementation to be:

size_t RuntimeUniformDescription::GetSize() const {
  size_t size = 0;
  if (padding_layout.empty()) {
    size = dimensions.rows * dimensions.cols * bit_width / 8u;
  } else {
    size = sizeof(float) * padding_layout.size();
  }
  if (array_elements.value_or(0) > 0) {
    // Covered by check on the line above.
    // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
    size *= array_elements.value();
  }
  return size;
}

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.

If it works it works for now I guess

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ugh, this is bothering me. For the vec3array in the test the current GetSize returns 24 bytes, but the proposed one returns 32 bytes. This causes the test to fail. This is not working as intended and shouldn't be landed until this is addressed. My brain is spent for now. I'll look monday =(

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Okay, got it sorted out. There is a difference between the size a uniform takes up in the float buffer from dart and the size it will take up on the GPU because there is no padding in the dart float buffer. I've gotten rid of the oddity and split the names apart. This should clear everything up. I'm not 100% happy with it but this gives the next person looking at the code an understanding of what is actually going on.

Comment thread engine/src/flutter/impeller/core/runtime_types.h Outdated
Comment thread engine/src/flutter/impeller/compiler/types.h
Comment thread engine/src/flutter/impeller/runtime_stage/runtime_stage_types.fbs Outdated

@b-luk b-luk 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.

lgtm as long as walley approves

@QuncCccccc

QuncCccccc commented Jan 26, 2026

Copy link
Copy Markdown
Contributor

Hi I'm the gardener this week:) and when I'm investigating the closed tree, I noticed there are some skia gold failures showing up. Could you help triage the images?
Screenshot 2026-01-26 at 2 53 02 PM

--EDIT:
Ah sorry the tree is green now but seems the gold website still shows un-triaged images😂.

@gaaclarke

Copy link
Copy Markdown
Member Author

Here's a link to those golden failures: https://flutter-gold.skia.org/search?blame=001000027422&corpus=flutter

There are definitely some problematic images. They are all in vulkan though, this test should have affected metal only. I'll see if I can reproduce them locally.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 27, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 27, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 27, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 27, 2026
@gaaclarke

Copy link
Copy Markdown
Member Author

Reason for revert: golden failures and known logical errors that didn't have test coverage

@gaaclarke gaaclarke added the revert Autorevert PR (with "Reason for revert:" comment) label Jan 27, 2026
auto-submit Bot pushed a commit that referenced this pull request Jan 27, 2026
@auto-submit auto-submit Bot removed the revert Autorevert PR (with "Reason for revert:" comment) label Jan 27, 2026
github-merge-queue Bot pushed a commit that referenced this pull request Jan 27, 2026
<!-- start_original_pr_link -->
Reverts: #181340
<!-- end_original_pr_link -->
<!-- start_initiating_author -->
Initiated by: gaaclarke
<!-- end_initiating_author -->
<!-- start_revert_reason -->
Reason for reverting: golden failures and known logical errors that
didn't have test coverage
<!-- end_revert_reason -->
<!-- start_original_pr_author -->
Original PR Author: gaaclarke
<!-- end_original_pr_author -->

<!-- start_reviewers -->
Reviewed By: {walley892, b-luk}
<!-- end_reviewers -->

<!-- start_revert_body -->
This change reverts the following previous change:
fixes #180873

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [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.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[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

<!-- end_revert_body -->

Co-authored-by: auto-submit[bot] <[email protected]>
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 27, 2026
@gaaclarke

Copy link
Copy Markdown
Member Author

@QuncCccccc I reverted this change and marked those goldens as negative

@QuncCccccc

Copy link
Copy Markdown
Contributor

@QuncCccccc I reverted this change and marked those goldens as negative

Thanks!

auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Jan 27, 2026
flutter/flutter@7165649...dfd92b7

2026-01-27 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Fixes metal vec3 uniform padding (#181340)" (flutter/flutter#181552)
2026-01-27 [email protected] Change update_engine_version documenation (flutter/flutter#181508)
2026-01-27 [email protected] Roll Packages from e712bfa to e37af11 (7 revisions) (flutter/flutter#181544)
2026-01-27 [email protected] Roll FreeType to 2.14.1 (flutter/flutter#181428)
2026-01-27 [email protected] Roll Skia from 566d202962d3 to c2754838b077 (3 revisions) (flutter/flutter#181536)
2026-01-27 [email protected] [Material] modernize time picker components with Dart 3 switch expressions (flutter/flutter#181356)
2026-01-27 [email protected] Roll Skia from 55a87b50a368 to 566d202962d3 (1 revision) (flutter/flutter#181528)
2026-01-27 [email protected] Roll Dart SDK from f55d4538469a to 4c7cb0a1d07d (2 revisions) (flutter/flutter#181526)
2026-01-27 [email protected] Replace `pub run` mentions with `dart run` (flutter/flutter#181317)
2026-01-27 [email protected] Roll Skia from 1a15ed6b17f2 to 55a87b50a368 (1 revision) (flutter/flutter#181523)
2026-01-27 [email protected] Marks linux_chrome_dev_mode to be unflaky (flutter/flutter#181352)
2026-01-27 [email protected] Roll Skia from f2c1aa140b79 to 1a15ed6b17f2 (2 revisions) (flutter/flutter#181514)
2026-01-27 [email protected] Fix Range Slider issue where indescrete ranges lead to out of range r… (flutter/flutter#181082)
2026-01-26 [email protected] Roll Dart SDK from ba23b5ed0a97 to f55d4538469a (2 revisions) (flutter/flutter#181512)
2026-01-26 [email protected] Add `alignment` to `SizeTransition` (flutter/flutter#177895)
2026-01-26 [email protected] Roll Fuchsia Linux SDK from T4qTEa3T5CCSCIoJY... to akraNGn2lw4T1msgZ... (flutter/flutter#181509)
2026-01-26 [email protected] Roll Skia from be280d242a60 to f2c1aa140b79 (3 revisions) (flutter/flutter#181504)
2026-01-26 [email protected] Fixes metal vec3 uniform padding (flutter/flutter#181340)
2026-01-26 [email protected] Fixes duplicated import in accessibility test library (flutter/flutter#181506)
2026-01-26 [email protected] [Reland] Don't strip symbols from `libapp.so` on android by default (flutter/flutter#181275)
2026-01-26 [email protected] Fix Gradle path in Android Platform Embedder README. (flutter/flutter#181501)
2026-01-26 [email protected] Remove unused `ActivityLifecycleListener` class (flutter/flutter#181406)

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 [email protected],[email protected] 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
github-merge-queue Bot pushed a commit that referenced this pull request Jan 28, 2026
Re-landing of #181340
incorporating #181550.

Vec3 uniforms on the metal backend are vec4-aligned. This PR accounts
for that padding.

---------

Co-authored-by: gaaclarke <[email protected]>
flutter-zl pushed a commit to flutter-zl/flutter that referenced this pull request Feb 10, 2026
fixes flutter#180873

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [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.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[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
flutter-zl pushed a commit to flutter-zl/flutter that referenced this pull request Feb 10, 2026
…181552)

<!-- start_original_pr_link -->
Reverts: flutter#181340
<!-- end_original_pr_link -->
<!-- start_initiating_author -->
Initiated by: gaaclarke
<!-- end_initiating_author -->
<!-- start_revert_reason -->
Reason for reverting: golden failures and known logical errors that
didn't have test coverage
<!-- end_revert_reason -->
<!-- start_original_pr_author -->
Original PR Author: gaaclarke
<!-- end_original_pr_author -->

<!-- start_reviewers -->
Reviewed By: {walley892, b-luk}
<!-- end_reviewers -->

<!-- start_revert_body -->
This change reverts the following previous change:
fixes flutter#180873

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [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.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[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

<!-- end_revert_body -->

Co-authored-by: auto-submit[bot] <[email protected]>
flutter-zl pushed a commit to flutter-zl/flutter that referenced this pull request Feb 10, 2026
Re-landing of flutter#181340
incorporating flutter#181550.

Vec3 uniforms on the metal backend are vec4-aligned. This PR accounts
for that padding.

---------

Co-authored-by: gaaclarke <[email protected]>
walley892 added a commit to walley892/flutter that referenced this pull request Feb 17, 2026
Re-landing of flutter#181340
incorporating flutter#181550.

Vec3 uniforms on the metal backend are vec4-aligned. This PR accounts
for that padding.

---------

Co-authored-by: gaaclarke <[email protected]>
gaaclarke added a commit to gaaclarke/flutter that referenced this pull request Feb 18, 2026
Reland `Enabled some disabled impeller fragment shader dart tests` (flutter#180788)

relands flutter#180759

changes since revert:
1) waits for async tests now
1) keeps 2 tests that don't pass on impeller disabled

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [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.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[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

Enables fragment shader test for impeller (fixes mat2 on vulkan) (flutter#181013)

This rearranges the packing of mat2 fields in structs which was causing
an existing fragment shader test to fail.

The test still doesn't work on Metal, but this PR fixes it for Vulkan. I
suspect the fix for metal may be similar but since the uniforms aren't
in a virtual struct the code isn't hitting this fix.

issue: flutter#180873

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [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.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[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

Account for vec3 padding in Metal (flutter#181563)

Re-landing of flutter#181340
incorporating flutter#181550.

Vec3 uniforms on the metal backend are vec4-aligned. This PR accounts
for that padding.

---------

Co-authored-by: gaaclarke <[email protected]>

Fixes getUniformX for Vulkan (flutter#181286)

Does what it says on the tin!

This PR adds struct member information to the runtime flatbuffer format.
This allows dart code to introspect structs at runtime.

Also modifies the runtime_stage tests to verify formats for all
supported uniform types.

Bubbles up struct member information to dart, and uses that information
to grab struct members in `getUniformX` related functions. This is
necessary on Vulkan because all uniforms are packaged into a single
struct. Also re-enables tests

- [ ] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [ ] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [ ] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [ ] I signed the [CLA].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [ ] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [ ] All existing and new tests are passing.

Organize and update fragment shader uniform tests. (flutter#181822)

Adds more comprehensive testing for setFloat and getUniform* on fragment
shaders.

Deletes redundant tests.

Fixes getUniformVecX indexing errors.

Update web ui fragment shader tests (flutter#181877)

Adds a bunch of tests for uniform setting functionality for custom
fragment shaders on the web.

Deletes redundant tests.

Fixes a discovered issue in the uniform offset calculation. We were
previously using the `location`, which is the integer offset of the
uniform, not the offset in floats.
gaaclarke added a commit to gaaclarke/flutter that referenced this pull request Feb 18, 2026
Reland `Enabled some disabled impeller fragment shader dart tests` (flutter#180788)

relands flutter#180759

changes since revert:
1) waits for async tests now
1) keeps 2 tests that don't pass on impeller disabled

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [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.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[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

Enables fragment shader test for impeller (fixes mat2 on vulkan) (flutter#181013)

This rearranges the packing of mat2 fields in structs which was causing
an existing fragment shader test to fail.

The test still doesn't work on Metal, but this PR fixes it for Vulkan. I
suspect the fix for metal may be similar but since the uniforms aren't
in a virtual struct the code isn't hitting this fix.

issue: flutter#180873

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [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.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

**Note**: The Flutter team is currently trialing the use of [Gemini Code
Assist for
GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code).
Comments from the `gemini-code-assist` bot should not be taken as
authoritative feedback from the Flutter team. If you find its comments
useful you can update your code accordingly, but if you are unsure or
disagree with the feedback, please feel free to wait for a Flutter team
member's review for guidance on which automated comments should be
addressed.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[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

Account for vec3 padding in Metal (flutter#181563)

Re-landing of flutter#181340
incorporating flutter#181550.

Vec3 uniforms on the metal backend are vec4-aligned. This PR accounts
for that padding.

---------

Co-authored-by: gaaclarke <[email protected]>

Fixes getUniformX for Vulkan (flutter#181286)

Does what it says on the tin!

This PR adds struct member information to the runtime flatbuffer format.
This allows dart code to introspect structs at runtime.

Also modifies the runtime_stage tests to verify formats for all
supported uniform types.

Bubbles up struct member information to dart, and uses that information
to grab struct members in `getUniformX` related functions. This is
necessary on Vulkan because all uniforms are packaged into a single
struct. Also re-enables tests

- [ ] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [ ] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [ ] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [ ] I signed the [CLA].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [ ] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [ ] All existing and new tests are passing.

Organize and update fragment shader uniform tests. (flutter#181822)

Adds more comprehensive testing for setFloat and getUniform* on fragment
shaders.

Deletes redundant tests.

Fixes getUniformVecX indexing errors.

Update web ui fragment shader tests (flutter#181877)

Adds a bunch of tests for uniform setting functionality for custom
fragment shaders on the web.

Deletes redundant tests.

Fixes a discovered issue in the uniform offset calculation. We were
previously using the `location`, which is the integer offset of the
uniform, not the offset in floats.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

enable 'FragmentShader shader with array uniforms renders correctly' for impeller

5 participants