From 7d05bcb0d5fb4c27671aec20ea742295c2612798 Mon Sep 17 00:00:00 2001 From: "Brian J. Tarricone" Date: Tue, 14 Jun 2022 19:37:32 -0700 Subject: [PATCH] Don't recreate monitors when we get a spurious randr notify Sometimes we get notifications from randr of changes where the new setup is exactly like the old. The main annoying time this happens is when the system resumes from suspend. This is probably why there's often a flash of the desktop right after resuming: we're destroying the old blanker windows and creating new ones, even though the setup is exactly the same. --- locker/src/monitor.rs | 177 ++++++++++++++++++++++++++++---------- locker/src/screensaver.rs | 55 +++++------- 2 files changed, 156 insertions(+), 76 deletions(-) diff --git a/locker/src/monitor.rs b/locker/src/monitor.rs index f98ac81..9acb72c 100644 --- a/locker/src/monitor.rs +++ b/locker/src/monitor.rs @@ -16,21 +16,27 @@ struct BacklightControl { pub struct Monitor<'a> { conn: &'a xcb::Connection, - pub root: x::Window, - pub black_gc: x::Gcontext, - output: randr::Output, - pub blanker_window: x::Window, - pub unlock_window: x::Window, + properties: MonitorProperties, + black_gc: x::Gcontext, + blanker_window: x::Window, + unlock_window: x::Window, blank_cursor: x::Cursor, - pub x: i16, - pub y: i16, - pub width: u16, - pub height: u16, backlight_control: Option, } -impl<'a> Monitor<'a> { - pub fn set_up_all(conn: &xcb::Connection) -> xcb::Result> { +#[derive(Debug, PartialEq)] +struct MonitorProperties { + root: x::Window, + black_pixel: u32, + output: randr::Output, + x: i16, + y: i16, + width: u16, + height: u16, +} + +impl MonitorProperties { + fn discover_all(conn: &xcb::Connection) -> xcb::Result> { let mut monitors = Vec::new(); for screen in conn.get_setup().roots() { let cookie = conn.send_request(&randr::GetScreenResources { @@ -50,7 +56,7 @@ impl<'a> Monitor<'a> { config_timestamp, }); let crtc_info = conn.wait_for_reply(cookie)?; - monitors.push(Monitor::new(conn, &screen, *output, &crtc_info)?); + monitors.push(MonitorProperties::new(&screen, *output, &crtc_info)); } } } @@ -58,50 +64,133 @@ impl<'a> Monitor<'a> { Ok(monitors) } - pub fn new(conn: &'a xcb::Connection, screen: &x::Screen, output: randr::Output, crtc_info: &randr::GetCrtcInfoReply) -> xcb::Result> { - let (blanker_window, unlock_window) = create_windows(conn, &screen, &crtc_info)?; - let blank_cursor = create_blank_cursor(conn, &screen)?; + fn new(screen: &x::Screen, output: randr::Output, crtc_info: &randr::GetCrtcInfoReply) -> MonitorProperties { + MonitorProperties { + root: screen.root(), + black_pixel: screen.black_pixel(), + output, + x: crtc_info.x(), + y: crtc_info.y(), + width: crtc_info.width(), + height: crtc_info.height(), + } + } +} + +impl<'a> Monitor<'a> { + pub fn discover_all(conn: &'a xcb::Connection) -> xcb::Result>> { + let monitors = MonitorProperties + ::discover_all(conn)? + .into_iter() + .map(|properties| Monitor::new(conn, properties)) + .collect::>, _>>()?; + Ok(monitors) + } + + pub fn maybe_rediscover(conn: &'a xcb::Connection, old_monitors: &Vec>) -> xcb::Result>>> { + let current_properties = MonitorProperties::discover_all(conn)?; + let mut diff = false; + for (current_properties, old_monitor) in current_properties.iter().zip(old_monitors) { + if *current_properties != old_monitor.properties { + diff = true; + break; + } + } + + if diff { + current_properties + .into_iter() + .map(|properties| Monitor::new(conn, properties)) + .collect::>, _>>() + .map(Some) + } else { + Ok(None) + } + } + + fn new(conn: &'a xcb::Connection, properties: MonitorProperties) -> xcb::Result> { + let (blanker_window, unlock_window) = create_windows(conn, &properties)?; + let blank_cursor = create_blank_cursor(conn, properties.root)?; let black_gc: x::Gcontext = conn.generate_id(); conn.send_and_check_request(&x::CreateGc { cid: black_gc, - drawable: x::Drawable::Window(screen.root()), + drawable: x::Drawable::Window(properties.root), value_list: &[ - x::Gc::Foreground(screen.black_pixel()), - x::Gc::Background(screen.black_pixel()), + x::Gc::Foreground(properties.black_pixel), + x::Gc::Background(properties.black_pixel), ], })?; - let backlight_control = find_backlight_control(conn, output); + let backlight_control = find_backlight_control(conn, properties.output); if let Err(err) = &backlight_control { warn!("Failed to find backlight control: {}", err); } Ok(Monitor { conn, - root: screen.root(), + properties, black_gc, - output, blanker_window, unlock_window, blank_cursor, - x: crtc_info.x(), - y: crtc_info.y(), - width: crtc_info.width(), - height: crtc_info.height(), backlight_control: backlight_control.ok().flatten(), }) } pub fn geometry(&self) -> x::Rectangle { x::Rectangle { - x: self.x, - y: self.y, - width: self.width, - height: self.height, + x: self.properties.x, + y: self.properties.y, + width: self.properties.width, + height: self.properties.height, } } + pub fn root(&self) -> x::Window { + self.properties.root + } + + pub fn x(&self) -> i16 { + self.properties.x + } + + pub fn y(&self) -> i16 { + self.properties.y + } + + pub fn width(&self) -> u16 { + self.properties.width + } + + pub fn height(&self) -> u16 { + self.properties.height + } + + pub fn blanker_window(&self) -> x::Window { + self.blanker_window + } + + pub fn unlock_window(&self) -> x::Window { + self.unlock_window + } + + pub fn paint(&self, ev: &x::ExposeEvent) -> xcb::Result<()> { + self.conn.send_and_check_request(&x::PolyFillRectangle { + drawable: x::Drawable::Window(ev.window()), + gc: self.black_gc, + rectangles: &[ + x::Rectangle { + x: ev.x() as i16, + y: ev.y() as i16, + width: ev.width(), + height: ev.height(), + }, + ], + })?; + Ok(()) + } + pub fn blank(&self) -> anyhow::Result<()> { let mut cookies = Vec::new(); cookies.push(self.conn.send_request_checked(&x::ConfigureWindow { @@ -246,7 +335,7 @@ impl<'a> Monitor<'a> { fn get_current_brightness(&self, backlight_control: &BacklightControl) -> anyhow::Result> { let cookie = self.conn.send_request(&randr::GetOutputProperty { - output: self.output, + output: self.properties.output, property: backlight_control.property, r#type: x::ATOM_INTEGER, long_offset: 0, @@ -265,7 +354,7 @@ impl<'a> Monitor<'a> { fn set_brightness(&self, backlight_control: &BacklightControl, level: i32) -> anyhow::Result<()> { self.conn.send_and_check_request(&randr::ChangeOutputProperty { - output: self.output, + output: self.properties.output, property: backlight_control.property, r#type: x::ATOM_INTEGER, mode: x::PropMode::Replace, @@ -284,27 +373,27 @@ impl<'a> Drop for Monitor<'a> { } } -fn create_windows(conn: &xcb::Connection, screen: &x::Screen, crtc_info: &randr::GetCrtcInfoReply) -> xcb::Result<(x::Window, x::Window)> { +fn create_windows(conn: &xcb::Connection, properties: &MonitorProperties) -> xcb::Result<(x::Window, x::Window)> { let blanker_window: x::Window = conn.generate_id(); let unlock_window: x::Window = conn.generate_id(); let mut cookies = Vec::new(); - debug!("creating blanker window 0x{:x}, {}x{}+{}+{}; unlock window 0x{:x}", blanker_window.resource_id(), crtc_info.width(), crtc_info.height(), crtc_info.x(), crtc_info.y(), unlock_window.resource_id()); + debug!("creating blanker window 0x{:x}, {}x{}+{}+{}; unlock window 0x{:x}", blanker_window.resource_id(), properties.width, properties.height, properties.x, properties.y, unlock_window.resource_id()); cookies.push(conn.send_request_checked(&x::CreateWindow { depth: x::COPY_FROM_PARENT as u8, wid: blanker_window, - parent: screen.root(), - x: crtc_info.x(), - y: crtc_info.y(), - width: crtc_info.width(), - height: crtc_info.height(), + parent: properties.root, + x: properties.x, + y: properties.y, + width: properties.width, + height: properties.height, border_width: 0, class: x::WindowClass::InputOutput, visual: x::COPY_FROM_PARENT, value_list: &[ - x::Cw::BackPixel(screen.black_pixel()), - x::Cw::BorderPixel(screen.black_pixel()), + x::Cw::BackPixel(properties.black_pixel), + x::Cw::BorderPixel(properties.black_pixel), x::Cw::OverrideRedirect(true), x::Cw::SaveUnder(true), x::Cw::EventMask(x::EventMask::KEY_PRESS | x::EventMask::KEY_RELEASE | x::EventMask::BUTTON_PRESS | x::EventMask::BUTTON_RELEASE | x::EventMask::POINTER_MOTION | x::EventMask::POINTER_MOTION_HINT | x::EventMask::EXPOSURE), @@ -337,8 +426,8 @@ fn create_windows(conn: &xcb::Connection, screen: &x::Screen, crtc_info: &randr: class: x::WindowClass::InputOutput, visual: x::COPY_FROM_PARENT, value_list: &[ - x::Cw::BackPixel(screen.black_pixel()), - x::Cw::BorderPixel(screen.black_pixel()), + x::Cw::BackPixel(properties.black_pixel), + x::Cw::BorderPixel(properties.black_pixel), x::Cw::OverrideRedirect(true), x::Cw::SaveUnder(true), x::Cw::EventMask(x::EventMask::KEY_PRESS | x::EventMask::KEY_RELEASE | x::EventMask::BUTTON_PRESS | x::EventMask::BUTTON_RELEASE | x::EventMask::POINTER_MOTION | x::EventMask::POINTER_MOTION_HINT | x::EventMask::STRUCTURE_NOTIFY | x::EventMask::EXPOSURE), @@ -366,13 +455,13 @@ fn create_windows(conn: &xcb::Connection, screen: &x::Screen, crtc_info: &randr: Ok((blanker_window, unlock_window)) } -fn create_blank_cursor(conn: &xcb::Connection, screen: &x::Screen) -> xcb::Result { +fn create_blank_cursor(conn: &xcb::Connection, root: x::Window) -> xcb::Result { let blank_cursor: x::Cursor = conn.generate_id(); let pixmap: x::Pixmap = conn.generate_id(); conn.send_and_check_request(&x::CreatePixmap { pid: pixmap, - drawable: x::Drawable::Window(screen.root()), + drawable: x::Drawable::Window(root), depth: 1, width: 1, height: 1, diff --git a/locker/src/screensaver.rs b/locker/src/screensaver.rs index 7213aaa..f6b2db6 100644 --- a/locker/src/screensaver.rs +++ b/locker/src/screensaver.rs @@ -83,7 +83,7 @@ impl<'a> Screensaver<'a> { command_atoms, command_handlers, blanker_state: BlankerState::Idle, - monitors: Monitor::set_up_all(conn)?, + monitors: Monitor::discover_all(conn)?, last_user_activity: Instant::now(), unlock_dialog: None, }) @@ -138,19 +138,21 @@ impl<'a> Screensaver<'a> { if !embedder_handled { match event { - xcb::Event::RandR(randr::Event::Notify(ev)) => { - debug!("Got xrandr notify event: {:#?}", ev); - self.monitors = Monitor::set_up_all(self.conn)?; - match self.blanker_state { - BlankerState::Idle => (), - BlankerState::Blanked => { - self.blanker_state = BlankerState::Idle; - self.blank_screen()?; - }, - BlankerState::Locked => { - self.blanker_state = BlankerState::Idle; - self.lock_screen()?; - }, + xcb::Event::RandR(randr::Event::Notify(_)) => { + if let Some(new_monitors) = Monitor::maybe_rediscover(self.conn, &self.monitors)? { + debug!("Got xrandr notify event; recreated monitors"); + self.monitors = new_monitors; + match self.blanker_state { + BlankerState::Idle => (), + BlankerState::Blanked => { + self.blanker_state = BlankerState::Idle; + self.blank_screen()?; + }, + BlankerState::Locked => { + self.blanker_state = BlankerState::Idle; + self.lock_screen()?; + }, + } } }, ev @ xcb::Event::Input(_) => self.handle_user_activity(ev)?, @@ -222,20 +224,9 @@ impl<'a> Screensaver<'a> { } }, xcb::Event::X(x::Event::Expose(ev)) => { - if let Some(monitor) = self.monitors.iter().find(|m| ev.window() == m.blanker_window || ev.window() == m.unlock_window) { + if let Some(monitor) = self.monitors.iter().find(|m| ev.window() == m.blanker_window() || ev.window() == m.unlock_window()) { debug!("got expose for {} at {}x{}+{}+{}", ev.window().resource_id(), ev.width(), ev.height(), ev.x(), ev.y()); - self.conn.send_and_check_request(&x::PolyFillRectangle { - drawable: x::Drawable::Window(ev.window()), - gc: monitor.black_gc, - rectangles: &[ - x::Rectangle { - x: ev.x() as i16, - y: ev.y() as i16, - width: ev.width(), - height: ev.height(), - }, - ], - })?; + monitor.paint(&ev)?; } }, ev @ xcb::Event::X(x::Event::MotionNotify(_)) => self.handle_user_activity(ev)?, @@ -343,23 +334,23 @@ impl<'a> Screensaver<'a> { let mut monitor_data = None; for monitor in &self.monitors { let cookie = self.conn.send_request(&x::QueryPointer { - window: monitor.root, + window: monitor.root(), }); let reply = self.conn.wait_for_reply(cookie)?; let px = reply.root_x() as i32; let py = reply.root_y() as i32; if reply.same_screen() - && px >= monitor.x as i32 && px < monitor.x as i32 + monitor.width as i32 - && py >= monitor.y as i32 && py < monitor.y as i32 + monitor.height as i32 + && px >= monitor.x() as i32 && px < monitor.x() as i32 + monitor.width() as i32 + && py >= monitor.y() as i32 && py < monitor.y() as i32 + monitor.height() as i32 { - monitor_data = Some((monitor.geometry(), monitor.blanker_window, monitor.unlock_window)); + monitor_data = Some((monitor.geometry(), monitor.blanker_window(), monitor.unlock_window())); break; } } let (monitor_geom, blanker_window, unlock_window) = monitor_data.unwrap_or_else(|| { warn!("Unable to determine which monitor pointer is on; using first one"); let monitor = self.monitors.iter().nth(0).unwrap(); - (monitor.geometry(), monitor.blanker_window, monitor.unlock_window) + (monitor.geometry(), monitor.blanker_window(), monitor.unlock_window()) }); for monitor in &self.monitors {