win32 host: APPWINDOW client size used the system DPI, never read back - #32
Merged
Merged
Conversation
create()'s width/height specify the CLIENT area, and Win32 takes an OUTER size, so widget_show grows the request by the non-client frame via AdjustWindowRectExForDpi. That conversion is DPI-dependent twice over - the client scales, and so do the border and title-bar metrics - but the host has to choose a DPI before the HWND exists, and it used GetDpiForSystem(). Nothing corrected it afterwards. When the estimate is wrong the client comes out wrong by the ratio of the two DPIs and stays wrong: WM_DPICHANGED fires when a window MOVES between monitors, not when it is born on one that disagrees with the system DPI. With GetDpiForSystem() = 144 and the frame landing on a 96 DPI monitor, a 940x400 request produced a 1416x617 client - the outer size computed for 144 (1432x656) minus the 96 DPI non-client frame. Not an exotic setup: a 150% primary with a 100% secondary hits it the moment a window opens on the secondary, and so does a remote session whose desktop DPI differs from the console's. Read GetDpiForWindow back once the HWND exists and, if it disagrees with the estimate, redo the client -> outer conversion at the real DPI. Before ShowWindow, so the correction is never visible as a resize flash. This is the same read-back the crossplatform host already does at the end of create_native_window - the native host simply never had it (#29 hardened the xpl side only). Found via an editor embedding neui: the shell's own APPWINDOW goes through this host (neui_get_api(nullptr) returns the native one when it is linked) while the embedded editor frame goes through the xpl host, so the editor was laid out correctly inside a window a third too large. Verified on a machine where GetDpiForSystem()=144 and the primary monitor is 96: the app's client area goes from 1416x617 to 940x400 (outer 1432x656 -> 956x439) and the UI fills the window. No regression test: this path needs a real frame from the native host, which only comes up under the WinMain that host provides, so a console harness like tests/embed_smoke_win32.cpp (xpl host, no WinMain needed) cannot reach it. Worth a WIN32-subsystem test harness as a follow-up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XLLVDvxV66uC4GiMtb7tHH
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.
Session::widget_showconverts the requested CLIENT size to the OUTER sizeWin32 wants, via
AdjustWindowRectExForDpi. That conversion is DPI-dependenttwice over — the client scales, and so do the border and title-bar metrics —
but the DPI has to be chosen before the HWND exists, so the host used
GetDpiForSystem()(hosts/win32/widgets.cpp:2851).Nothing corrected it afterwards. When the estimate is wrong, the client comes
out wrong by the ratio of the two DPIs and stays wrong:
WM_DPICHANGEDfires when a window moves between monitors, not when it is born on one that
disagrees with the system DPI.
Symptom
With
GetDpiForSystem()= 144 and the frame landing on a 96 DPI monitor, a940x400 request produces a 1416x617 client — the outer size computed for
144 (1432x656) minus the 96 DPI non-client frame.
This is not an exotic configuration. A 150% primary with a 100% secondary hits
it the moment a window opens on the secondary, and so does a remote session
whose desktop DPI differs from the console's. The xpl host's own comment in
create_native_windowalready calls out this case.Fix
Read
GetDpiForWindowback once the HWND exists and, if it disagrees with theestimate, redo the client → outer conversion at the real DPI. Placed before
ShowWindowso the correction is never visible as a resize flash.This is the same read-back the crossplatform host already does at the end of
create_native_window. The native host simply never had it — #29 hardened thexpl side only.
How it turned up
An editor embedding neui. The shell's own APPWINDOW goes through this host
(
neui_get_api(nullptr)returns the native one when it is linked) while theembedded editor frame goes through the xpl host. So the editor laid itself out
correctly inside a window a third too large, with the UI occupying the
top-left 940x400 of a 1416x617 client.
Verification
On a machine where
GetDpiForSystem()= 144 and the primary monitor is 96:and the UI fills the window.
No regression test — and why
This path needs a real frame from the native host, which only comes up under
the
WinMainthat host provides (hosts/win32/window.cpp). A console harnesslike
tests/embed_smoke_win32.cppcan't reach it — that one works because itdrives the xpl host, which needs no
WinMain. I tried an analogousappwindow_dpi_smoke_win32and it never gets an HWND back fromshow().A WIN32-subsystem test harness would cover it, but that is a bigger change than
this fix and wants its own way of reporting assertions without stdout. Flagging
it as a follow-up rather than guessing at it here.