On Wayland, fix Window::request_redraw being delayed

On Waylnad when asking for redraw before `MainEventsCleared`
would result for redraw being send on the next event loop tick,
which is not expectable given that it must be delivered on the same
event loop tick.

CHANGELOG.md

@@ -13,6 +13,7 @@ And please only add new entries to the top of this list, right below the `# Unre
 - On Windows, respect min/max inner sizes when creating the window.
 - For backwards compatibility, `Window` now (additionally) implements the old version (`0.4`) of the `HasRawWindowHandle` trait
 - On Windows, added support for `EventLoopWindowTarget::set_device_event_filter`.
+- On Wayland, fix user requested `WindowEvent::RedrawRequested` being delayed by a frame.
 
 # 0.27.1 (2022-07-30)
 

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

@@ -2,6 +2,7 @@ use std::cell::RefCell;
 use std::collections::HashMap;
 use std::error::Error;
 use std::io::Result as IOResult;
+use std::mem;
 use std::process;
 use std::rc::Rc;
 use std::time::{Duration, Instant};
@@ -26,7 +27,7 @@ use crate::platform_impl::EventLoopWindowTarget as PlatformEventLoopWindowTarget
 use super::env::{WindowingFeatures, WinitEnv};
 use super::output::OutputManager;
 use super::seat::SeatManager;
-use super::window::shim::{self, WindowUpdate};
+use super::window::shim::{self, WindowCompositorUpdate, WindowUserRequest};
 use super::{DeviceId, WindowId};
 
 mod proxy;
@@ -82,6 +83,9 @@ impl<T> EventLoopWindowTarget<T> {
 }
 
 pub struct EventLoop<T: 'static> {
+    /// Dispatcher of Wayland events.
+    pub wayland_dispatcher: WinitDispatcher,
+
     /// Event loop.
     event_loop: calloop::EventLoop<'static, WinitState>,
 
@@ -94,9 +98,6 @@ pub struct EventLoop<T: 'static> {
     /// Sender of user events.
     user_events_sender: calloop::channel::Sender<T>,
 
-    /// Dispatcher of Wayland events.
-    pub wayland_dispatcher: WinitDispatcher,
-
     /// Window target.
     window_target: RootEventLoopWindowTarget<T>,
 
@@ -164,17 +165,19 @@ impl<T: 'static> EventLoop<T> {
         let (event_loop_awakener, event_loop_awakener_source) = calloop::ping::make_ping()?;
 
         // Handler of window requests.
-        event_loop.handle().insert_source(
+        event_loop
-            event_loop_awakener_source,
+            .handle()
-            move |_, _, winit_state| {
+            .insert_source(event_loop_awakener_source, move |_, _, state| {
-                shim::handle_window_requests(winit_state);
+                // Drain events here as well to account for application doing batch event processing
-            },
+                // on RedrawEventsCleared.
-        )?;
+                shim::handle_window_requests(state);
+            })?;
 
         let event_loop_handle = event_loop.handle();
         let window_map = HashMap::new();
         let event_sink = EventSink::new();
-        let window_updates = HashMap::new();
+        let window_user_requests = HashMap::new();
+        let window_compositor_updates = HashMap::new();
 
         // Create event loop window target.
         let event_loop_window_target = EventLoopWindowTarget {
@@ -183,7 +186,8 @@ impl<T: 'static> EventLoop<T> {
             state: RefCell::new(WinitState {
                 window_map,
                 event_sink,
-                window_updates,
+                window_user_requests,
+                window_compositor_updates,
             }),
             event_loop_handle,
             output_manager,
@@ -236,7 +240,8 @@ impl<T: 'static> EventLoop<T> {
         // applications don't themselves have a formal suspend/resume lifecycle.
         callback(Event::Resumed, &self.window_target, &mut control_flow);
 
-        let mut window_updates: Vec<(WindowId, WindowUpdate)> = Vec::new();
+        let mut window_compositor_updates: Vec<(WindowId, WindowCompositorUpdate)> = Vec::new();
+        let mut window_user_requests: Vec<(WindowId, WindowUserRequest)> = Vec::new();
         let mut event_sink_back_buffer = Vec::new();
 
         // NOTE We break on errors from dispatches, since if we've got protocol error
@@ -357,25 +362,26 @@ impl<T: 'static> EventLoop<T> {
                 );
             }
 
-            // Process 'new' pending updates.
+            // Process 'new' pending updates from compositor.
             self.with_state(|state| {
-                window_updates.clear();
+                window_compositor_updates.clear();
-                window_updates.extend(
+                window_compositor_updates.extend(
                     state
-                        .window_updates
+                        .window_compositor_updates
                         .iter_mut()
-                        .map(|(wid, window_update)| (*wid, window_update.take())),
+                        .map(|(wid, window_update)| (*wid, mem::take(window_update))),
                 );
             });
 
-            for (window_id, window_update) in window_updates.iter_mut() {
+            for (window_id, window_compositor_update) in window_compositor_updates.iter_mut() {
-                if let Some(scale_factor) = window_update.scale_factor.map(|f| f as f64) {
+                if let Some(scale_factor) = window_compositor_update.scale_factor.map(|f| f as f64)
+                {
                     let mut physical_size = self.with_state(|state| {
                         let window_handle = state.window_map.get(window_id).unwrap();
                         let mut size = window_handle.size.lock().unwrap();
 
                         // Update the new logical size if it was changed.
-                        let window_size = window_update.size.unwrap_or(*size);
+                        let window_size = window_compositor_update.size.unwrap_or(*size);
                         *size = window_size;
 
                         window_size.to_physical(scale_factor)
@@ -397,26 +403,27 @@ impl<T: 'static> EventLoop<T> {
                     // We don't update size on a window handle since we'll do that later
                     // when handling size update.
                     let new_logical_size = physical_size.to_logical(scale_factor);
-                    window_update.size = Some(new_logical_size);
+                    window_compositor_update.size = Some(new_logical_size);
                 }
 
-                if let Some(size) = window_update.size.take() {
+                if let Some(size) = window_compositor_update.size.take() {
                     let physical_size = self.with_state(|state| {
                         let window_handle = state.window_map.get_mut(window_id).unwrap();
                         let mut window_size = window_handle.size.lock().unwrap();
 
                         // Always issue resize event on scale factor change.
-                        let physical_size =
+                        let physical_size = if window_compositor_update.scale_factor.is_none()
-                            if window_update.scale_factor.is_none() && *window_size == size {
+                            && *window_size == size
-                                // The size hasn't changed, don't inform downstream about that.
+                        {
-                                None
+                            // The size hasn't changed, don't inform downstream about that.
-                            } else {
+                            None
-                                *window_size = size;
+                        } else {
-                                let scale_factor =
+                            *window_size = size;
-                                    sctk::get_surface_scale_factor(window_handle.window.surface());
+                            let scale_factor =
-                                let physical_size = size.to_physical(scale_factor as f64);
+                                sctk::get_surface_scale_factor(window_handle.window.surface());
-                                Some(physical_size)
+                            let physical_size = size.to_physical(scale_factor as f64);
-                            };
+                            Some(physical_size)
+                        };
 
                         // We still perform all of those resize related logic even if the size
                         // hasn't changed, since GNOME relies on `set_geometry` calls after
@@ -425,7 +432,11 @@ impl<T: 'static> EventLoop<T> {
                         window_handle.window.refresh();
 
                         // Mark that refresh isn't required, since we've done it right now.
-                        window_update.refresh_frame = false;
+                        state
+                            .window_user_requests
+                            .get_mut(window_id)
+                            .unwrap()
+                            .refresh_frame = false;
 
                         physical_size
                     });
@@ -443,7 +454,8 @@ impl<T: 'static> EventLoop<T> {
                     }
                 }
 
-                if window_update.close_window {
+                // If the close is requested, send it here.
+                if window_compositor_update.close_window {
                     sticky_exit_callback(
                         Event::WindowEvent {
                             window_id: crate::window::WindowId(*window_id),
@@ -480,21 +492,40 @@ impl<T: 'static> EventLoop<T> {
                 &mut callback,
             );
 
+            // Apply user requests, so every event required resize and latter surface commit will
+            // be applied right before drawing. This will also ensure that every `RedrawRequested`
+            // event will be delivered in time.
+            self.with_state(|state| {
+                shim::handle_window_requests(state);
+            });
+
+            // Process 'new' pending updates from compositor.
+            self.with_state(|state| {
+                window_user_requests.clear();
+                window_user_requests.extend(
+                    state
+                        .window_user_requests
+                        .iter_mut()
+                        .map(|(wid, window_request)| (*wid, mem::take(window_request))),
+                );
+            });
+
             // Handle RedrawRequested events.
-            for (window_id, window_update) in window_updates.iter() {
+            for (window_id, mut window_request) in window_user_requests.iter() {
                 // Handle refresh of the frame.
-                if window_update.refresh_frame {
+                if window_request.refresh_frame {
                     self.with_state(|state| {
                         let window_handle = state.window_map.get_mut(window_id).unwrap();
                         window_handle.window.refresh();
-                        if !window_update.redraw_requested {
-                            window_handle.window.surface().commit();
-                        }
                     });
+
+                    // In general refreshing the frame requires surface commit, those force user
+                    // to redraw.
+                    window_request.redraw_requested = true;
                 }
 
                 // Handle redraw request.
-                if window_update.redraw_requested {
+                if window_request.redraw_requested {
                     sticky_exit_callback(
                         Event::RedrawRequested(crate::window::WindowId(*window_id)),
                         &self.window_target,

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

@@ -3,7 +3,9 @@
 use std::collections::HashMap;
 
 use super::EventSink;
-use crate::platform_impl::wayland::window::shim::{WindowHandle, WindowUpdate};
+use crate::platform_impl::wayland::window::shim::{
+    WindowCompositorUpdate, WindowHandle, WindowUserRequest,
+};
 use crate::platform_impl::wayland::WindowId;
 
 /// Wrapper to carry winit's state.
@@ -12,10 +14,14 @@ pub struct WinitState {
     /// event loop and forwarded downstream afterwards.
     pub event_sink: EventSink,
 
+    /// Window updates comming from the user requests. Those are separatelly dispatched right after
+    /// `MainEventsCleared`.
+    pub window_user_requests: HashMap<WindowId, WindowUserRequest>,
+
     /// Window updates, which are coming from SCTK or the compositor, which require
     /// calling back to the winit's downstream. They are handled right in the event loop,
     /// unlike the ones coming from buffers on the `WindowHandle`'s.
-    pub window_updates: HashMap<WindowId, WindowUpdate>,
+    pub window_compositor_updates: HashMap<WindowId, WindowCompositorUpdate>,
 
     /// Window map containing all SCTK windows. Since those windows aren't allowed
     /// to be sent to other threads, they live on the event loop's thread

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

@@ -30,7 +30,7 @@ use super::{EventLoopWindowTarget, WindowId};
 
 pub mod shim;
 
-use shim::{WindowHandle, WindowRequest, WindowUpdate};
+use shim::{WindowCompositorUpdate, WindowHandle, WindowRequest, WindowUserRequest};
 
 #[cfg(feature = "sctk-adwaita")]
 pub type WinitFrame = sctk_adwaita::AdwaitaFrame;
@@ -94,11 +94,20 @@ impl Window {
 
                 // Get the window that received the event.
                 let window_id = super::make_wid(&surface);
-                let mut window_update = winit_state.window_updates.get_mut(&window_id).unwrap();
+                let mut window_compositor_update = winit_state
+                    .window_compositor_updates
+                    .get_mut(&window_id)
+                    .unwrap();
+
+                // Mark that we need a frame refresh on the DPI change.
+                winit_state
+                    .window_user_requests
+                    .get_mut(&window_id)
+                    .unwrap()
+                    .refresh_frame = true;
 
                 // Set pending scale factor.
-                window_update.scale_factor = Some(scale);
+                window_compositor_update.scale_factor = Some(scale);
-                window_update.redraw_requested = true;
 
                 surface.set_buffer_scale(scale);
             })
@@ -128,11 +137,19 @@ impl Window {
                     use sctk::window::{Event, State};
 
                     let winit_state = dispatch_data.get::<WinitState>().unwrap();
-                    let mut window_update = winit_state.window_updates.get_mut(&window_id).unwrap();
+                    let mut window_compositor_update = winit_state
+                        .window_compositor_updates
+                        .get_mut(&window_id)
+                        .unwrap();
+
+                    let mut window_user_requests = winit_state
+                        .window_user_requests
+                        .get_mut(&window_id)
+                        .unwrap();
 
                     match event {
                         Event::Refresh => {
-                            window_update.refresh_frame = true;
+                            window_user_requests.refresh_frame = true;
                         }
                         Event::Configure { new_size, states } => {
                             let is_maximized = states.contains(&State::Maximized);
@@ -140,14 +157,13 @@ impl Window {
                             let is_fullscreen = states.contains(&State::Fullscreen);
                             fullscreen_clone.store(is_fullscreen, Ordering::Relaxed);
 
-                            window_update.refresh_frame = true;
+                            window_user_requests.refresh_frame = true;
-                            window_update.redraw_requested = true;
                             if let Some((w, h)) = new_size {
-                                window_update.size = Some(LogicalSize::new(w, h));
+                                window_compositor_update.size = Some(LogicalSize::new(w, h));
                             }
                         }
                         Event::Close => {
-                            window_update.close_window = true;
+                            window_compositor_update.close_window = true;
                         }
                     }
                 },
@@ -234,9 +250,9 @@ impl Window {
         let size = Arc::new(Mutex::new(LogicalSize::new(width, height)));
 
         // We should trigger redraw and commit the surface for the newly created window.
-        let mut window_update = WindowUpdate::new();
+        let mut window_user_request = WindowUserRequest::new();
-        window_update.refresh_frame = true;
+        window_user_request.refresh_frame = true;
-        window_update.redraw_requested = true;
+        window_user_request.redraw_requested = true;
 
         let window_id = super::make_wid(&surface);
         let window_requests = Arc::new(Mutex::new(Vec::with_capacity(64)));
@@ -262,9 +278,13 @@ impl Window {
             .event_sink
             .push_window_event(crate::event::WindowEvent::Focused(false), window_id);
 
+        // Add state for the window.
         winit_state
-            .window_updates
+            .window_user_requests
-            .insert(window_id, WindowUpdate::new());
+            .insert(window_id, window_user_request);
+        winit_state
+            .window_compositor_updates
+            .insert(window_id, WindowCompositorUpdate::new());
 
         let windowing_features = event_loop_window_target.windowing_features;
 

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

@@ -98,56 +98,38 @@ pub enum WindowRequest {
     Close,
 }
 
-/// Pending update to a window from SCTK window.
+// The window update comming from the compositor.
-#[derive(Debug, Clone, Copy)]
+#[derive(Default, Debug, Clone, Copy)]
-pub struct WindowUpdate {
+pub struct WindowCompositorUpdate {
     /// New window size.
     pub size: Option<LogicalSize<u32>>,
 
     /// New scale factor.
     pub scale_factor: Option<i32>,
 
+    /// Close the window.
+    pub close_window: bool,
+}
+
+impl WindowCompositorUpdate {
+    pub fn new() -> Self {
+        Default::default()
+    }
+}
+
+/// Pending update to a window requested by the user.
+#[derive(Default, Debug, Clone, Copy)]
+pub struct WindowUserRequest {
     /// Whether `redraw` was requested.
     pub redraw_requested: bool,
 
     /// Wether the frame should be refreshed.
     pub refresh_frame: bool,
-
-    /// Close the window.
-    pub close_window: bool,
 }
 
-impl WindowUpdate {
+impl WindowUserRequest {
     pub fn new() -> Self {
-        Self {
+        Default::default()
-            size: None,
-            scale_factor: None,
-            redraw_requested: false,
-            refresh_frame: false,
-            close_window: false,
-        }
-    }
-
-    pub fn take(&mut self) -> Self {
-        let size = self.size.take();
-        let scale_factor = self.scale_factor.take();
-
-        let redraw_requested = self.redraw_requested;
-        self.redraw_requested = false;
-
-        let refresh_frame = self.refresh_frame;
-        self.refresh_frame = false;
-
-        let close_window = self.close_window;
-        self.close_window = false;
-
-        Self {
-            size,
-            scale_factor,
-            redraw_requested,
-            refresh_frame,
-            close_window,
-        }
     }
 }
 
@@ -422,7 +404,8 @@ impl WindowHandle {
 #[inline]
 pub fn handle_window_requests(winit_state: &mut WinitState) {
     let window_map = &mut winit_state.window_map;
-    let window_updates = &mut winit_state.window_updates;
+    let window_user_requests = &mut winit_state.window_user_requests;
+    let window_compositor_updates = &mut winit_state.window_compositor_updates;
     let mut windows_to_close: Vec<WindowId> = Vec::new();
 
     // Process the rest of the events.
@@ -478,15 +461,15 @@ pub fn handle_window_requests(winit_state: &mut WinitState) {
                     window_handle.window.set_decorate(decorations);
 
                     // We should refresh the frame to apply decorations change.
-                    let window_update = window_updates.get_mut(window_id).unwrap();
+                    let window_request = window_user_requests.get_mut(window_id).unwrap();
-                    window_update.refresh_frame = true;
+                    window_request.refresh_frame = true;
                 }
                 #[cfg(feature = "sctk-adwaita")]
                 WindowRequest::CsdThemeVariant(theme) => {
                     window_handle.window.set_frame_config(theme.into());
 
-                    let window_update = window_updates.get_mut(window_id).unwrap();
+                    let window_requst = window_user_requests.get_mut(window_id).unwrap();
-                    window_update.refresh_frame = true;
+                    window_requst.refresh_frame = true;
                 }
                 #[cfg(not(feature = "sctk-adwaita"))]
                 WindowRequest::CsdThemeVariant(_) => {}
@@ -494,29 +477,29 @@ pub fn handle_window_requests(winit_state: &mut WinitState) {
                     window_handle.window.set_resizable(resizeable);
 
                     // We should refresh the frame to update button state.
-                    let window_update = window_updates.get_mut(window_id).unwrap();
+                    let window_request = window_user_requests.get_mut(window_id).unwrap();
-                    window_update.refresh_frame = true;
+                    window_request.refresh_frame = true;
                 }
                 WindowRequest::Title(title) => {
                     window_handle.window.set_title(title);
 
                     // We should refresh the frame to draw new title.
-                    let window_update = window_updates.get_mut(window_id).unwrap();
+                    let window_request = window_user_requests.get_mut(window_id).unwrap();
-                    window_update.refresh_frame = true;
+                    window_request.refresh_frame = true;
                 }
                 WindowRequest::MinSize(size) => {
                     let size = size.map(|size| (size.width, size.height));
                     window_handle.window.set_min_size(size);
 
-                    let window_update = window_updates.get_mut(window_id).unwrap();
+                    let window_request = window_user_requests.get_mut(window_id).unwrap();
-                    window_update.redraw_requested = true;
+                    window_request.refresh_frame = true;
                 }
                 WindowRequest::MaxSize(size) => {
                     let size = size.map(|size| (size.width, size.height));
                     window_handle.window.set_max_size(size);
 
-                    let window_update = window_updates.get_mut(window_id).unwrap();
+                    let window_request = window_user_requests.get_mut(window_id).unwrap();
-                    window_update.redraw_requested = true;
+                    window_request.refresh_frame = true;
                 }
                 WindowRequest::FrameSize(size) => {
                     if !window_handle.is_resizable.get() {
@@ -530,21 +513,21 @@ pub fn handle_window_requests(winit_state: &mut WinitState) {
                     window_handle.window.resize(size.width, size.height);
 
                     // We should refresh the frame after resize.
-                    let window_update = window_updates.get_mut(window_id).unwrap();
+                    let window_request = window_user_requests.get_mut(window_id).unwrap();
-                    window_update.refresh_frame = true;
+                    window_request.refresh_frame = true;
                 }
                 WindowRequest::PassthroughMouseInput(passthrough) => {
                     window_handle.passthrough_mouse_input(passthrough);
 
-                    let window_update = window_updates.get_mut(window_id).unwrap();
+                    let window_request = window_user_requests.get_mut(window_id).unwrap();
-                    window_update.refresh_frame = true;
+                    window_request.refresh_frame = true;
                 }
                 WindowRequest::Attention(request_type) => {
                     window_handle.set_user_attention(request_type);
                 }
                 WindowRequest::Redraw => {
-                    let window_update = window_updates.get_mut(window_id).unwrap();
+                    let window_request = window_user_requests.get_mut(window_id).unwrap();
-                    window_update.redraw_requested = true;
+                    window_request.redraw_requested = true;
                 }
                 WindowRequest::Close => {
                     // The window was requested to be closed.
@@ -561,7 +544,8 @@ pub fn handle_window_requests(winit_state: &mut WinitState) {
     // Close the windows.
     for window in windows_to_close {
         let _ = window_map.remove(&window);
-        let _ = window_updates.remove(&window);
+        let _ = window_user_requests.remove(&window);
+        let _ = window_compositor_updates.remove(&window);
     }
 }