From bda15dbb7bbed762ba2c9995f80f14731c8a309d Mon Sep 17 00:00:00 2001 From: Tim Kane Date: Tue, 12 Mar 2024 16:04:26 +1100 Subject: [PATCH 1/6] Resolve Xcode runtime warnings regarding access of NSView/NSWindow properties from a non-main thread This issue presents when rendering from a thread that is other than the window thread, more easily reproduced when disabling vsync This commit aims to populate the relevant data into backend_data, by way of dispatch_async (ocurring on the main thread). Those particular backend data items are protected by an NSlock There may be more elegant ways to do this Main Thread Checker imgui/backends/imgui_impl_osx.mm:608 -[NSView window] must be used from main thread only imgui/backends/imgui_impl_osx.mm:608 -[NSWindow backingScaleFactor] must be used from main thread only imgui/backends/imgui_impl_osx.mm:609 -[NSView bounds] must be used from main thread only --- backends/imgui_impl_osx.mm | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/backends/imgui_impl_osx.mm b/backends/imgui_impl_osx.mm index b13d15aef508..3b6e8e30d361 100644 --- a/backends/imgui_impl_osx.mm +++ b/backends/imgui_impl_osx.mm @@ -81,7 +81,13 @@ NSTextInputContext* InputContext; id Monitor; - ImGui_ImplOSX_Data() { memset(this, 0, sizeof(*this)); } + struct { + NSLock *lock; + CGFloat backingScaleFactor; + CGRect bounds; + } exclusive; + + ImGui_ImplOSX_Data() { memset(this, 0, sizeof(*this)); @autoreleasepool {this->exclusive.lock = [[NSLock alloc] init]; } } }; static ImGui_ImplOSX_Data* ImGui_ImplOSX_CreateBackendData() { return IM_NEW(ImGui_ImplOSX_Data)(); } From 1a11a3160d3a61c727d8f9e684152715db0a66d0 Mon Sep 17 00:00:00 2001 From: Tim Kane Date: Tue, 12 Mar 2024 16:05:19 +1100 Subject: [PATCH 2/6] Resolve Xcode runtime warnings regarding access of NSView/NSWindow properties from a non-main thread This issue presents when rendering from a thread that is other than the window thread, more easily reproduced when disabling vsync This commit aims to populate the relevant data into backend_data, by way of dispatch_async (ocurring on the main thread). Those particular backend data items are protected by an NSlock There may be more elegant ways to do this Main Thread Checker imgui/backends/imgui_impl_osx.mm:608 -[NSView window] must be used from main thread only imgui/backends/imgui_impl_osx.mm:608 -[NSWindow backingScaleFactor] must be used from main thread only imgui/backends/imgui_impl_osx.mm:609 -[NSView bounds] must be used from main thread only --- backends/imgui_impl_osx.mm | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/backends/imgui_impl_osx.mm b/backends/imgui_impl_osx.mm index 3b6e8e30d361..7ca87fcb55d2 100644 --- a/backends/imgui_impl_osx.mm +++ b/backends/imgui_impl_osx.mm @@ -611,8 +611,18 @@ void ImGui_ImplOSX_NewFrame(NSView* view) // Setup display size if (view) { - const float dpi = (float)[view.window backingScaleFactor]; - io.DisplaySize = ImVec2((float)view.bounds.size.width, (float)view.bounds.size.height); + dispatch_async(dispatch_get_main_queue(), ^{ + [bd->exclusive.lock lock]; + bd->exclusive.backingScaleFactor = view.window.backingScaleFactor; + bd->exclusive.bounds = view.bounds; + [bd->exclusive.lock unlock]; + }); + + [bd->exclusive.lock lock]; + const float dpi = (float) bd->exclusive.backingScaleFactor; + io.DisplaySize = ImVec2((float) bd->exclusive.bounds.size.width, bd->exclusive.bounds.size.height); + [bd->exclusive.lock unlock]; + io.DisplayFramebufferScale = ImVec2(dpi, dpi); } From c0623e97109ebf546409dfd0da23087ad162b905 Mon Sep 17 00:00:00 2001 From: Tim Kane Date: Thu, 14 Mar 2024 22:40:21 +1100 Subject: [PATCH 3/6] Needed a little more work to resolve a number of outstanding thread sanitiser concerns This is a little nasty.. I'm trying to keep all changes localised to imgui_impl_osx.mm Ideally the ImGui_IO_lock would live within ImGuiIO but I didn't want to muddy the waters Perhaps better if this lock primitive is delivered through BackendPlatformUserData, and only used where provided This change requires the client platform to explicitly lock the primitive from outside of ImGui around ImGui::NewFrame specifically, and additionally from anywhere on the client side (external to ImGui) that concurrently hooks into ImGuiIO That requires the client to define an extern lock (and use it appropriately) extern NSlock *ImGui_IO_lock If this lock were native to ImGui we could have ImGui::NewFrame() handle the lock on our behalf, negating the need for client side locking --- backends/imgui_impl_osx.mm | 42 ++++++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/backends/imgui_impl_osx.mm b/backends/imgui_impl_osx.mm index 7ca87fcb55d2..36ce449d206f 100644 --- a/backends/imgui_impl_osx.mm +++ b/backends/imgui_impl_osx.mm @@ -395,8 +395,12 @@ IMGUI_IMPL_API void ImGui_ImplOSX_NewFrame(void* _Nullable view) { #endif +NSLock *ImGui_IO_lock; + bool ImGui_ImplOSX_Init(NSView* view) { + ImGui_IO_lock = [[NSLock alloc] init]; + ImGuiIO& io = ImGui::GetIO(); ImGui_ImplOSX_Data* bd = ImGui_ImplOSX_CreateBackendData(); io.BackendPlatformUserData = (void*)bd; @@ -659,10 +663,15 @@ static ImGuiMouseSource GetMouseSource(NSEvent* event) } } + static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) { ImGuiIO& io = ImGui::GetIO(); + [ImGui_IO_lock lock]; + + bool WantCaptureMouse = io.WantCaptureMouse; + if (event.type == NSEventTypeLeftMouseDown || event.type == NSEventTypeRightMouseDown || event.type == NSEventTypeOtherMouseDown) { int button = (int)[event buttonNumber]; @@ -671,7 +680,8 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) io.AddMouseSourceEvent(GetMouseSource(event)); io.AddMouseButtonEvent(button, true); } - return io.WantCaptureMouse; + [ImGui_IO_lock unlock]; + return WantCaptureMouse; } if (event.type == NSEventTypeLeftMouseUp || event.type == NSEventTypeRightMouseUp || event.type == NSEventTypeOtherMouseUp) @@ -682,7 +692,8 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) io.AddMouseSourceEvent(GetMouseSource(event)); io.AddMouseButtonEvent(button, false); } - return io.WantCaptureMouse; + [ImGui_IO_lock unlock]; + return WantCaptureMouse; } if (event.type == NSEventTypeMouseMoved || event.type == NSEventTypeLeftMouseDragged || event.type == NSEventTypeRightMouseDragged || event.type == NSEventTypeOtherMouseDragged) @@ -697,7 +708,9 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) mousePoint = NSMakePoint(mousePoint.x, view.bounds.size.height - mousePoint.y); io.AddMouseSourceEvent(GetMouseSource(event)); io.AddMousePosEvent((float)mousePoint.x, (float)mousePoint.y); - return io.WantCaptureMouse; + + [ImGui_IO_lock unlock]; + return WantCaptureMouse; } if (event.type == NSEventTypeScrollWheel) @@ -715,7 +728,10 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) // scrollingDeltaY. When these are added to the current x and y positions of the scrolling view, // it appears to jump up or down. It can be observed in Preview, various JetBrains IDEs and here. if (event.phase == NSEventPhaseCancelled) + { + [ImGui_IO_lock unlock]; return false; + } double wheel_dx = 0.0; double wheel_dy = 0.0; @@ -740,20 +756,25 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) if (wheel_dx != 0.0 || wheel_dy != 0.0) io.AddMouseWheelEvent((float)wheel_dx, (float)wheel_dy); - return io.WantCaptureMouse; + [ImGui_IO_lock unlock]; + return WantCaptureMouse; } if (event.type == NSEventTypeKeyDown || event.type == NSEventTypeKeyUp) { if ([event isARepeat]) - return io.WantCaptureKeyboard; + { + [ImGui_IO_lock unlock]; + return WantCaptureMouse; + } int key_code = (int)[event keyCode]; ImGuiKey key = ImGui_ImplOSX_KeyCodeToImGuiKey(key_code); io.AddKeyEvent(key, event.type == NSEventTypeKeyDown); io.SetKeyEventNativeData(key, key_code, -1); // To support legacy indexing (<1.87 user code) - return io.WantCaptureKeyboard; + [ImGui_IO_lock unlock]; + return WantCaptureMouse; } if (event.type == NSEventTypeFlagsChanged) @@ -784,7 +805,10 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) case ImGuiKey_LeftAlt: mask = 0x0020; break; case ImGuiKey_RightAlt: mask = 0x0040; break; default: - return io.WantCaptureKeyboard; + { + [ImGui_IO_lock unlock]; + return WantCaptureMouse; + } } NSEventModifierFlags modifier_flags = [event modifierFlags]; @@ -792,9 +816,11 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) io.SetKeyEventNativeData(key, key_code, -1); // To support legacy indexing (<1.87 user code) } - return io.WantCaptureKeyboard; + [ImGui_IO_lock unlock]; + return WantCaptureMouse; } + [ImGui_IO_lock unlock]; return false; } From a30bc7e7949511753f947848b04222dda6ea6bb5 Mon Sep 17 00:00:00 2001 From: Tim Kane Date: Fri, 15 Mar 2024 00:02:43 +1100 Subject: [PATCH 4/6] Added a scoped lock, rather than explicitly unlocking at all return points --- backends/imgui_impl_osx.mm | 35 ++++++++++++++++++++++++----------- 1 file changed, 24 insertions(+), 11 deletions(-) diff --git a/backends/imgui_impl_osx.mm b/backends/imgui_impl_osx.mm index 36ce449d206f..cb8ca76b2672 100644 --- a/backends/imgui_impl_osx.mm +++ b/backends/imgui_impl_osx.mm @@ -395,12 +395,33 @@ IMGUI_IMPL_API void ImGui_ImplOSX_NewFrame(void* _Nullable view) { #endif + NSLock *ImGui_IO_lock; + +@interface ScopedLock : NSObject +@property (strong) NSLock *lockptr; +@end + +@implementation ScopedLock + +- (id)init:(NSLock *)incoming +{ + _lockptr = incoming; + [_lockptr lock]; + return self; +} + +- (void)dealloc { + [_lockptr unlock]; +} +@end + + bool ImGui_ImplOSX_Init(NSView* view) { ImGui_IO_lock = [[NSLock alloc] init]; - + ImGuiIO& io = ImGui::GetIO(); ImGui_ImplOSX_Data* bd = ImGui_ImplOSX_CreateBackendData(); io.BackendPlatformUserData = (void*)bd; @@ -488,6 +509,7 @@ bool ImGui_ImplOSX_Init(NSView* view) return true; } + void ImGui_ImplOSX_Shutdown() { ImGui_ImplOSX_Data* bd = ImGui_ImplOSX_GetBackendData(); @@ -668,7 +690,7 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) { ImGuiIO& io = ImGui::GetIO(); - [ImGui_IO_lock lock]; + ScopedLock *sclock = [[ScopedLock alloc] init:ImGui_IO_lock]; bool WantCaptureMouse = io.WantCaptureMouse; @@ -680,7 +702,6 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) io.AddMouseSourceEvent(GetMouseSource(event)); io.AddMouseButtonEvent(button, true); } - [ImGui_IO_lock unlock]; return WantCaptureMouse; } @@ -692,7 +713,6 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) io.AddMouseSourceEvent(GetMouseSource(event)); io.AddMouseButtonEvent(button, false); } - [ImGui_IO_lock unlock]; return WantCaptureMouse; } @@ -709,7 +729,6 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) io.AddMouseSourceEvent(GetMouseSource(event)); io.AddMousePosEvent((float)mousePoint.x, (float)mousePoint.y); - [ImGui_IO_lock unlock]; return WantCaptureMouse; } @@ -729,7 +748,6 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) // it appears to jump up or down. It can be observed in Preview, various JetBrains IDEs and here. if (event.phase == NSEventPhaseCancelled) { - [ImGui_IO_lock unlock]; return false; } @@ -756,7 +774,6 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) if (wheel_dx != 0.0 || wheel_dy != 0.0) io.AddMouseWheelEvent((float)wheel_dx, (float)wheel_dy); - [ImGui_IO_lock unlock]; return WantCaptureMouse; } @@ -764,7 +781,6 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) { if ([event isARepeat]) { - [ImGui_IO_lock unlock]; return WantCaptureMouse; } @@ -773,7 +789,6 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) io.AddKeyEvent(key, event.type == NSEventTypeKeyDown); io.SetKeyEventNativeData(key, key_code, -1); // To support legacy indexing (<1.87 user code) - [ImGui_IO_lock unlock]; return WantCaptureMouse; } @@ -816,11 +831,9 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) io.SetKeyEventNativeData(key, key_code, -1); // To support legacy indexing (<1.87 user code) } - [ImGui_IO_lock unlock]; return WantCaptureMouse; } - [ImGui_IO_lock unlock]; return false; } From 736071831f52dd72b36f05c7854bba9d66bbd294 Mon Sep 17 00:00:00 2001 From: Tim Kane Date: Fri, 15 Mar 2024 00:07:37 +1100 Subject: [PATCH 5/6] Revert unnecessary cruft --- backends/imgui_impl_osx.mm | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/backends/imgui_impl_osx.mm b/backends/imgui_impl_osx.mm index cb8ca76b2672..ae8068eb9f19 100644 --- a/backends/imgui_impl_osx.mm +++ b/backends/imgui_impl_osx.mm @@ -509,7 +509,6 @@ bool ImGui_ImplOSX_Init(NSView* view) return true; } - void ImGui_ImplOSX_Shutdown() { ImGui_ImplOSX_Data* bd = ImGui_ImplOSX_GetBackendData(); @@ -685,15 +684,12 @@ static ImGuiMouseSource GetMouseSource(NSEvent* event) } } - static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) { ImGuiIO& io = ImGui::GetIO(); ScopedLock *sclock = [[ScopedLock alloc] init:ImGui_IO_lock]; - bool WantCaptureMouse = io.WantCaptureMouse; - if (event.type == NSEventTypeLeftMouseDown || event.type == NSEventTypeRightMouseDown || event.type == NSEventTypeOtherMouseDown) { int button = (int)[event buttonNumber]; @@ -702,7 +698,7 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) io.AddMouseSourceEvent(GetMouseSource(event)); io.AddMouseButtonEvent(button, true); } - return WantCaptureMouse; + return io.WantCaptureMouse; } if (event.type == NSEventTypeLeftMouseUp || event.type == NSEventTypeRightMouseUp || event.type == NSEventTypeOtherMouseUp) @@ -713,7 +709,7 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) io.AddMouseSourceEvent(GetMouseSource(event)); io.AddMouseButtonEvent(button, false); } - return WantCaptureMouse; + return io.WantCaptureMouse; } if (event.type == NSEventTypeMouseMoved || event.type == NSEventTypeLeftMouseDragged || event.type == NSEventTypeRightMouseDragged || event.type == NSEventTypeOtherMouseDragged) @@ -729,7 +725,7 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) io.AddMouseSourceEvent(GetMouseSource(event)); io.AddMousePosEvent((float)mousePoint.x, (float)mousePoint.y); - return WantCaptureMouse; + return io.WantCaptureMouse; } if (event.type == NSEventTypeScrollWheel) @@ -774,14 +770,14 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) if (wheel_dx != 0.0 || wheel_dy != 0.0) io.AddMouseWheelEvent((float)wheel_dx, (float)wheel_dy); - return WantCaptureMouse; + return io.WantCaptureMouse; } if (event.type == NSEventTypeKeyDown || event.type == NSEventTypeKeyUp) { if ([event isARepeat]) { - return WantCaptureMouse; + return io.WantCaptureMouse; } int key_code = (int)[event keyCode]; @@ -789,7 +785,7 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) io.AddKeyEvent(key, event.type == NSEventTypeKeyDown); io.SetKeyEventNativeData(key, key_code, -1); // To support legacy indexing (<1.87 user code) - return WantCaptureMouse; + return io.WantCaptureMouse; } if (event.type == NSEventTypeFlagsChanged) @@ -822,7 +818,7 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) default: { [ImGui_IO_lock unlock]; - return WantCaptureMouse; + return io.WantCaptureMouse; } } @@ -831,7 +827,7 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) io.SetKeyEventNativeData(key, key_code, -1); // To support legacy indexing (<1.87 user code) } - return WantCaptureMouse; + return io.WantCaptureMouse; } return false; From 756feb8ab4bc77634360f36e96d912cf064c6238 Mon Sep 17 00:00:00 2001 From: Tim Kane Date: Fri, 15 Mar 2024 00:12:08 +1100 Subject: [PATCH 6/6] Additional cruft removal and inadvertent copy/paste errors --- backends/imgui_impl_osx.mm | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/backends/imgui_impl_osx.mm b/backends/imgui_impl_osx.mm index ae8068eb9f19..fcd61b6eb29c 100644 --- a/backends/imgui_impl_osx.mm +++ b/backends/imgui_impl_osx.mm @@ -724,7 +724,6 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) mousePoint = NSMakePoint(mousePoint.x, view.bounds.size.height - mousePoint.y); io.AddMouseSourceEvent(GetMouseSource(event)); io.AddMousePosEvent((float)mousePoint.x, (float)mousePoint.y); - return io.WantCaptureMouse; } @@ -743,9 +742,7 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) // scrollingDeltaY. When these are added to the current x and y positions of the scrolling view, // it appears to jump up or down. It can be observed in Preview, various JetBrains IDEs and here. if (event.phase == NSEventPhaseCancelled) - { return false; - } double wheel_dx = 0.0; double wheel_dy = 0.0; @@ -776,16 +773,14 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) if (event.type == NSEventTypeKeyDown || event.type == NSEventTypeKeyUp) { if ([event isARepeat]) - { - return io.WantCaptureMouse; - } + return io.WantCaptureKeyboard; int key_code = (int)[event keyCode]; ImGuiKey key = ImGui_ImplOSX_KeyCodeToImGuiKey(key_code); io.AddKeyEvent(key, event.type == NSEventTypeKeyDown); io.SetKeyEventNativeData(key, key_code, -1); // To support legacy indexing (<1.87 user code) - return io.WantCaptureMouse; + return io.WantCaptureKeyboard; } if (event.type == NSEventTypeFlagsChanged) @@ -816,10 +811,7 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) case ImGuiKey_LeftAlt: mask = 0x0020; break; case ImGuiKey_RightAlt: mask = 0x0040; break; default: - { - [ImGui_IO_lock unlock]; - return io.WantCaptureMouse; - } + return io.WantCaptureKeyboard; } NSEventModifierFlags modifier_flags = [event modifierFlags]; @@ -827,7 +819,7 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) io.SetKeyEventNativeData(key, key_code, -1); // To support legacy indexing (<1.87 user code) } - return io.WantCaptureMouse; + return io.WantCaptureKeyboard; } return false;