Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions shell/platform/windows/win32_dpi_helper.cc
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include "flutter/shell/platform/windows/win32_dpi_helper.h"

#include <iostream>

namespace flutter {

namespace {
Expand All @@ -18,7 +20,6 @@ Win32DpiHelper::Win32DpiHelper() {
if (user32_module_ == nullptr) {
return;
}

if (!AssignProcAddress(user32_module_, "EnableNonClientDpiScaling",
enable_non_client_dpi_scaling_)) {
return;
Expand All @@ -33,7 +34,6 @@ Win32DpiHelper::Win32DpiHelper() {
set_process_dpi_awareness_context_)) {
return;
}

permonitorv2_supported_ = true;
}

Expand All @@ -43,6 +43,20 @@ Win32DpiHelper::~Win32DpiHelper() {
}
}

void Win32DpiHelper::SetDpiAwerenessAllVersions() {
if (permonitorv2_supported_) {
SetProcessDpiAwarenessContext(DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2);
} else {
std::cerr << "Per Monitor V2 DPI awareness is not supported on this "
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this logged? It's not an error case, unless I'm not understanding the PR.

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 saw it as more of a fringe case, but it's true it's not an error. Makes sense to remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

"version of Windows. Setting System DPI awareness\n";
AssignProcAddress(user32_module_, "GetDpiForSystem", get_dpi_for_system_);
AssignProcAddress(user32_module_, "SetProcessDpiAware",
set_process_dpi_aware_);

SetProcessDPIAware();

Choose a reason for hiding this comment

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

Whilst it’s tempting to do this I do not recommend it. The host app should be in charge of it’s DPI mode, not flutter. For add to app scenarios you’ll have to support all DPI modes in the fullness of time. Also, it’s not recommended to set DPI programatically, that should be done in the calling app’s manifest. Recommend updating this to sniff the current DPI mode and act accordingly. You’ll have to put this logic in testbed / example tho

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From what I understand, setting the awareness in the application manifest will override this calls anyway. I added the calls to ensure that even if the host didn't provide it in the manifest, we would. I tested with Testbed, by setting a different awareness in the manifest and it took that value over the api call. What do you think?

}
}

bool Win32DpiHelper::IsPerMonitorV2Available() {
return permonitorv2_supported_;
}
Expand All @@ -61,6 +75,10 @@ UINT Win32DpiHelper::GetDpiForWindow(HWND hwnd) {
return get_dpi_for_window_(hwnd);
}

UINT Win32DpiHelper::GetDpiForSystem() {
return get_dpi_for_system_();
}

BOOL Win32DpiHelper::SetProcessDpiAwarenessContext(
DPI_AWARENESS_CONTEXT context) {
if (!permonitorv2_supported_) {
Expand All @@ -69,4 +87,8 @@ BOOL Win32DpiHelper::SetProcessDpiAwarenessContext(
return set_process_dpi_awareness_context_(context);
}

BOOL Win32DpiHelper::SetProcessDpiAware() {
return set_process_dpi_aware_();
}

} // namespace flutter
18 changes: 18 additions & 0 deletions shell/platform/windows/win32_dpi_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,35 @@ class Win32DpiHelper {
// Wrapper for OS functionality to return the DPI for |HWND|
UINT GetDpiForWindow(HWND);

// Wrapper for OS functionality to return the DPI for the System. Only used if
// Per Monitor V2 is not supported by the current Windows version.
UINT GetDpiForSystem();
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to the comment below: is there a reason to expose this separately rather than making it an internal detail of GetDpiForWindow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

GetDpiForWindow is also the name of the win32 api function, which is specific for Per Monitor V2. I didn't want to mix the names and create confusion on what it does. I could just change it to GetDpi, and under the hood, do either ..DpiForWindow/System accordingly. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

SGTM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


// Sets the current process to a specified DPI awareness context.
BOOL SetProcessDpiAwarenessContext(DPI_AWARENESS_CONTEXT);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we actually need three public functions here? Would it make sense to just expose a top-level SetProcessDpiAware (which would actually be what's currently called SetDpiAwarenessAllVersions, and the current function with that name being called something with System in it, maybe)? It's not clear to me if there are cases where a client would need to know the details of which mode is being set.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I agree, we just need to expose the one that would set it for all versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


// Sets the current process to System-level DPI awareness.
BOOL SetProcessDpiAware();

// Sets the DPI awareness for the application. For versions >= Windows 1703,
// use Per Monitor V2. For any older versions, use System.
//
// This call is overriden if DPI awareness is stated in the application
// manifest.
void SetDpiAwerenessAllVersions();
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo: Awareness

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


private:
using EnableNonClientDpiScaling_ = BOOL __stdcall(HWND);
using GetDpiForWindow_ = UINT __stdcall(HWND);
using SetProcessDpiAwarenessContext_ = BOOL __stdcall(DPI_AWARENESS_CONTEXT);
using SetProcessDpiAware_ = BOOL __stdcall();
using GetDpiForSystem_ = UINT __stdcall();

EnableNonClientDpiScaling_* enable_non_client_dpi_scaling_ = nullptr;
GetDpiForWindow_* get_dpi_for_window_ = nullptr;
SetProcessDpiAwarenessContext_* set_process_dpi_awareness_context_ = nullptr;
SetProcessDpiAware_* set_process_dpi_aware_ = nullptr;
GetDpiForSystem_* get_dpi_for_system_ = nullptr;

HMODULE user32_module_ = nullptr;
bool permonitorv2_supported_ = false;
Expand Down
36 changes: 18 additions & 18 deletions shell/platform/windows/win32_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,8 @@

namespace flutter {

Win32Window::Win32Window() {
// Assume Windows 10 1703 or greater for DPI handling. When running on a
// older release of Windows where this context doesn't exist, DPI calls will
// fail and Flutter rendering will be impacted until this is fixed.
// To handle downlevel correctly, dpi_helper must use the most recent DPI
// context available should be used: Windows 1703: Per-Monitor V2, 8.1:
// Per-Monitor V1, Windows 7: System See
// https://docs.microsoft.com/en-us/windows/win32/hidpi/high-dpi-desktop-application-development-on-windows
// for more information.
}
Win32Window::Win32Window() {}

Win32Window::~Win32Window() {
Destroy();
}
Expand All @@ -26,7 +18,11 @@ void Win32Window::InitializeChild(const char* title,
Destroy();
std::wstring converted_title = NarrowToWide(title);

WNDCLASS window_class = ResgisterWindowClass(converted_title);
// Set DPI awareness for all Windows versions. This call has to be made before
// the HWND is created.
dpi_helper_->SetDpiAwerenessAllVersions();

Choose a reason for hiding this comment

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

As mentioned above, don’t set the DPI awareness mode here but in the calling app.


WNDCLASS window_class = RegisterWindowClass(converted_title);

auto* result = CreateWindowEx(
0, window_class.lpszClassName, converted_title.c_str(),
Expand Down Expand Up @@ -54,7 +50,7 @@ std::wstring Win32Window::NarrowToWide(const char* source) {
return wideTitle;
}

WNDCLASS Win32Window::ResgisterWindowClass(std::wstring& title) {
WNDCLASS Win32Window::RegisterWindowClass(std::wstring& title) {
window_class_name_ = title;

WNDCLASS window_class{};
Expand Down Expand Up @@ -83,13 +79,17 @@ LRESULT CALLBACK Win32Window::WndProc(HWND const window,

auto that = static_cast<Win32Window*>(cs->lpCreateParams);

// Since the application is running in Per-monitor V2 mode, turn on
// automatic titlebar scaling
BOOL result = that->dpi_helper_->EnableNonClientDpiScaling(window);
if (result != TRUE) {
OutputDebugString(L"Failed to enable non-client area autoscaling");
if (that->dpi_helper_->IsPerMonitorV2Available()) {

Choose a reason for hiding this comment

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

Assuming you move the DPI mode setting logic into the client app, this would become a getter for the current DPI mode

// Since the application is running in Per-monitor V2 mode, turn on
// automatic titlebar scaling
BOOL result = that->dpi_helper_->EnableNonClientDpiScaling(window);
if (result != TRUE) {
OutputDebugString(L"Failed to enable non-client area autoscaling");
}
that->current_dpi_ = that->dpi_helper_->GetDpiForWindow(window);
} else {
that->current_dpi_ = that->dpi_helper_->GetDpiForSystem();
}
that->current_dpi_ = that->dpi_helper_->GetDpiForWindow(window);
that->window_handle_ = window;
} else if (Win32Window* that = GetThisFromHandle(window)) {
return that->MessageHandler(window, message, wparam, lparam);
Expand Down
2 changes: 1 addition & 1 deletion shell/platform/windows/win32_window.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class Win32Window {

// Registers a window class with default style attributes, cursor and
// icon.
WNDCLASS ResgisterWindowClass(std::wstring& title);
WNDCLASS RegisterWindowClass(std::wstring& title);

// OS callback called by message pump. Handles the WM_NCCREATE message which
// is passed when the non-client area is being created and enables automatic
Expand Down