From 41f6aacf4a898e17e63f5eab289c36624906cae7 Mon Sep 17 00:00:00 2001 From: Vladimir Ein Date: Fri, 20 Jun 2025 00:21:23 +0500 Subject: [PATCH] fix (win32): No more hanging up when the viewport it resized. Removed WM_SIZING as it is not needed and was processed incorrectly (#2401). Reworked the message loop so that it processes all pending messages rather than one message per frame (#544, #1571, #2357). Continue rendering when the viewport is being moved or resized by the user. Sending resize events, too (#1896, #2217). Removed unused width/height fields from mvViewport. --- src/dearpygui_commands.h | 2 +- src/mvViewport.h | 4 +- src/mvViewport_apple.mm | 7 +- src/mvViewport_linux.cpp | 4 +- src/mvViewport_win32.cpp | 265 +++++++++++++++++---------------------- 5 files changed, 121 insertions(+), 161 deletions(-) diff --git a/src/dearpygui_commands.h b/src/dearpygui_commands.h index 457f2884b..74deb8704 100644 --- a/src/dearpygui_commands.h +++ b/src/dearpygui_commands.h @@ -2156,7 +2156,7 @@ create_viewport(PyObject* self, PyObject* args, PyObject* kwargs) )) return GetPyNone(); - mvViewport* viewport = mvCreateViewport(width, height); + mvViewport* viewport = mvCreateViewport(); if (PyObject* item = PyDict_GetItemString(kwargs, "clear_color")) viewport->clearColor = ToColor(item); if (PyObject* item = PyDict_GetItemString(kwargs, "small_icon")) viewport->small_icon = ToString(item); if (PyObject* item = PyDict_GetItemString(kwargs, "large_icon")) viewport->large_icon = ToString(item); diff --git a/src/mvViewport.h b/src/mvViewport.h index c2b28c809..52e194b51 100644 --- a/src/mvViewport.h +++ b/src/mvViewport.h @@ -39,8 +39,6 @@ struct mvViewport // position/size b8 sizeDirty = false; b8 posDirty = false; - u32 width = 0; - u32 height = 0; u32 minwidth = 250; u32 minheight = 250; u32 maxwidth = 10000; @@ -56,7 +54,7 @@ struct mvViewport }; -mvViewport* mvCreateViewport (u32 width, u32 height); +mvViewport* mvCreateViewport (); void mvCleanupViewport (mvViewport& viewport); void mvShowViewport (mvViewport& viewport, b8 minimized, b8 maximized); void mvMaximizeViewport(mvViewport& viewport); diff --git a/src/mvViewport_apple.mm b/src/mvViewport_apple.mm index f468e6bfd..e6e41cf2d 100644 --- a/src/mvViewport_apple.mm +++ b/src/mvViewport_apple.mm @@ -11,11 +11,9 @@ #include mvViewport* -mvCreateViewport(unsigned width, unsigned height) +mvCreateViewport() { auto viewport = new mvViewport(); - viewport->width = width; - viewport->height = height; viewport->platformSpecifics = new mvViewportData(); return viewport; } @@ -236,9 +234,6 @@ viewportData->layer.drawableSize = CGSizeMake(width, height); id drawable = [viewportData->layer nextDrawable]; - viewport->width = (unsigned)width; - viewport->height = (unsigned)height; - id commandBuffer = [graphicsData->commandQueue commandBuffer]; graphicsData->renderPassDescriptor.colorAttachments[0].clearColor = MTLClearColorMake(viewport->clearColor.r, viewport->clearColor.g, diff --git a/src/mvViewport_linux.cpp b/src/mvViewport_linux.cpp index eb5c2b642..b8b6496e3 100644 --- a/src/mvViewport_linux.cpp +++ b/src/mvViewport_linux.cpp @@ -107,11 +107,9 @@ mvPrerender() } mvViewport* -mvCreateViewport(unsigned width, unsigned height) +mvCreateViewport() { mvViewport* viewport = new mvViewport(); - viewport->width = width; - viewport->height = height; viewport->platformSpecifics = new mvViewportData(); return viewport; } diff --git a/src/mvViewport_win32.cpp b/src/mvViewport_win32.cpp index 135823857..6e7757625 100644 --- a/src/mvViewport_win32.cpp +++ b/src/mvViewport_win32.cpp @@ -37,55 +37,86 @@ mvHandleModes(mvViewport& viewport) } +// Applies deferred changes to various viewport parameters set from Python via API static void -mvPrerender(mvViewport& viewport) +ApplyViewportParms(mvViewport& viewport) { - MV_PROFILE_SCOPE("Viewport prerender") + mvViewportData* viewportData = (mvViewportData*)viewport.platformSpecifics; - mvViewportData* viewportData = (mvViewportData*)viewport.platformSpecifics; + if (viewport.posDirty) + { + int horizontal_shift = get_horizontal_shift(viewportData->handle); + SetWindowPos(viewportData->handle, viewport.alwaysOnTop ? HWND_TOPMOST : HWND_TOP, viewport.xpos - horizontal_shift, viewport.ypos, 0, 0, SWP_SHOWWINDOW | SWP_NOSIZE); + viewport.posDirty = false; + } - if (viewportData->msg.message == WM_QUIT) - viewport.running = false; + if (viewport.sizeDirty) + { + SetWindowPos(viewportData->handle, viewport.alwaysOnTop ? HWND_TOPMOST : HWND_TOP, 0, 0, viewport.actualWidth, viewport.actualHeight, SWP_SHOWWINDOW | SWP_NOMOVE); + viewport.sizeDirty = false; + } + if (viewport.modesDirty) { - // TODO: we probably need a separate mutex for this - std::lock_guard lk(GContext->mutex); + viewportData->modes = WS_OVERLAPPED; - if (viewport.posDirty) - { - int horizontal_shift = get_horizontal_shift(viewportData->handle); - SetWindowPos(viewportData->handle, viewport.alwaysOnTop ? HWND_TOPMOST : HWND_TOP, viewport.xpos - horizontal_shift, viewport.ypos, 0, 0, SWP_SHOWWINDOW | SWP_NOSIZE); - viewport.posDirty = false; + if (viewport.resizable && viewport.decorated) viewportData->modes |= WS_THICKFRAME; + if (viewport.decorated) { + viewportData->modes |= WS_CAPTION | WS_SYSMENU | WS_MINIMIZEBOX | WS_MAXIMIZEBOX; } - - if (viewport.sizeDirty) - { - SetWindowPos(viewportData->handle, viewport.alwaysOnTop ? HWND_TOPMOST : HWND_TOP, 0, 0, viewport.actualWidth, viewport.actualHeight, SWP_SHOWWINDOW | SWP_NOMOVE); - viewport.sizeDirty = false; + else { + viewportData->modes |= WS_POPUP; } - if (viewport.modesDirty) - { - viewportData->modes = WS_OVERLAPPED; + SetWindowLongPtr(viewportData->handle, GWL_STYLE, viewportData->modes); + SetWindowPos(viewportData->handle, viewport.alwaysOnTop ? HWND_TOPMOST : HWND_NOTOPMOST, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE | SWP_SHOWWINDOW); + viewport.modesDirty = false; + } - if (viewport.resizable && viewport.decorated) viewportData->modes |= WS_THICKFRAME; - if (viewport.decorated) { - viewportData->modes |= WS_CAPTION | WS_SYSMENU | WS_MINIMIZEBOX | WS_MAXIMIZEBOX; - } - else { - viewportData->modes |= WS_POPUP; - } + if (viewport.titleDirty) + { + SetWindowTextA(viewportData->handle, viewport.title.c_str()); + viewport.titleDirty = false; + } +} - SetWindowLongPtr(viewportData->handle, GWL_STYLE, viewportData->modes); - SetWindowPos(viewportData->handle, viewport.alwaysOnTop ? HWND_TOPMOST : HWND_NOTOPMOST, 0, 0, 0, 0, SWP_NOMOVE | SWP_NOSIZE | SWP_SHOWWINDOW); - viewport.modesDirty = false; - } +static void +StartNewFrame() +{ + // Font manager is thread-unsafe, so we'd better sync it + std::lock_guard lk(GContext->mutex); - if (viewport.titleDirty) - { - SetWindowTextA(viewportData->handle, viewport.title.c_str()); - viewport.titleDirty = false; - } + if (mvToolManager::GetFontManager().isInvalid()) + { + mvToolManager::GetFontManager().rebuildAtlas(); + ImGui_ImplDX11_InvalidateDeviceObjects(); + mvToolManager::GetFontManager().updateAtlas(); + } + + // Start the Dear ImGui frame + ImGui_ImplDX11_NewFrame(); + ImGui_ImplWin32_NewFrame(); + + // Note: ImGui::NewFrame can conflict with get_text_size() on fonts: + // in particular, it can do SetCurrentFont() somewhere in the middle of + // get_text_size(), and thus ruin its measurements. + // That's why we cover NewFrame() with the mutex, too. + ImGui::NewFrame(); +} + + +static bool +mvPrerender(mvViewport& viewport) +{ + MV_PROFILE_SCOPE("Viewport prerender") + + mvViewportData* viewportData = (mvViewportData*)viewport.platformSpecifics; + + // An extra scope for mutex lock + { + // TODO: we probably need a separate mutex for this + std::lock_guard lk(GContext->mutex); + ApplyViewportParms(viewport); } // Poll and handle messages (inputs, window resize, etc.) @@ -97,40 +128,32 @@ mvPrerender(mvViewport& viewport) if (GContext->IO.waitForInput) ::WaitMessage(); - if (::PeekMessage(&viewportData->msg, nullptr, 0U, 0U, PM_REMOVE)) + while (::PeekMessage(&viewportData->msg, nullptr, 0U, 0U, PM_REMOVE)) { - ::TranslateMessage(&viewportData->msg); - ::DispatchMessage(&viewportData->msg); - //continue; - } - - { - // Font manager is thread-unsafe, so we'd better sync it - std::lock_guard lk(GContext->mutex); - - if (mvToolManager::GetFontManager().isInvalid()) + if (viewportData->msg.message == WM_QUIT) + { + viewport.running = false; + return false; + } + else { - mvToolManager::GetFontManager().rebuildAtlas(); - ImGui_ImplDX11_InvalidateDeviceObjects(); - mvToolManager::GetFontManager().updateAtlas(); + ::TranslateMessage(&viewportData->msg); + ::DispatchMessage(&viewportData->msg); } } - // Start the Dear ImGui frame - ImGui_ImplDX11_NewFrame(); - ImGui_ImplWin32_NewFrame(); - ImGui::NewFrame(); - + StartNewFrame(); + return true; } +const UINT_PTR resizeTimerID = 1; + static LRESULT mvHandleMsg(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam) noexcept { if (ImGui_ImplWin32_WndProcHandler(hWnd, msg, wParam, lParam)) return true; - static UINT_PTR puIDEvent = 0; - mvViewport* viewport = GContext->viewport; mvGraphics& graphics = GContext->graphics; mvViewportData* viewportData = (mvViewportData*)viewport->platformSpecifics; @@ -139,64 +162,6 @@ mvHandleMsg(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam) noexcept switch (msg) { - case WM_PAINT: - { - if (GContext->frame > 0) - { - - RECT rect; - RECT crect; - int awidth = 0; - int aheight = 0; - int cwidth = 0; - int cheight = 0; - if (GetWindowRect(hWnd, &rect)) - { - awidth = rect.right - rect.left; - aheight = rect.bottom - rect.top; - } - - if (GetClientRect(hWnd, &crect)) - { - cwidth = crect.right - crect.left; - cheight = crect.bottom - crect.top; - } - - { - std::lock_guard lk(GContext->mutex); - - viewport->actualWidth = awidth; - viewport->actualHeight = aheight; - - - GContext->viewport->clientHeight = cheight; - GContext->viewport->clientWidth = cwidth; - - //GContext->viewport->resized = true; - mvOnResize(); - GContext->viewport->resized = false; - - if (mvToolManager::GetFontManager().isInvalid()) - { - mvToolManager::GetFontManager().rebuildAtlas(); - ImGui_ImplDX11_InvalidateDeviceObjects(); - mvToolManager::GetFontManager().updateAtlas(); - } - } - // Start the Dear ImGui frame - ImGui_ImplDX11_NewFrame(); - ImGui_ImplWin32_NewFrame(); - ImGui::NewFrame(); - Render(); - present(graphics, GContext->viewport->clearColor, GContext->viewport->vsync); - } - // must be called for the OS to do its thing - PAINTSTRUCT tPaint; - HDC tDeviceContext = BeginPaint(hWnd, &tPaint); - EndPaint(hWnd, &tPaint); - break; - } - case WM_GETMINMAXINFO: { // TODO: lock the mutex? @@ -209,19 +174,28 @@ mvHandleMsg(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam) noexcept break; } - case WM_MOVING: + case WM_MOVE: { std::lock_guard lk(GContext->mutex); - int horizontal_shift = get_horizontal_shift(viewportData->handle); - RECT rect = *(RECT*)(lParam); - viewport->xpos = rect.left + horizontal_shift; - viewport->ypos = rect.top; + // We explicitly ignore all WM_MOVE messages until the rendering loop + // starts. This is because on Windows 10 and later, the coordinates passed + // to CreateWindow don't point to the *visible* top left corner of the window + // (see DWMWA_EXTENDED_FRAME_BOUNDS - there's transparent padding around the window). + // ApplyViewportParms, called from mvPrerender, moves the viewport to correct + // coordinates, but this will only happen if we don't spoil xpos/ypos in a WM_MOVE + // sent before the first call to mvPrerender (e.g. in response to ShowWindow). + RECT rect; + if (GContext->frame > 0 && GetWindowRect(hWnd, &rect)) + { + int horizontal_shift = get_horizontal_shift(viewportData->handle); + viewport->xpos = rect.left + horizontal_shift; + viewport->ypos = rect.top; + } break; } case WM_SIZE: - case WM_SIZING: if (graphicsData != nullptr && wParam != SIZE_MINIMIZED) { @@ -247,25 +221,10 @@ mvHandleMsg(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam) noexcept viewport->actualWidth = awidth; viewport->actualHeight = aheight; + viewport->clientWidth = cwidth; + viewport->clientHeight = cheight; - - if (viewport->decorated) - { - GContext->viewport->clientHeight = cheight; - GContext->viewport->clientWidth = cwidth; - } - else - { - GContext->viewport->clientHeight = cheight; - GContext->viewport->clientWidth = cwidth; - } - - GContext->viewport->resized = true; - //mvOnResize(); - - // I believe this are only used for the error logger - viewport->width = (UINT)LOWORD(lParam); - viewport->height = (UINT)HIWORD(lParam); + viewport->resized = true; if (viewport->decorated) resize_swapchain(graphics, (int)(UINT)LOWORD(lParam), (int)(UINT)HIWORD(lParam)); @@ -280,21 +239,32 @@ mvHandleMsg(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam) noexcept // Timer events can still be caught so here we add a timer so we // can continue rendering when catching the WM_TIMER event. // Timer is killed in the WM_EXITSIZEMOVE case below. - puIDEvent = SetTimer(NULL, puIDEvent, USER_TIMER_MINIMUM, NULL); - SetTimer(hWnd, puIDEvent, USER_TIMER_MINIMUM, NULL); + SetTimer(hWnd, resizeTimerID, USER_TIMER_MINIMUM, NULL); break; } case WM_EXITSIZEMOVE: { - KillTimer(hWnd, puIDEvent); + KillTimer(hWnd, resizeTimerID); break; } case WM_TIMER: { - if (wParam == puIDEvent) - mvOnResize(); + if (wParam == resizeTimerID) + { + // TODO: we probably need a separate mutex for ApplyViewportParms + std::lock_guard lk(GContext->mutex); + ApplyViewportParms(*viewport); + StartNewFrame(); + Render(); + present(graphics, viewport->clearColor, viewport->vsync); + if (viewport->resized) + { + mvOnResize(); + viewport->resized = false; + } + } break; } case WM_SYSCOMMAND: @@ -361,11 +331,9 @@ mvHandleMsg(HWND hWnd, UINT msg, WPARAM wParam, LPARAM lParam) noexcept } mvViewport* -mvCreateViewport(unsigned width, unsigned height) +mvCreateViewport() { mvViewport* viewport = new mvViewport(); - viewport->width = width; - viewport->height = height; viewport->platformSpecifics = new mvViewportData(); return viewport; } @@ -496,7 +464,8 @@ mvCleanupViewport(mvViewport& viewport) void mvRenderFrame() { - mvPrerender(*GContext->viewport); + if (!mvPrerender(*GContext->viewport)) + return; Render(); present(GContext->graphics, GContext->viewport->clearColor, GContext->viewport->vsync); }