Merge pull request #157 from mitchmindtree/macos_fixes

Several fixes for the macOS backend

src/platform/macos/events_loop.rs

@@ -6,7 +6,7 @@ use std;
 
 
 pub struct EventsLoop {
-    pub windows: std::sync::Mutex<Vec<std::sync::Arc<Window>>>,
+    pub windows: std::sync::Mutex<Vec<std::sync::Weak<Window>>>,
     pub pending_events: std::sync::Mutex<std::collections::VecDeque<Event>>,
     modifiers: std::sync::Mutex<Modifiers>,
     interrupted: std::sync::atomic::AtomicBool,
@@ -19,7 +19,7 @@ pub struct EventsLoop {
     //
     // This is *only* `Some` for the duration of a call to either of these methods and will be
     // `None` otherwise.
-    pub user_callback: UserCallback,
+    user_callback: UserCallback,
 }
 
 struct Modifiers {
@@ -61,15 +61,19 @@ impl UserCallback {
 
     // 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 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();
+    //
+    // Note that the callback may not always be `Some`. This is because some `NSWindowDelegate`
+    // callbacks can be triggered by means other than `NSApp().sendEvent`. For example, if a window
+    // is destroyed or created during a call to the user's callback, the `WindowDelegate` methods
+    // may be called with `windowShouldClose` or `windowDidResignKey`.
+    unsafe fn call_with_event(&self, event: Event) {
+        let callback = match self.mutex.lock().unwrap().take() {
+            Some(callback) => callback,
+            None => return,
+        };
         (*callback)(event);
         *self.mutex.lock().unwrap() = Some(callback);
     }
@@ -117,9 +121,7 @@ impl EventsLoop {
         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);
-                }
+                self.call_user_callback_with_pending_events();
 
                 let pool = foundation::NSAutoreleasePool::new(cocoa::base::nil);
 
@@ -161,9 +163,7 @@ impl EventsLoop {
         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);
-                }
+                self.call_user_callback_with_pending_events();
 
                 let pool = foundation::NSAutoreleasePool::new(cocoa::base::nil);
 
@@ -217,6 +217,46 @@ impl EventsLoop {
         }
     }
 
+    // Removes the window with the given `Id` from the `windows` list.
+    //
+    // This is called when a window is either `Closed` or `Drop`ped.
+    pub fn find_and_remove_window(&self, id: super::window::Id) {
+        if let Ok(mut windows) = self.windows.lock() {
+            windows.retain(|w| match w.upgrade() {
+                Some(w) => w.id() != id,
+                None => true,
+            });
+        }
+    }
+
+    fn call_user_callback_with_pending_events(&self) {
+        loop {
+            let event = match self.pending_events.lock().unwrap().pop_front() {
+                Some(event) => event,
+                None => return,
+            };
+            unsafe {
+                self.user_callback.call_with_event(event);
+            }
+        }
+    }
+
+    // Calls the user callback if one exists.
+    //
+    // Otherwise, stores the event in the `pending_events` queue.
+    //
+    // This is necessary for the case when `WindowDelegate` callbacks are triggered during a call
+    // to the user's callback.
+    pub fn call_user_callback_with_event_or_store_in_pending(&self, event: Event) {
+        if self.user_callback.mutex.lock().unwrap().is_some() {
+            unsafe {
+                self.user_callback.call_with_event(event);
+            }
+        } else {
+            self.pending_events.lock().unwrap().push_back(event);
+        }
+    }
+
     // Convert some given `NSEvent` into a winit `Event`.
     unsafe fn ns_event_to_event(&self, ns_event: cocoa::base::id) -> Option<Event> {
         if ns_event == cocoa::base::nil {
@@ -236,8 +276,6 @@ impl EventsLoop {
         let event_type = ns_event.eventType();
         let ns_window = ns_event.window();
         let window_id = super::window::get_window_id(ns_window);
-        let windows = self.windows.lock().unwrap();
-        let maybe_window = windows.iter().find(|window| window_id == window.id());
 
         // FIXME: Document this. Why do we do this? Seems like it passes on events to window/app.
         // If we don't do this, window does not become main for some reason.
@@ -246,16 +284,23 @@ impl EventsLoop {
             _ => appkit::NSApp().sendEvent_(ns_event),
         }
 
+        let windows = self.windows.lock().unwrap();
+        let maybe_window = windows.iter()
+            .filter_map(std::sync::Weak::upgrade)
+            .find(|window| window_id == window.id());
+
         let into_event = |window_event| Event::WindowEvent {
             window_id: ::WindowId(window_id),
             event: window_event,
         };
 
         // Returns `Some` window if one of our windows is the key window.
-        let maybe_key_window = || windows.iter().find(|window| {
-            let is_key_window: cocoa::base::BOOL = msg_send![*window.window, isKeyWindow];
-            is_key_window == cocoa::base::YES
-        });
+        let maybe_key_window = || windows.iter()
+            .filter_map(std::sync::Weak::upgrade)
+            .find(|window| {
+                let is_key_window: cocoa::base::BOOL = msg_send![*window.window, isKeyWindow];
+                is_key_window == cocoa::base::YES
+            });
 
         match event_type {
 
@@ -586,4 +631,4 @@ fn event_mods(event: cocoa::base::id) -> ModifiersState {
         alt: flags.contains(appkit::NSAlternateKeyMask),
         logo: flags.contains(appkit::NSCommandKeyMask),
     }
-}
+}

src/platform/macos/mod.rs

@@ -26,7 +26,8 @@ impl Window2 {
     {
         let weak_events_loop = ::std::sync::Arc::downgrade(&events_loop);
         let window = ::std::sync::Arc::new(try!(Window::new(weak_events_loop, attributes, pl_attribs)));
-        events_loop.windows.lock().unwrap().push(window.clone());
+        let weak_window = ::std::sync::Arc::downgrade(&window);
+        events_loop.windows.lock().unwrap().push(weak_window);
         Ok(Window2 { window: window })
     }
 

src/platform/macos/window.rs

@@ -43,12 +43,7 @@ impl WindowDelegate {
     fn class() -> *const Class {
         use std::os::raw::c_void;
 
-        // Emits an event via the `EventsLoop`'s callback.
-        //
-        // 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).
+        // Emits an event via the `EventsLoop`'s callback or stores it in the pending queue.
         unsafe fn emit_event(state: &mut DelegateState, window_event: WindowEvent) {
             let window_id = get_window_id(*state.window);
             let event = Event::WindowEvent {
@@ -57,15 +52,30 @@ impl WindowDelegate {
             };
 
             if let Some(events_loop) = state.events_loop.upgrade() {
-                events_loop.user_callback.call_with_event(event);
+                events_loop.call_user_callback_with_event_or_store_in_pending(event);
             }
         }
 
+        // Called when the window is resized or when the window was moved to a different screen.
+        unsafe fn emit_resize_event(state: &mut DelegateState) {
+            let rect = NSView::frame(*state.view);
+            let scale_factor = NSWindow::backingScaleFactor(*state.window) as f32;
+            let width = (scale_factor * rect.size.width as f32) as u32;
+            let height = (scale_factor * rect.size.height as f32) as u32;
+            emit_event(state, WindowEvent::Resized(width, height));
+        }
+
         extern fn window_should_close(this: &Object, _: Sel, _: id) -> BOOL {
             unsafe {
                 let state: *mut c_void = *this.get_ivar("winitState");
                 let state = &mut *(state as *mut DelegateState);
                 emit_event(state, WindowEvent::Closed);
+
+                // Remove the window from the events_loop.
+                if let Some(events_loop) = state.events_loop.upgrade() {
+                    let window_id = get_window_id(*state.window);
+                    events_loop.find_and_remove_window(window_id);
+                }
             }
             YES
         }
@@ -74,11 +84,15 @@ impl WindowDelegate {
             unsafe {
                 let state: *mut c_void = *this.get_ivar("winitState");
                 let state = &mut *(state as *mut DelegateState);
-                let rect = NSView::frame(*state.view);
-                let scale_factor = NSWindow::backingScaleFactor(*state.window) as f32;
-                let width = (scale_factor * rect.size.width as f32) as u32;
-                let height = (scale_factor * rect.size.height as f32) as u32;
-                emit_event(state, WindowEvent::Resized(width, height));
+                emit_resize_event(state);
+            }
+        }
+
+        extern fn window_did_change_screen(this: &Object, _: Sel, _: id) {
+            unsafe {
+                let state: *mut c_void = *this.get_ivar("winitState");
+                let state = &mut *(state as *mut DelegateState);
+                emit_resize_event(state);
             }
         }
 
@@ -113,6 +127,8 @@ impl WindowDelegate {
                 window_should_close as extern fn(&Object, Sel, id) -> BOOL);
             decl.add_method(sel!(windowDidResize:),
                 window_did_resize as extern fn(&Object, Sel, id));
+            decl.add_method(sel!(windowDidChangeScreen:),
+                window_did_change_screen as extern fn(&Object, Sel, id));
 
             decl.add_method(sel!(windowDidBecomeKey:),
                 window_did_become_key as extern fn(&Object, Sel, id));
@@ -173,8 +189,15 @@ impl Drop for Window {
         // Remove this window from the `EventLoop`s list of windows.
         let id = self.id();
         if let Some(ev) = self.delegate.state.events_loop.upgrade() {
-            let mut windows = ev.windows.lock().unwrap();
-            windows.retain(|w| w.id() != id)
+            ev.find_and_remove_window(id);
+        }
+
+        // Close the window if it has not yet been closed.
+        let nswindow = *self.window;
+        if nswindow != nil {
+            unsafe {
+                msg_send![nswindow, close];
+            }
         }
     }
 }