On Wayland, reduce amount of spurious wakeups

Mark it as breaking, since some clients relied on that behavior, simply
because dispatching clients queue always woke up a winit, meaning that
they won't be able to use user events for this sake.

CHANGELOG.md

@@ -8,6 +8,7 @@ And please only add new entries to the top of this list, right below the `# Unre
 
 # Unreleased
 
+- **Breaking:** on Wayland, dispatching user created wayland queue won't wake up the loop unless winit has event to send back.
 - Removed platform-specific extensions that should be retrieved through `raw-window-handle` trait implementations instead:
   - `platform::windows::HINSTANCE`.
   - `WindowExtWindows::hinstance`.

src/platform_impl/linux/wayland/event_loop/mod.rs

@@ -90,8 +90,15 @@ impl<T: 'static> EventLoop<T> {
         // Register Wayland source.
         let wayland_source = WaylandSource::new(event_queue)?;
         let wayland_dispatcher =
-            calloop::Dispatcher::new(wayland_source, |_, queue, winit_state| {
+            calloop::Dispatcher::new(wayland_source, |_, queue, winit_state: &mut WinitState| {
-                queue.dispatch_pending(winit_state)
+                let result = queue.dispatch_pending(winit_state);
+                if result.is_ok()
+                    && (!winit_state.events_sink.is_empty()
+                        || !winit_state.window_compositor_updates.is_empty())
+                {
+                    winit_state.dispatched_events = true;
+                }
+                result
             });
 
         event_loop
@@ -102,21 +109,25 @@ impl<T: 'static> EventLoop<T> {
         let pending_user_events = Rc::new(RefCell::new(Vec::new()));
         let pending_user_events_clone = pending_user_events.clone();
         let (user_events_sender, user_events_channel) = calloop::channel::channel();
-        event_loop
+        event_loop.handle().insert_source(
-            .handle()
+            user_events_channel,
-            .insert_source(user_events_channel, move |event, _, _| {
+            move |event, _, winit_state: &mut WinitState| {
                 if let calloop::channel::Event::Msg(msg) = event {
+                    winit_state.dispatched_events = true;
                     pending_user_events_clone.borrow_mut().push(msg);
                 }
-            })?;
+            },
+        )?;
 
         // An event's loop awakener to wake up for window events from winit's windows.
         let (event_loop_awakener, event_loop_awakener_source) = calloop::ping::make_ping()?;
-        event_loop
+        event_loop.handle().insert_source(
-            .handle()
+            event_loop_awakener_source,
-            .insert_source(event_loop_awakener_source, move |_, _, _| {
+            move |_, _, winit_state: &mut WinitState| {
                 // No extra handling is required, we just need to wake-up.
-            })?;
+                winit_state.dispatched_events = true;
+            },
+        )?;
 
         let window_target = EventLoopWindowTarget {
             connection: connection.clone(),
@@ -220,49 +231,101 @@ impl<T: 'static> EventLoop<T> {
     where
         F: FnMut(Event<T>, &RootEventLoopWindowTarget<T>, &mut ControlFlow),
     {
-        let start = Instant::now();
+        let cause = loop {
+            let start = Instant::now();
 
-        // TODO(rib): remove this workaround and instead make sure that the calloop
+            // TODO(rib): remove this workaround and instead make sure that the calloop
-        // WaylandSource correctly implements the cooperative prepare_read protocol
+            // WaylandSource correctly implements the cooperative prepare_read protocol
-        // that support multithreaded wayland clients that may all read from the
+            // that support multithreaded wayland clients that may all read from the
-        // same socket.
+            // same socket.
-        //
+            //
-        // During the run of the user callback, some other code monitoring and reading the
+            // During the run of the user callback, some other code monitoring and reading the
-        // Wayland socket may have been run (mesa for example does this with vsync), if that
+            // Wayland socket may have been run (mesa for example does this with vsync), if that
-        // is the case, some events may have been enqueued in our event queue.
+            // is the case, some events may have been enqueued in our event queue.
-        //
+            //
-        // If some messages are there, the event loop needs to behave as if it was instantly
+            // If some messages are there, the event loop needs to behave as if it was instantly
-        // woken up by messages arriving from the Wayland socket, to avoid delaying the
+            // woken up by messages arriving from the Wayland socket, to avoid delaying the
-        // dispatch of these events until we're woken up again.
+            // dispatch of these events until we're woken up again.
-        let instant_wakeup = {
+            let instant_wakeup = {
-            let mut wayland_source = self.wayland_dispatcher.as_source_mut();
+                let mut wayland_source = self.wayland_dispatcher.as_source_mut();
-            let queue = wayland_source.queue();
+                let queue = wayland_source.queue();
-            let state = match &mut self.window_target.p {
+                let state = match &mut self.window_target.p {
-                PlatformEventLoopWindowTarget::Wayland(window_target) => {
+                    PlatformEventLoopWindowTarget::Wayland(window_target) => {
-                    window_target.state.get_mut()
+                        window_target.state.get_mut()
+                    }
+                    #[cfg(x11_platform)]
+                    _ => unreachable!(),
+                };
+
+                match queue.dispatch_pending(state) {
+                    Ok(dispatched) => {
+                        state.dispatched_events |= !state.events_sink.is_empty()
+                            || !state.window_compositor_updates.is_empty();
+                        dispatched > 0
+                    }
+                    Err(error) => {
+                        error!("Error dispatching wayland queue: {}", error);
+                        self.control_flow = ControlFlow::ExitWithCode(1);
+                        return;
+                    }
                 }
-                #[cfg(x11_platform)]
-                _ => unreachable!(),
             };
 
-            match queue.dispatch_pending(state) {
+            timeout = if instant_wakeup {
-                Ok(dispatched) => dispatched > 0,
+                Some(Duration::ZERO)
-                Err(error) => {
+            } else {
-                    error!("Error dispatching wayland queue: {}", error);
+                let control_flow_timeout = match self.control_flow {
-                    self.control_flow = ControlFlow::ExitWithCode(1);
+                    ControlFlow::Wait => None,
-                    return;
+                    ControlFlow::Poll => Some(Duration::ZERO),
-                }
+                    ControlFlow::WaitUntil(wait_deadline) => {
-            }
+                        Some(wait_deadline.saturating_duration_since(start))
-        };
+                    }
+                    // This function shouldn't have to handle any requests to exit
+                    // the application (there should be no need to poll for events
+                    // if the application has requested to exit) so we consider
+                    // it a bug in the backend if we ever see `ExitWithCode` here.
+                    ControlFlow::ExitWithCode(_code) => unreachable!(),
+                };
+                min_timeout(control_flow_timeout, timeout)
+            };
 
-        timeout = if instant_wakeup {
+            // NOTE Ideally we should flush as the last thing we do before polling
-            Some(Duration::ZERO)
+            // to wait for events, and this should be done by the calloop
-        } else {
+            // WaylandSource but we currently need to flush writes manually.
-            let control_flow_timeout = match self.control_flow {
+            let _ = self.connection.flush();
-                ControlFlow::Wait => None,
+
-                ControlFlow::Poll => Some(Duration::ZERO),
+            if let Err(error) = self.loop_dispatch(timeout) {
-                ControlFlow::WaitUntil(wait_deadline) => {
+                // NOTE We exit on errors from dispatches, since if we've got protocol error
-                    Some(wait_deadline.saturating_duration_since(start))
+                // libwayland-client/wayland-rs will inform us anyway, but crashing downstream is not
+                // really an option. Instead we inform that the event loop got destroyed. We may
+                // communicate an error that something was terminated, but winit doesn't provide us
+                // with an API to do that via some event.
+                // Still, we set the exit code to the error's OS error code, or to 1 if not possible.
+                let exit_code = error.raw_os_error().unwrap_or(1);
+                self.control_flow = ControlFlow::ExitWithCode(exit_code);
+                return;
+            }
+
+            // NB: `StartCause::Init` is handled as a special case and doesn't need
+            // to be considered here
+            let cause = match self.control_flow {
+                ControlFlow::Poll => StartCause::Poll,
+                ControlFlow::Wait => StartCause::WaitCancelled {
+                    start,
+                    requested_resume: None,
+                },
+                ControlFlow::WaitUntil(deadline) => {
+                    if Instant::now() < deadline {
+                        StartCause::WaitCancelled {
+                            start,
+                            requested_resume: Some(deadline),
+                        }
+                    } else {
+                        StartCause::ResumeTimeReached {
+                            start,
+                            requested_resume: deadline,
+                        }
+                    }
                 }
                 // This function shouldn't have to handle any requests to exit
                 // the application (there should be no need to poll for events
@@ -270,52 +333,14 @@ impl<T: 'static> EventLoop<T> {
                 // it a bug in the backend if we ever see `ExitWithCode` here.
                 ControlFlow::ExitWithCode(_code) => unreachable!(),
             };
-            min_timeout(control_flow_timeout, timeout)
-        };
 
-        // NOTE Ideally we should flush as the last thing we do before polling
+            // Reduce spurious wake-ups.
-        // to wait for events, and this should be done by the calloop
+            let dispatched_events = self.with_state(|state| state.dispatched_events);
-        // WaylandSource but we currently need to flush writes manually.
+            if matches!(cause, StartCause::WaitCancelled { .. }) && !dispatched_events {
-        let _ = self.connection.flush();
+                continue;
-
-        if let Err(error) = self.loop_dispatch(timeout) {
-            // NOTE We exit on errors from dispatches, since if we've got protocol error
-            // libwayland-client/wayland-rs will inform us anyway, but crashing downstream is not
-            // really an option. Instead we inform that the event loop got destroyed. We may
-            // communicate an error that something was terminated, but winit doesn't provide us
-            // with an API to do that via some event.
-            // Still, we set the exit code to the error's OS error code, or to 1 if not possible.
-            let exit_code = error.raw_os_error().unwrap_or(1);
-            self.control_flow = ControlFlow::ExitWithCode(exit_code);
-            return;
-        }
-
-        // NB: `StartCause::Init` is handled as a special case and doesn't need
-        // to be considered here
-        let cause = match self.control_flow {
-            ControlFlow::Poll => StartCause::Poll,
-            ControlFlow::Wait => StartCause::WaitCancelled {
-                start,
-                requested_resume: None,
-            },
-            ControlFlow::WaitUntil(deadline) => {
-                if Instant::now() < deadline {
-                    StartCause::WaitCancelled {
-                        start,
-                        requested_resume: Some(deadline),
-                    }
-                } else {
-                    StartCause::ResumeTimeReached {
-                        start,
-                        requested_resume: deadline,
-                    }
-                }
             }
-            // This function shouldn't have to handle any requests to exit
+
-            // the application (there should be no need to poll for events
+            break cause;
-            // if the application has requested to exit) so we consider
-            // it a bug in the backend if we ever see `ExitWithCode` here.
-            ControlFlow::ExitWithCode(_code) => unreachable!(),
         };
 
         self.single_iteration(&mut callback, cause);
@@ -531,6 +556,11 @@ impl<T: 'static> EventLoop<T> {
             }
         }
 
+        // Reset the hint that we've dispatched events.
+        self.with_state(|state| {
+            state.dispatched_events = false;
+        });
+
         // This is always the last event we dispatch before poll again
         sticky_exit_callback(
             Event::AboutToWait,

src/platform_impl/linux/wayland/event_loop/sink.rs

@@ -20,6 +20,12 @@ impl EventSink {
         Default::default()
     }
 
+    /// Return `true` if there're pending events.
+    #[inline]
+    pub fn is_empty(&self) -> bool {
+        self.window_events.is_empty()
+    }
+
     /// Add new device event to a queue.
     #[inline]
     pub fn push_device_event(&mut self, event: DeviceEvent, device_id: DeviceId) {

src/platform_impl/linux/wayland/state.rs

@@ -1,5 +1,6 @@
 use std::cell::RefCell;
 use std::error::Error;
+use std::sync::atomic::Ordering;
 use std::sync::{Arc, Mutex};
 
 use fnv::FnvHashMap;
@@ -104,6 +105,10 @@ pub struct WinitState {
 
     /// Loop handle to re-register event sources, such as keyboard repeat.
     pub loop_handle: LoopHandle<'static, Self>,
+
+    /// Whether we have dispatched events to the user thus we want to
+    /// send `AboutToWait` and normally wakeup the user.
+    pub dispatched_events: bool,
 }
 
 impl WinitState {
@@ -167,6 +172,8 @@ impl WinitState {
             monitors: Arc::new(Mutex::new(monitors)),
             events_sink: EventSink::new(),
             loop_handle,
+            // Make it true by default.
+            dispatched_events: true,
         })
     }
 
@@ -328,6 +335,18 @@ impl CompositorHandler for WinitState {
             None => return,
         };
 
+        // In case we have a redraw requested we must indicate the wake up.
+        if self
+            .window_requests
+            .get_mut()
+            .get(&window_id)
+            .unwrap()
+            .redraw_requested
+            .load(Ordering::Relaxed)
+        {
+            self.dispatched_events = true;
+        }
+
         window.lock().unwrap().frame_callback_received();
     }
 }

src/platform_impl/linux/wayland/window/mod.rs

@@ -287,10 +287,14 @@ impl Window {
 
     #[inline]
     pub fn request_redraw(&self) {
-        self.window_requests
+        if self
+            .window_requests
             .redraw_requested
-            .store(true, Ordering::Relaxed);
+            .compare_exchange(false, true, Ordering::Relaxed, Ordering::Relaxed)
-        self.event_loop_awakener.ping();
+            .is_ok()
+        {
+            self.event_loop_awakener.ping();
+        }
     }
 
     #[inline]