Report a correct display ID in the window metrics event on win32#174156
Report a correct display ID in the window metrics event on win32#174156mattkae merged 8 commits intoflutter:masterfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request correctly adds the display ID to the WindowMetricsEvent on Win32. The refactoring in DisplayMonitor to extract FromMonitor is a good improvement for code clarity and reuse. The changes are applied consistently across the codebase, including updates to constructors, tests, and mocks. I have one suggestion to improve the fallback logic in FlutterWindow::GetDisplay for better logging and clarity in an edge case.
| FlutterWindow::FlutterWindow( | ||
| int width, | ||
| int height, | ||
| std::shared_ptr<DisplayMonitor> const& display_monitor, |
There was a problem hiding this comment.
Hm maybe we should rename this to DisplayManager. "Monitor" is a bit on an overloaded term here, and we're using it for more than just listening to changes.
There was a problem hiding this comment.
Could we do that in another PR? Just want to limit the scope, I can add that to my list as a follow up
There was a problem hiding this comment.
Yup that sounds good to me 👍
There was a problem hiding this comment.
Thanks for fixing this! LGTM but please make sure there's a test somewhere that the display ID is plumbed to the window metric: https://github.com/flutter/flutter/pull/174156/files#r2289414502
Pull Request is not mergeable
…tter#174156) ## What's new? Now that we're sending down "true" display information to the framework, we also need to send down the correct display id in the WindowMetricsEvent for the view so that the display resolves properly. This pull request adds that :) ## How to test Here's a silly app that makes sure that our `Display` is legit: ```dart // Copyright 2014 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. import 'package:flutter/widgets.dart'; void main() => runApp(W()); class W extends StatelessWidget { @OverRide Widget build(BuildContext context) { final x = View.of(context); print(x.display.id); return const Center( child: Text('Hello, world!', key: Key('title'), textDirection: TextDirection.ltr), ); } } ``` ## 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.
…tter#174156) ## What's new? Now that we're sending down "true" display information to the framework, we also need to send down the correct display id in the WindowMetricsEvent for the view so that the display resolves properly. This pull request adds that :) ## How to test Here's a silly app that makes sure that our `Display` is legit: ```dart // Copyright 2014 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. import 'package:flutter/widgets.dart'; void main() => runApp(W()); class W extends StatelessWidget { @OverRide Widget build(BuildContext context) { final x = View.of(context); print(x.display.id); return const Center( child: Text('Hello, world!', key: Key('title'), textDirection: TextDirection.ltr), ); } } ``` ## 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.
flutter/flutter@d2ac021...26bb33b 2025-08-22 [email protected] [HCPP] Clean up overlay layer when last frame had overlay content and current doesn't (flutter/flutter#173881) 2025-08-22 [email protected] Migrate more files to use WidgetStateProperty (flutter/flutter#174176) 2025-08-22 [email protected] Roll Skia from 75fef9fb3ed7 to cb15e1452399 (1 revision) (flutter/flutter#174255) 2025-08-22 [email protected] Roll Skia from 006241a7fbe1 to 75fef9fb3ed7 (1 revision) (flutter/flutter#174254) 2025-08-22 [email protected] Skip wasm build when dry run is disabled and --wasm is not specified. (flutter/flutter#174184) 2025-08-22 [email protected] Roll Dart SDK from c153c5259e62 to 4f9623f024ab (2 revisions) (flutter/flutter#174250) 2025-08-22 [email protected] Roll Skia from d70087007490 to 006241a7fbe1 (2 revisions) (flutter/flutter#174252) 2025-08-22 [email protected] Roll Skia from c09589f7ca69 to d70087007490 (22 revisions) (flutter/flutter#174245) 2025-08-21 [email protected] [ Widget Preview ] Add regression test for issue 173895 (flutter/flutter#174037) 2025-08-21 [email protected] Improve xcresult comment and naming (flutter/flutter#173129) 2025-08-21 [email protected] Update `.gemini/styleguide.md` to encourage `master`-only (flutter/flutter#174065) 2025-08-21 [email protected] [ Widget Preview ] Fix crash when attempting to provide non-const params to a `Preview` (flutter/flutter#174242) 2025-08-21 [email protected] Roll Skia from 721e68fe652a to c09589f7ca69 (12 revisions) (flutter/flutter#174162) 2025-08-21 [email protected] Roll Dart SDK from 0d0a0c394381 to c153c5259e62 (7 revisions) (flutter/flutter#174235) 2025-08-21 [email protected] [ Tool ] Throw `ToolExit` when asset entries use absolute paths (flutter/flutter#174230) 2025-08-21 [email protected] Roll Dart SDK from 0d0a0c394381 to c153c5259e62 (7 revisions) (flutter/flutter#174227) 2025-08-21 [email protected] [ Tool ] Cleanup widget preview and frontend server shutdown (flutter/flutter#173863) 2025-08-21 [email protected] Use an alternative to `git describe` for `master` version resolution (flutter/flutter#174088) 2025-08-21 [email protected] Report a correct display ID in the window metrics event on win32 (flutter/flutter#174156) 2025-08-21 [email protected] Revert "Update the AccessibilityPlugin::Announce method to account fo… (flutter/flutter#174223) 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
…tter#174156) ## What's new? Now that we're sending down "true" display information to the framework, we also need to send down the correct display id in the WindowMetricsEvent for the view so that the display resolves properly. This pull request adds that :) ## How to test Here's a silly app that makes sure that our `Display` is legit: ```dart // Copyright 2014 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. import 'package:flutter/widgets.dart'; void main() => runApp(W()); class W extends StatelessWidget { @OverRide Widget build(BuildContext context) { final x = View.of(context); print(x.display.id); return const Center( child: Text('Hello, world!', key: Key('title'), textDirection: TextDirection.ltr), ); } } ``` ## 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.
…tter#174156) ## What's new? Now that we're sending down "true" display information to the framework, we also need to send down the correct display id in the WindowMetricsEvent for the view so that the display resolves properly. This pull request adds that :) ## How to test Here's a silly app that makes sure that our `Display` is legit: ```dart // Copyright 2014 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. import 'package:flutter/widgets.dart'; void main() => runApp(W()); class W extends StatelessWidget { @OverRide Widget build(BuildContext context) { final x = View.of(context); print(x.display.id); return const Center( child: Text('Hello, world!', key: Key('title'), textDirection: TextDirection.ltr), ); } } ``` ## 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.
…tter#174156) ## What's new? Now that we're sending down "true" display information to the framework, we also need to send down the correct display id in the WindowMetricsEvent for the view so that the display resolves properly. This pull request adds that :) ## How to test Here's a silly app that makes sure that our `Display` is legit: ```dart // Copyright 2014 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. import 'package:flutter/widgets.dart'; void main() => runApp(W()); class W extends StatelessWidget { @OverRide Widget build(BuildContext context) { final x = View.of(context); print(x.display.id); return const Center( child: Text('Hello, world!', key: Key('title'), textDirection: TextDirection.ltr), ); } } ``` ## 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.
…tter#174156) ## What's new? Now that we're sending down "true" display information to the framework, we also need to send down the correct display id in the WindowMetricsEvent for the view so that the display resolves properly. This pull request adds that :) ## How to test Here's a silly app that makes sure that our `Display` is legit: ```dart // Copyright 2014 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. import 'package:flutter/widgets.dart'; void main() => runApp(W()); class W extends StatelessWidget { @OverRide Widget build(BuildContext context) { final x = View.of(context); print(x.display.id); return const Center( child: Text('Hello, world!', key: Key('title'), textDirection: TextDirection.ltr), ); } } ``` ## 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. # Conflicts: # engine/src/flutter/shell/platform/windows/testing/mock_window.cc
…tter#174156) ## What's new? Now that we're sending down "true" display information to the framework, we also need to send down the correct display id in the WindowMetricsEvent for the view so that the display resolves properly. This pull request adds that :) ## How to test Here's a silly app that makes sure that our `Display` is legit: ```dart // Copyright 2014 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. import 'package:flutter/widgets.dart'; void main() => runApp(W()); class W extends StatelessWidget { @OverRide Widget build(BuildContext context) { final x = View.of(context); print(x.display.id); return const Center( child: Text('Hello, world!', key: Key('title'), textDirection: TextDirection.ltr), ); } } ``` ## 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.
What's new?
Now that we're sending down "true" display information to the framework, we also need to send down the correct display id in the WindowMetricsEvent for the view so that the display resolves properly. This pull request adds that :)
How to test
Here's a silly app that makes sure that our
Displayis legit:Pre-launch Checklist
///).