Wayland: rework the event loop & expose readiness signal (#298)

* wayland: don't create a second event_queue

As each EventsLoop has its own context, this is no longer necessary.

* wayland: buffer events rather than direct dispatch

Changes the behavior of the event loop to first internally
buffer the events generated by the wayland handlers, and then
dispatch them to the client's closure.

- It simplifies the event loop logic
- It makes it possible for the user to call window methods such as
  `set_title()` or `set_inner_size()` without causing a deadlock

* wayland: add is_ready() & fix protocol errors

Adds a `is_ready()` method to the windows to advertize
when it is legal to start drawing, and fix a few wayland
protocol mishandling in the process.

CHANGELOG.md

@@ -1,6 +1,8 @@
 # Unreleased
 
 - Uniformize keyboard scancode values accross Wayland and X11 (#297).
+- Internal rework of the wayland event loop
+- Added method `os::linux::WindowExt::is_ready`
 
 # Version 0.8.1 (2017-09-22)
 

src/os/unix.rs

@@ -112,6 +112,17 @@ pub trait WindowExt {
     ///
     /// The pointer will become invalid when the glutin `Window` is destroyed.
     fn get_wayland_display(&self) -> Option<*mut libc::c_void>;
+
+    /// Check if the window is ready for drawing
+    ///
+    /// On wayland, drawing on a surface before the server has configured
+    /// it using a special event is illegal. As a result, you should wait
+    /// until this method returns `true`.
+    ///
+    /// Once it starts returning `true`, it can never return `false` again.
+    ///
+    /// If the window is X11-based, this will just always return `true`.
+    fn is_ready(&self) -> bool;
 }
 
 impl WindowExt for Window {
@@ -175,6 +186,14 @@ impl WindowExt for Window {
             _ => None
         }
     }
+
+    #[inline]
+    fn is_ready(&self) -> bool {
+        match self.window {
+            LinuxWindow::Wayland(ref w) => w.is_ready(),
+            LinuxWindow::X(_) => true
+        }
+    }
 }
 
 /// Additional methods on `WindowBuilder` that are specific to Unix.

src/platform/linux/mod.rs

@@ -325,7 +325,7 @@ impl EventsLoop {
 
     pub fn new_wayland() -> Result<EventsLoop, ()> {
         wayland::WaylandContext::init()
-            .map(|ctx| EventsLoop::Wayland(wayland::EventsLoop::new(Arc::new(ctx))))
+            .map(|ctx| EventsLoop::Wayland(wayland::EventsLoop::new(ctx)))
             .ok_or(())
     }
 

src/platform/linux/wayland/context.rs

@@ -114,7 +114,6 @@ impl wl_registry::Handler for WaylandEnv {
     {
         if interface == wl_output::WlOutput::interface_name() {
             // intercept outputs
-            // this "expect" cannot trigger (see https://github.com/vberger/wayland-client-rs/issues/69)
             let output = self.registry.bind::<wl_output::WlOutput>(1, name);
             evqh.register::<_, WaylandEnv>(&output, self.my_id);
             self.monitors.push(OutputInfo::new(output, name));
@@ -201,7 +200,7 @@ declare_handler!(WaylandEnv, wl_output::Handler, wl_output::WlOutput);
 
 pub struct WaylandContext {
     pub display: wl_display::WlDisplay,
-    evq: Mutex<EventQueue>,
+    pub evq: Mutex<EventQueue>,
     env_id: usize,
 }
 
@@ -237,6 +236,14 @@ impl WaylandContext {
         })
     }
 
+    pub fn read_events(&self) {
+        let evq_guard = self.evq.lock().unwrap();
+        // read some events from the socket if some are waiting & queue is empty
+        if let Some(guard) = evq_guard.prepare_read() {
+            guard.read_events().expect("Wayland connection unexpectedly lost");
+        }
+    }
+
     pub fn dispatch_pending(&self) {
         let mut guard = self.evq.lock().unwrap();
         guard.dispatch_pending().expect("Wayland connection unexpectedly lost");
@@ -293,8 +300,8 @@ impl WaylandContext {
         evq.register::<_, InitialBufferHandler>(&buffer, initial_buffer_handler_id);
     }
 
-    pub fn create_window<H: wayland_window::Handler>(&self, width: u32, height: u32)
+    pub fn create_window<H: wayland_window::Handler>(&self, width: u32, height: u32, decorated: bool)
-        -> (Arc<wl_surface::WlSurface>, wayland_window::DecoratedSurface<H>)
+        -> (Arc<wl_surface::WlSurface>, wayland_window::DecoratedSurface<H>, bool)
     {
         let mut guard = self.evq.lock().unwrap();
         let (surface, decorated, xdg) = {
@@ -308,7 +315,7 @@ impl WaylandContext {
                 &env.inner.shm,
                 env.get_shell(),
                 env.get_seat(),
-                false
+                decorated
             ).expect("Failed to create a tmpfile buffer.");
             let xdg = match env.get_shell() {
                 &Shell::Xdg(_) => true,
@@ -325,7 +332,7 @@ impl WaylandContext {
             // from the compositor
             self.blank_surface(&surface, &mut *guard, width as i32, height as i32);
         }
-        (surface, decorated)
+        (surface, decorated, xdg)
     }
 }
 

src/platform/linux/wayland/event_loop.rs

@@ -26,19 +26,15 @@ use super::keyboard::KbdHandler;
 ///
 /// Failure to do so is unsafe™
 pub struct EventsLoopSink {
-    callback: Box<FnMut(::Event)>
+    buffer: VecDeque<::Event>
 }
 
 unsafe impl Send for EventsLoopSink { }
 
 impl EventsLoopSink {
-    pub fn new() -> (EventsLoopSink, Arc<Mutex<VecDeque<::Event>>>) {
+    pub fn new() -> EventsLoopSink{EventsLoopSink {
-        let buffer = Arc::new(Mutex::new(VecDeque::new()));
+            buffer: VecDeque::new()
-        let buffer_clone = buffer.clone();
+        }
-        let sink = EventsLoopSink {
-            callback: Box::new(move |evt| { println!("TEMP: {:?}", evt); buffer.lock().unwrap().push_back(evt)}),
-        };
-        (sink, buffer_clone)
     }
 
     pub fn send_event(&mut self, evt: ::WindowEvent, wid: WindowId) {
@@ -46,37 +42,27 @@ impl EventsLoopSink {
             event: evt,
             window_id: ::WindowId(::platform::WindowId::Wayland(wid))
         };
-        (self.callback)(evt)
+        self.buffer.push_back(evt);
     }
 
-    // This function is only safe of the set callback is unset before exclusive
+    pub fn send_raw_event(&mut self, evt: ::Event) {
-    // access to the wayland EventQueue is finished.
+        self.buffer.push_back(evt);
-    //
-    // The callback also cannot be used any longer as long as it has not been
-    // cleared from the Sink.
-    unsafe fn set_callback(&mut self, cb: Box<FnMut(::Event)>) -> Box<FnMut(::Event)> {
-        ::std::mem::replace(&mut self.callback, cb)
     }
 
-    fn with_callback<F>(&mut self, f: F)
+    fn empty_with<F>(&mut self, callback: &mut F) where F: FnMut(::Event) {
-        where F: FnOnce(&mut FnMut(::Event)),
+        for evt in self.buffer.drain(..) {
-    {
+            callback(evt)
-        f(&mut *self.callback)
+        }
     }
 }
 
 pub struct EventsLoop {
-    // the global wayland context
+    // the wayland context
     ctxt: Arc<WaylandContext>,
-    // our EventQueue
-    evq: Arc<Mutex<EventQueue>>,
     // ids of the DecoratedHandlers of the surfaces we know
     decorated_ids: Mutex<Vec<(usize, Arc<wl_surface::WlSurface>)>>,
-    // our sink, receiver of callbacks, shared with some handlers
+    // our sink, shared with some handlers, buffering the events
     sink: Arc<Mutex<EventsLoopSink>>,
-    // a buffer in which events that were dispatched internally are stored
-    // until the user next dispatches events
-    buffer: Arc<Mutex<VecDeque<::Event>>>,
     // trigger cleanup of the dead surfaces
     cleanup_needed: Arc<AtomicBool>,
     // Whether or not there is a pending `Awakened` event to be emitted.
@@ -114,17 +100,16 @@ impl EventsLoopProxy {
 }
 
 impl EventsLoop {
-    pub fn new(ctxt: Arc<WaylandContext>) -> EventsLoop {
+    pub fn new(mut ctxt: WaylandContext) -> EventsLoop {
-        let mut evq = ctxt.display.create_event_queue();
+        let sink = Arc::new(Mutex::new(EventsLoopSink::new()));
-        let (sink, buffer) = EventsLoopSink::new();
+        let inputh = InputHandler::new(&ctxt, sink.clone());
-        let sink = Arc::new(Mutex::new(sink));
+        let hid = ctxt.evq.get_mut().unwrap().add_handler_with_init(inputh);
-        let hid = evq.add_handler_with_init(InputHandler::new(&ctxt, sink.clone()));
+        let ctxt = Arc::new(ctxt);
+
         EventsLoop {
             ctxt: ctxt,
-            evq: Arc::new(Mutex::new(evq)),
             decorated_ids: Mutex::new(Vec::new()),
             sink: sink,
-            buffer: buffer,
             pending_wakeup: Arc::new(AtomicBool::new(false)),
             cleanup_needed: Arc::new(AtomicBool::new(false)),
             hid: hid
@@ -144,41 +129,39 @@ impl EventsLoop {
     }
 
     // some internals that Window needs access to
-    pub fn get_window_init(&self) -> (Arc<Mutex<EventQueue>>, Arc<AtomicBool>) {
+    pub fn get_window_init(&self) -> Arc<AtomicBool> {
-        (self.evq.clone(), self.cleanup_needed.clone())
+        self.cleanup_needed.clone()
     }
 
     pub fn register_window(&self, decorated_id: usize, surface: Arc<wl_surface::WlSurface>) {
-        self.buffer.lock().unwrap().push_back(::Event::WindowEvent {
-            window_id: ::WindowId(::platform::WindowId::Wayland(make_wid(&surface))),
-            event: ::WindowEvent::Refresh
-        });
         self.decorated_ids.lock().unwrap().push((decorated_id, surface.clone()));
-        let mut guard = self.evq.lock().unwrap();
+        let mut guard = self.ctxt.evq.lock().unwrap();
         let mut state = guard.state();
         state.get_mut_handler::<InputHandler>(self.hid).windows.push(surface);
     }
 
-    fn process_resize(evq: &mut EventQueue, ids: &[(usize, Arc<wl_surface::WlSurface>)], callback: &mut FnMut(::Event))
+    fn process_resize(evq: &mut EventQueue, ids: &[(usize, Arc<wl_surface::WlSurface>)], sink: &mut EventsLoopSink)
     {
         let mut state = evq.state();
         for &(decorated_id, ref window) in ids {
             let decorated = state.get_mut_handler::<DecoratedSurface<DecoratedHandler>>(decorated_id);
             if let Some((w, h)) = decorated.handler().as_mut().and_then(|h| h.take_newsize()) {
                 decorated.resize(w as i32, h as i32);
-                callback(
+                sink.send_event(
-                    ::Event::WindowEvent {
+                     ::WindowEvent::Resized(w,h),
-                        window_id: ::WindowId(::platform::WindowId::Wayland(make_wid(&window))),
+                     make_wid(&window)
-                        event: ::WindowEvent::Resized(w,h)
+                );
-                    }
+            }
+            if decorated.handler().as_mut().map(|h| h.take_refresh()).unwrap_or(false) {
+                sink.send_event(
+                     ::WindowEvent::Refresh,
+                     make_wid(&window)
                 );
             }
             if decorated.handler().as_ref().map(|h| h.is_closed()).unwrap_or(false) {
-                 callback(
+                 sink.send_event(
-                    ::Event::WindowEvent {
+                     ::WindowEvent::Closed,
-                        window_id: ::WindowId(::platform::WindowId::Wayland(make_wid(&window))),
+                     make_wid(&window)
-                        event: ::WindowEvent::Closed
-                    }
                 );
 
             }
@@ -187,7 +170,7 @@ impl EventsLoop {
 
     fn prune_dead_windows(&self) {
         self.decorated_ids.lock().unwrap().retain(|&(_, ref w)| w.status() == Liveness::Alive);
-        let mut evq_guard = self.evq.lock().unwrap();
+        let mut evq_guard = self.ctxt.evq.lock().unwrap();
         let mut state = evq_guard.state();
         let handler = state.get_mut_handler::<InputHandler>(self.hid);
         handler.windows.retain(|w| w.status() == Liveness::Alive);
@@ -198,11 +181,12 @@ impl EventsLoop {
         }
     }
 
-    fn empty_buffer<F>(&self, callback: &mut F) where F: FnMut(::Event) {
+    fn post_dispatch_triggers(&self) {
-        let mut guard = self.buffer.lock().unwrap();
+        let mut evq_guard = self.ctxt.evq.lock().unwrap();
-        for evt in guard.drain(..) {
+        let mut sink_guard = self.sink.lock().unwrap();
-            callback(evt)
+        let ids_guard = self.decorated_ids.lock().unwrap();
-        }
+        self.emit_pending_wakeup(&mut sink_guard);
+        Self::process_resize(&mut evq_guard, &ids_guard, &mut sink_guard);
     }
 
     pub fn poll_events<F>(&mut self, mut callback: F)
@@ -211,41 +195,15 @@ impl EventsLoop {
         // send pending requests to the server...
         self.ctxt.flush();
 
-        // first of all, get exclusive access to this event queue
+        // dispatch any pre-buffered events
-        let mut evq_guard = self.evq.lock().unwrap();
+        self.sink.lock().unwrap().empty_with(&mut callback);
 
-        // dispatch pre-buffered events
+        // try dispatching events without blocking
-        self.empty_buffer(&mut callback);
+        self.ctxt.read_events();
-
-        // read some events from the socket if some are waiting & queue is empty
-        if let Some(guard) = evq_guard.prepare_read() {
-            guard.read_events().expect("Wayland connection unexpectedly lost");
-        }
-
-        // set the callback into the sink
-        // we extend the lifetime of the closure to 'static to be able to put it in
-        // the sink, but we'll explicitly drop it at the end of this function, so it's fine
-        let static_cb = unsafe { ::std::mem::transmute(Box::new(callback) as Box<FnMut(_)>) };
-        let old_cb = unsafe { self.sink.lock().unwrap().set_callback(static_cb) };
-
-        // then do the actual dispatching
         self.ctxt.dispatch_pending();
-        evq_guard.dispatch_pending().expect("Wayland connection unexpectedly lost");
+        self.post_dispatch_triggers();
 
-        self.emit_pending_wakeup();
+        self.sink.lock().unwrap().empty_with(&mut callback);
-
-        {
-            let mut sink_guard = self.sink.lock().unwrap();
-
-            // events where probably dispatched, process resize
-            let ids_guard = self.decorated_ids.lock().unwrap();
-            sink_guard.with_callback(
-                |cb| Self::process_resize(&mut evq_guard, &ids_guard, cb)
-            );
-
-            // replace the old noop callback
-            unsafe { sink_guard.set_callback(old_cb) };
-        }
 
         if self.cleanup_needed.swap(false, atomic::Ordering::Relaxed) {
             self.prune_dead_windows()
@@ -258,9 +216,6 @@ impl EventsLoop {
         // send pending requests to the server...
         self.ctxt.flush();
 
-        // first of all, get exclusive access to this event queue
-        let mut evq_guard = self.evq.lock().unwrap();
-
         // Check for control flow by wrapping the callback.
         let control_flow = ::std::cell::Cell::new(ControlFlow::Continue);
         let mut callback = |event| if let ControlFlow::Break = callback(event) {
@@ -268,24 +223,15 @@ impl EventsLoop {
         };
 
         // dispatch pre-buffered events
-        self.empty_buffer(&mut callback);
+        self.sink.lock().unwrap().empty_with(&mut callback);
-
-        // set the callback into the sink
-        // we extend the lifetime of the closure to 'static to be able to put it in
-        // the sink, but we'll explicitly drop it at the end of this function, so it's fine
-        let static_cb = unsafe { ::std::mem::transmute(Box::new(callback) as Box<FnMut(_)>) };
-        let old_cb = unsafe { self.sink.lock().unwrap().set_callback(static_cb) };
 
         loop {
+            // dispatch events blocking if needed
             self.ctxt.dispatch();
-            evq_guard.dispatch_pending().expect("Wayland connection unexpectedly lost");
+            self.post_dispatch_triggers();
 
-            self.emit_pending_wakeup();
+            // empty buffer of events
-
+            self.sink.lock().unwrap().empty_with(&mut callback);
-            let ids_guard = self.decorated_ids.lock().unwrap();
-            self.sink.lock().unwrap()
-                .with_callback(|cb| Self::process_resize(&mut evq_guard, &ids_guard, cb));
-            self.ctxt.flush();
 
             if self.cleanup_needed.swap(false, atomic::Ordering::Relaxed) {
                 self.prune_dead_windows()
@@ -295,15 +241,12 @@ impl EventsLoop {
                 break;
             }
         }
-
-        // replace the old noop callback
-        unsafe { self.sink.lock().unwrap().set_callback(old_cb) };
     }
 
     // If an `EventsLoopProxy` has signalled a wakeup, emit an event and reset the flag.
-    fn emit_pending_wakeup(&self) {
+    fn emit_pending_wakeup(&self, sink: &mut EventsLoopSink) {
         if self.pending_wakeup.load(atomic::Ordering::Relaxed) {
-            self.sink.lock().unwrap().with_callback(|cb| cb(::Event::Awakened));
+            sink.send_raw_event(::Event::Awakened);
             self.pending_wakeup.store(false, atomic::Ordering::Relaxed);
         }
     }

src/platform/linux/wayland/window.rs

@@ -1,8 +1,8 @@
 use std::sync::{Arc, Mutex};
-use std::sync::atomic::AtomicBool;
+use std::sync::atomic::{Ordering, AtomicBool};
 use std::cmp;
 
-use wayland_client::{EventQueue, EventQueueHandle, Proxy};
+use wayland_client::{EventQueueHandle, Proxy};
 use wayland_client::protocol::{wl_display,wl_surface};
 
 use {CreationError, MouseCursor, CursorState, WindowAttributes};
@@ -18,8 +18,6 @@ use super::wayland_window::DecoratedSurface;
 pub struct Window {
     // the global wayland context
     ctxt: Arc<WaylandContext>,
-    // the EventQueue of our EventsLoop
-    evq: Arc<Mutex<EventQueue>>,
     // signal to advertize the EventsLoop when we are destroyed
     cleanup_signal: Arc<AtomicBool>,
     // our wayland surface
@@ -44,27 +42,31 @@ impl Window {
         let ctxt = evlp.context().clone();
         let (width, height) = attributes.dimensions.unwrap_or((800,600));
 
-        let (surface, decorated) = ctxt.create_window::<DecoratedHandler>(width, height);
+        let (surface, decorated, xdg) = ctxt.create_window::<DecoratedHandler>(width, height, attributes.decorations);
 
         // init DecoratedSurface
-        let (evq, cleanup_signal) = evlp.get_window_init();
+        let cleanup_signal = evlp.get_window_init();
+
+        let mut fullscreen_monitor = None;
+        if let Some(RootMonitorId { inner: PlatformMonitorId::Wayland(ref monitor_id) }) = attributes.fullscreen {
+            ctxt.with_output(monitor_id.clone(), |output| {
+                fullscreen_monitor = output.clone();
+            });
+        }
+
         let decorated_id = {
-            let mut evq_guard = evq.lock().unwrap();
+            let mut evq_guard = ctxt.evq.lock().unwrap();
             // store the DecoratedSurface handler
             let decorated_id = evq_guard.add_handler_with_init(decorated);
             {
                 let mut state = evq_guard.state();
                 // initialize the DecoratedHandler
                 let decorated = state.get_mut_handler::<DecoratedSurface<DecoratedHandler>>(decorated_id);
-                *(decorated.handler()) = Some(DecoratedHandler::new());
+                *(decorated.handler()) = Some(DecoratedHandler::new(!xdg));
 
                 // set fullscreen if necessary
-                if let Some(RootMonitorId { inner: PlatformMonitorId::Wayland(ref monitor_id) }) = attributes.fullscreen {
+                if let Some(output) = fullscreen_monitor {
-                    ctxt.with_output(monitor_id.clone(), |output| {
+                    decorated.set_fullscreen(Some(&output));
-                        decorated.set_fullscreen(Some(output))
-                    });
-                } else if attributes.decorations {
-                    decorated.set_decorate(true);
                 }
                 // Finally, set the decorations size
                 decorated.resize(width as i32, height as i32);
@@ -76,7 +78,6 @@ impl Window {
         };
         let me = Window {
             ctxt: ctxt,
-            evq: evq,
             cleanup_signal: cleanup_signal,
             surface: surface,
             size: Mutex::new((width, height)),
@@ -95,7 +96,7 @@ impl Window {
     }
 
     pub fn set_title(&self, title: &str) {
-        let mut guard = self.evq.lock().unwrap();
+        let mut guard = self.ctxt.evq.lock().unwrap();
         let mut state = guard.state();
         let decorated = state.get_mut_handler::<DecoratedSurface<DecoratedHandler>>(self.decorated_id);
         decorated.set_title(title.into())
@@ -136,7 +137,7 @@ impl Window {
     #[inline]
     // NOTE: This will only resize the borders, the contents must be updated by the user
     pub fn set_inner_size(&self, x: u32, y: u32) {
-        let mut guard = self.evq.lock().unwrap();
+        let mut guard = self.ctxt.evq.lock().unwrap();
         let mut state = guard.state();
         let mut decorated = state.get_mut_handler::<DecoratedSurface<DecoratedHandler>>(self.decorated_id);
         decorated.resize(x as i32, y as i32);
@@ -213,25 +214,36 @@ impl Window {
 
         find
     }
+
+    pub fn is_ready(&self) -> bool {
+        let mut guard = self.ctxt.evq.lock().unwrap();
+        let mut state = guard.state();
+        let mut decorated = state.get_mut_handler::<DecoratedSurface<DecoratedHandler>>(self.decorated_id);
+        decorated.handler().as_ref().unwrap().configured
+    }
 }
 
 impl Drop for Window {
     fn drop(&mut self) {
         self.surface.destroy();
-        self.cleanup_signal.store(true, ::std::sync::atomic::Ordering::Relaxed);
+        self.cleanup_signal.store(true, Ordering::Relaxed);
     }
 }
 
 pub struct DecoratedHandler {
     newsize: Option<(u32, u32)>,
+    refresh: bool,
     closed: bool,
+    configured: bool
 }
 
 impl DecoratedHandler {
-    fn new() -> DecoratedHandler {
+    fn new(configured: bool) -> DecoratedHandler {
         DecoratedHandler {
             newsize: None,
+            refresh: false,
             closed: false,
+            configured: configured
         }
     }
 
@@ -239,6 +251,12 @@ impl DecoratedHandler {
         self.newsize.take()
     }
 
+    pub fn take_refresh(&mut self) -> bool {
+        let refresh = self.refresh;
+        self.refresh = false;
+        refresh
+    }
+
     pub fn is_closed(&self) -> bool { self.closed }
 }
 
@@ -248,9 +266,9 @@ impl wayland_window::Handler for DecoratedHandler {
                  _cfg: wayland_window::Configure,
                  newsize: Option<(i32, i32)>)
     {
-        if let Some((w, h)) = newsize {
+        self.newsize = newsize.map(|(w, h)| (w as u32, h as u32));
-            self.newsize = Some((w as u32, h as u32));
+        self.refresh = true;
-        }
+        self.configured = true;
     }
 
     fn close(&mut self, _: &mut EventQueueHandle) {