Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash with Metal/SDL when switching to fullscreen #4414

Open
ghost opened this issue Aug 10, 2021 · 15 comments
Open

Crash with Metal/SDL when switching to fullscreen #4414

ghost opened this issue Aug 10, 2021 · 15 comments

Comments

@ghost
Copy link

ghost commented Aug 10, 2021

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
Copy link
Owner

ocornut commented Aug 11, 2021

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
Copy link
Author

ghost commented Aug 21, 2021

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
Copy link
Owner

ocornut commented Aug 23, 2021

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
Copy link

Mielie commented Oct 6, 2021

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
Copy link

Mielie commented Oct 6, 2021

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
Copy link

Mielie commented Dec 16, 2021

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
Copy link
Contributor

rokups commented Feb 3, 2022

@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
Copy link

Mielie commented Feb 3, 2022

@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
Copy link
Contributor

rokups commented Feb 3, 2022

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
Copy link

Mielie commented Feb 3, 2022

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
Copy link
Contributor

rokups commented Feb 3, 2022

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
Copy link

Mielie commented Feb 3, 2022

Yup, tried all the options under scaled...

@Mielie
Copy link

Mielie commented Feb 3, 2022

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
Copy link

Mielie commented Apr 9, 2023

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants