Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Win - DPI awareness #156

Closed
wants to merge 1 commit into from
Closed

Conversation

lofcz
Copy link

@lofcz lofcz commented Jan 14, 2025

This adds DPI awareness on Windows, mitigating the blur on high DPI displays.

Before (blurry):
image

After (sharp):
image

@lofcz lofcz force-pushed the feat-win-dpi-awareness branch from f018260 to 3bf4f80 Compare January 14, 2025 02:43
make compiler happy
Copy link
Owner

@btzy btzy left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! But I'm not sure if this should be the responsibility of NFDe.

The DPI awareness setting on Windows affects the entire process, which means that enabling DPI awareness will affect other windows. This is undesirable because other windows may not be written to be DPI aware, and so they may display text and images at an unintended size (usually they are displayed too small, because the DPI-unaware process won't have code to scale up the size).

In my opinion, setting the DPI awareness of the process should be the responsibility of whoever owns the main() function, because only they know whether all their dependencies support DPI-aware mode, and what things they need to do (if any) to enable DPI awareness on all their dependencies.

As NFDe only opens system dialog windows, once you enable DPI awareness for your process, dialog windows will automatically be opened in DPI-aware mode without any further code changes.

#include <ShellScalingApi.h>
#pragma comment(lib, "Shcore.lib")

#define SET_DPI_AWARENESS() \
Copy link
Owner

Choose a reason for hiding this comment

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

Why can't this be a regular C++ function (instead of a macro)?

(SetProcessDpiAwarenessContextPtr)(procAddr); \
if (setProcessDpiAwarenessContext) \
{ \
setProcessDpiAwarenessContext(DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2); \
Copy link
Owner

Choose a reason for hiding this comment

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

Do you know what happens if you pass DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2 to a user32.dll implementation that doesn't know about it (i.e. before Windows 10 version 1703)?

@@ -358,6 +379,9 @@ nfdresult_t NFD_OpenDialogN(nfdnchar_t** outPath,
nfdresult_t NFD_OpenDialogN_With_Impl(nfdversion_t version,
nfdnchar_t** outPath,
const nfdopendialognargs_t* args) {

SET_DPI_AWARENESS();
Copy link
Owner

Choose a reason for hiding this comment

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

We only need to set DPI awareness once per process, and it stays set for the lifetime of the program. Therefore, I think the proper place to put the code is in the NFD_Init function, which is expected to be called once, before calling any other function, instead of within each dialog call (which will be run every time you open a dialog).

@btzy
Copy link
Owner

btzy commented Feb 23, 2025

Closing this - as per my earlier comment, I think it is inappropriate for NFDe to fudge with the process-wide DPI awareness setting.

@btzy btzy closed this Feb 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants