Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Gets the DPI for all awareness mode and older Windows versions #15951

Merged
merged 10 commits into from
Jan 28, 2020

Conversation

franciscojma86
Copy link
Contributor

@franciscojma86 franciscojma86 commented Jan 23, 2020

A continuation of #12735.

Looks for the most modern API GetDpiForWindow first, but falls back to older APIs if not available. If none of the APIs are available, default to a 96 DPI.

Removed existing "SetDpi*" APIs since the DPI awareness should be set by the runner app. Also removed EnableNonClientDpiScaling since it should be called when the top-level window is created, which happens in the runner app.

Tested: Changed the runner's manifest to all awareness modes, and tried all possible APIs. Haven't tested in non-Windows 10 machines.

Related to google/flutter-desktop-embedding#572

Copy link
Contributor

@stuartmorgan-g stuartmorgan-g left a comment

Choose a reason for hiding this comment

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

Just minor nits.

@clarkezone
Copy link

Had a quick skim through but need to do a more careful review. One alarm-bell is that you appear to no longer be enabling non-client scaling which means you aren’t leveraging PerMonitor V2 on up-level machines which is a regression to what’s checked in. Is that a deliberate decision?

return;
}

if (!AssignProcAddress(user32_module_, "SetProcessDpiAwarenessContext",
set_process_dpi_awareness_context_)) {
return;

Choose a reason for hiding this comment

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

SetProcessDpiAwarenessContext is the most recent version of the API.. any reason why you chose not to use this if available?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just removed it since it's not being used anywhere, and we would want the dpi awareness to be set by the runner app, in the manifest anyway. Is there any reason to keep it?

Choose a reason for hiding this comment

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

OK, yes good point :-)

@franciscojma86
Copy link
Contributor Author

Yeah, I left a comment in the description, but should've pointed it out in the code. I removed it since it can only be called for the creation of the top-level window, which is created in the runner. As far as I can tell, that wall wasn't really doing anything. Do I understand that correctly?

@clarkezone
Copy link

Yes, good point, I was reading this in isolation and not considering the runner side of the fence.

@franciscojma86 franciscojma86 added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Jan 24, 2020
@fluttergithubbot fluttergithubbot merged commit 0de80f4 into flutter:master Jan 28, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 28, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 28, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 28, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 29, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 29, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jan 29, 2020
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Jan 29, 2020
flutter/engine@c4229bf...ec32966

git log c4229bf..ec32966 --first-parent --oneline
2020-01-28 [email protected] Revert "Always make gpu thread different from platform thread regardless of platform view (#16068)" (flutter/engine#16161)
2020-01-28 [email protected] Roll fuchsia/sdk/core/mac-amd64 from gNitp... to 5fMtM... (flutter/engine#16153)
2020-01-28 [email protected] Revert "Disable setting a library tag handler." (flutter/engine#16157)
2020-01-28 [email protected] Always make gpu thread different from platform thread regardless of platform view (flutter/engine#16068)
2020-01-28 [email protected] Add test to ensure that concurrent message loops have at least one workers. (flutter/engine#16074)
2020-01-28 [email protected] Revert breaking PRs (flutter/engine#16148)
2020-01-28 [email protected] Roll fuchsia/sdk/core/linux-amd64 from 8Ns10... to 2rLoq... (flutter/engine#15971)
2020-01-28 [email protected] Gets the DPI for all awareness mode and older Windows versions (flutter/engine#15951)
2020-01-28 [email protected] Hold a mutex when updating all CanPostTaskToAllNativeThreads::Captures members. (flutter/engine#16085)
2020-01-28 [email protected] Disable setting a library tag handler. (flutter/engine#16086)
2020-01-28 [email protected] Web PargraphStyle TextHeightBehavior integration (flutter/engine#16075)
2020-01-28 [email protected] Fix flake by making thread ID tracking in CanPostTaskToAllNativeThreads thread safe. (flutter/engine#16081)
2020-01-28 [email protected] Remove buggy assertion in EmbedderTest::CanPostTaskToAllNativeThreads. (flutter/engine#16071)
2020-01-27 [email protected] Remove tonic/platform. (flutter/engine#16062)
2020-01-27 [email protected] Allow embedders to schedule a callback on all engine managed threads. (flutter/engine#15980)
2020-01-27 [email protected] Engine/LibTxt/dart:ui impl of TextHeightBehavior (flutter/engine#15087)
2020-01-27 [email protected] Roll src/third_party/skia f1b2b42613cb..9c1d30dd163e (8 commits) (flutter/engine#16060)
2020-01-27 [email protected] Remove stale recipe changelog. (flutter/engine#15985)


If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected] on the revert to ensure that a human
is aware of the problem.

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
NoamDev added a commit to NoamDev/engine that referenced this pull request Feb 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants