imgui icon indicating copy to clipboard operation
imgui copied to clipboard

Win32 DX11 monitor Work Size desync issue

Open lailoken opened this issue 10 months ago β€’ 17 comments

Version/Branch of Dear ImGui:

Version 1.91.9, Branch: docking

Back-ends:

imgui_impl_dx11 + imgui_impl_win32

Compiler, OS:

MSVC

Full config/build information:

Dear ImGui 1.91.9 WIP (19184)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 4, sizeof(ImDrawVert): 20
define: __cplusplus=199711
define: IMGUI_DISABLE_OBSOLETE_FUNCTIONS
define: _WIN32
define: _WIN64
define: _MSC_VER=1940
define: _MSVC_LANG=202002
define: IMGUI_HAS_VIEWPORT
define: IMGUI_HAS_DOCK
--------------------------------
io.BackendPlatformName: imgui_impl_win32
io.BackendRendererName: imgui_impl_dx11
io.ConfigFlags: 0x0000C481
 NavEnableKeyboard
 DockingEnable
 ViewportsEnable
 DpiEnableScaleViewports
 DpiEnableScaleFonts
io.ConfigViewportsNoAutoMerge
io.ConfigViewportsNoDefaultParent
io.ConfigDockingWithShift
io.ConfigNavCaptureKeyboard
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x00001C0E
 HasMouseCursors
 HasSetMousePos
 PlatformHasViewports
 HasMouseHoveredViewport
 RendererHasVtxOffset
 RendererHasViewports
--------------------------------
io.Fonts: 11 fonts, Flags: 0x00000000, TexSize: 1024,1024
io.DisplaySize: 1795.00,1161.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------
style.WindowPadding: 8.00,8.00
style.WindowBorderSize: 1.00
style.FramePadding: 4.00,3.00
style.FrameRounding: 0.00
style.FrameBorderSize: 0.00
style.ItemSpacing: 8.00,4.00
style.ItemInnerSpacing: 4.00,4.00

Details:

I've been seeing some desync issues happening on occasion, and they may or may not all be related to each other.

One of the cases is when working with ImGui over RDP, when switching back one user reported that the mouse showed the hover over controls that were about 20px below where the mouse was (once the window was moved it was correct again).

And another user commented that our hovering app bar was not above the windows taskbar as it usually is, but right over it (and no amount of dragging fixed that).

I believe (perhaps both) these are related to the fact that the monitor layout is different and changes (in several ways). ImGui already handled the WM_DISPLAYCHANGE message, so this confused me. However, something else I saw when switching from and to RDP is that after connecting, Windows goes wild reordering windows (and the task bar) for a few more seconds, thus even though ImGui received the WM_DISPLAYCHANGE shortly after connecting, there may still be additional messages needed.

Indeed, I also saw that resizing the windows task bar did not update ImGui's internal metrics of monitor work sizes real-time as it should, so I added some debugging to see the messages being dispatched.

I'm not sure if it is the correct message, but I also added WM_GETMINMAXINFO to:

    case WM_DISPLAYCHANGE:
    case WM_GETMINMAXINFO:
        bd->WantUpdateMonitors = true;
        return 0;

And this at least solved how ImGui responds to taskbar changes.

I'm not an experienced windows programmer, and if anyone else can see an issue with this, perhaps there is a better message, but I believe this will address that shortfall.

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

No response

lailoken avatar Feb 19 '25 22:02 lailoken

Oops, I think I knew about this oversight but forgot to report it :( Thanks for the reminder!

WM_GETMINMAXINFO isn't the right message for this as that'll constantly refresh the monitor layout as the window is moved and resized.

My private backend looks for WM_SETTINGCHANGE with wParam == SPI_SETWORKAREA, IE:

case WM_SETTINGCHANGE:
    if (wParam != SPI_SETWORKAREA)
        break;
case WM_DISPLAYCHANGE:
    bd->WantUpdateMonitors = true;
    return 0;

Can you confirm that fixes things for you?

PathogenDavid avatar Feb 20 '25 00:02 PathogenDavid

Thanks, this seems to work (with a small change)

Initially I did not see any reference to WM_SETTINGCHANGE when resizing my app bar. This is my debug log for reference, I have now also logged the wParam field to see when there is a possible SPI_SETWORKAREA:

Sub  WndProc: WM_WININICHANGE (0x001a) 0x2f 0x0000000000000000 - (0000021451E20F30)
Main WndProc: WM_WININICHANGE (0x001a) 0x0000000000000000
Sub  WndProc: WM_WINDOWPOSCHANGING (0x0046) 0x0 0x00000020856fe6f0 - (0000021451E20F30)
Sub  WndProc: WM_WINDOWPOSCHANGED (0x0047) 0x0 0x00000020856fe6f0 - (0000021451E20F30)
Sub  WndProc: WM_MOVE (0x0003) 0x0 0x0000000002dd0dc8 - (0000021451E20F30)

I notice that:

#define WM_WININICHANGE                 0x001A
#if(WINVER >= 0x0400)
#define WM_SETTINGCHANGE                WM_WININICHANGE
#endif /* WINVER >= 0x0400 */
  • It's there for compatibility, new applications should emit WM_SETTINGCHANGE as per the manual pages.

But I think a more backward compatible version would be to use WM_WININICHANGE for ImGui's purposes:

case WM_WININICHANGE:
    if (wParam != SPI_SETWORKAREA)
        break;
case WM_DISPLAYCHANGE:
    bd->WantUpdateMonitors = true;
    return 0;

This fixes the workarea issues, and may or may not fix the other offset issues users have reported (although I have been unable to reproduce their issues to test).

This is a good addition then nonetheless.

lailoken avatar Feb 20 '25 17:02 lailoken

Reopening as this should be in our backend.

ocornut avatar Feb 20 '25 17:02 ocornut

But I think a more backward compatible version would be to use WM_WININICHANGE for ImGui's purposes:

WINVER 0x0400 is Windows NT 4.0 so probably not compatibility worth worrying about. WM_DISPLAYCHANGE itself already needs WINVER >= 0x0400, so the backend already doesn't support Windows 98 😁

PathogenDavid avatar Feb 20 '25 18:02 PathogenDavid

Yes, WM_SETTINGCHANGE should be good then. πŸ‘

lailoken avatar Feb 20 '25 22:02 lailoken

Thank you both for investigating! I have pushed change ea59440 (attributed to David).

TL;DR: work area change may trigger a WM_SETTINGCHANGE's SPI_SETWORKAREA message.

    case WM_DISPLAYCHANGE:
        bd->WantUpdateMonitors = true;
        return 0;
++  case WM_SETTINGCHANGE:
++      if (wParam == SPI_SETWORKAREA)
++         bd->WantUpdateMonitors = true;
++      return 0;
    }

I was curious and noticed neither GLFW neither SDL3 used WM_SETTINGCHANGE for that purpose. GLFW has a monitor callback so presumably it might want to do the same thing as work area is part of monitor? (@dougbinks ?) https://github.com/glfw/glfw/blob/e7ea71be039836da3a98cea55ae5569cb5eb885c/src/win32_init.c#L340

        case WM_DISPLAYCHANGE:
            _glfwPollMonitorsWin32();
            break;

For SDL3 it's harder for me to tell. From my point of view which is the same one of an app using SDL, I'm waiting for the following messages to refresh my list of display and work area:

case SDL_EVENT_DISPLAY_ORIENTATION:
case SDL_EVENT_DISPLAY_ADDED:
case SDL_EVENT_DISPLAY_REMOVED:
case SDL_EVENT_DISPLAY_MOVED:
case SDL_EVENT_DISPLAY_CONTENT_SCALE_CHANGED:
{
    bd->WantUpdateMonitors = true;
    return true;
}

And it doesn't look like WM_SETTINGCHANGE's SPI_SETWORKAREA has any particular effect: https://github.com/libsdl-org/SDL/blob/3293eb1a162d99914c7737b8e7a3ea4d7a97e841/src/video/windows/SDL_windowsevents.c#L2173

   case WM_DISPLAYCHANGE:
    {
        // Reacquire displays if any were added or removed
        WIN_RefreshDisplays(SDL_GetVideoDevice());
    } break;

So even if SDL_GetDisplayUsableBounds() value are not going through a cache (it's not the case for WIN_GetDisplayUsableBounds() at least) i am not sure if user is expected to poll this or if it is worth adding a specific event if there's none, or bundling into an existing event.

Sorry for pinging cc @slouken you might find this useful.

ocornut avatar Feb 21 '25 16:02 ocornut

I was curious and noticed neither GLFW neither SDL3 used WM_SETTINGCHANGE for that purpose. GLFW has a monitor callback so presumably it might want to do the same thing as work area is part of monitor

GLFW doesn't have a work area callback, only a work area query. Maximized windows automatically get resized by the OS with WM_SIZE, and mouse coordinates do not depend on monitor work area. As far as I can see changing the work area doesn't cause issues for ImGui on GLFW, though I've only tested by auto-hiding the Taskbar so far.

So at the moment there's no reason to add WM_SETTINGCHANGE event handling to GLFW unless an issue crops up.

dougbinks avatar Feb 21 '25 16:02 dougbinks

To clarify, the patch we applied was specifically for this sub-issue:

I also saw that resizing the windows task bar did not update ImGui's internal metrics of monitor work sizes real-time as it should, so I added some debugging to see the messages being dispatched.

And I brain-farted that there was a wider issue in this topic (which, to clarify, has nothing to do with GLFW or SDL) so it was hasty to me to close, reopening.

GLFW doesn't have a work area callback, only a work area query

Indeed. But as glfwGetMonitorWorkarea() takes monitor as input it seems to be a property of the monitor, from user's point of view.

ocornut avatar Feb 21 '25 16:02 ocornut

Indeed. But as glfwGetMonitorWorkarea() takes monitor as input it seems to be a property of the monitor, from user's point of view.

I agree it's a property of a monitor, but the GLFW monitor callback is for connect/disconnect events rather than property changes. At the moment we have a number of monitor properties which can be polled, but we don't have events for them changing. Adding callbacks for these property changing is a good idea, but it's a pretty large endeavour given the number of properties and OSs to cover.

I've added an issue to track this: https://github.com/glfw/glfw/issues/2691

dougbinks avatar Feb 21 '25 17:02 dougbinks

Just to clarify i can totally do without. We could rework backends to update work area once a frame and call it a day. I merely wanted to tag you for your thoughts. But you are right it may not be glfw nor sdl job to notify of those changes.

ocornut avatar Feb 21 '25 17:02 ocornut

I just did a text and noticed that our Linux SDL backend did not adjust workarea on Gnome launcher resizing either. Not a big issue for us, but I think it may also need to be addressed at some point.

lailoken avatar Feb 21 '25 17:02 lailoken

I've kind of given up on the fidelity of SDL work-area metrics, need to double-check and fall back in my app to compensate for invalid data.

This ticket case-in-point: https://github.com/libsdl-org/SDL/issues/11407#issuecomment-2595593102

lailoken avatar Feb 21 '25 17:02 lailoken

I think it’s an issue on our end that we can fix in the backend, as no one gave a specific suggestion that those values changing was tied to monitor/display changing. EDIT Never mind, your issue point to something different.

ocornut avatar Feb 21 '25 17:02 ocornut

Pushed 1a7b594 to update monitors and work area every frames with GLFW, SDL2, SDL3 backends. Confirmed that it made work area correct when resizing the Windows task-bar.

ocornut avatar Feb 21 '25 18:02 ocornut

Updating monitor every frames is causing performances issues with SDL3 on Mac (and presumably with SDL2) : #8558 I'll post to the SDL thread https://github.com/libsdl-org/SDL/issues/11407#issuecomment-2595593102 but I think we might need either an event for this, either to address the performance issue.

ocornut avatar Apr 09 '25 10:04 ocornut

I have posted a new SDL issue https://github.com/libsdl-org/SDL/issues/12785 And will work on reverting the change to limit the polling to Win32 platform for now.

ocornut avatar Apr 09 '25 10:04 ocornut

I have pushed b811c42 to workaround work-area refreshingon Windows for SDL2/SDL3 backends.

ocornut avatar Apr 09 '25 12:04 ocornut