Merge pull request #316 from binsoftware/cocoa-fixes

Cocoa fixes: memory leaks, monitor handling, is_current()

src/android/mod.rs

@@ -14,6 +14,7 @@ use std::collections::VecDeque;
 use Api;
 use BuilderAttribs;
 use GlRequest;
+use native_monitor::NativeMonitorId;
 
 pub struct Window {
     display: ffi::egl::types::EGLDisplay,
@@ -41,6 +42,10 @@ impl MonitorID {
         Some("Primary".to_string())
     }
 
+    pub fn get_native_identifier(&self) -> NativeMonitorId {
+        NativeMonitorId::Unavailable
+    }
+
     pub fn get_dimensions(&self) -> (u32, u32) {
         unimplemented!()
     }

src/cocoa/mod.rs

@@ -8,6 +8,7 @@ use libc;
 use Api;
 use BuilderAttribs;
 use GlRequest;
+use native_monitor::NativeMonitorId;
 
 use cocoa::base::{Class, id, YES, NO, NSUInteger, nil, objc_allocateClassPair, class, objc_registerClassPair};
 use cocoa::base::{selector, msg_send, msg_send_stret, class_addMethod, class_addIvar};
@@ -29,6 +30,7 @@ use std::str::FromStr;
 use std::str::from_utf8;
 use std::sync::Mutex;
 use std::ascii::AsciiExt;
+use std::ops::Deref;
 
 use events::Event::{Awakened, MouseInput, MouseMoved, ReceivedCharacter, KeyboardInput, MouseWheel};
 use events::ElementState::{Pressed, Released};
@@ -50,9 +52,9 @@ static mut alt_pressed: bool = false;
 
 struct DelegateState {
     is_closed: bool,
-    context: id,
+    context: IdRef,
-    view: id,
+    view: IdRef,
-    window: id,
+    window: IdRef,
     handler: Option<fn(u32, u32)>,
 }
 
@@ -89,11 +91,11 @@ impl WindowDelegate {
                 let state = &mut *delegate.get_state();
                 mem::forget(delegate);
 
-                let _: id = msg_send()(state.context, selector("update"));
+                let _: id = msg_send()(*state.context, selector("update"));
 
                 if let Some(handler) = state.handler {
-                    let rect = NSView::frame(state.view);
+                    let rect = NSView::frame(*state.view);
-                    let scale_factor = NSWindow::backingScaleFactor(state.window) as f32;
+                    let scale_factor = NSWindow::backingScaleFactor(*state.window) as f32;
                     (handler)((scale_factor * rect.size.width as f32) as u32,
                               (scale_factor * rect.size.height as f32) as u32);
                 }
@@ -160,9 +162,9 @@ impl WindowDelegate {
 }
 
 pub struct Window {
-    view: id,
+    view: IdRef,
-    window: id,
+    window: IdRef,
-    context: id,
+    context: IdRef,
     delegate: WindowDelegate,
     resize: Option<fn(u32, u32)>,
 
@@ -221,9 +223,9 @@ impl<'a> Iterator for PollEventsIterator<'a> {
                 // to the user application, by passing handler through to the delegate state.
                 let mut ds = DelegateState {
                     is_closed: self.window.is_closed.get(),
-                    context: self.window.context,
+                    context: self.window.context.clone(),
-                    window: self.window.window,
+                    window: self.window.window.clone(),
-                    view: self.window.view,
+                    view: self.window.view.clone(),
                     handler: self.window.resize,
                 };
                 self.window.delegate.set_state(&mut ds);
@@ -249,7 +251,7 @@ impl<'a> Iterator for PollEventsIterator<'a> {
                     } else {
                         self.window.view.convertPoint_fromView_(window_point, nil)
                     };
-                    let view_rect = NSView::frame(self.window.view);
+                    let view_rect = NSView::frame(*self.window.view);
                     let scale_factor = self.window.hidpi_factor();
                     Some(MouseMoved(((scale_factor * view_point.x as f32) as i32,
                                     (scale_factor * (view_rect.size.height - view_point.y) as f32) as i32)))
@@ -358,12 +360,12 @@ impl Window {
             Some(window) => window,
             None         => { return Err(OsError(format!("Couldn't create NSWindow"))); },
         };
-        let view = match Window::create_view(window) {
+        let view = match Window::create_view(*window) {
             Some(view) => view,
             None       => { return Err(OsError(format!("Couldn't create NSView"))); },
         };
 
-        let context = match Window::create_context(view, builder.vsync, builder.gl_version) {
+        let context = match Window::create_context(*view, builder.vsync, builder.gl_version) {
             Some(context) => context,
             None          => { return Err(OsError(format!("Couldn't create OpenGL context"))); },
         };
@@ -377,11 +379,12 @@ impl Window {
             }
         }
 
+        let window_id = *window;
         let window = Window {
             view: view,
             window: window,
             context: context,
-            delegate: WindowDelegate::new(window),
+            delegate: WindowDelegate::new(window_id),
             resize: None,
 
             is_closed: Cell::new(false),
@@ -404,17 +407,43 @@ impl Window {
         }
     }
 
-    fn create_window(dimensions: (u32, u32), title: &str, monitor: Option<MonitorID>) -> Option<id> {
+    fn create_window(dimensions: (u32, u32), title: &str, monitor: Option<MonitorID>) -> Option<IdRef> {
         unsafe {
-            let frame = if monitor.is_some() {
+            let screen = monitor.map(|monitor_id| {
-                let screen = NSScreen::mainScreen(nil);
+                let native_id = match monitor_id.get_native_identifier() {
-                NSScreen::frame(screen)
+                    NativeMonitorId::Numeric(num) => num,
-            } else {
+                    _ => panic!("OS X monitors should always have a numeric native ID")
+                };
+                let matching_screen = {
+                    let screens = NSScreen::screens(nil);
+                    let count: NSUInteger = msg_send()(screens, selector("count"));
+                    let key = IdRef::new(NSString::alloc(nil).init_str("NSScreenNumber"));
+                    let mut matching_screen: Option<id> = None;
+                    for i in range(0, count) {
+                        let screen = msg_send()(screens, selector("objectAtIndex:"), i as NSUInteger);
+                        let device_description = NSScreen::deviceDescription(screen);
+                        let value = msg_send()(device_description, selector("objectForKey:"), *key);
+                        if value != nil {
+                            let screen_number: NSUInteger = msg_send()(value, selector("unsignedIntValue"));
+                            if screen_number as u32 == native_id {
+                                matching_screen = Some(screen);
+                                break;
+                            }
+                        }
+                    }
+                    matching_screen
+                };
+                matching_screen.unwrap_or(NSScreen::mainScreen(nil))
+            });
+            let frame = match screen {
+                Some(screen) => NSScreen::frame(screen),
+                None => {
                     let (width, height) = dimensions;
                     NSRect::new(NSPoint::new(0., 0.), NSSize::new(width as f64, height as f64))
+                }
             };
 
-            let masks = if monitor.is_some() {
+            let masks = if screen.is_some() {
                 NSBorderlessWindowMask as NSUInteger
             } else {
                 NSTitledWindowMask as NSUInteger |
@@ -423,44 +452,39 @@ impl Window {
                 NSResizableWindowMask as NSUInteger
             };
 
-            let window = NSWindow::alloc(nil).initWithContentRect_styleMask_backing_defer_(
+            let window = IdRef::new(NSWindow::alloc(nil).initWithContentRect_styleMask_backing_defer_(
                 frame,
                 masks,
                 NSBackingStoreBuffered,
                 NO,
-            );
+            ));
-
+            window.non_nil().map(|window| {
-            if window == nil {
+                let title = IdRef::new(NSString::alloc(nil).init_str(title));
-                None
+                window.setTitle_(*title);
-            } else {
-                let title = NSString::alloc(nil).init_str(title);
-                window.setTitle_(title);
                 window.setAcceptsMouseMovedEvents_(YES);
-                if monitor.is_some() {
+                if screen.is_some() {
                     window.setLevel_(NSMainMenuWindowLevel as i64 + 1);
                 }
                 else {
                     window.center();
                 }
-                Some(window)
+                window
-            }
+            })
         }
     }
 
-    fn create_view(window: id) -> Option<id> {
+    fn create_view(window: id) -> Option<IdRef> {
         unsafe {
-            let view = NSView::alloc(nil).init();
+            let view = IdRef::new(NSView::alloc(nil).init());
-            if view == nil {
+            view.non_nil().map(|view| {
-                None
-            } else {
                 view.setWantsBestResolutionOpenGLSurface_(YES);
-                window.setContentView_(view);
+                window.setContentView_(*view);
-                Some(view)
+                view
-            }
+            })
         }
     }
 
-    fn create_context(view: id, vsync: bool, gl_version: GlRequest) -> Option<id> {
+    fn create_context(view: id, vsync: bool, gl_version: GlRequest) -> Option<IdRef> {
         let profile = match gl_version {
             GlRequest::Latest => NSOpenGLProfileVersion4_1Core as u32,
             GlRequest::Specific(Api::OpenGl, (1 ... 2, _)) => NSOpenGLProfileVersionLegacy as u32,
@@ -485,22 +509,18 @@ impl Window {
                 0
             ];
 
-            let pixelformat = NSOpenGLPixelFormat::alloc(nil).initWithAttributes_(&attributes);
+            let pixelformat = IdRef::new(NSOpenGLPixelFormat::alloc(nil).initWithAttributes_(&attributes));
-            if pixelformat == nil {
+            pixelformat.non_nil().map(|pixelformat| {
-                return None;
+                let context = IdRef::new(NSOpenGLContext::alloc(nil).initWithFormat_shareContext_(*pixelformat, nil));
-            }
+                context.non_nil().map(|context| {
-
-            let context = NSOpenGLContext::alloc(nil).initWithFormat_shareContext_(pixelformat, nil);
-            if context == nil {
-                None
-            } else {
                     context.setView_(view);
                     if vsync {
                         let value = 1;
                         context.setValues_forParameter_(&value, NSOpenGLContextParameter::NSOpenGLCPSwapInterval);
                     }
-                Some(context)
+                    context
-            }
+                })
+            }).unwrap_or(None)
         }
     }
 
@@ -510,22 +530,22 @@ impl Window {
 
     pub fn set_title(&self, title: &str) {
         unsafe {
-            let title = NSString::alloc(nil).init_str(title);
+            let title = IdRef::new(NSString::alloc(nil).init_str(title));
-            self.window.setTitle_(title);
+            self.window.setTitle_(*title);
         }
     }
 
     pub fn show(&self) {
-        unsafe { NSWindow::makeKeyAndOrderFront_(self.window, nil); }
+        unsafe { NSWindow::makeKeyAndOrderFront_(*self.window, nil); }
     }
 
     pub fn hide(&self) {
-        unsafe { NSWindow::orderOut_(self.window, nil); }
+        unsafe { NSWindow::orderOut_(*self.window, nil); }
     }
 
     pub fn get_position(&self) -> Option<(i32, i32)> {
         unsafe {
-            let content_rect = NSWindow::contentRectForFrameRect_(self.window, NSWindow::frame(self.window));
+            let content_rect = NSWindow::contentRectForFrameRect_(*self.window, NSWindow::frame(*self.window));
             // NOTE: coordinate system might be inconsistent with other backends
             Some((content_rect.origin.x as i32, content_rect.origin.y as i32))
         }
@@ -534,27 +554,27 @@ impl Window {
     pub fn set_position(&self, x: i32, y: i32) {
         unsafe {
             // NOTE: coordinate system might be inconsistent with other backends
-            NSWindow::setFrameOrigin_(self.window, NSPoint::new(x as f64, y as f64));
+            NSWindow::setFrameOrigin_(*self.window, NSPoint::new(x as f64, y as f64));
         }
     }
 
     pub fn get_inner_size(&self) -> Option<(u32, u32)> {
         unsafe {
-            let view_frame = NSView::frame(self.view);
+            let view_frame = NSView::frame(*self.view);
             Some((view_frame.size.width as u32, view_frame.size.height as u32))
         }
     }
 
     pub fn get_outer_size(&self) -> Option<(u32, u32)> {
         unsafe {
-            let window_frame = NSWindow::frame(self.window);
+            let window_frame = NSWindow::frame(*self.window);
             Some((window_frame.size.width as u32, window_frame.size.height as u32))
         }
     }
 
     pub fn set_inner_size(&self, width: u32, height: u32) {
         unsafe {
-            NSWindow::setContentSize_(self.window, NSSize::new(width as f64, height as f64));
+            NSWindow::setContentSize_(*self.window, NSSize::new(width as f64, height as f64));
         }
     }
 
@@ -585,12 +605,20 @@ impl Window {
     }
 
     pub unsafe fn make_current(&self) {
-        let _: id = msg_send()(self.context, selector("update"));
+        let _: id = msg_send()(*self.context, selector("update"));
         self.context.makeCurrentContext();
     }
 
     pub fn is_current(&self) -> bool {
-        unimplemented!()
+        unsafe {
+            let current = NSOpenGLContext::currentContext(nil);
+            if current != nil {
+                let is_equal: bool = msg_send()(current, selector("isEqual:"), *self.context);
+                is_equal
+            } else {
+                false
+            }
+        }
     }
 
     pub fn get_proc_address(&self, _addr: &str) -> *const () {
@@ -663,7 +691,7 @@ impl Window {
 
     pub fn hidpi_factor(&self) -> f32 {
         unsafe {
-            NSWindow::backingScaleFactor(self.window) as f32
+            NSWindow::backingScaleFactor(*self.window) as f32
         }
     }
 
@@ -671,3 +699,57 @@ impl Window {
         unimplemented!();
     }
 }
+
+struct IdRef(id);
+
+impl IdRef {
+    fn new(i: id) -> IdRef {
+        IdRef(i)
+    }
+
+    fn retain(i: id) -> IdRef {
+        if i != nil {
+            unsafe { msg_send::<()>()(i, selector("retain")) };
+        }
+        IdRef(i)
+    }
+
+    fn non_nil(self) -> Option<IdRef> {
+        if self.0 == nil { None } else { Some(self) }
+    }
+}
+
+impl Drop for IdRef {
+    fn drop(&mut self) {
+        if self.0 != nil {
+            unsafe { msg_send::<()>()(self.0, selector("release")) };
+        }
+    }
+}
+
+impl Deref for IdRef {
+    type Target = id;
+    fn deref<'a>(&'a self) -> &'a id {
+        &self.0
+    }
+}
+
+impl Clone for IdRef {
+    fn clone(&self) -> IdRef {
+        if self.0 != nil {
+            unsafe { msg_send::<()>()(self.0, selector("retain")) };
+        }
+        IdRef(self.0)
+    }
+
+    fn clone_from(&mut self, source: &IdRef) {
+        if source.0 != nil {
+            unsafe { msg_send::<()>()(source.0, selector("retain")) };
+        }
+        if self.0 != nil {
+            unsafe { msg_send::<()>()(self.0, selector("release")) };
+        }
+        self.0 = source.0;
+    }
+}
+

src/cocoa/monitor.rs

@@ -1,5 +1,6 @@
 use core_graphics::display;
 use std::collections::VecDeque;
+use native_monitor::NativeMonitorId;
 
 pub struct MonitorID(u32);
 
@@ -35,6 +36,11 @@ impl MonitorID {
         Some(format!("Monitor #{}", screen_num))
     }
 
+    pub fn get_native_identifier(&self) -> NativeMonitorId {
+        let MonitorID(display_id) = *self;
+        NativeMonitorId::Numeric(display_id)
+    }
+
     pub fn get_dimensions(&self) -> (u32, u32) {
         let MonitorID(display_id) = *self;
         let dimension = unsafe {

src/lib.rs

@@ -51,6 +51,8 @@ pub use headless::{HeadlessRendererBuilder, HeadlessContext};
 pub use window::{WindowBuilder, Window, WindowProxy, PollEventsIterator, WaitEventsIterator};
 #[cfg(feature = "window")]
 pub use window::{AvailableMonitorsIter, MonitorID, get_available_monitors, get_primary_monitor};
+#[cfg(feature = "window")]
+pub use native_monitor::NativeMonitorId;
 
 #[cfg(all(not(target_os = "windows"), not(target_os = "linux"), not(target_os = "macos"), not(target_os = "android")))]
 use this_platform_is_not_supported;
@@ -324,3 +326,20 @@ impl<'a> BuilderAttribs<'a> {
                       .expect("Could not find compliant pixel format")
     }
 }
+
+mod native_monitor {
+    /// Native platform identifier for a monitor. Different platforms use fundamentally different types
+    /// to represent a monitor ID.
+    #[derive(PartialEq, Eq)]
+    pub enum NativeMonitorId {
+        /// Cocoa and X11 use a numeric identifier to represent a monitor.
+        Numeric(u32),
+
+        /// Win32 uses a Unicode string to represent a monitor.
+        Name(String),
+
+        /// Other platforms (Android) don't support monitor identification.
+        Unavailable
+    }
+}
+

src/win32/monitor.rs

@@ -3,6 +3,8 @@ use user32;
 
 use std::collections::VecDeque;
 
+use native_monitor::NativeMonitorId;
+
 /// Win32 implementation of the main `MonitorID` object.
 pub struct MonitorID {
     /// The system name of the monitor.
@@ -113,6 +115,11 @@ impl MonitorID {
         Some(self.readable_name.clone())
     }
 
+    /// See the docs of the crate root file.
+    pub fn get_native_identifier(&self) -> NativeMonitorId {
+        NativeMonitorId::Name(self.readable_name.clone())
+    }
+
     /// See the docs if the crate root file.
     pub fn get_dimensions(&self) -> (u32, u32) {
         // TODO: retreive the dimensions every time this is called

src/window.rs

@@ -7,6 +7,7 @@ use CreationError;
 use Event;
 use GlRequest;
 use MouseCursor;
+use native_monitor::NativeMonitorId;
 
 use gl_common;
 use libc;
@@ -509,6 +510,12 @@ impl MonitorID {
         id.get_name()
     }
 
+    /// Returns the native platform identifier for this monitor.
+    pub fn get_native_identifier(&self) -> NativeMonitorId {
+        let &MonitorID(ref id) = self;
+        id.get_native_identifier()
+    }
+
     /// Returns the number of pixels currently displayed on the monitor.
     pub fn get_dimensions(&self) -> (u32, u32) {
         let &MonitorID(ref id) = self;

src/x11/window/monitor.rs

@@ -2,6 +2,7 @@ use std::ptr;
 use std::collections::VecDeque;
 use super::super::ffi;
 use super::ensure_thread_init;
+use native_monitor::NativeMonitorId;
 
 pub struct MonitorID(pub u32);
 
@@ -43,6 +44,11 @@ impl MonitorID {
         Some(format!("Monitor #{}", screen_num))
     }
 
+    pub fn get_native_identifier(&self) -> NativeMonitorId {
+        let MonitorID(screen_num) = *self;
+        NativeMonitorId::Numeric(screen_num)
+    }
+
     pub fn get_dimensions(&self) -> (u32, u32) {
         let dimensions = unsafe {
             let display = ffi::XOpenDisplay(ptr::null());