X11: Refactor Drawing and event loop handling

src/platform/x11/error.rs

@@ -54,6 +54,7 @@ pub enum Error {
     MainThreadRecvResult,
     Calloop(calloop::Error),
     RequestFromMainThreadFailed(RequestFailed),
+    SendMainThread,
     #[cfg(feature = "opengl")]
     XLib(crate::wrappers::xlib::XLibError),
     #[cfg(feature = "opengl")]
@@ -81,6 +82,7 @@ impl Display for Error {
             }
             Error::Calloop(e) => e.fmt(f),
             Error::RequestFromMainThreadFailed(e) => e.fmt(f),
+            Error::SendMainThread => FatalError::SendMainThread.fmt(f),
             #[cfg(feature = "opengl")]
             Error::XLib(e) => e.fmt(f),
             #[cfg(feature = "opengl")]
@@ -157,6 +159,15 @@ impl From<RequestFailed> for Error {
     }
 }
 
+impl From<FatalError> for Error {
+    fn from(value: FatalError) -> Self {
+        match value {
+            FatalError::Connection(e) => Self::Connection(e),
+            FatalError::SendMainThread => Self::SendMainThread,
+        }
+    }
+}
+
 #[cfg(feature = "opengl")]
 impl From<crate::wrappers::xlib::XLibError> for Error {
     fn from(value: crate::wrappers::xlib::XLibError) -> Self {

src/platform/x11/event_loop.rs

@@ -13,7 +13,7 @@ use crate::wrappers::xkbcommon::XkbcommonState;
 use crate::{Event, MouseButton, MouseEvent, ScrollDelta, WindowEvent, WindowHandler, WindowSize};
 use calloop::generic::Generic;
 use calloop::timer::{TimeoutAction, Timer};
-use calloop::{Interest, LoopSignal, Mode, PostAction};
+use calloop::{Interest, LoopHandle, LoopSignal, Mode, PostAction};
 use dpi::{PhysicalPosition, PhysicalSize};
 use std::rc::Rc;
 use std::sync::mpsc;
@@ -52,6 +52,7 @@ pub(crate) struct EventLoop {
     window: Rc<WindowInner>,
 
     new_physical_size: Option<PhysicalSize<u16>>,
+    exposed: bool,
 
     loop_signal: LoopSignal,
 
@@ -64,8 +65,6 @@ pub(crate) struct EventLoop {
     main_thread: Option<MainThreadCaller>,
 }
 
-const FRAME_INTERVAL: Duration = Duration::from_millis(15);
-
 impl EventLoop {
     pub fn new(
         window: Rc<WindowInner>, handler: Box<dyn WindowHandler>,
@@ -75,9 +74,7 @@ impl EventLoop {
     ) -> Result<Self, Error> {
         let loop_handle = inner.handle();
 
-        loop_handle
-            .insert_source(Timer::from_duration(FRAME_INTERVAL), |i, _, e| e.handle_frame(i))
-            .map_err(|e| e.error)?;
+        Self::setup_fallback_frame_timer(&loop_handle)?;
 
         loop_handle
             .insert_source(
@@ -94,6 +91,7 @@ impl EventLoop {
             loop_signal: inner.get_signal(),
             handler,
             new_physical_size: None,
+            exposed: false,
             drag_n_drop: DragNDropState::NoCurrentSession,
             xkb_state: XkbcommonState::new(&window.connection),
             run_error: None,
@@ -105,17 +103,62 @@ impl EventLoop {
     }
 
     #[inline]
-    fn drain_xcb_events(&mut self) -> Result<(), FatalError> {
-        // the X server has a tendency to send spurious/extraneous configure notify events when a
-        // window is resized, and we need to batch those together and just send one resize event
-        // when they've all been coalesced.
-        self.new_physical_size = None;
-
+    fn drain_xcb_events(&mut self) -> Result<bool, ConnectionError> {
+        let mut event_received = false;
         while let Some(event) = self.window.connection.conn.poll_for_event()? {
+            event_received = true;
             self.handle_xcb_event(event)?;
         }
 
-        self.handle_coalesced_resize_events()
+        Ok(event_received)
+    }
+
+    fn setup_fallback_frame_timer(
+        loop_handle: &LoopHandle<'_, Self>,
+    ) -> Result<(), calloop::Error> {
+        const FRAME_INTERVAL: Duration = Duration::from_millis(15);
+
+        fn handle_frame(evloop: &mut EventLoop, previous_deadline: Instant) -> TimeoutAction {
+            evloop.exposed = true;
+
+            // We'll try to keep a consistent frame pace. If the last frame couldn't be processed in
+            // the expected frame time, this will throttle down to prevent multiple frames from
+            // being queued up.
+
+            let now = Instant::now();
+            let next_deadline = if previous_deadline + FRAME_INTERVAL >= now {
+                now + FRAME_INTERVAL
+            } else {
+                previous_deadline + FRAME_INTERVAL
+            };
+
+            TimeoutAction::ToInstant(next_deadline)
+        }
+
+        loop_handle
+            .insert_source(Timer::from_duration(FRAME_INTERVAL), |i, _, e| handle_frame(e, i))
+            .map_err(|e| e.error)?;
+
+        Ok(())
+    }
+
+    fn handle_redraw(&mut self) {
+        if !self.exposed {
+            return;
+        }
+        self.exposed = false;
+
+        if !self.window.is_mapped.get() {
+            return;
+        }
+
+        if let Err(e) = self.handler.on_frame() {
+            self.trigger_fatal_error(e.into());
+            return;
+        }
+
+        // Any socket error will be handled in the next poll
+        let _ = self.window.connection.conn.flush();
     }
 
     fn handle_coalesced_resize_events(&mut self) -> Result<(), FatalError> {
@@ -143,6 +186,9 @@ impl EventLoop {
                     previous: WindowSize::from_physical(previous.cast(), scale_factor),
                 })?;
             }
+
+            // Immediately schedule a redraw, do not wait for an "expose" event
+            self.exposed = true;
         }
 
         Ok(())
@@ -178,6 +224,13 @@ impl EventLoop {
         self.loop_signal.wakeup();
     }
 
+    fn trigger_fatal_error(&mut self, error: Error) {
+        if self.run_error.is_none() {
+            self.run_error = Some(error);
+        }
+        self.stop_now();
+    }
+
     fn handle_request(&mut self, req: WindowThreadRequest) -> Result<(), Error> {
         match req {
             WindowThreadRequest::Resize(new_size) => {
@@ -223,30 +276,28 @@ impl EventLoop {
         Ok(PostAction::Continue)
     }
 
-    fn handle_frame(&mut self, previous_deadline: Instant) -> TimeoutAction {
-        if let Err(e) = self.handler.on_frame() {
-            self.run_error = Some(e.into());
-            self.stop_now();
-            return TimeoutAction::Drop;
+    fn handle_idle(&mut self) {
+        if let Err(e) = self.try_handle_idle() {
+            self.trigger_fatal_error(e.into());
         }
+    }
 
-        // We'll try to keep a consistent frame pace. If the last frame couldn't be processed in
-        // the expected frame time, this will throttle down to prevent multiple frames from
-        // being queued up.
+    fn try_handle_idle(&mut self) -> Result<(), FatalError> {
+        // Check for any events in the internal buffers before going to sleep:
+        self.drain_xcb_events()?;
 
-        let now = Instant::now();
-        let next_deadline = if previous_deadline + FRAME_INTERVAL >= now {
-            now + FRAME_INTERVAL
-        } else {
-            previous_deadline + FRAME_INTERVAL
-        };
+        loop {
+            self.handle_coalesced_resize_events()?;
+            self.handle_redraw();
 
-        TimeoutAction::ToInstant(next_deadline)
-    }
+            if !self.drain_xcb_events()? {
+                break;
+            }
+        }
 
-    fn handle_idle(&mut self) {
-        // Check for any events in the internal buffers before going to sleep:
-        let _ = self.drain_xcb_events();
+        self.window.connection.conn.flush()?;
+
+        Ok(())
     }
 
     pub fn run(mut self, mut inner: calloop::EventLoop<Self>) -> Result<(), Error> {
@@ -329,14 +380,17 @@ impl EventLoop {
                 }
             }
 
-            XEvent::ConfigureNotify(event) => {
-                let new_physical_size = PhysicalSize::new(event.width, event.height);
+            XEvent::Error(e) => {
+                warn!("Received leftover X11 error: {:?}", e);
+            }
 
-                if self.new_physical_size.is_some() || new_physical_size != self.window.get_size() {
-                    self.new_physical_size = Some(new_physical_size);
-                }
+            XEvent::ConfigureNotify(event) => {
+                // These are coalesced and then handled asynchronously at the end of the event loop
+                self.new_physical_size = Some(PhysicalSize::new(event.width, event.height));
             }
 
+            XEvent::Expose(_) => self.exposed = true,
+
             ////
             // mouse
             ////
@@ -416,6 +470,12 @@ impl EventLoop {
                 self.handle_event(Event::Window(WindowEvent::Unfocused));
             }
 
+            XEvent::MapNotify(_) => {
+                self.window.is_mapped.set(true);
+                self.exposed = true;
+            }
+            XEvent::UnmapNotify(_) => self.window.is_mapped.set(false),
+
             _ => {}
         }
 

src/platform/x11/window_shared.rs

@@ -56,6 +56,7 @@ pub(crate) struct WindowInner {
     pub(crate) visual_id: Visualid,
 
     pub(crate) is_focused: Cell<bool>,
+    pub(crate) is_mapped: Cell<bool>,
     pub(crate) loop_signal: LoopSignal,
 
     pub(crate) main_thread_shared: Arc<WindowThreadShared>,
@@ -134,6 +135,7 @@ impl WindowInner {
             loop_signal: ev_loop.get_signal(),
 
             is_focused: false.into(),
+            is_mapped: false.into(),
             main_thread_shared: shared,
 
             #[cfg(feature = "opengl")]
@@ -165,7 +167,11 @@ impl WindowInner {
 
     pub fn store_size(&self, size: PhysicalSize<u16>) -> PhysicalSize<u16> {
         let previous = self.window_size.replace(size);
-        self.main_thread_shared.set_size(size);
+
+        if previous != size {
+            self.main_thread_shared.set_size(size);
+        }
+
         previous
     }
 

src/platform/x11/xcb_window.rs

@@ -62,12 +62,12 @@ impl XcbWindow {
         Ok(Self { window_id, connection })
     }
 
-    pub fn map_window(&self) -> Result<VoidCookie<'_, XCBConnection>, ReplyOrIdError> {
-        Ok(self.connection.conn.map_window(self.window_id.get())?)
+    pub fn map_window(&self) -> Result<VoidCookie<'_, XCBConnection>, ConnectionError> {
+        self.connection.conn.map_window(self.window_id.get())
     }
 
-    pub fn unmap_window(&self) -> Result<VoidCookie<'_, XCBConnection>, ReplyOrIdError> {
-        Ok(self.connection.conn.unmap_window(self.window_id.get())?)
+    pub fn unmap_window(&self) -> Result<VoidCookie<'_, XCBConnection>, ConnectionError> {
+        self.connection.conn.unmap_window(self.window_id.get())
     }
 
     pub fn resize(
@@ -90,41 +90,40 @@ impl XcbWindow {
         )
     }
 
-    pub fn set_title(&self, title: &str) -> Result<VoidCookie<'_, XCBConnection>, ReplyOrIdError> {
-        Ok(self.connection.conn.change_property8(
+    pub fn set_title(&self, title: &str) -> Result<VoidCookie<'_, XCBConnection>, ConnectionError> {
+        self.connection.conn.change_property8(
             PropMode::REPLACE,
             self.window_id.get(),
             AtomEnum::WM_NAME,
             AtomEnum::STRING,
             title.as_bytes(),
-        )?)
+        )
     }
 
-    pub fn enable_wm_protocols(&self) -> Result<VoidCookie<'_, XCBConnection>, ReplyOrIdError> {
-        Ok(self.connection.conn.change_property32(
+    pub fn enable_wm_protocols(&self) -> Result<VoidCookie<'_, XCBConnection>, ConnectionError> {
+        self.connection.conn.change_property32(
             PropMode::REPLACE,
             self.window_id.get(),
             self.connection.atoms.WM_PROTOCOLS,
             AtomEnum::ATOM,
             &[self.connection.atoms.WM_DELETE_WINDOW],
-        )?)
+        )
     }
 
-    pub fn enable_dnd_protocols(&self) -> Result<VoidCookie<'_, XCBConnection>, ReplyOrIdError> {
-        Ok(self.connection.conn.change_property32(
+    pub fn enable_dnd_protocols(&self) -> Result<VoidCookie<'_, XCBConnection>, ConnectionError> {
+        self.connection.conn.change_property32(
             PropMode::REPLACE,
             self.window_id.get(),
             self.connection.atoms.XdndAware,
             AtomEnum::ATOM,
             &[5u32], // Latest version; hasn't changed since 2002
-        )?)
+        )
     }
 
     pub fn set_size_hints(
         &self, size_hints: WmSizeHints,
-    ) -> Result<VoidCookie<'_, XCBConnection>, ReplyOrIdError> {
-        Ok(size_hints
-            .set_normal_hints(&self.connection.conn as &XCBConnection, self.window_id.get())?)
+    ) -> Result<VoidCookie<'_, XCBConnection>, ConnectionError> {
+        size_hints.set_normal_hints(&self.connection.conn as &XCBConnection, self.window_id.get())
     }
 
     #[inline]