Complete macOS docking and viewport support

[stuartcarnie]

trafficstars

This PR builds on the work of https://github.com/ocornut/imgui/pull/2778 to provide complete docking and viewport support for macOS. This PR address the following issues:

• ✅ Correct viewport window focus. Borderless windows can become key windows, allowing individual viewports to be brought forward:

  

• ✅ Fix a performance / hang issue when a window is completely occluded, which resulted in nextDrawable hanging for about 1 second, causing ImGui to become unresponsive.

• ✅ Fix crash in Metal backend when ImDrawCmd->ElemCount == 0, which would occur when showing modal windows

• ✅ Correct behaviour when dragging viewports across multiple monitors

• ✅ Handle high-frequency, momentum mouse wheel scroll events, for a native macOS feel when flicking a trackpad or magic mouse to scroll a view:

  

• ✅ Handle monitor reconfigure events

• ✅ Rebased and merged on latest docking branch

[stuartcarnie]

@ocornut this branch is ready for review to add complete docking support to macOS. I have found it works really well and performs better than GLFW and SDL2 on macOS, maintaining 60 fps even when there are several windows opened.

[rokups]

Hey, thanks for this PR! I am testing it out and i experience some issues still.

Viewport windows are moved together with main viewport window.

1. Drag "Hello, world!" window out of main viewport.
2. Drag main viewport window. Observed: "Hello, world!" is moved by same amount as main viewport is dragged. Expected: "Hello, world!" window should remain stationary.

Window focus does not work 100 correct.

1. Drag "Hello, world!" window out of main viewport.
2. Click "Hello, world!" window to focus it (titlebar becomes blue).
3. Focus a third party window like terminal or finder. Observed: "Hello, world!" window titlebar remains blue. Expected: "Hello, world!" window titlebar should become black.

[stuartcarnie]

Thanks for the feedback. I have a couple of questions.

Hey, thanks for this PR! I am testing it out and i experience some issues still.

Viewport windows are moved together with main viewport window.

1. Drag "Hello, world!" window out of main viewport.
2. Drag main viewport window. Observed: "Hello, world!" is moved by same amount as main viewport is dragged. Expected: "Hello, world!" window should remain stationary.

What OS version? I am not observing that behaviour on macOS 12.3 (or any version of 12 prior).

Window focus does not work 100 correct.

1. Drag "Hello, world!" window out of main viewport.
2. Click "Hello, world!" window to focus it (titlebar becomes blue).
3. Focus a third party window like terminal or finder. Observed: "Hello, world!" window titlebar remains blue. Expected: "Hello, world!" window titlebar should become black.

I think we can fix it; however, that behaviour is the same when the window is inside the viewport:

Hopefully we can send an app-focus event to ImGui to resolve it.

[stuartcarnie]

@rokups

Window focus does not work 100 correct.

1. Drag "Hello, world!" window out of main viewport.
2. Click "Hello, world!" window to focus it (titlebar becomes blue).
3. Focus a third party window like terminal or finder. Observed: "Hello, world!" window titlebar remains blue. Expected: "Hello, world!" window titlebar should become black.

This appears to be an ImGui issue. I have observed the same behaviour testing with GLFW and SDL docking versions. The OSX backend does send the App active / inactive events, but ImGui does not change the window title.

[rokups]

What OS version?

I am testing on 10.13.

This appears to be an ImGui issue.

Indeed it is! Ignore this comment then. Most important thing is backend sending these events. We will fix focus issue in the core.

[rokups]

Thanks for replies @stuartcarnie :pray: Could you please point me to changes that implement these things you listed?

• Fix a performance / hang issue when a window is completely occluded, which resulted in nextDrawable hanging for about 1 second, causing ImGui to become unresponsive.
• Fix crash in Metal backend when ImDrawCmd->ElemCount == 0, which would occur when showing modal windows

[stuartcarnie]

@rokups sure thing!

Thanks for replies @stuartcarnie 🙏 Could you please point me to changes that implement these things you listed?

• Fix a performance / hang issue when a window is completely occluded, which resulted in nextDrawable hanging for about 1 second, causing ImGui to become unresponsive.

https://github.com/stuartcarnie/imgui/blob/6a27fa53f06f5f8511b8fe82594aaef51c9d6646/backends/imgui_impl_metal.mm#L283-L290

• Fix crash in Metal backend when ImDrawCmd->ElemCount == 0, which would occur when showing modal windows

I think this is fixed now, but here is the check:

https://github.com/stuartcarnie/imgui/blob/6a27fa53f06f5f8511b8fe82594aaef51c9d6646/backends/imgui_impl_metal.mm#L732-L733

[ocornut]

Merged in master d58b8414, e66fc22 Merged in docking 6868d11 (+ small bits d666a1d) All as reworked by @rokups from #4821 (which itself is based on #2778) Thanks @metarutaiga and @stuartcarnie !

Before we close this topic #4821 could Rokups write a recap of all open questions/issues and what hasn't been merged? Some might end up in separate issues.

At minimum I know the mouse stuff hasn't been merged. I suspect ImGui::UpdateInputEvents() has a bug, in the else if (e->Type == ImGuiInputEventType_MouseWheel) block, if (trickle_fast_inputs && (mouse_wheeled || mouse_button_changed != 0)) should IHMO be changed to if (trickle_fast_inputs && (mouse_moved || mouse_button_changed != 0)) to both merge multiple wheel events and trickle wheel events after a mouse pos event.

[rokups]

In addition to momentum mouse change, improvement to MTLStorageModeManaged texture mode detection was also left out. Primary reason is that it used a compiler feature @available(), which wasnt guaranteed to be available on older compilers. We need to reevaluate whether we have to switch back to preprocessor defines or if our decided version constraints (no support below v10.12 Sierra) allow using these expressions. Also, from what i read in the documentation it made me doubt whether we need MTLStorageModeShared at all, considering only data exchange between GPU and CPU happens on font texture upload after rasterization and we can very well do it manually, making MTLStorageModeManaged not necessary.

[stuartcarnie]

@rokups MTLStorageModeManaged is probably not necessary, unless benchmarks and profiling says otherwise

[ocornut]

I suspect ImGui::UpdateInputEvents() has a bug, in the else if (e->Type == ImGuiInputEventType_MouseWheel) block, if (trickle_fast_inputs && (mouse_wheeled || mouse_button_changed != 0)) should IHMO be changed to if (trickle_fast_inputs && (mouse_moved || mouse_button_changed != 0)) to both merge multiple wheel events and trickle wheel events after a mouse pos event.

I pushed e346059 with this change now.

[ocornut]

Hello Stuart, some elements of this branch were not merged hence why we kept the PR open.

[stuartcarnie]

Ahh, sorry @ocornut - I have restored the branch and re-opened the PR