From f1e62a5cad38b49974217a500ac1076797f60734 Mon Sep 17 00:00:00 2001 From: max Date: Fri, 10 May 2024 13:37:50 +0300 Subject: [PATCH 1/7] defer certain events on macOS to avoid re-entrant calls --- src/macos/view.rs | 4 ++-- src/macos/window.rs | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/macos/view.rs b/src/macos/view.rs index 8b765b4..3d5ffc5 100644 --- a/src/macos/view.rs +++ b/src/macos/view.rs @@ -249,14 +249,14 @@ extern "C" fn become_first_responder(this: &Object, _sel: Sel) -> BOOL { is_key_window == YES }; if is_key_window { - state.trigger_event(Event::Window(WindowEvent::Focused)); + state.defer_event(Event::Window(WindowEvent::Focused)); } YES } extern "C" fn resign_first_responder(this: &Object, _sel: Sel) -> BOOL { let state = unsafe { WindowState::from_view(this) }; - state.trigger_event(Event::Window(WindowEvent::Unfocused)); + state.defer_event(Event::Window(WindowEvent::Unfocused)); YES } diff --git a/src/macos/window.rs b/src/macos/window.rs index f945518..0d101d9 100644 --- a/src/macos/window.rs +++ b/src/macos/window.rs @@ -1,4 +1,5 @@ use std::cell::{Cell, RefCell}; +use std::collections::VecDeque; use std::ffi::c_void; use std::ptr; use std::rc::Rc; @@ -267,6 +268,7 @@ impl<'a> Window<'a> { keyboard_state: KeyboardState::new(), frame_timer: Cell::new(None), window_info: Cell::new(window_info), + deferred_events: RefCell::default(), }); let window_state_ptr = Rc::into_raw(Rc::clone(&window_state)); @@ -360,6 +362,10 @@ pub(super) struct WindowState { frame_timer: Cell>, /// The last known window info for this window. pub window_info: Cell, + + /// Events that should be triggered before calling `on_frame`. This is needed to avoid mutably + /// borrowing the fields from `WindowState` more than once. + deferred_events: RefCell>, } impl WindowState { @@ -383,7 +389,15 @@ impl WindowState { self.window_handler.borrow_mut().on_event(&mut window, event) } + pub(super) fn defer_event(&self, event: Event) { + self.deferred_events.borrow_mut().push_back(event); + } + pub(super) fn trigger_frame(&self) { + for event in self.deferred_events.borrow_mut().drain(..) { + self.trigger_event(event); + } + let mut window = crate::Window::new(Window { inner: &self.window_inner }); self.window_handler.borrow_mut().on_frame(&mut window); } From f95cd99d6701b24b90d918f78942a201bbc9f307 Mon Sep 17 00:00:00 2001 From: max Date: Sun, 12 May 2024 11:03:00 +0300 Subject: [PATCH 2/7] immediately trigger deferred event if borrowing succeeds --- src/macos/window.rs | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/src/macos/window.rs b/src/macos/window.rs index 0d101d9..6d5776b 100644 --- a/src/macos/window.rs +++ b/src/macos/window.rs @@ -1,4 +1,4 @@ -use std::cell::{Cell, RefCell}; +use std::cell::{Cell, RefCell, RefMut}; use std::collections::VecDeque; use std::ffi::c_void; use std::ptr; @@ -386,20 +386,26 @@ impl WindowState { pub(super) fn trigger_event(&self, event: Event) -> EventStatus { let mut window = crate::Window::new(Window { inner: &self.window_inner }); - self.window_handler.borrow_mut().on_event(&mut window, event) + let mut window_handler = self.window_handler.borrow_mut(); + let status = window_handler.on_event(&mut window, event); + self.trigger_deferred_events(&mut window_handler); + status } pub(super) fn defer_event(&self, event: Event) { - self.deferred_events.borrow_mut().push_back(event); + if let Ok(mut window_handler) = self.window_handler.try_borrow_mut() { + let mut window = crate::Window::new(Window { inner: &self.window_inner }); + window_handler.on_event(&mut window, event); + } else { + self.deferred_events.borrow_mut().push_back(event); + } } pub(super) fn trigger_frame(&self) { - for event in self.deferred_events.borrow_mut().drain(..) { - self.trigger_event(event); - } - let mut window = crate::Window::new(Window { inner: &self.window_inner }); - self.window_handler.borrow_mut().on_frame(&mut window); + let mut window_handler = self.window_handler.borrow_mut(); + self.trigger_deferred_events(&mut window_handler); + window_handler.on_frame(&mut window); } pub(super) fn keyboard_state(&self) -> &KeyboardState { @@ -433,6 +439,13 @@ impl WindowState { (*window_state_ptr).frame_timer.set(Some(timer)); } + + fn trigger_deferred_events(&self, window_handler: &mut RefMut<'_, Box>) { + for event in self.deferred_events.borrow_mut().drain(..) { + let mut window = crate::Window::new(Window { inner: &self.window_inner }); + window_handler.on_event(&mut window, event); + } + } } unsafe impl<'a> HasRawWindowHandle for Window<'a> { From fc4bff8380b6293200664296b2ef56f4d82cc2f9 Mon Sep 17 00:00:00 2001 From: max Date: Sun, 12 May 2024 20:24:34 +0300 Subject: [PATCH 3/7] nicer signature --- src/macos/window.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/macos/window.rs b/src/macos/window.rs index 6d5776b..ab3576c 100644 --- a/src/macos/window.rs +++ b/src/macos/window.rs @@ -388,7 +388,7 @@ impl WindowState { let mut window = crate::Window::new(Window { inner: &self.window_inner }); let mut window_handler = self.window_handler.borrow_mut(); let status = window_handler.on_event(&mut window, event); - self.trigger_deferred_events(&mut window_handler); + self.trigger_deferred_events(window_handler.as_mut()); status } @@ -404,7 +404,7 @@ impl WindowState { pub(super) fn trigger_frame(&self) { let mut window = crate::Window::new(Window { inner: &self.window_inner }); let mut window_handler = self.window_handler.borrow_mut(); - self.trigger_deferred_events(&mut window_handler); + self.trigger_deferred_events(window_handler.as_mut()); window_handler.on_frame(&mut window); } @@ -440,7 +440,7 @@ impl WindowState { (*window_state_ptr).frame_timer.set(Some(timer)); } - fn trigger_deferred_events(&self, window_handler: &mut RefMut<'_, Box>) { + fn trigger_deferred_events(&self, window_handler: &mut dyn WindowHandler) { for event in self.deferred_events.borrow_mut().drain(..) { let mut window = crate::Window::new(Window { inner: &self.window_inner }); window_handler.on_event(&mut window, event); From 6018b449dbe73d69ac027789dc56bd54bdb46a4c Mon Sep 17 00:00:00 2001 From: max Date: Sun, 12 May 2024 20:30:26 +0300 Subject: [PATCH 4/7] rename method: defer_event -> trigger_deferrable_event --- src/macos/view.rs | 4 ++-- src/macos/window.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/macos/view.rs b/src/macos/view.rs index 3d5ffc5..e8a8829 100644 --- a/src/macos/view.rs +++ b/src/macos/view.rs @@ -249,14 +249,14 @@ extern "C" fn become_first_responder(this: &Object, _sel: Sel) -> BOOL { is_key_window == YES }; if is_key_window { - state.defer_event(Event::Window(WindowEvent::Focused)); + state.trigger_deferrable_event(Event::Window(WindowEvent::Focused)); } YES } extern "C" fn resign_first_responder(this: &Object, _sel: Sel) -> BOOL { let state = unsafe { WindowState::from_view(this) }; - state.defer_event(Event::Window(WindowEvent::Unfocused)); + state.trigger_deferrable_event(Event::Window(WindowEvent::Unfocused)); YES } diff --git a/src/macos/window.rs b/src/macos/window.rs index ab3576c..02d7522 100644 --- a/src/macos/window.rs +++ b/src/macos/window.rs @@ -392,7 +392,7 @@ impl WindowState { status } - pub(super) fn defer_event(&self, event: Event) { + pub(super) fn trigger_deferrable_event(&self, event: Event) { if let Ok(mut window_handler) = self.window_handler.try_borrow_mut() { let mut window = crate::Window::new(Window { inner: &self.window_inner }); window_handler.on_event(&mut window, event); From 398e945221cdff0ca1349ed739f3d25c324e11fb Mon Sep 17 00:00:00 2001 From: max Date: Mon, 13 May 2024 17:12:27 +0300 Subject: [PATCH 5/7] clean up, add comments --- src/macos/window.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/macos/window.rs b/src/macos/window.rs index 02d7522..d9e1938 100644 --- a/src/macos/window.rs +++ b/src/macos/window.rs @@ -1,4 +1,4 @@ -use std::cell::{Cell, RefCell, RefMut}; +use std::cell::{Cell, RefCell}; use std::collections::VecDeque; use std::ffi::c_void; use std::ptr; @@ -363,8 +363,7 @@ pub(super) struct WindowState { /// The last known window info for this window. pub window_info: Cell, - /// Events that should be triggered before calling `on_frame`. This is needed to avoid mutably - /// borrowing the fields from `WindowState` more than once. + /// Events that will be triggered at the end of `window_handler`'s borrow. deferred_events: RefCell>, } @@ -384,6 +383,8 @@ impl WindowState { state } + /// Trigger the event immediately and return the event status. + /// Will panic if `window_handler` is already borrowed (see `trigger_deferrable_event`). pub(super) fn trigger_event(&self, event: Event) -> EventStatus { let mut window = crate::Window::new(Window { inner: &self.window_inner }); let mut window_handler = self.window_handler.borrow_mut(); @@ -392,6 +393,9 @@ impl WindowState { status } + /// Trigger the event immediately if `window_handler` can be borrowed mutably, + /// otherwise add the event to a queue that will be cleared once `window_handler`'s mutable borrow ends. + /// As this method might result in the event triggering asynchronously, it can't reliably return the event status. pub(super) fn trigger_deferrable_event(&self, event: Event) { if let Ok(mut window_handler) = self.window_handler.try_borrow_mut() { let mut window = crate::Window::new(Window { inner: &self.window_inner }); @@ -441,8 +445,8 @@ impl WindowState { } fn trigger_deferred_events(&self, window_handler: &mut dyn WindowHandler) { + let mut window = crate::Window::new(Window { inner: &self.window_inner }); for event in self.deferred_events.borrow_mut().drain(..) { - let mut window = crate::Window::new(Window { inner: &self.window_inner }); window_handler.on_event(&mut window, event); } } From fc180ff23c312b07f0682873ad2a55c51f820eea Mon Sep 17 00:00:00 2001 From: max Date: Mon, 13 May 2024 22:20:39 +0300 Subject: [PATCH 6/7] address review comments --- src/macos/window.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/macos/window.rs b/src/macos/window.rs index d9e1938..f9f19fb 100644 --- a/src/macos/window.rs +++ b/src/macos/window.rs @@ -389,7 +389,7 @@ impl WindowState { let mut window = crate::Window::new(Window { inner: &self.window_inner }); let mut window_handler = self.window_handler.borrow_mut(); let status = window_handler.on_event(&mut window, event); - self.trigger_deferred_events(window_handler.as_mut()); + self.send_deferred_events(window_handler.as_mut()); status } @@ -400,6 +400,7 @@ impl WindowState { if let Ok(mut window_handler) = self.window_handler.try_borrow_mut() { let mut window = crate::Window::new(Window { inner: &self.window_inner }); window_handler.on_event(&mut window, event); + self.send_deferred_events(window_handler.as_mut()); } else { self.deferred_events.borrow_mut().push_back(event); } @@ -408,7 +409,7 @@ impl WindowState { pub(super) fn trigger_frame(&self) { let mut window = crate::Window::new(Window { inner: &self.window_inner }); let mut window_handler = self.window_handler.borrow_mut(); - self.trigger_deferred_events(window_handler.as_mut()); + self.send_deferred_events(window_handler.as_mut()); window_handler.on_frame(&mut window); } @@ -444,9 +445,9 @@ impl WindowState { (*window_state_ptr).frame_timer.set(Some(timer)); } - fn trigger_deferred_events(&self, window_handler: &mut dyn WindowHandler) { + fn send_deferred_events(&self, window_handler: &mut dyn WindowHandler) { let mut window = crate::Window::new(Window { inner: &self.window_inner }); - for event in self.deferred_events.borrow_mut().drain(..) { + while let Some(event) = self.deferred_events.borrow_mut().pop_front() { window_handler.on_event(&mut window, event); } } From 480fc8ed52fd7a3117e57decf558ccf6a029522b Mon Sep 17 00:00:00 2001 From: max Date: Wed, 29 May 2024 20:18:53 +0300 Subject: [PATCH 7/7] fix borrow logic when sending deferred events --- src/macos/window.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/macos/window.rs b/src/macos/window.rs index f9f19fb..97bc27f 100644 --- a/src/macos/window.rs +++ b/src/macos/window.rs @@ -447,8 +447,13 @@ impl WindowState { fn send_deferred_events(&self, window_handler: &mut dyn WindowHandler) { let mut window = crate::Window::new(Window { inner: &self.window_inner }); - while let Some(event) = self.deferred_events.borrow_mut().pop_front() { - window_handler.on_event(&mut window, event); + loop { + let next_event = self.deferred_events.borrow_mut().pop_front(); + if let Some(event) = next_event { + window_handler.on_event(&mut window, event); + } else { + break; + } } } }