Use modern DPI awareness settings #13254

VeselovAlex · 2022-01-18T12:56:43Z

Link to issue number:

Fixes #13370
Fixes #6722
Fixes #3875
Fixes #12070
Fixes #7083
Likely fixes #9531, otherwise close as stale/can't reproduce

Summary of the issue:

When DPI for a monitor is not set to 100%, or when using multiple monitors with different DPI settings,
NVDA would:

misplace highlight frames
have inaccurate mouse tracking
have inaccurate touch screen interaction

NVDA currently sets the DPI awareness via a Windows API call introduced in Windows Vista.
It is recommended to set DPI through the app manifest, rather than Windows API calls where possible.

Newer settings for DPI awareness have been introduced since Windows Vista.
Windows 8 introduced multiple monitor DPI awareness.
Windows 10 introduced a richer version of multiple monitor DPI awareness.

Description of how this pull request fixes the issue:

Background docs: https://docs.microsoft.com/en-us/windows/win32/hidpi/high-dpi-desktop-application-development-on-windows#per-monitor-and-per-monitor-v2-dpi-awareness

When running as an executable, NVDA sets DPI awareness via the app manifest.
When running through source, NVDA sets DPI awareness via Windows API calls.
The most modern method available is used to set DPI awareness.

For Windows 7, DPI awareness is unlikely to improve. There may be fixes from setting it via the app manifest instead of via Windows API calls.
For Windows 8 and newer, NVDA has per monitor DPI awareness
For Windows 10 1703 and newer, NVDA has advanced per monitor DPI awareness, including:
- Child window DPI change notifications
- Scaling of non-client area - All windows will automatically have their non-client area drawn in a DPI sensitive fashion. Calls to EnableNonClientDpiScaling are unnecessary.
- Scaling of Win32 menus - All NTUSER menus created in Per Monitor v2 contexts will be scaling in a per-monitor fashion.
- Dialog Scaling - Win32 dialogs created in Per Monitor v2 contexts will automatically respond to DPI changes.
- Improved scaling of comctl32 controls - Various comctl32 controls have improved DPI scaling behavior in Per Monitor v2 contexts.
- Improved theming behavior - UxTheme handles opened in the context of a Per Monitor v2 window will operate in terms of the DPI associated with that window.

Testing strategy:

Manual testing

Using

different DPI settings across 2 monitors
- 100/125% DPI 2560x1400 (larger monitor)
- 200% DPI 2560x1400 (smaller screen)
Windows 11 (NVDA source and installed)
Windows 10 (older than 1703) (NVDA installed) - this has not been tested across all applications, just a smoke test
Windows 7 should have unchanged behaviour from this PR

Known issues with pull request:

#7915, #13440 and #8076 are still remaining issues.

Change log entries:

Bug fixes
- NVDA is now DPI aware when using multiple monitors.
There are several fixes for using a DPI setting higher than 100% or multiple monitors.
Issues may still exist with versions of Windows older than Windows 10 1809.
Applications which NVDA interacts with also need to be DPI aware. (#13254)
  - Visual highlighting frames should now be correctly placed in most applications. (#13370, #3875, #12070)
  - Touch screen interaction should now be accurate for most applications. (#7083)
  - Mouse tracking should now work for most applications.
  Note there are still known issues with Chrome. (#6722, #7915)
  -

Code Review Checklist:

Pull Request description:
- description is up to date
- change log entries
Testing:
- Manual testing
API is compatible with existing add-ons.
Documentation:
UX of all users considered:
- Speech
- Braille
- Low Vision
- Different web browsers
- Localization in other languages / culture than English

feerrenrut · 2022-02-16T09:36:20Z

Hi @VeselovAlex, welcome to the NVDA project! Thanks for your contribution.

This looks like a good change, however it would help if there was more information about the cause. To help with this we use a pull request template which outlines the information that we require. Would you be able to fill this out for us?

I can confirm this rectifies the visual highlighting in the case outlined in the description on my machine.

we can also set dpi awareness programatically before creating any windows (is nwda.pyw suitable for this purpose?).

There is a call to ctypes.windll.user32.SetProcessDPIAware() in source/core.py at about line 427
However, docs for SetProcessDpiAwarenessContext recommend setting this via the manifest.

This seems to depend on DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2, from the docs:

Per Monitor v2 was made available in the Creators Update of Windows 10, and is not available on earlier versions of the operating system.

Can you update the description to cover what will happen for Windows versions before the "Creators Update" (1703)?

Just an FYI for future contributions, the contributing guidelines recommend creating an issue / commenting on an issue before you start. The intention is to

prevent contributors from wasting their time if the change won't be accepted
prevent duplicate efforts
align expectations

You may have done this, but there is no link to an issue (requested in the PR template).

feerrenrut · 2022-02-21T05:34:51Z

Further action is required on this PR, converting it to a draft until this is resolved:

Per Monitor v2 isn't supported before Windows 10 1703, NVDA needs to support older Windows versions. What is the behavior here with this change?
Should the call to ctypes.windll.user32.SetProcessDPIAware() be removed?

VeselovAlex · 2022-02-21T17:54:26Z

Hi @feerrenrut, thanks for the feedback and sorry for the delay. I've update PR description, added PerMonitor as a fallback and removed explicit SetProcessDPIAware call. This solution has some limitations described above, if we should switch back to explicit calls please let me know.

BTW, there is a fixed issue JDK-8279227 with HiDPI support in JDK and the fix will be released soon (OpenJDK 18 or 17.4). This fix will affect NVDA too

feerrenrut · 2022-03-02T08:09:54Z

Hi @VeselovAlex, thanks for the updates. Do you know why the manifest approach is preferred? I'm guessing the manifest approach is helpful for supporting older versions of Windows?
Aside from maintenance cost, is there any reason not have both in code function calls to set this AND manifest values?

It would be good if we could support developers (who will mostly run from the script), but not at the cost of users.

feerrenrut · 2022-03-23T03:34:45Z

Reading the docs again, I think we should do both (use the manifest, and set in code). Setting from code should only be active for source copies, and will need to take into account the Windows version. This may seem pointless, but the value is in supporting developers. It's easy to imagine a situation where a developer is trying to track down a breakage in 'picking' (getting the UI element at screen coords) that only occurs with the source copy. Keeping the manifest approach for executable versions lowers the risk that we get Windows capabilities wrong for end users.

feerrenrut · 2022-06-16T12:34:06Z

@VeselovAlex If you don't mind, I'm going to look into this myself. I may push several commits to get this PR moving again.

seanbudd · 2022-07-14T06:41:00Z

Test if this also fixes: #6722

This change makes NVDA DPI-aware to fix wrong size and position of accessible elements on the second display if its DPI differs from the first one's

AppVeyorBot · 2022-07-17T19:13:14Z

PASS: Translation comments check.
PASS: Unit tests.
PASS: Lint check.
FAIL: System tests. See test results for more information.
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/9x1n29npa5fb2qv9/artifacts/output/nvda_snapshot_pr13254-25883,513e340e.exe

See test results for failed build of commit 513e340e56

AppVeyorBot · 2022-07-17T20:54:15Z

PASS: Translation comments check.
PASS: Unit tests.
PASS: Lint check.
FAIL: System tests. See test results for more information.
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/7lppiq1fbwrbqyen/artifacts/output/nvda_snapshot_pr13254-25885,8cf134e3.exe

See test results for failed build of commit 8cf134e323

VeselovAlex · 2022-07-17T21:19:36Z

I've added a DPI awareness setup when NVDA is running as a script. Also #6722 seems to be fixed with this PR

source/core.py

AppVeyorBot · 2022-08-22T02:53:23Z

PASS: Translation comments check.
PASS: Unit tests.
PASS: Lint check.
FAIL: System tests. See test results for more information.
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/2k2cxuaslrtu7nh2/artifacts/output/nvda_snapshot_pr13254-26250,83193c21.exe

See test results for failed build of commit 83193c2148

seanbudd · 2022-08-23T06:20:39Z

This PR has now been tested and is ready for review.

feerrenrut

Good work here, looks solid. Can you clarify whether items in the description under "testing" are tested but not fixed, or not yet tested. I have assumed the former.

feerrenrut · 2022-08-29T11:33:42Z

source/core.py

@@ -427,8 +427,9 @@ def main():
 	Finally, it starts the wx main loop.
 	"""
 	log.debug("Core starting")
-
-	ctypes.windll.user32.SetProcessDPIAware()
+	if globalVars.runningAsSource:


Please add a comment here to highlight that this is handled via the manifest for packaged versions.

seanbudd · 2022-08-30T00:31:39Z

@feerrenrut - correct, these are things tested but not fixed fully

…PI awareness (#14161) fix-up of #13254 Summary of the issue: After #13254 got merged NVDA failed to start from sources on Windows 7 with the following exception: CRITICAL - __main__ (16:59:34.992) - MainThread (6908): core failure Traceback (most recent call last): File "nvda.pyw", line 393, in <module> core.main() File "core.py", line 436, in main setDPIAwareness() File "winAPI\dpiAwareness.py", line 67, in setDPIAwareness hResult = ctypes.windll.shcore.SetProcessDpiAwareness(PROCESS_PER_MONITOR_DPI_AWARE) File "C:\Python37\lib\ctypes\__init__.py", line 434, in __getattr__ dll = self._dlltype(name) File "C:\Python37\lib\ctypes\__init__.py", line 364, in __init__ self._handle = _dlopen(self._name, mode) OSError: [WinError 126] The specified module cannot be found While we handle different versions of Windows by catching AttributeError if the given function for setting DPI awareness is not present on Windows 7 shcore is missing and ctypes raises a different error for missing libraries. Description of user facing changes NVDA can once again start from sources when running on Windows 7. Description of development approach In addition to catching AttributeError we also catch WindowsError and inspect its error code. If it is caused by the missing library appropriate info is logged and the function continues to use legacy method of setting DPI awareness. Note that AttributeError has to be handled as well to allow NVDA to start on Windows 8/Server 2012.

VeselovAlex requested a review from a team as a code owner January 18, 2022 12:56

VeselovAlex requested a review from michaelDCurran January 18, 2022 12:56

This comment was marked as outdated.

Sign in to view

feerrenrut marked this pull request as draft February 21, 2022 05:34

VeselovAlex marked this pull request as ready for review February 21, 2022 17:47

This comment was marked as outdated.

Sign in to view

feerrenrut added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Mar 18, 2022

seanbudd requested review from feerrenrut and removed request for michaelDCurran June 9, 2022 05:44

seanbudd added the blocked/needs-code-review label Jun 20, 2022

seanbudd added the merge-early Merge Early in a developer cycle label Jun 28, 2022

seanbudd assigned feerrenrut Jun 28, 2022

seanbudd linked an issue Jul 14, 2022 that may be closed by this pull request

Mouse tracking does not work well with display scaling. #6722

Closed

VeselovAlex and others added 3 commits July 17, 2022 21:59

Add DPI awareness settings to app manifest

1a7253e

This change makes NVDA DPI-aware to fix wrong size and position of accessible elements on the second display if its DPI differs from the first one's

Make lint test green

2c1ef46

Add PerMonitor fallback, remove SetProcessDPIAware

45f0db3

Setup DPI awareness if NVDA is running as a script

ea9e1e3

VeselovAlex force-pushed the dpi-awareness branch from 97dece1 to ea9e1e3 Compare July 17, 2022 20:09

Lint fixes

d142fae

seanbudd reviewed Jul 17, 2022

View reviewed changes

source/core.py Outdated Show resolved Hide resolved

seanbudd mentioned this pull request Aug 17, 2022

Some apps refuse to return physical coordinates for high DPI settings when NVDA is DPI aware #3875

Closed

improve docs

2101e70

seanbudd mentioned this pull request Aug 18, 2022

Ensure indeterminate progress dialogs centred on screen before visible #14024

Merged

5 tasks

seanbudd added 2 commits August 19, 2022 18:38

Merge branch 'master' into dpi-awareness

01ed8e5

fix up runningAsSource check

fdc8bfe

seanbudd added the audience/low-vision PR or issue is relevant to sighted or low vision users label Aug 23, 2022

rename constants for readibility

dae3dc5

seanbudd mentioned this pull request Aug 23, 2022

DisplayModel getting bounding rect from offset sometimes inaccurate on higher DPI #9531

Closed

seanbudd removed the blocked/needs-testing label Aug 23, 2022

seanbudd mentioned this pull request Aug 23, 2022

NVDA cannot move mouse pointer to current navigator object in list view in the Reaper #13440

Open

seanbudd mentioned this pull request Aug 23, 2022

Mouse tracking does not work in Chromium #8076

Open

seanbudd added the displayScaling Display scaling and DPI awareness label Aug 23, 2022

seanbudd changed the title ~~Add DPI awareness settings to app manifest~~ Use modern DPI awareness settings Aug 23, 2022

feerrenrut approved these changes Aug 29, 2022

View reviewed changes

seanbudd added 2 commits August 30, 2022 10:32

Merge branch 'master' into dpi-awareness

ef7d40b

add comment

d3b5166

feerrenrut approved these changes Aug 30, 2022

View reviewed changes

seanbudd added 2 commits August 30, 2022 13:31

update changes

c80cc66

Merge branch 'master' into dpi-awareness

6c6e82f

seanbudd merged commit 28b4756 into nvaccess:master Aug 30, 2022

cary-rowen mentioned this pull request Aug 30, 2022

Mouse routing does not work properly when using Object navigation in Chrome #7915

Open

seanbudd added a commit that referenced this pull request Aug 31, 2022

Fixup changes for #13254 now that #7915 is re-opened

72287be

seanbudd mentioned this pull request Aug 31, 2022

Fixup changes for #13254 now that #7915 is re-opened #14091

Merged

seanbudd added a commit that referenced this pull request Aug 31, 2022

Fixup changes for #13254 now that #7915 is re-opened (#14091)

f843cbe

lukaszgo1 mentioned this pull request Sep 18, 2022

No longer fail to start NVDA from sources on Windows 7 when setting DPI awareness #14161

Merged

6 tasks

seanbudd mentioned this pull request Dec 20, 2023

Character navigation behaves inaccurately in Tera Term 5.0 but works in 4.x #15929

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use modern DPI awareness settings #13254

Use modern DPI awareness settings #13254

VeselovAlex commented Jan 18, 2022 •

edited by seanbudd

This comment was marked as outdated.

feerrenrut commented Feb 16, 2022

feerrenrut commented Feb 21, 2022

VeselovAlex commented Feb 21, 2022

This comment was marked as outdated.

feerrenrut commented Mar 2, 2022

feerrenrut commented Mar 23, 2022

feerrenrut commented Jun 16, 2022

seanbudd commented Jul 14, 2022

AppVeyorBot commented Jul 17, 2022

AppVeyorBot commented Jul 17, 2022

VeselovAlex commented Jul 17, 2022

AppVeyorBot commented Aug 22, 2022

seanbudd commented Aug 23, 2022

feerrenrut left a comment

feerrenrut Aug 29, 2022

seanbudd commented Aug 30, 2022

Use modern DPI awareness settings #13254

Use modern DPI awareness settings #13254

Conversation

VeselovAlex commented Jan 18, 2022 • edited by seanbudd

Link to issue number:

Summary of the issue:

Description of how this pull request fixes the issue:

Testing strategy:

Known issues with pull request:

Change log entries:

Code Review Checklist:

This comment was marked as outdated.

feerrenrut commented Feb 16, 2022

feerrenrut commented Feb 21, 2022

VeselovAlex commented Feb 21, 2022

This comment was marked as outdated.

feerrenrut commented Mar 2, 2022

feerrenrut commented Mar 23, 2022

feerrenrut commented Jun 16, 2022

seanbudd commented Jul 14, 2022

AppVeyorBot commented Jul 17, 2022

AppVeyorBot commented Jul 17, 2022

VeselovAlex commented Jul 17, 2022

AppVeyorBot commented Aug 22, 2022

seanbudd commented Aug 23, 2022

feerrenrut left a comment

Choose a reason for hiding this comment

feerrenrut Aug 29, 2022

Choose a reason for hiding this comment

seanbudd commented Aug 30, 2022

VeselovAlex commented Jan 18, 2022 •

edited by seanbudd