From ad1843aea6e2a3a6f185a47a4d3ad650ee15eed8 Mon Sep 17 00:00:00 2001 From: John Nunley Date: Fri, 29 Dec 2023 08:13:06 -0800 Subject: [PATCH] On X11, query for higher Xrandr version This appears to be the solution for the elusive #3335 issue. Previously, in the Xlib backend, we used the "XRRQueryVersion" function to query for the Xrandr version, and used that to determine whether we should use the "GetScreenResources" call or the "GetScreenResourcesCurrent" call. However, we passed the version "0, 0" into "XRRQueryVersion". Previously with Xlib this wasn't a problem, as Xlib ignores the version you pass in and substitutes it with the version of RandR it expects. https://gitlab.freedesktop.org/xorg/lib/libxrandr/-/blob/master/src/Xrandr.c?ref_type=heads#L386-387 The way that "XRRQueryVersion" is implemented on the server end, it compares the version passed into the request with the version supported by the server. If the server's version is greater than the client version, it just returns the client version. If the client's version is greater, it passes the server's version. Since we were passing in "0, 0" this means that the server returned "0, 0". https://gitlab.freedesktop.org/xorg/xserver/-/blob/master/randr/rrdispatch.c?ref_type=heads#L50-59 To determine whether we use "GetScreenResources" or "GetScreenResourcesCurrent", we compare the version returned by the server against "1, 3". Since we got "0, 0"- a version of XRandR so old it doesn't even exist- we use "GetScreenResources". The problem manifests in that "GetScreenResources" can take several seconds to query the screen state based on the current hardware configuration. On the other hand, "GetScreenResourcesCurrent" is fast; it uses the server's hardware cache if it is available. This problem is visible in XTrace. On the latest `master`: ``` 000:<:00c2: 12: RANDR-Request(140,0): QueryVersion major-version=0 minor-version=0 000:>:00c2:32: Reply to QueryVersion: major-version=0 minor-version=0 000:<:00c3: 8: RANDR-Request(140,8): GetScreenResources window=0x0000076e 000:>:00c3:1600: Reply to GetScreenResources: ``` On the `v0.28.0` tag: ``` 000:<:0019: 12: RANDR-Request(140,0): QueryVersion major-version=1 minor-version=6 000:>:0019:32: Reply to QueryVersion: major-version=1 minor-version=6 ...later 000:<:002d: 8: RANDR-Request(140,25): GetScreenResourcesCurrent window=0x0000076e 000:>:002d:1600: Reply to GetScreenResourcesCurrent ``` This commit fixes this issue by requesting "1, 3" instead. This returns the version we expect, where we can now use "GetScreenResourcesCurrent" properly. Fixes #3335 Signed-off-by: John Nunley --- CHANGELOG.md | 1 + src/platform_impl/linux/x11/monitor.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be0cfef8..ae6ed751 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Unreleased` header. # Unreleased +- On X11, reduce the amount of time spent fetching screen resources. - On Windows, macOS, X11, Wayland and Web, implement setting images as cursors. See the `custom_cursors.rs` example. - **Breaking:** Remove `Window::set_cursor_icon` - Add `WindowBuilder::with_cursor` and `Window::set_cursor` which takes a `CursorIcon` or `CustomCursor` diff --git a/src/platform_impl/linux/x11/monitor.rs b/src/platform_impl/linux/x11/monitor.rs index 09e42d66..e5859ac3 100644 --- a/src/platform_impl/linux/x11/monitor.rs +++ b/src/platform_impl/linux/x11/monitor.rs @@ -344,7 +344,7 @@ impl ScreenResources { conn: &impl x11rb::connection::Connection, root: &x11rb::protocol::xproto::Screen, ) -> Result { - let version = conn.randr_query_version(0, 0)?.reply()?; + let version = conn.randr_query_version(1, 3)?.reply()?; if (version.major_version == 1 && version.minor_version >= 3) || version.major_version > 1 { let reply = conn