From 6508586fc2bc03b8ae58bc164db773fd669a8f0a Mon Sep 17 00:00:00 2001 From: Tim Kane Date: Tue, 12 Mar 2024 16:05:19 +1100 Subject: [PATCH 1/5] 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 06a6aff8b..0afbf4028 100644 --- a/backends/imgui_impl_osx.mm +++ b/backends/imgui_impl_osx.mm @@ -625,8 +625,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 6054b1a2a12194cfe9810060bb77ef0fb9e6ba3e Mon Sep 17 00:00:00 2001 From: Tim Kane Date: Thu, 14 Mar 2024 22:40:21 +1100 Subject: [PATCH 2/5] 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 0afbf4028..94b83be6d 100644 --- a/backends/imgui_impl_osx.mm +++ b/backends/imgui_impl_osx.mm @@ -402,8 +402,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(); ImGuiPlatformIO& platform_io = ImGui::GetPlatformIO(); IMGUI_CHECKVERSION(); @@ -673,6 +677,7 @@ static ImGuiMouseSource GetMouseSource(NSEvent* event) } } + static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) { // Only process events from the window containing ImGui view @@ -680,6 +685,10 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) return false; 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]; @@ -688,7 +697,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) @@ -699,7 +709,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) @@ -714,7 +725,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) @@ -732,7 +745,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; @@ -757,20 +773,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) @@ -801,15 +822,20 @@ 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; + } } io.AddKeyEvent(key, (modifier_flags & mask) != 0); 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 f35459a0f96dcd5697c09d8ab60e7f36f9a1bca1 Mon Sep 17 00:00:00 2001 From: Tim Kane Date: Fri, 15 Mar 2024 00:02:43 +1100 Subject: [PATCH 3/5] 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 94b83be6d..0f1f110db 100644 --- a/backends/imgui_impl_osx.mm +++ b/backends/imgui_impl_osx.mm @@ -402,12 +402,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(); ImGuiPlatformIO& platform_io = ImGui::GetPlatformIO(); IMGUI_CHECKVERSION(); @@ -502,6 +523,7 @@ bool ImGui_ImplOSX_Init(NSView* view) return true; } + void ImGui_ImplOSX_Shutdown() { ImGui_ImplOSX_Data* bd = ImGui_ImplOSX_GetBackendData(); @@ -685,7 +707,7 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) return false; ImGuiIO& io = ImGui::GetIO(); - [ImGui_IO_lock lock]; + ScopedLock *sclock = [[ScopedLock alloc] init:ImGui_IO_lock]; bool WantCaptureMouse = io.WantCaptureMouse; @@ -697,7 +719,6 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) io.AddMouseSourceEvent(GetMouseSource(event)); io.AddMouseButtonEvent(button, true); } - [ImGui_IO_lock unlock]; return WantCaptureMouse; } @@ -709,7 +730,6 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) io.AddMouseSourceEvent(GetMouseSource(event)); io.AddMouseButtonEvent(button, false); } - [ImGui_IO_lock unlock]; return WantCaptureMouse; } @@ -726,7 +746,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; } @@ -746,7 +765,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; } @@ -773,7 +791,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; } @@ -781,7 +798,6 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) { if ([event isARepeat]) { - [ImGui_IO_lock unlock]; return WantCaptureMouse; } @@ -790,7 +806,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; } @@ -831,11 +846,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 5d621088a6e25bd63b9fe6cedc4136a06367ec10 Mon Sep 17 00:00:00 2001 From: Tim Kane Date: Fri, 15 Mar 2024 00:07:37 +1100 Subject: [PATCH 4/5] 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 0f1f110db..e73948959 100644 --- a/backends/imgui_impl_osx.mm +++ b/backends/imgui_impl_osx.mm @@ -523,7 +523,6 @@ bool ImGui_ImplOSX_Init(NSView* view) return true; } - void ImGui_ImplOSX_Shutdown() { ImGui_ImplOSX_Data* bd = ImGui_ImplOSX_GetBackendData(); @@ -699,7 +698,6 @@ static ImGuiMouseSource GetMouseSource(NSEvent* event) } } - static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) { // Only process events from the window containing ImGui view @@ -709,8 +707,6 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) 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]; @@ -719,7 +715,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) @@ -730,7 +726,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) @@ -746,7 +742,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) @@ -791,14 +787,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]; @@ -806,7 +802,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) @@ -839,14 +835,14 @@ static bool ImGui_ImplOSX_HandleEvent(NSEvent* event, NSView* view) default: { [ImGui_IO_lock unlock]; - return WantCaptureMouse; + return io.WantCaptureMouse; } } io.AddKeyEvent(key, (modifier_flags & mask) != 0); io.SetKeyEventNativeData(key, key_code, -1); // To support legacy indexing (<1.87 user code) } - return WantCaptureMouse; + return io.WantCaptureMouse; } return false; From b7d1f7f86ea1f6440a11dbc90cbbede20e199252 Mon Sep 17 00:00:00 2001 From: Tim Kane Date: Fri, 15 Mar 2024 00:12:08 +1100 Subject: [PATCH 5/5] 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 e73948959..a7df57413 100644 --- a/backends/imgui_impl_osx.mm +++ b/backends/imgui_impl_osx.mm @@ -741,7 +741,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; } @@ -760,9 +759,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; @@ -793,16 +790,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) @@ -833,16 +828,13 @@ 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; } io.AddKeyEvent(key, (modifier_flags & mask) != 0); io.SetKeyEventNativeData(key, key_code, -1); // To support legacy indexing (<1.87 user code) } - return io.WantCaptureMouse; + return io.WantCaptureKeyboard; } return false;