Skip to content

Adds Vulkan sync validation test run - #195596

Merged
auto-submit[bot] merged 2 commits into
flutter:masterfrom
gaaclarke:vulkan-sync-test-shard
Oct 30, 2026
Merged

auto-submit[bot] merged 2 commits into
flutter:masterfrom
gaaclarke:vulkan-sync-test-shard

Conversation

@gaaclarke

@gaaclarke gaaclarke commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

Adds new test run that just runs a handful of tests with vulkan synchronization validation layers on. The synchronization layers are heavy so we opted for this instead of running every tests with them on.

fixes #195502

Pre-launch Checklist

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

If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.

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.

@gaaclarke
gaaclarke force-pushed the vulkan-sync-test-shard branch from b36d37b to 20b3156 Compare October 29, 2026 16:18
@gaaclarke
gaaclarke marked this pull request as ready for review October 29, 2026 16:45

@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 new CI target and test suite for running a select set of Impeller unit tests with Vulkan synchronization validation enabled to detect barrier issues. It adds the necessary command-line switches, updates the Vulkan context initialization to conditionally enable synchronization validation, and defines the test runner logic in Python. The review feedback identifies two instances in the Python test runner script where lines exceed the 80-character limit, violating the Google Python Style Guide, and suggests wrapping them.

Comment on lines +674 to +677
def run_impeller_vulkan_sync_validation_tests(
build_dir: str, build_name: str, executable_filter: typing.Optional[typing.List[str]],
coverage: bool
) -> None:

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

This line exceeds the 80-character limit specified by the Google Python Style Guide. Please wrap the function arguments to adhere to the style guide.

def run_impeller_vulkan_sync_validation_tests(
    build_dir: str,
    build_name: str,
    executable_filter: typing.Optional[typing.List[str]],
    coverage: bool,
) -> None:
References
  1. Python code should follow the Google Python Style Guide, which enforces an 80-character line limit. (link)

xvfb.stop_virtual_x(build_name)

if 'impeller-vulkan-sync-validation' in types:
run_impeller_vulkan_sync_validation_tests(build_dir, args.variant, engine_filter, args.coverage)

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

This line exceeds the 80-character limit specified by the Google Python Style Guide. Please wrap the function call arguments to adhere to the style guide.

Suggested change
run_impeller_vulkan_sync_validation_tests(build_dir, args.variant, engine_filter, args.coverage)
run_impeller_vulkan_sync_validation_tests(
build_dir, args.variant, engine_filter, args.coverage
)
References
  1. Python code should follow the Google Python Style Guide, which enforces an 80-character line limit. (link)

@gaaclarke gaaclarke added the CICD Run CI/CD label Oct 29, 2026
@gaaclarke gaaclarke changed the title Adds Vulkan sync validation test shard Adds Vulkan sync validation test run Oct 29, 2026
@gaaclarke

gaaclarke commented Oct 29, 2026 •

Copy link
Copy Markdown
Member Author

@andywolff andywolff 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. However, looking at the passing led run timeline, it only took 10 seconds to run the actual test part of these tests. I wonder if my information about the cost of this validation is overblown

@gaaclarke gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 30, 2026
@auto-submit
auto-submit Bot added this pull request to the merge queue Oct 30, 2026
Merged via the queue into flutter:master with commit 184243b Oct 30, 2026
24 of 25 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 30, 2026
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Oct 31, 2026
flutter/flutter@5e805bf...ee8b482

2026-10-31 [email protected] Roll Skia from 9bab765df357 to 607de4b035dc (1 revision) (flutter/flutter#195728)
2026-10-31 [email protected] Roll Skia from 284054bec98a to 9bab765df357 (2 revisions) (flutter/flutter#195726)
2026-10-31 [email protected] Roll Skia from 3a3f56f7ddd4 to 284054bec98a (1 revision) (flutter/flutter#195724)
2026-10-31 [email protected] Roll Skia from 466994d27b1e to 3a3f56f7ddd4 (1 revision) (flutter/flutter#195719)
2026-10-30 [email protected] Fix XcodeProjectInterpreter dependency cycle in flutter_tools (flutter/flutter#195694)
2026-10-30 [email protected] Document that Positioned also works inside an Overlay (flutter/flutter#190607)
2026-10-30 [email protected] Roll Skia from ce0b85b46db6 to 466994d27b1e (3 revisions) (flutter/flutter#195711)
2026-10-30 [email protected] Adds Vulkan sync validation test run (flutter/flutter#195596)
2026-10-30 [email protected] [Android] Let the raster thread use every core of a chip without little cores (flutter/flutter#195071)
2026-10-30 [email protected] Roll Skia from 121d04c01e2d to ce0b85b46db6 (6 revisions) (flutter/flutter#195699)
2026-10-30 [email protected] [web] Anchor platform view slots to top-left to fix RTL document layout (flutter/flutter#194601)
2026-10-30 [email protected] Update golden file testing docs (flutter/flutter#195449)
2026-10-30 [email protected] [tool] Migrate Cache.flutterRoot static state to instance configuration (flutter/flutter#190795)
2026-10-30 [email protected] Apply view metrics overrides below the framework (flutter/flutter#194311)
2026-10-30 [email protected] Roll Skia from c0c46ee9f938 to 121d04c01e2d (9 revisions) (flutter/flutter#195687)
2026-10-30 [email protected] Use uniforms in the Porter Duff blend shader instead of specialization constants (flutter/flutter#195458)
2026-10-30 [email protected] [flutter_tools] Inject dependencies into iOS, macOS, and Windows application packages (flutter/flutter#194047)

If this roll has caused a breakage, revert this CL and set the roller
to dry run mode 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
dayanruben pushed a commit to dayanruben/flutter that referenced this pull request Nov 3, 2026
…95689)

> [!NOTE]
> This depends on flutter#195596 to add
the testing infrastructure

This PR adds dependencies to vulkan subpasses when a depth/stencil
buffer is used. It was found to be a problem when enabling the
synchronization validation layers.

fixes flutter#195599

## 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 in-code documentation (doc comments with
`///`).
- [x] If this PR introduces a new feature or capability, I created and
linked a website documentation issue or PR in [flutter/website] (or
verified none is needed).
- [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].

If this change needs to override an active code freeze, provide a
comment explaining why. The code freeze workflow can be overridden by
code reviewers. See pinned issues for any active code freezes with
guidance.

**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
[AI contribution guidelines]:
https://github.com/flutter/flutter/blob/main/CONTRIBUTING.ai.md
[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
[flutter/website]: https://github.com/flutter/website
[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
gmackall pushed a commit to gmackall/flutter that referenced this pull request Nov 3, 2026
Adds new test run that just runs a handful of tests with vulkan
synchronization validation layers on. The synchronization layers are
heavy so we opted for this instead of running every tests with them on.

fixes flutter#195502

## 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 in-code documentation (doc comments with
`///`).
- [x] If this PR introduces a new feature or capability, I created and
linked a website documentation issue or PR in [flutter/website] (or
verified none is needed).
- [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].

If this change needs to override an active code freeze, provide a
comment explaining why. The code freeze workflow can be overridden by
code reviewers. See pinned issues for any active code freezes with
guidance.

**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
[AI contribution guidelines]:
https://github.com/flutter/flutter/blob/main/CONTRIBUTING.ai.md
[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
[flutter/website]: https://github.com/flutter/website
[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
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.

[Impeller] Set up a CI shard focused on vulkan sync validation

2 participants