Revert "linux/x11: Reduce input latency and ensure rerender priority (#13355)" (#13465)

This reverts commit f69c8ca74e after it
has already been partially reverted in
https://github.com/zed-industries/zed/pull/13458.

Why the revert?

The changes in that commit/PR fix one type of problem — dropping of
frames when being blasted with input events — but trades it for another
one that I can't explain yet: when the system is under load, then input
becomes _laggy_ and input events seem to be delayed.

Two examples of how that shows up:

1. When the system is under load* and you hold down the `down` key to
scroll, then lift the finger, the cursor stops sometimes. If you then
produce another input event by jiggling the mouse cursor you'll see more
`down`-key events coming up and the cursor moving down. It feels as if
the event loop is not being woken up even though there are still events.
I suspect it might have something to do with XIM, because if it's
disabled, it seems as if problems become less severe.
2. When the system is under load* and you click-and-drag a selection in
the editor, you can see how the selection is delayed and takes 500ms-1s
to catch up to where the cursor is.

* system under load: start Zed, then in another terminal window create a
release build of Zed, for example.

With the changes reverted, the failure mode looks different: we skip
frames. But that, I think, is the better of two bad options, because
skipping frames means that you see what's happening vs. input events
seemingly still coming in seconds after you stopped using the keyboard.

Release Notes:

- N/A
This commit is contained in:
Thorsten Ball 2024-06-24 15:31:49 +02:00 committed by GitHub
parent 40748b0a15
commit 82435075a5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 59 additions and 78 deletions

View File

@ -13,6 +13,7 @@ use util::ResultExt;
use x11rb::connection::{Connection, RequestConnection};
use x11rb::cursor;
use x11rb::errors::ConnectionError;
use x11rb::protocol::randr::ConnectionExt as _;
use x11rb::protocol::xinput::ConnectionExt;
use x11rb::protocol::xkb::ConnectionExt as _;
use x11rb::protocol::xproto::{ChangeWindowAttributesAux, ConnectionExt as _};
@ -45,7 +46,7 @@ use crate::platform::linux::xdg_desktop_portal::{Event as XDPEvent, XDPEventSour
pub(super) const XINPUT_MASTER_DEVICE: u16 = 1;
pub(crate) struct WindowRef {
pub window: X11WindowStatePtr,
window: X11WindowStatePtr,
refresh_event_token: RegistrationToken,
}
@ -297,33 +298,7 @@ impl X11Client {
{
let xcb_connection = xcb_connection.clone();
move |_readiness, _, client| {
let windows = client
.0
.borrow()
.windows
.values()
.map(|window_ref| window_ref.window.clone())
.collect::<Vec<_>>();
while let Some(event) = xcb_connection.poll_for_event()? {
for window in &windows {
let last_render_at;
let refresh_rate;
{
let window_state = window.state.borrow();
last_render_at = window_state.last_render_at;
refresh_rate = window_state.refresh_rate;
}
if let Some(last_render_at) = last_render_at {
if last_render_at.elapsed() >= refresh_rate {
window.refresh();
}
} else {
window.refresh();
}
}
let mut state = client.0.borrow_mut();
if state.ximc.is_none() || state.xim_handler.is_none() {
drop(state);
@ -980,18 +955,60 @@ impl LinuxClient for X11Client {
state.common.appearance,
)?;
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 window = window.0.clone();
let refresh_rate = window.state.borrow().refresh_rate;
move |mut instant, (), _| {
window.refresh();
let refresh_duration = mode_refresh_rate(mode);
move |mut instant, (), client| {
let state = client.0.borrow_mut();
state
.xcb_connection
.send_event(
false,
x_window,
xproto::EventMask::EXPOSURE,
xproto::ExposeEvent {
response_type: xproto::EXPOSE_EVENT,
sequence: 0,
window: x_window,
x: 0,
y: 0,
width: 0,
height: 0,
count: 1,
},
)
.unwrap();
let _ = state.xcb_connection.flush().unwrap();
// Take into account that some frames have been skipped
let now = Instant::now();
while instant < now {
instant += refresh_rate;
instant += refresh_duration;
}
calloop::timer::TimeoutAction::ToInstant(instant)
}
@ -1129,6 +1146,15 @@ impl LinuxClient for X11Client {
}
}
// Adatpted 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 {
let millihertz = mode.dot_clock as u64 * 1_000 / (mode.htotal as u64 * mode.vtotal as u64);
let micros = 1_000_000_000 / millihertz;
log::info!("Refreshing at {} micros", micros);
Duration::from_micros(micros)
}
fn fp3232_to_f32(value: xinput::Fp3232) -> f32 {
value.integral as f32 + value.frac as f32 / u32::MAX as f32
}

View File

@ -14,7 +14,6 @@ use util::{maybe, ResultExt};
use x11rb::{
connection::Connection,
protocol::{
randr::{self, ConnectionExt as _},
xinput::{self, ConnectionExt as _},
xproto::{
self, ClientMessageEvent, ConnectionExt as _, EventMask, TranslateCoordinatesReply,
@ -32,7 +31,6 @@ use std::{
ptr::NonNull,
rc::Rc,
sync::{self, Arc},
time::{Duration, Instant},
};
use super::{X11Display, XINPUT_MASTER_DEVICE};
@ -161,8 +159,6 @@ pub struct Callbacks {
pub struct X11WindowState {
pub destroyed: bool,
pub last_render_at: Option<Instant>,
pub refresh_rate: Duration,
client: X11ClientStatePtr,
executor: ForegroundExecutor,
atoms: XcbAtoms,
@ -401,31 +397,6 @@ impl X11WindowState {
};
xcb_connection.map_window(x_window).unwrap();
let screen_resources = 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 = 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_rate = mode_refresh_rate(mode);
Ok(Self {
client,
executor,
@ -442,8 +413,6 @@ impl X11WindowState {
appearance,
handle,
destroyed: false,
last_render_at: None,
refresh_rate,
})
}
@ -613,11 +582,6 @@ impl X11WindowStatePtr {
let mut cb = self.callbacks.borrow_mut();
if let Some(ref mut fun) = cb.request_frame {
fun();
self.state
.borrow_mut()
.last_render_at
.replace(Instant::now());
}
}
@ -1064,12 +1028,3 @@ impl PlatformWindow for X11Window {
false
}
}
// Adatpted 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 {
let millihertz = mode.dot_clock as u64 * 1_000 / (mode.htotal as u64 * mode.vtotal as u64);
let micros = 1_000_000_000 / millihertz;
log::info!("Refreshing at {} micros", micros);
Duration::from_micros(micros)
}