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

Correct PerMonitorV2 DPI for popups without targets #8209

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

miloush
Copy link
Contributor

@miloush miloush commented Sep 12, 2023

Fixes #3874

Description

When creating a HWND for a popup, it needs to be positioned at the correct monitor to receive its DPI in PerMonitorV2 mode, as noticed and implemented here:

// We many not have attempted to position the popup yet
//
// If we don't have prior position information and we are currently running in Per-Monitor DPI Aware mode,
// we should build the window by specifying a point on the current monitor.
// Doing so ensures that the underlying HWND is created with the right DPI. Otherwise, the HWND that is created at
// (0,0) and then shown on another monitor with a different DPI, will immediately receive a WM_DPICHANGED message. This
// will in turn cause the HWND to be resized, and its layout to be updated. This layout-update can result in the dismissal
// of the popup itself, esp. if this Popup is rooted on another Popup.
//
// PopupInitialPlacementHelper.GetPlacementOrigin() will return (0,0) when running in SystemAware and Unaware mode.
// When running in Per-Monitor DPI Aware mode, this method will obtain the screen coordinates of the (left, top) of
// the Display on which the PopupRoot is situated, and return that value here.

However, when the PlacemenetTarget of the popup is null, these required steps are not done, since the GetPlacementTargetOriginInScreenCoordinates gives up and returns 0,0. The popup is then properly positioned after the window is created, which is too late - the winow is potentially created on the wrong monitor, with consequences as above.

This PR updates GetPlacementTargetOriginInScreenCoordinates to estimate the position that would be used later to position the popup when there is no target. This ensures that the window is created on the correct monitor and the popup will receive correct DPI values.

Customer Impact

Only customers using PerMonitorV2 and monitors with at least two different DPIs are affected. For those customers, not taking this PR would mean that popups opened without setting any PlacementTarget will open with the primary's monitor DPI.

Regression

No.

Testing

Compiled and run using .NET 8.0.0-preview.7.23375.6 with PerMonitorV2 manifest. Verified that setting ContextMenu.IsOpen to true exhibits the bug on a monitor with different DPI and does not when the PR is applied, using various popup positioning.

Risk

I believe this is low, the scenario when the new code executed is very narrow. All of the following must be true:

  1. Application opted into per monitor V2 DPI awarenes
  2. Apllication is running in a multi-monitor setup with varying DPI configuration
  3. The popup does not have any PlacementTarget set (this shouldn't be common)
  4. The popup is opened programatically (e.g. setting IsOpen=true)

Without the PR, the window is created at 0,0 in this case. With the PR, the window is created at a different coordinate. Either way, the window is positioned afterwards to the correct location, so the only observable change is the monitor on which the Win32 window is initially created.

Microsoft Reviewers: Open in CodeFlow

@miloush miloush requested a review from a team as a code owner September 12, 2023 17:19
@ghost ghost assigned miloush Sep 12, 2023
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Sep 12, 2023
@ghost ghost requested review from dipeshmsft and singhashish-wpf September 12, 2023 17:19
@ghost ghost added the Community Contribution A label for all community Contributions label Sep 12, 2023
@mayphi
Copy link

mayphi commented Jan 10, 2025

Any update on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community Contribution A label for all community Contributions PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ContextMenu/MenuItem, Dpi in PerMonitorV2 mode, scaling error in WPF with programmatic opening
3 participants