[conhost] Fix WM_GETDPISCALEDSIZE handler, use provided size instead of current window size - #18268
Merged
Dustin L. Howett (DHowett) merged 6 commits intoJan 10, 2025
Conversation
Member
|
(Marking this one blocked per e-mail) |
Evan Koschik (ekoschik)
force-pushed
the
user/evkoschi/getDpiScaledSize
branch
from
December 20, 2024 00:05
2bfd71c to
f6093ec
Compare
Contributor
Author
|
@microsoft-github-policy-service agree
|
Dustin L. Howett (DHowett)
requested changes
Jan 7, 2025
| } | ||
| if (WI_IsFlagSet(dwStyle, WS_VSCROLL)) | ||
| { | ||
| prectWindow->bottom += ServiceLocator::LocateHighDpiApi<WindowDpiApi>()->GetSystemMetricsForDpi(SM_CXVSCROLL, dpi); |
There was a problem hiding this comment.
shouldn't this be prectWindow->right?
Comment on lines
253
to
254
|
|
||
| // Format our final suggestion for consumption. |
There was a problem hiding this comment.
Suggested change
| // Format our final suggestion for consumption. |
No longer applicable
| pSuggestionSize->cy = rectProposed.height(); | ||
| SIZE* pSizeNew = (SIZE*)lParam; | ||
| UINT dpiNew = (WORD)wParam; | ||
| if (!_HandleGetDpiScaledSize(dpiNew, pSizeNew)) |
There was a problem hiding this comment.
Could this be reduced to return _HandleGetDpiScaledSize(dpiNew, pSizeNew); relying on the compiler to do the transformation between bool and BOOL for us?
Or at worst ? TRUE : FALSE?
There was a problem hiding this comment.
wait, don't we need to UnlockConsole even if this fails?!
Evan Koschik (ekoschik)
force-pushed
the
user/evkoschi/getDpiScaledSize
branch
from
January 9, 2025 23:25
f6093ec to
50dc2ee
Compare
Dustin L. Howett (DHowett)
approved these changes
Jan 9, 2025
Leonard Hecker (lhecker)
approved these changes
Jan 9, 2025
Leonard Hecker (lhecker)
approved these changes
Jan 10, 2025
Dustin L. Howett (DHowett)
approved these changes
Jan 10, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The conhost window uses the window message WM_GETDPISCALEDSIZE to scale its client rect non-linearly. This is done to keep the rows and columns from changing when the window changes (font sizes scale non-linearly). If you size the window such that the text perfectly fits the width (and cursor is on the first row of the next line), dragging the window between monitors with different DPIs should NOT change how much of the text fits on each line.
https://learn.microsoft.com/en-us/windows/win32/hidpi/wm-getdpiscaledsize
The current code is assuming that the size that should be scaled is the current window size. This is sometimes the case, for example when dragging a window between monitors, but it is not always the case. This message can sometimes contain a size that is different from the window's current size. For example, if the window is maximized, minimized, or snapped (the size in these cases is the normal rect, or restore rect).
The msdn page above does (now) call this out, though it is possible that this was added after this conhost code was added...
This incorrect assumption can cause the conhost window to be unexpectedly large/small in some cases. For example:
Expected: The window should restore to the original logical size, with the text perfectly fitting one line.
Actual: The window restores to another size; it is the snapped size on the original monitor (the size of the window at the time it was changing DPI, in step 4 above).
References and Relevant Issues
This message (WM_GETDPISCALEDSIZE) is not widely used, but it is used by dialogs (user32!CreateDialog), since they also size their windows using font sizes. The code in this change borrows from the code in the dialog manager, user32!GetDialogDpiScaledSize.
Detailed Description of the Pull Request / Additional comments
The WM_GETDPISCALEDSIZE message contains the new DPI and the new size, which is an in/out parameter. It starts as the new window size, scaled to the window's current DPI, and is expected to be scaled to the new DPI.
The client area (the part with the text) is NOT scaled linearly. For example, if the font at 100% DPI has a height of 7, it could have a height of 15 at 200%. (And if it did have a height of 14, linearly scaled, it would surely not be linearly scaled at 150%, since fonts cannot have a height of 10.5.) To pick the right size, we need to resolve the font at the new DPI and use its actual size to scale the client area.
To keep the amount of text in the window the same, we need to remove the non-client area of the window (caption bars, resize borders, etc). The non-client area is outside the area with the text, and its size depends on the window's DPI and window styles. To remove it and add it back, we need to: