ocornut / imgui

Dear ImGui: Bloat-free Graphical User interface for C++ with minimal dependencies
MIT License
59.1k stars 10.08k forks source link

Crash with Metal/SDL when switching to fullscreen #4414

Open ghost opened 3 years ago

ghost commented 3 years ago
Dear ImGui 1.84 WIP (18313)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=201402
define: __APPLE__
define: __GNUC__=4
define: __clang_version__=12.0.5 (clang-1205.0.22.11)
--------------------------------
io.BackendPlatformName: imgui_impl_sdl
io.BackendRendererName: imgui_impl_metal
io.ConfigFlags: 0x00000003
 NavEnableKeyboard
 NavEnableGamepad
io.ConfigMacOSXBehaviors
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x0000000E
 HasMouseCursors
 HasSetMousePos
 RendererHasVtxOffset
--------------------------------
io.Fonts: 2 fonts, Flags: 0x00000000, TexSize: 512,256
io.DisplaySize: 1280.00,720.00
io.DisplayFramebufferScale: 2.00,2.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

Version/Branch of Dear ImGui:

Version: #f99fe72 Branch: master

Back-end/Renderer/Compiler/OS

Back-ends: imgui_impl_sdl.cpp + imgui_impl_metal.cpp Compiler: Xcode Clang 12.0.5 Operating System: macOS 11.5.1

My Issue/Question:

When switching to fullscreen via the standard keyboard shortcut ⌃⌘F, the application crashes, asserting in imgui_impl_metal.mm, line 523:

                    // Apply scissor/clipping rectangle
                    MTLScissorRect scissorRect =
                    {
                        .x = NSUInteger(clip_rect.x),
                        .y = NSUInteger(clip_rect.y),
                        .width = NSUInteger(clip_rect.z - clip_rect.x),
                        .height = NSUInteger(clip_rect.w - clip_rect.y)
                    };
                    [commandEncoder setScissorRect:scissorRect]; // Asserts here

The assertion appears to come from Metal itself:

validateMTLScissorRect:2649: failed assertion (rect.x(0) + rect.width(2880))(2880) must be <= render pass width(2560)' validateMTLScissorRect:2649: failed assertion (rect.x(0) + rect.width(2880))(2880) must be <= render pass width(2560)'

Screenshots/Video

Nothing to screenshot here as it's not a visual bug.

Standalone, minimal, complete and verifiable example: example.zip

ocornut commented 3 years ago

Hello, Thanks for reporting this. Can you confirm it happens with our unmodified demo app? It looks like imgui side was already told about the new framebuffer size, but underlying Metal buffers weren't setup properly.

ghost commented 3 years ago

It's not happening with the demo no, however the example is 148 lines of code, so hopefully that's enough for you to figure out the issue. There's nothing special happening in that code that could cause the crash, and there's no crash occurring without imgui present.

ocornut commented 3 years ago

Well if our main.mm works and not yours I guess it would make sense to try to understand the difference. Saying "there's nothing special happening" merely means "I don't understand yet what's causing this".

Given the reported issue, it feels to me that Metal and Dear ImGui are not in sync in term of display size. I don't have access to a Metal device to investigate further myself.

Mielie commented 2 years ago

I'm also having this issue. Using imgui_impl_sdl.cpp + imgui_impl_metal.cpp with clang 13.0.0 compiling on an m1 Mac.

For me this is happening with the demo program without any modifications. I am using the dockable branch.

Console error is exactly the same as described by the previous reporter.

Mielie commented 2 years ago

This doesn't appear to be limited to enabling fullscreen, it also occurs when the window is resized by the user. If you drag to make the window larger it works, but if you drag to make the window smaller it causes the same assertion to fail.

Mielie commented 2 years ago

This is only an issue for me when I am running my code from Xcode in the debug mode. If I compile and execute outside of Xcode then I have no issues at all.

I have found that I can successfully get the code to run in debug mode as well by disabling metal API validation in Xcode (edit schema -> run -> metal api validation).

rokups commented 2 years ago

@Mielie what OS version are you using? Also maybe you are using a retina screen? If yes - maybe issue goes away if you switch to non-hdpi resolution in OS settings?

So far i could not replicate this issue. Im testing on 10.13 and non-hdpi screen.

Mielie commented 2 years ago

@rokups The problem first started for me on Big Sur (which is when I started this project). It is still happening on Monterey (now on 12.2). I'm using an M1 MacBook Pro, so it does have a Retina display and I've tried all the different scaled options and I still get the assert unless I disabled API validation.

rokups commented 2 years ago

I've tried all the different scaled options

Can you try unscaled resolution? It may be possible that issue stems from some incorrect handling of this scaling. Even though i could not trigger it when switching to a scaled resolution myself, maybe it is a problem which im getting saved from by my hardware somehow.

Mielie commented 2 years ago

I normally have it set to default, but I've also tried the scaled options. There doesn't seem to be a way to select a specific resolution: resolution

rokups commented 2 years ago

Ah... Yeah what i meant is a resolution under "Scaled" option, which does not have "HDPI" next to it. I guess you tried that already... 🤔

Mielie commented 2 years ago

Yup, tried all the options under scaled...

Mielie commented 2 years ago

Just tried connecting an external display using HDMI. I set it as the main display and a non HDPI resolution (1024x768) and still got the issue.

Mielie commented 1 year ago

I note that this issue is now closed, and the fix in ImGui.cpp mentioned has removed the assert when you reduce the application window size, however, I am still getting the same assert when I attempt to switch the app to fullscreen.

radgeRayden commented 7 months ago

This problem also happens with the wgpu/sdl2 backend on linux nvidia. What I've been told is that the issue lies in the timing of the window size query; it is done automatically when one begins a new sdl frame, but the size might already be different from the one used to reconfigure the surface after an attempt at acquiring the next frame image fails. Locally I've worked around this by editing the DrawData struct prior to sending it to wgpu, replacing the DisplaySize field with a canonical size I query at the beginning of the frame and is used for the surface configuration as well.