Repository navigation
[windows] Add OpenGL golden test against ANGLE at D3D11 feature level 10_0 - #194321
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to cap the D3D11 feature level on Windows using the FLUTTER_WINDOWS_MAX_D3D_FEATURE_LEVEL environment variable, enabling testing of older GPU emulation. It adds a new CI target, integration tests, and C++ engine support to parse and apply this feature level cap. Feedback was provided regarding handling empty environment variables to prevent false-positive warnings, and replacing ASSERT_TRUE with EXPECT_TRUE in unit tests to ensure environment cleanup is always executed.
github-actions
Bot
added
engine
gaaclarke
left a comment
gaaclarke
left a comment
There was a problem hiding this comment.
Nice! If we want to keep the environment variable route I'd like the engine to print out something in flutter_windows_engine.cc that the integration test looks for to verify that we've actually successfully communicated the d3d level. The way this is written now someone could easily remove that line in flutter_windows_engine.cc and no one would notice that we severed the communication. Better yet if the thing that printed out was something like a glGetInteger that verifies the setting was used.
|
|
||
| // Parses a feature level written as "<major>_<minor>", such as "10_0". | ||
| // Returns nullopt if |value| is not in that form. | ||
| std::optional<egl::D3DFeatureLevel> ParseD3DFeatureLevel( |
There was a problem hiding this comment.
return absl::StatusOregl::D3DFeatureLevel
| FML_LOG(WARNING) << "Capping ANGLE's D3D11 feature level to " | ||
| << max_d3d_feature_level->major << "_" | ||
| << max_d3d_feature_level->minor << "."; |
There was a problem hiding this comment.
I was thinking we want to assert that we have this at the dart level. That's what we do with the impeller backend controls. What we have now is fine as long as we are asserting we are logging that this is working. I think we should do 1 of 2 things:
- Assert this log message at the dart level
- Add a c++ test that makes sure this gets printed
There was a problem hiding this comment.
considering you latest comment, it sounds like adding the assert for the print in the c++ test will be easiest
There was a problem hiding this comment.
Done. Added an assert for the log line in the c++ test.
If you still think we should also check for this in the dart integration test, let me know and I can add it. But it adds like ~80 lines to integration_tests.dart to make this one single test have custom code to do a pre-test flutter run and check the logs.
I added a log line to flutter_windows_engine which prints the requested feature level cap. I looked into making the integration test look for this log line, but that turns out to be difficult because the integration tests run with |
flutter-dashboard
Bot
removed
the
autosubmit
flutter/flutter@5c7e45a...a3d71d8 2026-10-16 [email protected] 0 Initialize variables in the DlComplexityPathReceiver helper (flutter/flutter#194540) 2026-10-16 [email protected] Roll Fuchsia Test Scripts from V-qQL4158Nw6L-QLi... to 2T18KC7-LDQtmQiLP... (flutter/flutter#194527) 2026-10-16 [email protected] Remove BorderMaskBlur (flutter/flutter#194508) 2026-10-16 [email protected] Roll Skia from a577264dc087 to 470b8816a261 (1 revision) (flutter/flutter#194526) 2026-10-16 [email protected] [ios] Use DFS traversal to find engines of a scene automatically (rather than manually manage the engines list) (flutter/flutter#192457) 2026-10-16 [email protected] [Skia][DisplayList] Map local bounds through the local matrix in DlLocalMatrixImageFilter (flutter/flutter#193963) 2026-10-16 [email protected] [framework] Fix avoid_setters_without_getters warnings on mixin declarations (flutter/flutter#193856) 2026-10-16 [email protected] Roll Dart SDK from 84686ea04815 to 55a734a01fb1 (3 revisions) (flutter/flutter#194522) 2026-10-16 [email protected] Roll Skia from d131b7789482 to a577264dc087 (7 revisions) (flutter/flutter#194514) 2026-10-16 [email protected] Roll Fuchsia GN SDK from PDJz3b3CL31uoLOkD... to kFdyRnojbVy0cxvRZ... (flutter/flutter#194510) 2026-10-16 [email protected] Roll GN to f9eca75ae554 (flutter/flutter#194471) 2026-10-16 [email protected] refactor(flutter_tools): migrate RunCommand and RunCommandBase to OptionDescriptor and OptionBundle (flutter/flutter#194362) 2026-10-15 [email protected] refactor(flutter_tools): finish TestCommand OptionDescriptor and OptionBundle migration (flutter/flutter#194361) 2026-10-15 [email protected] [iOS][a11y] Fix VoiceOver announcement for unfocused password fields (flutter/flutter#192851) 2026-10-15 [email protected] Roll pub packages (flutter/flutter#194477) 2026-10-15 [email protected] refactor(flutter_tools): finish DriveCommand OptionDescriptor migration (flutter/flutter#194360) 2026-10-15 [email protected] [flutter_tools] Roll package:vm_service to 15.4.0 (flutter/flutter#194467) 2026-10-15 [email protected] refactor(flutter_tools): migrate AttachCommand to OptionDescriptor and OptionBundle (flutter/flutter#194359) 2026-10-15 [email protected] [flutter_tools] Inject dependencies into update-packages and package_map (flutter/flutter#194046) 2026-10-15 [email protected] [impeller] parallelize opengles shader compiles (flutter/flutter#194022) 2026-10-15 [email protected] Roll pub packages (flutter/flutter#194469) 2026-10-15 [email protected] Restrict web dev server to loopback by default (flutter/flutter#187595) 2026-10-15 [email protected] Roll Packages from a1ed23c to da40eaf (33 revisions) (flutter/flutter#194459) 2026-10-15 [email protected] [web] Render soft hyphens on Skwasm (flutter/flutter#193850) 2026-10-15 [email protected] refactor(flutter_tools): migrate AssembleCommand to OptionDescriptor (flutter/flutter#194358) 2026-10-15 [email protected] [windows] Add OpenGL golden test against ANGLE at D3D11 feature level 10_0 (flutter/flutter#194321) 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
… 10_0 (flutter#194321) Adds a test that runs the existing engine_integration_golden_test, but with a D3D11 10_0 feature level cap. This is the follow up PR mentioned in flutter#194114. Together these two PRs gives us basic smoke testing coverage on 10_0 of the rendering pipeline from Flutter app code, through Impeller rendering, to presenting on the swapchain. The feature level cap is read directly from the environment variables by `windows/flutter_project_bundle.cc`. I don't know if this is the best way to do this, because it's doing it different than other "engine switches". Reading the environment variable directly is similar to how `FLUTTER_LINUX_RENDERER` is read in the [linux embedder](https://github.com/flutter/flutter/blob/debf408b622cb24c5992cff3d9c9142be79eab40/engine/src/flutter/shell/platform/linux/fl_engine.cc#L696). An alternative would be to expose it as a flag in flutter tool and piping it all the way through to here. That would be a lot more changes and plumbing, and it makes the flag a little more public, but if you prefer that way, let me know and I'll change this. I ran this locally on a Windows VM, with a `host_debug_unopt` engine build. It caught flutter#194317, so it's already acting as a useful 10_0 smoke test. I confirmed that with the fix (flutter#194319), it runs and passes as expected (but without doing a real skia-gold validation check). Part of flutter#192899 ## 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. - [ ] 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
Adds a test that runs the existing engine_integration_golden_test, but with a D3D11 10_0 feature level cap. This is the follow up PR mentioned in #194114. Together these two PRs gives us basic smoke testing coverage on 10_0 of the rendering pipeline from Flutter app code, through Impeller rendering, to presenting on the swapchain.
The feature level cap is read directly from the environment variables by
windows/flutter_project_bundle.cc. I don't know if this is the best way to do this, because it's doing it different than other "engine switches". Reading the environment variable directly is similar to howFLUTTER_LINUX_RENDERERis read in the linux embedder. An alternative would be to expose it as a flag in flutter tool and piping it all the way through to here. That would be a lot more changes and plumbing, and it makes the flag a little more public, but if you prefer that way, let me know and I'll change this.I ran this locally on a Windows VM, with a
host_debug_unoptengine build. It caught #194317, so it's already acting as a useful 10_0 smoke test. I confirmed that with the fix (#194319), it runs and passes as expected (but without doing a real skia-gold validation check).Part of #192899
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-assistbot 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.