From e6c46ae755033189153697f2fdc508f419ad20f1 Mon Sep 17 00:00:00 2001 From: defiantnerd <97224712+defiantnerd@users.noreply.github.com> Date: Tue, 25 Aug 2026 12:46:17 +0200 Subject: [PATCH] xpl host (win32): first RESIZE after frame creation reported physical px (#29) CreateWindowExW sends WM_SIZE before it returns (for a WS_CHILD embedded PLUGWINDOW it always does), but the xpl host only assigned wd.dpi *after* the call returned. The WM_SIZE handler converts the physical client size to logical with that dpi, so at its 96 default the conversion was an identity and the client's first NEUI_EVENT_RESIZE carried PHYSICAL pixels: a frame created 940x400 reported 1410x600 at 150%. A client laying out from the resize event - the documented way to react to a size change - then laid out 1.5x too big, which in an embedded editor pushes the whole UI off-screen. The win32 native host does not have the bug because it seeds wd->dpi in its WM_CREATE, one message earlier; the xpl host now does the same. - WM_CREATE: seed wd->dpi from GetDpiForWindow(hwnd) before the creation-time WM_SIZE can read it. - create_native_window: seed wd.dpi before CreateWindowExW from the best pre-creation estimate (embed parent / dialog owner DPI, else system DPI) and size the outer window from it. WM_GETMINMAXINFO arrives before WM_NCCREATE, so it too was scaling MIN_/MAX_ tracking sizes as if the frame were at 96 DPI; an embedded frame now also lands at the parent's real scale on the first try instead of via the correction round-trip. - WM_SIZE: convert with MulDiv (round-to-nearest, the exact inverse of the MulDiv used when sizing the window) instead of a truncating float divide, which lost a pixel at 125% (logical 941 -> 1176 phys -> 940). - Harden the division-by-zero paths GetDpiForWindow's 0-on-failure return feeds (the issue's aside): phys_to_log, platform_get_scale_factor, the post-creation read-back, Session::on_dpi_changed, d2d_create_context. Adds tests/embed_smoke_win32.cpp, a fake-DAW harness mirroring the existing Linux / macOS embed smokes (built, not ctest-registered): it embeds a PLUGWINDOW in a foreign HWND, drives it from the fake DAW's own pump, and asserts the client-area contract plus a logical-px first RESIZE. On this 150% display it fails before the fix (first RESIZE 1410x600) and passes after (940x400). Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 2 +- backends/d2d/d2d_backend.cpp | 1 + hosts/crossplatform/host.cpp | 2 + hosts/crossplatform/platform_win32.cpp | 87 ++++++++--- tests/CMakeLists.txt | 15 ++ tests/embed_smoke_win32.cpp | 197 +++++++++++++++++++++++++ 6 files changed, 282 insertions(+), 22 deletions(-) create mode 100644 tests/embed_smoke_win32.cpp diff --git a/CLAUDE.md b/CLAUDE.md index 4f5e3db..9468031 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -24,7 +24,7 @@ cmake -B out/build -G Xcode && cmake --build out/build --config Debug Outputs - Windows: `out/build/Debug/{neui_example.exe, neui.lib, neui-win32host.lib, neui-xplhost.lib, neui-backend-d2d.lib}`. macOS: `out/build/Debug/{neui_example.app, libneui.a}` + per-subdir `libneui-*.a`. Example apps (CMake targets): `neui_example`, `neui_grid_example`, `neui_section_scroll_example`, `neui_surface_example`, `neui_dnd_example`, `neui_dnd_source_example`, `neui_tabview_example`, `neui_font_loading_example`, `neui_filter_knob_example`, `neui_path_example`, `neui_overlay_example` (transparent CUSTOMDRAW overlay - DirectComposition on win32), `neui_arc_example` (value-driven arc / ring / pie compound layer), and `readme_example`. -**Tests**: `tests/` is a Tier-1 header-only unit suite (`neui_tests`) over the portable logic in `hosts/shared/*.h` - links no host and no backend, builds everywhere including the null platform. Toggle with `-DNEUI_BUILD_TESTS=OFF`. Run directly or via `ctest --test-dir out/build -C Debug`. Linux-only extra targets: `neui_cairo_smoke` (offscreen Cairo, ctest-registered) and `neui_embed_smoke` (fake-DAW embedding, needs a live X display). +**Tests**: `tests/` is a Tier-1 header-only unit suite (`neui_tests`) over the portable logic in `hosts/shared/*.h` - links no host and no backend, builds everywhere including the null platform. Toggle with `-DNEUI_BUILD_TESTS=OFF`. Run directly or via `ctest --test-dir out/build -C Debug`. Linux-only extra targets: `neui_cairo_smoke` (offscreen Cairo, ctest-registered) and `neui_embed_smoke` (fake-DAW embedding, needs a live X display). Windows-only: `neui_embed_smoke_win32` (fake-DAW embedding into a foreign HWND; asserts the client-area contract and that the first `NEUI_EVENT_RESIZE` is in logical px - only meaningful on a scaled display; built but not ctest-registered). ## Per-platform host + backend selection diff --git a/backends/d2d/d2d_backend.cpp b/backends/d2d/d2d_backend.cpp index 9f65d7f..b87af3a 100644 --- a/backends/d2d/d2d_backend.cpp +++ b/backends/d2d/d2d_backend.cpp @@ -786,6 +786,7 @@ namespace neui_d2d_backend HWND hwnd = reinterpret_cast(native_handle); UINT dpi = GetDpiForWindow(hwnd); + if (dpi == 0) dpi = 96; // failure -> 1:1; SetDpi(0, 0) is invalid auto* ctx = new D2DContext(); ctx->hwnd = hwnd; diff --git a/hosts/crossplatform/host.cpp b/hosts/crossplatform/host.cpp index d0717c1..dfa3f58 100644 --- a/hosts/crossplatform/host.cpp +++ b/hosts/crossplatform/host.cpp @@ -887,6 +887,8 @@ namespace xpl_host { auto* wd = get_widget(widget_index); if (!wd) return; + // Never store 0 - every physical <-> logical conversion divides by it. + if (new_dpi == 0) new_dpi = 96; wd->dpi = new_dpi; if (wd->render_ctx && _backend) _backend->update_dpi(wd->render_ctx, new_dpi); diff --git a/hosts/crossplatform/platform_win32.cpp b/hosts/crossplatform/platform_win32.cpp index fc0709e..1652a47 100644 --- a/hosts/crossplatform/platform_win32.cpp +++ b/hosts/crossplatform/platform_win32.cpp @@ -79,8 +79,12 @@ namespace xpl_host static inline int mouse_y(LPARAM lp) { return static_cast(static_cast(HIWORD(lp))); } // Convert a physical pixel coordinate to logical (96 DPI baseline). + // dpi == 0 is normalised to 96: GetDpiForWindow returns 0 on failure, and a + // frame's dpi is only populated once its HWND exists - neither may divide + // by zero. static inline float phys_to_log(int phys, uint32_t dpi) { + if (dpi == 0) dpi = 96; return static_cast(phys) * 96.0f / static_cast(dpi); } @@ -391,8 +395,22 @@ namespace xpl_host auto* wud = get_wud(hwnd); if (wud) { auto* wd = wud->session->get_widget(wud->widget_index); - if (wd && wd->type && !strcmp(wd->type, NEUI_W_APPWINDOW)) - ++g_appwindow_count; + if (wd) { + // Stash the HWND's real per-monitor DPI here, BEFORE the + // creation-time WM_SIZE arrives (CreateWindowExW sends WM_CREATE + // first, then WM_SIZE, all before it returns). That handler converts + // the physical client size to logical with wd->dpi - with dpi still + // at its 96 default the conversion is an identity and the client + // would see the first NEUI_EVENT_RESIZE in PHYSICAL px (940x400 + // reported as 1410x600 at 150%). create_native_window refines this + // to the same value right after CreateWindowExW returns; this is the + // assignment the early messages need. Mirrors the win32 native + // host's WM_CREATE (hosts/win32/window.cpp). + UINT hwnd_dpi = GetDpiForWindow(hwnd); + wd->dpi = hwnd_dpi ? hwnd_dpi : 96; + if (wd->type && !strcmp(wd->type, NEUI_W_APPWINDOW)) + ++g_appwindow_count; + } } // Give this window Win32 keyboard focus so WM_KEYDOWN/WM_CHAR arrive // here. NOT for a DAW-embedded frame (WS_CHILD): merely opening a @@ -579,8 +597,15 @@ namespace xpl_host auto* fwd = wud->session->get_widget(wud->widget_index); if (fwd) { - int w_log = static_cast(phys_to_log(static_cast(w_phys), fwd->dpi)); - int h_log = static_cast(phys_to_log(static_cast(h_phys), fwd->dpi)); + // WM_CREATE seeded fwd->dpi before the first WM_SIZE; fall back to + // the live HWND in case this frame was never routed through there. + UINT dpi = fwd->dpi ? fwd->dpi : GetDpiForWindow(hwnd); + if (dpi == 0) dpi = 96; + // MulDiv (round-to-nearest) is the exact inverse of the + // MulDiv(logical, dpi, 96) used when sizing the window; truncating + // here loses a pixel (logical 941 -> 1176 phys -> 940 at 125%). + int w_log = MulDiv(static_cast(w_phys), 96, static_cast(dpi)); + int h_log = MulDiv(static_cast(h_phys), 96, static_cast(dpi)); fwd->width = w_log; fwd->height = h_log; @@ -1376,19 +1401,35 @@ namespace xpl_host { if (!native_handle) return 1.0f; UINT dpi = GetDpiForWindow(static_cast(native_handle)); + if (dpi == 0) dpi = 96; // GetDpiForWindow failed - report 1:1, not 0 return static_cast(dpi) / 96.0f; } // Helper: create a HWND and attach a D2D render context to wd. - // Logical coordinates in wd are converted to physical pixels using the - // system DPI before the window exists, then refined to per-monitor DPI after. + // Logical coordinates in wd are converted to physical pixels using the best + // pre-creation DPI estimate, then refined to per-monitor DPI after. static void create_native_window(Session* session, uint32_t widget_index, WidgetData& wd, DWORD style, DWORD ex_style, HWND owner_hwnd = nullptr) { - // Use system DPI as the best estimate of the target monitor's DPI before - // the window is created. The actual per-monitor DPI is read back afterwards. - UINT sys_dpi = GetDpiForSystem(); + // Best estimate of the target monitor's DPI before the window exists. An + // embedded frame's parent and a dialog's owner are already on the target + // monitor, so their DPI is exact and lands the window at the right scale + // on the first try (no post-creation correction round-trip); otherwise + // fall back to the system DPI. The actual per-monitor DPI is read back + // afterwards. + UINT create_dpi = 0; + if (wd.embed_parent) + create_dpi = GetDpiForWindow(reinterpret_cast(wd.embed_parent)); + else if (owner_hwnd) + create_dpi = GetDpiForWindow(owner_hwnd); + if (create_dpi == 0) create_dpi = GetDpiForSystem(); + if (create_dpi == 0) create_dpi = 96; + // Seed wd.dpi so the messages CreateWindowExW sends before it returns see + // a real DPI. WM_GETMINMAXINFO in particular arrives before WM_NCCREATE - + // ahead of any chance to read GetDpiForWindow - and would otherwise scale + // NEUI_ATTR_MIN_/MAX_ tracking sizes as if the frame were at 96 DPI. + wd.dpi = create_dpi; // neui's create() width/height specify the CLIENT (content) area - the same // contract as the win32 native host (hosts/win32/widgets.cpp) and macOS @@ -1416,7 +1457,7 @@ namespace xpl_host AdjustWindowRectExForDpi(&wr, style, has_menu ? TRUE : FALSE, ex_style, dpi); return SIZE{ wr.right - wr.left, wr.bottom - wr.top }; }; - SIZE outer = outer_for_dpi(sys_dpi); + SIZE outer = outer_for_dpi(create_dpi); auto* wud = new WindowUserData{ session, widget_index }; @@ -1425,8 +1466,8 @@ namespace xpl_host k_wndclass, L"", style, - MulDiv(wd.x, sys_dpi, 96), - MulDiv(wd.y, sys_dpi, 96), + MulDiv(wd.x, create_dpi, 96), + MulDiv(wd.y, create_dpi, 96), outer.cx, outer.cy, owner_hwnd, @@ -1438,17 +1479,21 @@ namespace xpl_host if (!hwnd) { delete wud; return; } // Read back the actual per-monitor DPI now that the window has a monitor. - wd.dpi = GetDpiForWindow(hwnd); + // GetDpiForWindow returns 0 on failure - keep the seeded estimate then, so + // no coordinate conversion ever divides by zero. + UINT actual_dpi = GetDpiForWindow(hwnd); + wd.dpi = actual_dpi ? actual_dpi : create_dpi; wd.native_handle = hwnd; - // If the window's actual monitor DPI differs from GetDpiForSystem() (e.g. - // the primary is at 100% but the window opened on a 150% secondary, or - // vice versa), our initial CreateWindowExW used the wrong scaling. The - // D2D render target uses the per-window DPI for widget coordinates, so a - // mismatch leaves the outer window sized at one scale and the widget - // coordinate space at another - visible as margins that don't match - // left/top. Resize using the actual window DPI to keep both in sync. - if (wd.dpi != sys_dpi && wd.dpi != 0) { + // If the window's actual monitor DPI differs from the pre-creation + // estimate (e.g. the primary is at 100% but the window opened on a 150% + // secondary, or vice versa), our initial CreateWindowExW used the wrong + // scaling. The D2D render target uses the per-window DPI for widget + // coordinates, so a mismatch leaves the outer window sized at one scale + // and the widget coordinate space at another - visible as margins that + // don't match left/top. Resize using the actual window DPI to keep both + // in sync. + if (wd.dpi != create_dpi) { SIZE outer2 = outer_for_dpi(wd.dpi); SetWindowPos(hwnd, nullptr, 0, 0, outer2.cx, outer2.cy, diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index bb5a0d7..a95358f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -62,6 +62,21 @@ if(APPLE AND NOT NEUI_IOS) neui neui-xplhost neui-macoshost "-framework AppKit") endif() +# Windows-only: DAW-embedding harness (fake host HWND slot, embeds a +# PLUGWINDOW via NEUI_API_EMBED and drives it from the fake DAW's own message +# pump). Also guards issue #29 - the first NEUI_EVENT_RESIZE must be in +# logical pixels. Needs a GUI session and only differs from an identity check +# on a scaled display, so it is built but NOT registered with ctest; run +# ./tests//neui_embed_smoke_win32 manually. +if(WIN32) + add_executable(neui_embed_smoke_win32 embed_smoke_win32.cpp) + target_include_directories(neui_embed_smoke_win32 PRIVATE + ${PROJECT_SOURCE_DIR}/include) + target_compile_features(neui_embed_smoke_win32 PRIVATE cxx_std_17) + target_link_libraries(neui_embed_smoke_win32 PRIVATE + neui neui-xplhost neui-win32host) +endif() + # Linux-only: Cairo software backend offscreen smoke test. Unlike neui_tests # this links a real backend (neui-backend-cairo), so it is gated to the # platforms where that backend is built. diff --git a/tests/embed_smoke_win32.cpp b/tests/embed_smoke_win32.cpp new file mode 100644 index 0000000..ec9d474 --- /dev/null +++ b/tests/embed_smoke_win32.cpp @@ -0,0 +1,197 @@ +// Acceptance harness for the win32 DAW-embedding path (issue #29). +// +// Plays the role of a DAW: creates a foreign "host parent" HWND, embeds a neui +// PLUGWINDOW under it via the public NEUI_API_EMBED interface, then drives the +// UI purely from the DAW's own message pump (no neui-owned event loop - the +// win32 embedded frame is an ordinary WS_CHILD, so PeekMessage/DispatchMessage +// on this thread already services it). +// +// What it asserts: +// 1. NEUI_API_EMBED is exposed and set_parent accepts the foreign HWND. +// 2. The embedded child HWND is really parented under the DAW window. +// 3. The child's PHYSICAL client size matches the requested LOGICAL size +// scaled by the frame's DPI - the "create() size is the client area" +// contract. +// 4. **The first NEUI_EVENT_RESIZE reports LOGICAL pixels** (issue #29). On +// win32, CreateWindowExW sends WM_SIZE for a WS_CHILD before it returns, +// i.e. before the caller can read the new HWND's DPI back - so this is +// the one event most likely to escape with physical pixels in it. A +// client that lays out from the resize event would push its whole UI off +// the visible area of an embedded editor. +// +// Checks 3 + 4 only differ from a trivial identity when the display is +// scaled (125% / 150% / ...): at 96 DPI logical == physical. Run it on a +// scaled display to exercise the regression it was written for. +// +// Windows-only; needs a GUI session, so it is built but NOT registered with +// ctest - run ./tests//neui_embed_smoke_win32 manually. + +#include + +#include + +#include +#include + +static const int k_plug_w = 940; // logical, at 96 DPI +static const int k_plug_h = 400; + +static int g_resize_count = 0; +static int g_first_w = 0; +static int g_first_h = 0; + +static bool onevent(void*, neui_event_t* ev) +{ + if (ev && ev->type == NEUI_EVENT_RESIZE) { + if (g_resize_count == 0) { + g_first_w = ev->data.resize.width; + g_first_h = ev->data.resize.height; + } + ++g_resize_count; + std::printf(" RESIZE #%d: %d x %d\n", g_resize_count, + ev->data.resize.width, ev->data.resize.height); + } + return false; +} + +static neui_widget_client_t g_wc = { NEUI_VERSION, nullptr, onevent }; +static void* iface(void*, const char* n) +{ return std::strcmp(n, NEUI_API_WIDGETS) ? nullptr : (void*)&g_wc; } +static neui_client_t g_client = { NEUI_VERSION, iface }; + +// A DAW is normally per-monitor-v2 aware and the plugin inherits that context, +// so opt in here too - without it the process is DPI-virtualised and every +// GetDpiForWindow reports 96, which would make the DPI checks vacuous. +static void opt_into_per_monitor_dpi() +{ + using SetCtxFn = BOOL (WINAPI*)(void*); + HMODULE u32 = GetModuleHandleW(L"user32.dll"); + if (!u32) return; + auto set_ctx = reinterpret_cast( + reinterpret_cast( + GetProcAddress(u32, "SetProcessDpiAwarenessContext"))); + // -4 == DPI_AWARENESS_CONTEXT_PER_MONITOR_AWARE_V2 + if (set_ctx) set_ctx(reinterpret_cast(static_cast(-4))); +} + +// Pump the DAW's message loop for `ms`, exactly as a plugin host would. +static void daw_pump(DWORD ms) +{ + DWORD end = GetTickCount() + ms; + for (;;) { + MSG msg; + while (PeekMessageW(&msg, nullptr, 0, 0, PM_REMOVE)) { + TranslateMessage(&msg); + DispatchMessageW(&msg); + } + if (GetTickCount() >= end) break; + Sleep(10); + } +} + +int main() +{ + opt_into_per_monitor_dpi(); + + // ---- Fake DAW host window ----------------------------------------------- + WNDCLASSEXW wc = {}; + wc.cbSize = sizeof(wc); + wc.lpfnWndProc = DefWindowProcW; + wc.hInstance = GetModuleHandleW(nullptr); + wc.hCursor = LoadCursorW(nullptr, reinterpret_cast(IDC_ARROW)); + wc.hbrBackground = CreateSolidBrush(RGB(32, 32, 32)); + wc.lpszClassName = L"neui.fake.daw.host"; + RegisterClassExW(&wc); + + UINT sys_dpi = GetDpiForSystem(); + if (sys_dpi == 0) sys_dpi = 96; + HWND parent = CreateWindowExW( + 0, wc.lpszClassName, L"fake daw host", WS_OVERLAPPEDWINDOW, + 40, 40, + MulDiv(k_plug_w + 40, static_cast(sys_dpi), 96), + MulDiv(k_plug_h + 80, static_cast(sys_dpi), 96), + nullptr, nullptr, wc.hInstance, nullptr); + if (!parent) { std::printf("FAIL: could not create the fake DAW window\n"); return 1; } + ShowWindow(parent, SW_SHOWNORMAL); + UpdateWindow(parent); + daw_pump(200); + + UINT parent_dpi = GetDpiForWindow(parent); + if (parent_dpi == 0) parent_dpi = 96; + std::printf("fake DAW parent HWND=%p, dpi=%u (%.0f%% scaling)\n", + (void*)parent, parent_dpi, parent_dpi * 100.0 / 96.0); + + // ---- Embed a neui PLUGWINDOW under it ----------------------------------- + neui_init(); + neui_api_t* api = neui_get_api("neui.host.crossplatform"); + if (!api) { std::printf("FAIL: no crossplatform host registered\n"); return 1; } + neui_session_t sess = api->create_session(&g_client, nullptr); + auto* w = (neui_widget_api_t*)api->get_interface(sess, NEUI_API_WIDGETS); + auto* embed = (neui_embed_api_t*)api->get_interface(sess, NEUI_API_EMBED); + if (!w) { std::printf("FAIL: no NEUI_API_WIDGETS\n"); return 1; } + if (!embed) { std::printf("FAIL: no NEUI_API_EMBED\n"); return 1; } + + neui_widget_t plug = w->create(sess, widget_none, NEUI_W_PLUGWINDOW, + 0, 0, k_plug_w, k_plug_h, nullptr); + neui_widget_t btn = w->create(sess, plug, NEUI_W_BUTTON, 20, 20, 160, 32, nullptr); + w->set_text(sess, btn, "Embedded!"); + + if (!embed->set_parent(sess, plug, (void*)parent)) { + std::printf("FAIL: set_parent rejected the DAW HWND\n"); + return 1; + } + + w->show(sess, plug); // embedded: never run() / pump_once() + daw_pump(600); + + int failures = 0; + + // ---- 1. child really parented under the DAW window ---------------------- + HWND child = GetWindow(parent, GW_CHILD); + if (!child) { + std::printf("FAIL: no child HWND under the DAW parent\n"); + ++failures; + } else { + std::printf("PASS: embedded child HWND=%p under the DAW parent\n", (void*)child); + } + + // ---- 2. client-area contract: physical client == logical * scale -------- + if (child) { + RECT rc = {}; + GetClientRect(child, &rc); + int want_w = MulDiv(k_plug_w, static_cast(parent_dpi), 96); + int want_h = MulDiv(k_plug_h, static_cast(parent_dpi), 96); + int got_w = rc.right - rc.left; + int got_h = rc.bottom - rc.top; + if (got_w != want_w || got_h != want_h) { + std::printf("FAIL: child client is %dx%d physical, expected %dx%d " + "(%dx%d logical at %u dpi)\n", + got_w, got_h, want_w, want_h, k_plug_w, k_plug_h, parent_dpi); + ++failures; + } else { + std::printf("PASS: child client is %dx%d physical = %dx%d logical at %u dpi\n", + got_w, got_h, k_plug_w, k_plug_h, parent_dpi); + } + } + + // ---- 3. issue #29: the first RESIZE must be in LOGICAL pixels ----------- + if (g_resize_count == 0) { + std::printf("FAIL: no NEUI_EVENT_RESIZE reached the client\n"); + ++failures; + } else if (g_first_w != k_plug_w || g_first_h != k_plug_h) { + std::printf("FAIL: first RESIZE reported %dx%d, expected %dx%d logical" + "%s\n", g_first_w, g_first_h, k_plug_w, k_plug_h, + (g_first_w == MulDiv(k_plug_w, static_cast(parent_dpi), 96)) + ? " - those are PHYSICAL pixels (issue #29)" : ""); + ++failures; + } else { + std::printf("PASS: first RESIZE reported %dx%d logical\n", g_first_w, g_first_h); + } + + w->destroy(sess, plug); + api->destroy(sess); + DestroyWindow(parent); + + std::printf(failures ? "\nFAILED (%d)\n" : "\nOK\n", failures); + return failures ? 1 : 0; +}