x11: Halt periodic refresh for windows that aren't visible (#32775)

Michael Sloan created

This adds handling of UnmapNotify / MapNotify / VisibilityNotify to
track whether windows are visible. When hidden, the refresh loop is
halted until visible again. Often these refreshes were just checking if
the window is dirty, but I believe it sometimes did a full re-render for
things that change without user interaction (cursor blink, animations).

This also changes handling of Expose events to set a flag indicating the
next refresh should have `require_presentation: true`.

Release Notes:

- x11: No longer refreshes windows that aren't visible.

Change summary

crates/gpui/src/platform/linux/x11/client.rs | 224 ++++++++++++++++-----
crates/gpui/src/platform/linux/x11/window.rs |  15 
2 files changed, 172 insertions(+), 67 deletions(-)

Detailed changes

crates/gpui/src/platform/linux/x11/client.rs 🔗

@@ -28,7 +28,7 @@ use x11rb::{
     protocol::xkb::ConnectionExt as _,
     protocol::xproto::{
         AtomEnum, ChangeWindowAttributesAux, ClientMessageData, ClientMessageEvent,
-        ConnectionExt as _, EventMask, KeyPressEvent,
+        ConnectionExt as _, EventMask, KeyPressEvent, Visibility,
     },
     protocol::{Event, randr, render, xinput, xkb, xproto},
     resource_manager::Database,
@@ -78,7 +78,10 @@ pub(crate) const XINPUT_ALL_DEVICE_GROUPS: xinput::DeviceId = 1;
 
 pub(crate) struct WindowRef {
     window: X11WindowStatePtr,
-    refresh_event_token: RegistrationToken,
+    refresh_state: Option<RefreshState>,
+    expose_event_received: bool,
+    last_visibility: Visibility,
+    is_mapped: bool,
 }
 
 impl WindowRef {
@@ -95,6 +98,16 @@ impl Deref for WindowRef {
     }
 }
 
+enum RefreshState {
+    Hidden {
+        refresh_rate: Duration,
+    },
+    PeriodicRefresh {
+        refresh_rate: Duration,
+        event_loop_token: RegistrationToken,
+    },
+}
+
 #[derive(Debug)]
 #[non_exhaustive]
 pub enum EventHandlerError {
@@ -220,7 +233,14 @@ impl X11ClientStatePtr {
         let mut state = client.0.borrow_mut();
 
         if let Some(window_ref) = state.windows.remove(&x_window) {
-            state.loop_handle.remove(window_ref.refresh_event_token);
+            match window_ref.refresh_state {
+                Some(RefreshState::PeriodicRefresh {
+                    event_loop_token, ..
+                }) => {
+                    state.loop_handle.remove(event_loop_token);
+                }
+                _ => {}
+            }
         }
         if state.mouse_focused_window == Some(x_window) {
             state.mouse_focused_window = None;
@@ -550,10 +570,9 @@ impl X11Client {
             }
 
             for window in windows_to_refresh.into_iter() {
-                if let Some(window) = self.get_window(window) {
-                    window.refresh(RequestFrameOptions {
-                        require_presentation: true,
-                    });
+                let mut state = self.0.borrow_mut();
+                if let Some(window) = state.windows.get_mut(&window) {
+                    window.expose_event_received = true;
                 }
             }
 
@@ -661,6 +680,27 @@ impl X11Client {
 
     fn handle_event(&self, event: Event) -> Option<()> {
         match event {
+            Event::UnmapNotify(event) => {
+                let mut state = self.0.borrow_mut();
+                if let Some(window_ref) = state.windows.get_mut(&event.window) {
+                    window_ref.is_mapped = false;
+                }
+                state.update_refresh_loop(event.window);
+            }
+            Event::MapNotify(event) => {
+                let mut state = self.0.borrow_mut();
+                if let Some(window_ref) = state.windows.get_mut(&event.window) {
+                    window_ref.is_mapped = true;
+                }
+                state.update_refresh_loop(event.window);
+            }
+            Event::VisibilityNotify(event) => {
+                let mut state = self.0.borrow_mut();
+                if let Some(window_ref) = state.windows.get_mut(&event.window) {
+                    window_ref.last_visibility = event.state;
+                }
+                state.update_refresh_loop(event.window);
+            }
             Event::ClientMessage(event) => {
                 let window = self.get_window(event.window)?;
                 let [atom, arg1, arg2, arg3, arg4] = event.data.as_data32();
@@ -1383,61 +1423,12 @@ impl LinuxClient for X11Client {
             )
             .unwrap();
 
-        let screen_resources = state
-            .xcb_connection
-            .randr_get_screen_resources(x_window)
-            .unwrap()
-            .reply()
-            .expect("Could not find available screens");
-
-        let mode = screen_resources
-            .crtcs
-            .iter()
-            .find_map(|crtc| {
-                let crtc_info = state
-                    .xcb_connection
-                    .randr_get_crtc_info(*crtc, x11rb::CURRENT_TIME)
-                    .ok()?
-                    .reply()
-                    .ok()?;
-
-                screen_resources
-                    .modes
-                    .iter()
-                    .find(|m| m.id == crtc_info.mode)
-            })
-            .expect("Unable to find screen refresh rate");
-
-        let refresh_event_token = state
-            .loop_handle
-            .insert_source(calloop::timer::Timer::immediate(), {
-                let refresh_duration = mode_refresh_rate(mode);
-                move |mut instant, (), client| {
-                    let xcb_connection = {
-                        let state = client.0.borrow_mut();
-                        let xcb_connection = state.xcb_connection.clone();
-                        if let Some(window) = state.windows.get(&x_window) {
-                            let window = window.window.clone();
-                            drop(state);
-                            window.refresh(Default::default());
-                        }
-                        xcb_connection
-                    };
-                    client.process_x11_events(&xcb_connection).log_err();
-
-                    // Take into account that some frames have been skipped
-                    let now = Instant::now();
-                    while instant < now {
-                        instant += refresh_duration;
-                    }
-                    calloop::timer::TimeoutAction::ToInstant(instant)
-                }
-            })
-            .expect("Failed to initialize refresh timer");
-
         let window_ref = WindowRef {
             window: window.0.clone(),
-            refresh_event_token,
+            refresh_state: None,
+            expose_event_received: false,
+            last_visibility: Visibility::UNOBSCURED,
+            is_mapped: false,
         };
 
         state.windows.insert(x_window, window_ref);
@@ -1618,6 +1609,119 @@ impl LinuxClient for X11Client {
     }
 }
 
+impl X11ClientState {
+    fn update_refresh_loop(&mut self, x_window: xproto::Window) {
+        let Some(window_ref) = self.windows.get_mut(&x_window) else {
+            return;
+        };
+        let is_visible = window_ref.is_mapped
+            && !matches!(window_ref.last_visibility, Visibility::FULLY_OBSCURED);
+        match (is_visible, window_ref.refresh_state.take()) {
+            (false, refresh_state @ Some(RefreshState::Hidden { .. }))
+            | (false, refresh_state @ None)
+            | (true, refresh_state @ Some(RefreshState::PeriodicRefresh { .. })) => {
+                window_ref.refresh_state = refresh_state;
+            }
+            (
+                false,
+                Some(RefreshState::PeriodicRefresh {
+                    refresh_rate,
+                    event_loop_token,
+                }),
+            ) => {
+                self.loop_handle.remove(event_loop_token);
+                window_ref.refresh_state = Some(RefreshState::Hidden { refresh_rate });
+            }
+            (true, Some(RefreshState::Hidden { refresh_rate })) => {
+                let event_loop_token = self.start_refresh_loop(x_window, refresh_rate);
+                let Some(window_ref) = self.windows.get_mut(&x_window) else {
+                    return;
+                };
+                window_ref.refresh_state = Some(RefreshState::PeriodicRefresh {
+                    refresh_rate,
+                    event_loop_token,
+                });
+            }
+            (true, None) => {
+                let screen_resources = self
+                    .xcb_connection
+                    .randr_get_screen_resources_current(x_window)
+                    .unwrap()
+                    .reply()
+                    .expect("Could not find available screens");
+
+                // Ideally this would be re-queried when the window changes screens, but there
+                // doesn't seem to be an efficient / straightforward way to do this. Should also be
+                // updated when screen configurations change.
+                let refresh_rate = mode_refresh_rate(
+                    screen_resources
+                        .crtcs
+                        .iter()
+                        .find_map(|crtc| {
+                            let crtc_info = self
+                                .xcb_connection
+                                .randr_get_crtc_info(*crtc, x11rb::CURRENT_TIME)
+                                .ok()?
+                                .reply()
+                                .ok()?;
+
+                            screen_resources
+                                .modes
+                                .iter()
+                                .find(|m| m.id == crtc_info.mode)
+                        })
+                        .expect("Unable to find screen refresh rate"),
+                );
+
+                let event_loop_token = self.start_refresh_loop(x_window, refresh_rate);
+                let Some(window_ref) = self.windows.get_mut(&x_window) else {
+                    return;
+                };
+                window_ref.refresh_state = Some(RefreshState::PeriodicRefresh {
+                    refresh_rate,
+                    event_loop_token,
+                });
+            }
+        }
+    }
+
+    #[must_use]
+    fn start_refresh_loop(
+        &self,
+        x_window: xproto::Window,
+        refresh_rate: Duration,
+    ) -> RegistrationToken {
+        self.loop_handle
+            .insert_source(calloop::timer::Timer::immediate(), {
+                move |mut instant, (), client| {
+                    let xcb_connection = {
+                        let mut state = client.0.borrow_mut();
+                        let xcb_connection = state.xcb_connection.clone();
+                        if let Some(window) = state.windows.get_mut(&x_window) {
+                            let expose_event_received = window.expose_event_received;
+                            window.expose_event_received = false;
+                            let window = window.window.clone();
+                            drop(state);
+                            window.refresh(RequestFrameOptions {
+                                require_presentation: expose_event_received,
+                            });
+                        }
+                        xcb_connection
+                    };
+                    client.process_x11_events(&xcb_connection).log_err();
+
+                    // Take into account that some frames have been skipped
+                    let now = Instant::now();
+                    while instant < now {
+                        instant += refresh_rate;
+                    }
+                    calloop::timer::TimeoutAction::ToInstant(instant)
+                }
+            })
+            .expect("Failed to initialize refresh timer")
+    }
+}
+
 // Adapted from:
 // https://docs.rs/winit/0.29.11/src/winit/platform_impl/linux/x11/monitor.rs.html#103-111
 pub fn mode_refresh_rate(mode: &randr::ModeInfo) -> Duration {

crates/gpui/src/platform/linux/x11/window.rs 🔗

@@ -20,7 +20,7 @@ use x11rb::{
     protocol::{
         sync,
         xinput::{self, ConnectionExt as _},
-        xproto::{self, ClientMessageEvent, ConnectionExt, EventMask, TranslateCoordinatesReply},
+        xproto::{self, ClientMessageEvent, ConnectionExt, TranslateCoordinatesReply},
     },
     wrapper::ConnectionExt as _,
     xcb_ffi::XCBConnection,
@@ -407,7 +407,8 @@ impl X11WindowState {
                     | xproto::EventMask::FOCUS_CHANGE
                     | xproto::EventMask::KEY_PRESS
                     | xproto::EventMask::KEY_RELEASE
-                    | EventMask::PROPERTY_CHANGE,
+                    | xproto::EventMask::PROPERTY_CHANGE
+                    | xproto::EventMask::VISIBILITY_CHANGE,
             );
 
         let mut bounds = params.bounds.to_device_pixels(scale_factor);
@@ -785,7 +786,7 @@ impl X11Window {
             self.0.xcb.send_event(
                 false,
                 state.x_root_window,
-                EventMask::SUBSTRUCTURE_REDIRECT | EventMask::SUBSTRUCTURE_NOTIFY,
+                xproto::EventMask::SUBSTRUCTURE_REDIRECT | xproto::EventMask::SUBSTRUCTURE_NOTIFY,
                 message,
             ),
         )
@@ -836,7 +837,7 @@ impl X11Window {
             self.0.xcb.send_event(
                 false,
                 state.x_root_window,
-                EventMask::SUBSTRUCTURE_REDIRECT | EventMask::SUBSTRUCTURE_NOTIFY,
+                xproto::EventMask::SUBSTRUCTURE_REDIRECT | xproto::EventMask::SUBSTRUCTURE_NOTIFY,
                 message,
             ),
         )?;
@@ -921,7 +922,7 @@ impl X11WindowStatePtr {
         state.fullscreen = false;
         state.maximized_vertical = false;
         state.maximized_horizontal = false;
-        state.hidden = true;
+        state.hidden = false;
 
         for atom in atoms {
             if atom == state.atoms._NET_WM_STATE_FOCUSED {
@@ -1343,7 +1344,7 @@ impl PlatformWindow for X11Window {
             self.0.xcb.send_event(
                 false,
                 state.x_root_window,
-                EventMask::SUBSTRUCTURE_REDIRECT | EventMask::SUBSTRUCTURE_NOTIFY,
+                xproto::EventMask::SUBSTRUCTURE_REDIRECT | xproto::EventMask::SUBSTRUCTURE_NOTIFY,
                 message,
             ),
         )
@@ -1452,7 +1453,7 @@ impl PlatformWindow for X11Window {
             self.0.xcb.send_event(
                 false,
                 state.x_root_window,
-                EventMask::SUBSTRUCTURE_REDIRECT | EventMask::SUBSTRUCTURE_NOTIFY,
+                xproto::EventMask::SUBSTRUCTURE_REDIRECT | xproto::EventMask::SUBSTRUCTURE_NOTIFY,
                 message,
             ),
         )