From 3ce7904e018f4b0ce033220a948e7480d5972d2e Mon Sep 17 00:00:00 2001 From: mitchmindtree Date: Sun, 5 Feb 2017 12:51:09 +1100 Subject: [PATCH] Wrap the temporarily stored user callback in a type to enforce invariants This also removes the need for "box"ing the callback in favour of storing a raw `*mut` pointer. We can do this by ensuring that we never store the pointer for longer than the lifetime of the user callback, which is the duration of a call to `poll_events` or `run_forever`. Also removes old commented out event code from the window module. --- src/platform/macos/events_loop.rs | 147 ++++++++++------- src/platform/macos/window.rs | 266 +----------------------------- tests/events_loop.rs | 11 ++ 3 files changed, 109 insertions(+), 315 deletions(-) create mode 100644 tests/events_loop.rs diff --git a/src/platform/macos/events_loop.rs b/src/platform/macos/events_loop.rs index 59796d52..41d10237 100644 --- a/src/platform/macos/events_loop.rs +++ b/src/platform/macos/events_loop.rs @@ -11,10 +11,15 @@ pub struct EventsLoop { modifiers: std::sync::Mutex, interrupted: std::sync::atomic::AtomicBool, - /// The user's event callback given via either the `poll_events` or `run_forever` method. This - /// will only be `Some` for the duration of whichever of these methods has been called and will - /// always be `None` otherwise. - pub callback: std::sync::Mutex>> + // The user event callback given via either of the `poll_events` or `run_forever` methods. + // + // We store the user's callback here so that it may be accessed by each of the window delegate + // callbacks (e.g. resize, close, etc) for the duration of a call to either of the + // `poll_events` or `run_forever` methods. + // + // This is *only* `Some` for the duration of a call to either of these methods and will be + // `None` otherwise. + pub user_callback: UserCallback, } struct Modifiers { @@ -24,6 +29,60 @@ struct Modifiers { alt_pressed: bool, } +// Wrapping the user callback in a type allows us to: +// +// - ensure the callback pointer is never accidentally cloned +// - ensure that only the `EventsLoop` can `store` and `drop` the callback pointer +// - `unsafe impl Send` and `Sync` so that `Send` and `Sync` can be implemented for `EventsLoop`. +pub struct UserCallback { + mutex: std::sync::Mutex>, +} + + +unsafe impl Send for UserCallback {} +unsafe impl Sync for UserCallback {} + +impl UserCallback { + + // Here we store user's `callback` behind the mutex so that they may be safely shared between + // each of the window delegates. + // + // In order to make sure that the pointer is always valid, we must manually guarantee that it + // is dropped before the callback itself is dropped. Thus, this should *only* be called at the + // beginning of a call to `poll_events` and `run_forever`, both of which *must* drop the + // callback at the end of their scope using `drop_callback`. + fn store(&self, callback: &mut F) + where F: FnMut(Event) + { + let trait_object = callback as &mut FnMut(Event); + let trait_object_ptr = trait_object as *const FnMut(Event) as *mut FnMut(Event); + *self.mutex.lock().unwrap() = Some(trait_object_ptr); + } + + // Emits the given event via the user-given callback. + // + // This is *only* called within the `poll_events` and `run_forever` methods so we know that it + // is safe to `unwrap` the last callback without causing a panic as there must be at least one + // callback stored. + // + // This is unsafe as it requires dereferencing the pointer to the user-given callback. We + // guarantee this is safe by ensuring the `UserCallback` never lives longer than the user-given + // callback. + pub unsafe fn call_with_event(&self, event: Event) { + let callback: *mut FnMut(Event) = self.mutex.lock().unwrap().take().unwrap(); + (*callback)(event); + *self.mutex.lock().unwrap() = Some(callback); + } + + // Used to drop the user callback pointer at the end of the `poll_events` and `run_forever` + // methods. This is done to enforce our guarantee that the top callback will never live longer + // than the call to either `poll_events` or `run_forever` to which it was given. + fn drop(&self) { + self.mutex.lock().unwrap().take(); + } + +} + impl EventsLoop { @@ -39,29 +98,29 @@ impl EventsLoop { pending_events: std::sync::Mutex::new(std::collections::VecDeque::new()), modifiers: std::sync::Mutex::new(modifiers), interrupted: std::sync::atomic::AtomicBool::new(false), - callback: std::sync::Mutex::new(None), + user_callback: UserCallback { mutex: std::sync::Mutex::new(None) }, } } - pub fn poll_events(&self, callback: F) + pub fn poll_events(&self, mut callback: F) where F: FnMut(Event), { unsafe { if !msg_send![cocoa::base::class("NSThread"), isMainThread] { panic!("Events can only be polled from the main thread on macOS"); } - - self.store_callback(callback); } + self.user_callback.store(&mut callback); + // Loop as long as we have pending events to return. loop { - // First, yield all pending events. - while let Some(event) = self.pending_events.lock().unwrap().pop_front() { - self.emit_event(event); - } - unsafe { + // First, yield all pending events. + while let Some(event) = self.pending_events.lock().unwrap().pop_front() { + self.user_callback.call_with_event(event); + } + let pool = foundation::NSAutoreleasePool::new(cocoa::base::nil); // Poll for the next event, returning `nil` if there are none. @@ -77,18 +136,16 @@ impl EventsLoop { match event { // Call the user's callback. - Some(event) => self.emit_event(event), + Some(event) => self.user_callback.call_with_event(event), None => break, } } } - // Drop the callback to enforce our guarantee that it will never live longer than the - // duration of this method. - self.callback.lock().unwrap().take(); + self.user_callback.drop(); } - pub fn run_forever(&self, callback: F) + pub fn run_forever(&self, mut callback: F) where F: FnMut(Event) { self.interrupted.store(false, std::sync::atomic::Ordering::Relaxed); @@ -97,17 +154,17 @@ impl EventsLoop { if !msg_send![cocoa::base::class("NSThread"), isMainThread] { panic!("Events can only be polled from the main thread on macOS"); } - - self.store_callback(callback); } - loop { - // First, yield all pending events. - while let Some(event) = self.pending_events.lock().unwrap().pop_front() { - self.emit_event(event); - } + self.user_callback.store(&mut callback); + loop { unsafe { + // First, yield all pending events. + while let Some(event) = self.pending_events.lock().unwrap().pop_front() { + self.user_callback.call_with_event(event); + } + let pool = foundation::NSAutoreleasePool::new(cocoa::base::nil); // Wait for the next event. Note that this function blocks during resize. @@ -117,21 +174,24 @@ impl EventsLoop { foundation::NSDefaultRunLoopMode, cocoa::base::YES); - if let Some(event) = self.ns_event_to_event(ns_event) { - self.emit_event(event); - } + let maybe_event = self.ns_event_to_event(ns_event); + // Release the pool before calling the top callback in case the user calls either + // `run_forever` or `poll_events` within the callback. let _: () = msg_send![pool, release]; + + if let Some(event) = maybe_event { + self.user_callback.call_with_event(event); + } } if self.interrupted.load(std::sync::atomic::Ordering::Relaxed) { + self.interrupted.store(false, std::sync::atomic::Ordering::Relaxed); break; } } - // Drop the callback to enforce our guarantee that it will never live longer than the - // duration of this method. - self.callback.lock().unwrap().take(); + self.user_callback.drop(); } pub fn interrupt(&self) { @@ -157,31 +217,6 @@ impl EventsLoop { } } - // Here we store user's `callback` behind the `EventsLoop`'s mutex so that it may be safely - // shared between each of the window delegates. - // - // In order to store the `callback` within the `Eventsloop` as a trait object, we must - // `Box` the callback. Normally this would require that `F: 'static`, however we know that - // the callback cannot live longer than the lifetime of this method. Thus, we use `unsafe` - // to work around this requirement and enforce this guarantee ourselves. - // - // This should *only* be called at the beginning of `poll_events` and `run_forever`, both of - // which *must* drop the callback at the end of their scope. - unsafe fn store_callback(&self, callback: F) - where F: FnMut(Event) - { - let boxed: Box = Box::new(callback); - let boxed: Box = std::mem::transmute(boxed as Box); - *self.callback.lock().unwrap() = Some(boxed); - } - - // Emits the given event via the user-given callback. - fn emit_event(&self, event: Event) { - if let Ok(mut callback) = self.callback.lock() { - callback.as_mut().unwrap()(event); - } - } - // Convert some given `NSEvent` into a winit `Event`. unsafe fn ns_event_to_event(&self, ns_event: cocoa::base::id) -> Option { if ns_event == cocoa::base::nil { diff --git a/src/platform/macos/window.rs b/src/platform/macos/window.rs index df020bb6..04a973b4 100644 --- a/src/platform/macos/window.rs +++ b/src/platform/macos/window.rs @@ -43,12 +43,13 @@ impl WindowDelegate { fn class() -> *const Class { use std::os::raw::c_void; - // Emits an event via the `EventsLoop`. + // Emits an event via the `EventsLoop`'s callback. // - // If the `EventsLoop` callback is `Some` the event is immediately emitted via the callback. - // - // If it is `None`, it is pushed to the back of the `EventsLoop`'s `pending_events` deque. - fn emit_event(state: &mut DelegateState, window_event: Event) { + // The `Eventloop`'s callback should always be `Some` while the `WindowDelegate`'s methods + // are called as the delegate methods should only be called during a call to + // `nextEventMatchingMask` (called via EventsLoop::poll_events and + // EventsLoop::run_forever). + unsafe fn emit_event(state: &mut DelegateState, window_event: Event) { let window_id = get_window_id(*state.window); let event = ::Event::WindowEvent { window_id: ::WindowId(window_id), @@ -56,16 +57,7 @@ impl WindowDelegate { }; if let Some(events_loop) = state.events_loop.upgrade() { - if let Ok(mut callback) = events_loop.callback.lock() { - if let Some(callback) = callback.as_mut() { - callback(event); - return; - } - } - - if let Ok(mut pending_events) = events_loop.pending_events.lock() { - pending_events.push_back(event); - } + events_loop.user_callback.call_with_event(event); } } @@ -634,247 +626,3 @@ impl Clone for IdRef { IdRef(self.0) } } - -// #[allow(non_snake_case, non_upper_case_globals)] -// unsafe fn NSEventToEvent(window: &Window, nsevent: id) -> Option { -// if nsevent == nil { return None; } -// -// let event_type = nsevent.eventType(); -// appkit::NSApp().sendEvent_(if let appkit::NSKeyDown = event_type { nil } else { nsevent }); -// -// match event_type { -// appkit::NSLeftMouseDown => { Some(Event::MouseInput(ElementState::Pressed, MouseButton::Left)) }, -// appkit::NSLeftMouseUp => { Some(Event::MouseInput(ElementState::Released, MouseButton::Left)) }, -// appkit::NSRightMouseDown => { Some(Event::MouseInput(ElementState::Pressed, MouseButton::Right)) }, -// appkit::NSRightMouseUp => { Some(Event::MouseInput(ElementState::Released, MouseButton::Right)) }, -// appkit::NSOtherMouseDown => { Some(Event::MouseInput(ElementState::Pressed, MouseButton::Middle)) }, -// appkit::NSOtherMouseUp => { Some(Event::MouseInput(ElementState::Released, MouseButton::Middle)) }, -// appkit::NSMouseEntered => { Some(Event::MouseEntered) }, -// appkit::NSMouseExited => { Some(Event::MouseLeft) }, -// appkit::NSMouseMoved | -// appkit::NSLeftMouseDragged | -// appkit::NSOtherMouseDragged | -// appkit::NSRightMouseDragged => { -// let window_point = nsevent.locationInWindow(); -// let cWindow: id = msg_send![nsevent, window]; -// let view_point = if cWindow == nil { -// let window_rect = window.window.convertRectFromScreen_(NSRect::new(window_point, NSSize::new(0.0, 0.0))); -// window.view.convertPoint_fromView_(window_rect.origin, nil) -// } else { -// window.view.convertPoint_fromView_(window_point, nil) -// }; -// let view_rect = NSView::frame(*window.view); -// let scale_factor = window.hidpi_factor(); -// -// Some(Event::MouseMoved((scale_factor * view_point.x as f32) as i32, -// (scale_factor * (view_rect.size.height - view_point.y) as f32) as i32)) -// }, -// appkit::NSKeyDown => { -// let mut events = VecDeque::new(); -// let received_c_str = nsevent.characters().UTF8String(); -// let received_str = CStr::from_ptr(received_c_str); -// for received_char in from_utf8(received_str.to_bytes()).unwrap().chars() { -// events.push_back(Event::ReceivedCharacter(received_char)); -// } -// -// let vkey = to_virtual_key_code(NSEvent::keyCode(nsevent)); -// events.push_back(Event::KeyboardInput(ElementState::Pressed, NSEvent::keyCode(nsevent) as u8, vkey)); -// let event = events.pop_front(); -// window.delegate.state.pending_events.lock().unwrap().extend(events.into_iter()); -// event -// }, -// appkit::NSKeyUp => { -// let vkey = to_virtual_key_code(NSEvent::keyCode(nsevent)); -// -// Some(Event::KeyboardInput(ElementState::Released, NSEvent::keyCode(nsevent) as u8, vkey)) -// }, -// appkit::NSFlagsChanged => { -// let mut events = VecDeque::new(); -// let shift_modifier = Window::modifier_event(nsevent, appkit::NSShiftKeyMask, events::VirtualKeyCode::LShift, SHIFT_PRESSED); -// if shift_modifier.is_some() { -// SHIFT_PRESSED = !SHIFT_PRESSED; -// events.push_back(shift_modifier.unwrap()); -// } -// let ctrl_modifier = Window::modifier_event(nsevent, appkit::NSControlKeyMask, events::VirtualKeyCode::LControl, CTRL_PRESSED); -// if ctrl_modifier.is_some() { -// CTRL_PRESSED = !CTRL_PRESSED; -// events.push_back(ctrl_modifier.unwrap()); -// } -// let win_modifier = Window::modifier_event(nsevent, appkit::NSCommandKeyMask, events::VirtualKeyCode::LWin, WIN_PRESSED); -// if win_modifier.is_some() { -// WIN_PRESSED = !WIN_PRESSED; -// events.push_back(win_modifier.unwrap()); -// } -// let alt_modifier = Window::modifier_event(nsevent, appkit::NSAlternateKeyMask, events::VirtualKeyCode::LAlt, ALT_PRESSED); -// if alt_modifier.is_some() { -// ALT_PRESSED = !ALT_PRESSED; -// events.push_back(alt_modifier.unwrap()); -// } -// let event = events.pop_front(); -// window.delegate.state.pending_events.lock().unwrap().extend(events.into_iter()); -// event -// }, -// appkit::NSScrollWheel => { -// use events::MouseScrollDelta::{LineDelta, PixelDelta}; -// let scale_factor = window.hidpi_factor(); -// let delta = if nsevent.hasPreciseScrollingDeltas() == YES { -// PixelDelta(scale_factor * nsevent.scrollingDeltaX() as f32, -// scale_factor * nsevent.scrollingDeltaY() as f32) -// } else { -// LineDelta(scale_factor * nsevent.scrollingDeltaX() as f32, -// scale_factor * nsevent.scrollingDeltaY() as f32) -// }; -// let phase = match nsevent.phase() { -// appkit::NSEventPhaseMayBegin | appkit::NSEventPhaseBegan => TouchPhase::Started, -// appkit::NSEventPhaseEnded => TouchPhase::Ended, -// _ => TouchPhase::Moved, -// }; -// Some(Event::MouseWheel(delta, phase)) -// }, -// appkit::NSEventTypePressure => { -// Some(Event::TouchpadPressure(nsevent.pressure(), nsevent.stage())) -// }, -// appkit::NSApplicationDefined => { -// match nsevent.subtype() { -// appkit::NSEventSubtype::NSApplicationActivatedEventType => { Some(Event::Awakened) } -// _ => { None } -// } -// }, -// _ => { None }, -// } -// } -// -// pub fn to_virtual_key_code(code: u16) -> Option { -// Some(match code { -// 0x00 => events::VirtualKeyCode::A, -// 0x01 => events::VirtualKeyCode::S, -// 0x02 => events::VirtualKeyCode::D, -// 0x03 => events::VirtualKeyCode::F, -// 0x04 => events::VirtualKeyCode::H, -// 0x05 => events::VirtualKeyCode::G, -// 0x06 => events::VirtualKeyCode::Z, -// 0x07 => events::VirtualKeyCode::X, -// 0x08 => events::VirtualKeyCode::C, -// 0x09 => events::VirtualKeyCode::V, -// //0x0a => World 1, -// 0x0b => events::VirtualKeyCode::B, -// 0x0c => events::VirtualKeyCode::Q, -// 0x0d => events::VirtualKeyCode::W, -// 0x0e => events::VirtualKeyCode::E, -// 0x0f => events::VirtualKeyCode::R, -// 0x10 => events::VirtualKeyCode::Y, -// 0x11 => events::VirtualKeyCode::T, -// 0x12 => events::VirtualKeyCode::Key1, -// 0x13 => events::VirtualKeyCode::Key2, -// 0x14 => events::VirtualKeyCode::Key3, -// 0x15 => events::VirtualKeyCode::Key4, -// 0x16 => events::VirtualKeyCode::Key6, -// 0x17 => events::VirtualKeyCode::Key5, -// 0x18 => events::VirtualKeyCode::Equals, -// 0x19 => events::VirtualKeyCode::Key9, -// 0x1a => events::VirtualKeyCode::Key7, -// 0x1b => events::VirtualKeyCode::Minus, -// 0x1c => events::VirtualKeyCode::Key8, -// 0x1d => events::VirtualKeyCode::Key0, -// 0x1e => events::VirtualKeyCode::RBracket, -// 0x1f => events::VirtualKeyCode::O, -// 0x20 => events::VirtualKeyCode::U, -// 0x21 => events::VirtualKeyCode::LBracket, -// 0x22 => events::VirtualKeyCode::I, -// 0x23 => events::VirtualKeyCode::P, -// 0x24 => events::VirtualKeyCode::Return, -// 0x25 => events::VirtualKeyCode::L, -// 0x26 => events::VirtualKeyCode::J, -// 0x27 => events::VirtualKeyCode::Apostrophe, -// 0x28 => events::VirtualKeyCode::K, -// 0x29 => events::VirtualKeyCode::Semicolon, -// 0x2a => events::VirtualKeyCode::Backslash, -// 0x2b => events::VirtualKeyCode::Comma, -// 0x2c => events::VirtualKeyCode::Slash, -// 0x2d => events::VirtualKeyCode::N, -// 0x2e => events::VirtualKeyCode::M, -// 0x2f => events::VirtualKeyCode::Period, -// 0x30 => events::VirtualKeyCode::Tab, -// 0x31 => events::VirtualKeyCode::Space, -// 0x32 => events::VirtualKeyCode::Grave, -// 0x33 => events::VirtualKeyCode::Back, -// //0x34 => unkown, -// 0x35 => events::VirtualKeyCode::Escape, -// 0x36 => events::VirtualKeyCode::RWin, -// 0x37 => events::VirtualKeyCode::LWin, -// 0x38 => events::VirtualKeyCode::LShift, -// //0x39 => Caps lock, -// //0x3a => Left alt, -// 0x3b => events::VirtualKeyCode::LControl, -// 0x3c => events::VirtualKeyCode::RShift, -// //0x3d => Right alt, -// 0x3e => events::VirtualKeyCode::RControl, -// //0x3f => Fn key, -// //0x40 => F17 Key, -// 0x41 => events::VirtualKeyCode::Decimal, -// //0x42 -> unkown, -// 0x43 => events::VirtualKeyCode::Multiply, -// //0x44 => unkown, -// 0x45 => events::VirtualKeyCode::Add, -// //0x46 => unkown, -// 0x47 => events::VirtualKeyCode::Numlock, -// //0x48 => KeypadClear, -// 0x49 => events::VirtualKeyCode::VolumeUp, -// 0x4a => events::VirtualKeyCode::VolumeDown, -// 0x4b => events::VirtualKeyCode::Divide, -// 0x4c => events::VirtualKeyCode::NumpadEnter, -// //0x4d => unkown, -// 0x4e => events::VirtualKeyCode::Subtract, -// //0x4f => F18 key, -// //0x50 => F19 Key, -// 0x51 => events::VirtualKeyCode::NumpadEquals, -// 0x52 => events::VirtualKeyCode::Numpad0, -// 0x53 => events::VirtualKeyCode::Numpad1, -// 0x54 => events::VirtualKeyCode::Numpad2, -// 0x55 => events::VirtualKeyCode::Numpad3, -// 0x56 => events::VirtualKeyCode::Numpad4, -// 0x57 => events::VirtualKeyCode::Numpad5, -// 0x58 => events::VirtualKeyCode::Numpad6, -// 0x59 => events::VirtualKeyCode::Numpad7, -// //0x5a => F20 Key, -// 0x5b => events::VirtualKeyCode::Numpad8, -// 0x5c => events::VirtualKeyCode::Numpad9, -// //0x5d => unkown, -// //0x5e => unkown, -// //0x5f => unkown, -// 0x60 => events::VirtualKeyCode::F5, -// 0x61 => events::VirtualKeyCode::F6, -// 0x62 => events::VirtualKeyCode::F7, -// 0x63 => events::VirtualKeyCode::F3, -// 0x64 => events::VirtualKeyCode::F8, -// 0x65 => events::VirtualKeyCode::F9, -// //0x66 => unkown, -// 0x67 => events::VirtualKeyCode::F11, -// //0x68 => unkown, -// 0x69 => events::VirtualKeyCode::F13, -// //0x6a => F16 Key, -// 0x6b => events::VirtualKeyCode::F14, -// //0x6c => unkown, -// 0x6d => events::VirtualKeyCode::F10, -// //0x6e => unkown, -// 0x6f => events::VirtualKeyCode::F12, -// //0x70 => unkown, -// 0x71 => events::VirtualKeyCode::F15, -// 0x72 => events::VirtualKeyCode::Insert, -// 0x73 => events::VirtualKeyCode::Home, -// 0x74 => events::VirtualKeyCode::PageUp, -// 0x75 => events::VirtualKeyCode::Delete, -// 0x76 => events::VirtualKeyCode::F4, -// 0x77 => events::VirtualKeyCode::End, -// 0x78 => events::VirtualKeyCode::F2, -// 0x79 => events::VirtualKeyCode::PageDown, -// 0x7a => events::VirtualKeyCode::F1, -// 0x7b => events::VirtualKeyCode::Left, -// 0x7c => events::VirtualKeyCode::Right, -// 0x7d => events::VirtualKeyCode::Down, -// 0x7e => events::VirtualKeyCode::Up, -// //0x7f => unkown, -// -// _ => return None, -// }) -// } diff --git a/tests/events_loop.rs b/tests/events_loop.rs new file mode 100644 index 00000000..4623b80e --- /dev/null +++ b/tests/events_loop.rs @@ -0,0 +1,11 @@ +extern crate winit; + +// A part of the API requirement for `EventsLoop` is that it is `Send` + `Sync`. +// +// This short test will only compile if the `EventsLoop` is `Send` + `Sync`. +#[test] +fn send_sync() { + fn check_send_sync(_: T) {} + let events_loop = winit::EventsLoop::new(); + check_send_sync(events_loop); +}