Makes impeller the default renderer for windows. by gaaclarke · Pull Request #188140 · flutter/flutter

gaaclarke · 2026-06-18T00:35:09Z

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.

gemini-code-assist

Code Review

This pull request enables Impeller by default on Windows and updates the integration and unit tests accordingly, including adding tests for disabling Impeller via project flags and command-line switches. Feedback recommends explicitly verifying that the VM service completer completes in the integration tests to prevent false positives on timeouts, and simplifying a redundant conditional check in the engine initialization code.

gemini-code-assist · 2026-06-18T00:36:54Z

  // Check for impeller support.
  auto& switches = project_->GetSwitches();
-  bool enable_impeller = false;
+  bool enable_impeller = true;


Since enable_impeller is now initialized to true by default, the subsequent if block checking for FlutterImpellerSwitch::Enabled is redundant. We can simplify this entire block to a single line:

bool enable_impeller = project_->impeller_switch() != FlutterImpellerSwitch::Disabled;

This is cleaner, more concise, and avoids redundant checks.

References

Suggest simplification and refactoring to enhance readability and maintainability (Repository Style Guide, lines 30 and 34). ^(link)

loic-sharma

LGTM

gaaclarke · 2026-06-18T00:46:00Z

oops, i meant to make this a draft. the test isn't passing yet

flutter-dashboard · 2026-06-18T00:49:01Z

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

message

gaaclarke · 2026-06-18T01:26:49Z

@loic-sharma thanks for the quick review, i've responded to gemini's feedback, we should be good now

gemini-code-assist

Code Review

This pull request enables Impeller by default on Windows and updates the corresponding integration and unit tests to verify default enablement, disabling via project flag, and disabling via command-line flag. A review comment points out a copy-paste error in the integration test where a timeout error message incorrectly references "Step 2" instead of "Step 3".

gemini-code-assist · 2026-06-18T01:27:06Z

+        if (!completer.isCompleted) {
+          res = TaskResult.failure('Step 2 timed out waiting for VM service');
+          return;
+        }


In Step 3, the timeout error message still refers to "Step 2". It should be updated to "Step 3" to avoid confusion when debugging test failures.

Suggested change

if (!completer.isCompleted) {

res = TaskResult.failure('Step 2 timed out waiting for VM service');

return;

}

if (!completer.isCompleted) {

res = TaskResult.failure('Step 3 timed out waiting for VM service');

return;

}

gaaclarke · 2026-06-18T15:19:52Z

whoops, there are apropos test failures

gaaclarke · 2026-06-18T16:03:24Z

I filed a followup issue to fix the tests that require skia's software renderer at a later date: #188173

TLDR It's probably easiest to start linking in swiftshader.

loic-sharma · 2026-06-18T20:22:14Z

  properties_.assets_path = context.GetAssetsPath().c_str();
  properties_.icu_data_path = context.GetIcuDataPath().c_str();
  properties_.aot_library_path = context.GetAotLibraryPath().c_str();
+  properties_.impeller_switch = DisabledImpeller;


For all these DisabledImpeller lines, could we add a TODO comment that links to #188173?

I may have the PR with the fix up for review before the new CI run can finish running from me adding the TODO comment haha. I'm just cleaning it up right now.

loic-sharma

LGTM but could you add TODO comments where we force DisabledImpeller?

gaaclarke · 2026-06-18T20:32:32Z

LGTM but could you add TODO comments where we force DisabledImpeller?

Talked offline, omitting TODO since the fix is already going into review as a followup: #188188

flutter/flutter@15963bc...707e50f 2026-06-19 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 1E2qOlNnC2Ucn-1oV... to 9E-AJV4lUJH_Fmw66... (flutter/flutter#188253) 2026-06-19 engine-flutter-autoroll@skia.org Roll Dart SDK from d5c2c0ae32b4 to ae150461f388 (2 revisions) (flutter/flutter#188248) 2026-06-19 engine-flutter-autoroll@skia.org Roll Dart SDK from 51ef8e837416 to d5c2c0ae32b4 (1 revision) (flutter/flutter#188239) 2026-06-18 30870216+gaaclarke@users.noreply.github.com Makes impeller the default renderer for windows. (flutter/flutter#188140) 2026-06-18 engine-flutter-autoroll@skia.org Roll Dart SDK from 5883736e7670 to 51ef8e837416 (1 revision) (flutter/flutter#188181) 2026-06-18 engine-flutter-autoroll@skia.org Roll Skia from 1ae2466c9ea5 to 5fbb9bbd889c (1 revision) (flutter/flutter#188177) 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 bmparr@google.com,stuartmorgan@google.com 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

fixes flutter#187737 ## 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. - [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.  [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

Makes impeller the default renderer for windows.

1b21cad

gaaclarke requested a review from a team as a code owner June 18, 2026 00:35

flutter-dashboard Bot added the CICD Run CI/CD label Jun 18, 2026

github-actions Bot added engine flutter/engine related. See also e: labels. platform-windows Building on or for Windows specifically a: desktop Running on desktop team-windows Owned by the Windows platform team labels Jun 18, 2026

gemini-code-assist Bot reviewed Jun 18, 2026

View reviewed changes

loic-sharma previously approved these changes Jun 18, 2026

View reviewed changes

gaaclarke marked this pull request as draft June 18, 2026 00:46

made sure we saw the vm service before declaring we didn't see impeller

5050d25

message

gaaclarke dismissed loic-sharma’s stale review via 5050d25 June 18, 2026 01:25

github-actions Bot removed the CICD Run CI/CD label Jun 18, 2026

gaaclarke marked this pull request as ready for review June 18, 2026 01:26

gaaclarke added the CICD Run CI/CD label Jun 18, 2026

gaaclarke requested a review from loic-sharma June 18, 2026 01:26

gemini-code-assist Bot reviewed Jun 18, 2026

View reviewed changes

gaaclarke added 2 commits June 18, 2026 08:48

disabled impeller for some tests

efbfb6a

specifies skia should use software renderer

e3f1dc4

github-actions Bot added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) and removed CICD Run CI/CD labels Jun 18, 2026

gaaclarke mentioned this pull request Jun 18, 2026

[windows]: some flutter_windows_unittests need skia's software renderer #188173

Closed

gaaclarke added the CICD Run CI/CD label Jun 18, 2026

loic-sharma reviewed Jun 18, 2026

View reviewed changes

loic-sharma approved these changes Jun 18, 2026

View reviewed changes

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 18, 2026

auto-submit Bot added this pull request to the merge queue Jun 18, 2026

Merged via the queue into flutter:master with commit 1f213bb Jun 18, 2026
206 checks passed

flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 18, 2026

This was referenced Jun 19, 2026

Roll Flutter from 15963bc04c2b to 707e50f2f9e4 (6 revisions) flutter/packages#11944

Closed

Roll Flutter from 15963bc04c2b to 707e50f2f9e4 (6 revisions) flutter/packages#11945

Merged

Uh oh!

Conversation

gaaclarke commented Jun 18, 2026

Pre-launch Checklist

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

gemini-code-assist Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

loic-sharma left a comment

Choose a reason for hiding this comment

Uh oh!

gaaclarke commented Jun 18, 2026

Uh oh!

flutter-dashboard Bot commented Jun 18, 2026

Uh oh!

gaaclarke commented Jun 18, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

gaaclarke commented Jun 18, 2026

Uh oh!

gaaclarke commented Jun 18, 2026

Uh oh!

loic-sharma Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

gaaclarke Jun 18, 2026

Choose a reason for hiding this comment

Uh oh!

loic-sharma left a comment

Choose a reason for hiding this comment

Uh oh!

gaaclarke commented Jun 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants