From d64ff4961ba5ef0eef16954d7aef5dc4754851ef Mon Sep 17 00:00:00 2001 From: rustdesk Date: Wed, 23 Sep 2026 10:16:31 +0800 Subject: [PATCH] cursor: name a shape by what it looks like, not by its handle A cursor id was the platform's handle for the shape: HCURSOR on Windows, the XFixes serial on X11. Apps mint a new handle for a shape they have shown before (Chrome, Electron, Qt custom cursors), and an X11 animated cursor gets one per frame, so one arrow arrived under thousands of ids over a session. The controlled side cached and sent it again under each, and the controller decoded, rendered and kept it again under each, and grew by gigabytes over weeks of connection. The controlled side now names a shape by an xxh3 hash of its size, hotspot and pixels, taken once when a handle is first seen, before the pixels are compressed. The per-connection send already sends a shape once and afterwards only its id, so a shape now crosses once however many handles it has. The id is opaque in the protocol, so controllers of any version accept it. The DRM backend already named shapes by content with FNV-1a; it uses the same hash now. A peer before 1.5.0 still names shapes by handle, and the fix has to work against every deployed peer. For those peers the controller hashes each CursorData's compressed colors with its geometry: one peer compresses the same pixels to the same bytes, so a shape seen before is recognised without being decompressed. The UI gets that shape once, under one id, and every later handle for it is passed on as a cursor_id of that id. The map from the peer's ids is kept for the connection, unbounded, since the peer sends a shape once and may select any id it named again; an entry is two integers. From 1.5.0 the controller passes the peer's ids through unchanged, on the old path. The web core in flutter/web does the same in its own tree. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01EZ49AbZJYfm8NTp5yDPMab --- Cargo.lock | 8 ++ Cargo.toml | 1 + libs/scrap/Cargo.toml | 3 +- libs/scrap/src/common/drm_reader.rs | 12 ++- src/client/io_loop.rs | 134 +++++++++++++++++++++++++++- src/common.rs | 17 ++++ src/server/drm_capturer.rs | 4 +- src/server/input_service.rs | 9 ++ 8 files changed, 177 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fcac8e5ad..efb2299cf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7196,6 +7196,7 @@ dependencies = [ "wol-rs", "x11-clipboard 0.8.1", "x11rb 0.12.0", + "xxhash-rust", "zstd", ] @@ -7444,6 +7445,7 @@ dependencies = [ "tracing", "webm", "winapi 0.3.9", + "xxhash-rust", "zbus", ] @@ -10806,6 +10808,12 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b9cc00251562a284751c9973bace760d86c0276c471b4be569fe6b068ee97a56" +[[package]] +name = "xxhash-rust" +version = "0.8.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aee1b19627c7c60102ab80d3a9cbe18de90bfe03bfa6c3715447681f0e8c8af6" + [[package]] name = "yasna" version = "0.5.2" diff --git a/Cargo.toml b/Cargo.toml index 9fedd8cc1..76461d696 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -63,6 +63,7 @@ lazy_static = "1.4" sha2 = "0.10" repng = "0.2" zstd = "0.13" +xxhash-rust = { version = "0.8", features = ["xxh3"] } parity-tokio-ipc = { git = "https://github.com/rustdesk-org/parity-tokio-ipc" } magnum-opus = { git = "https://github.com/rustdesk-org/magnum-opus" } dasp = { version = "0.11", features = ["signal", "interpolate-linear", "interpolate"], optional = true } diff --git a/libs/scrap/Cargo.toml b/libs/scrap/Cargo.toml index 135391312..fdd674d94 100644 --- a/libs/scrap/Cargo.toml +++ b/libs/scrap/Cargo.toml @@ -20,7 +20,7 @@ wayland = ["gstreamer", "gstreamer-app", "gstreamer-video", "dbus", "tracing", " # Depends on `wayland`: the three drm modules live inside the `#[cfg(feature = "wayland")]` arm of # common/mod.rs, so `scrap/drm` on its own would compile nothing. The root crate happens to always # enable `scrap/wayland`, which is what hid this. -drm = ["wayland", "base/wayland_probe"] +drm = ["wayland", "base/wayland_probe", "dep:xxhash-rust"] mediacodec = ["ndk"] linux-pkg-config = ["dep:pkg-config"] hwcodec = ["dep:hwcodec"] @@ -69,6 +69,7 @@ gstreamer = { version = "0.16", optional = true } gstreamer-app = { version = "0.16", features = ["v1_10"], optional = true } gstreamer-video = { version = "0.16", optional = true } zbus = { version = "3.15", optional = true } +xxhash-rust = { version = "0.8", features = ["xxh3"], optional = true } [dependencies.hwcodec] git = "https://github.com/rustdesk-org/hwcodec" diff --git a/libs/scrap/src/common/drm_reader.rs b/libs/scrap/src/common/drm_reader.rs index a269ffe89..b75fe12b7 100644 --- a/libs/scrap/src/common/drm_reader.rs +++ b/libs/scrap/src/common/drm_reader.rs @@ -50,18 +50,18 @@ pub struct CursorSnapshot { /// unchanged but whose provenance flipped is a different cursor as far as the client is concerned, /// and without this it would be deduped away and never sent. fn cursor_id(hash: u64, width: u32, height: u32, hotx: i32, hoty: i32, hot_measured: bool) -> u64 { - let mut id = hash; + let mut hasher = xxhash_rust::xxh3::Xxh3::new(); for v in [ + hash, width as u64, height as u64, hotx as u32 as u64, hoty as u32 as u64, hot_measured as u64, ] { - id ^= v; - id = id.wrapping_mul(1099511628211); + hasher.update(&v.to_le_bytes()); } - id + hasher.digest() } /// Encodes a provenance answer for the one-shot log below. -1 is "nothing reported yet". @@ -649,16 +649,14 @@ impl DrmReader { let ch = c.height as i32; let n = (cw * ch) as usize; let src = std::slice::from_raw_parts(c.pixels, n); - let mut hash: u64 = 1469598103934665603; let mut colors = Vec::with_capacity(n * 4); for &p in src.iter() { colors.push(((p >> 16) & 0xff) as u8); colors.push(((p >> 8) & 0xff) as u8); colors.push((p & 0xff) as u8); colors.push(((p >> 24) & 0xff) as u8); - hash ^= p as u64; - hash = hash.wrapping_mul(1099511628211); } + let hash = xxhash_rust::xxh3::xxh3_64(&colors); // Ask the library where the hotspot came from instead of guessing from the // coordinates. `hot_x/hot_y == (0, 0)` means two opposite things - the plane // exposes no HOTSPOT_X/Y (every bare-metal driver), or it exposes them and the diff --git a/src/client/io_loop.rs b/src/client/io_loop.rs index b4d0a6c64..e76749848 100644 --- a/src/client/io_loop.rs +++ b/src/client/io_loop.rs @@ -61,7 +61,7 @@ use hbb_common::{ use hbb_common::{tokio::sync::Mutex as TokioMutex, ResultType}; use scrap::CodecFormat; use std::{ - collections::HashMap, + collections::{HashMap, HashSet}, ffi::c_void, num::NonZeroI64, path::PathBuf, @@ -96,6 +96,7 @@ pub struct Remote { chroma: Arc>>, last_record_state: bool, sent_close_reason: bool, + cursor_dedupe: CursorDedupe, } #[derive(Default)] @@ -145,6 +146,7 @@ impl Remote { chroma: Default::default(), last_record_state: false, sent_close_reason: false, + cursor_dedupe: Default::default(), } } @@ -1523,6 +1525,13 @@ impl Remote { } _ => {} }, + Some(message::Union::CursorData(cd)) if self.dedupes_cursors() => { + self.set_cursor_data_by_content(cd); + } + Some(message::Union::CursorId(id)) if self.dedupes_cursors() => { + self.handler + .set_cursor_id(self.cursor_dedupe.id(id).to_string()); + } Some(message::Union::CursorData(cd)) => { let id = cd.id; match decode_cursor_data(cd) { @@ -2560,6 +2569,28 @@ impl Remote { msg.set_misc(misc); self.sender.send(Data::Message(msg)).ok(); } + + fn dedupes_cursors(&self) -> bool { + !crate::is_peer_naming_cursors_by_content(self.handler.lc.read().unwrap().version) + } + + /// A shape the UI already has, under whatever handle, is only selected. + fn set_cursor_data_by_content(&mut self, cd: CursorData) { + let peer_id = cd.id; + let id = self.cursor_dedupe.name(&cd); + if self.cursor_dedupe.is_shown(id) { + self.handler.set_cursor_id(id.to_string()); + return; + } + match decode_cursor_data(cd) { + Ok(mut cd) => { + cd.id = id; + self.cursor_dedupe.shown(id); + self.handler.set_cursor_data(cd); + } + Err(err) => log::warn!("Rejected cursor {peer_id}: {err}"), + } + } } // Both UI handlers receive validated, uncompressed RGBA from the receive loop. @@ -2603,6 +2634,37 @@ fn decode_cursor_data(data: CursorData) -> hbb_common::ResultType { Ok(cd) } +/// Gives the UI one id per shape for a peer that names shapes by handle, so the UI decodes and +/// keeps a shape once however many handles it arrives under. Nothing is dropped for the +/// connection: the peer sends a shape once and may select any handle it named again. +#[derive(Default)] +struct CursorDedupe { + ids: HashMap, + shown: HashSet, +} + +impl CursorDedupe { + /// Hashes the compressed colors: one peer compresses the same pixels to the same bytes, and + /// a shape seen before is then never decompressed again. + fn name(&mut self, cd: &CursorData) -> u64 { + let id = crate::cursor_content_id(cd.width, cd.height, cd.hotx, cd.hoty, &cd.colors); + self.ids.insert(cd.id, id); + id + } + + fn id(&self, peer_id: u64) -> u64 { + self.ids.get(&peer_id).copied().unwrap_or(peer_id) + } + + fn is_shown(&self, id: u64) -> bool { + self.shown.contains(&id) + } + + fn shown(&mut self, id: u64) { + self.shown.insert(id); + } +} + struct RemoveJob { files: Vec, path: String, @@ -2725,3 +2787,73 @@ mod tests { ); } } + +#[cfg(test)] +mod cursor_dedupe_tests { + use super::*; + + fn shape(id: u64, hotx: i32, colors: &[u8]) -> CursorData { + CursorData { + id, + hotx, + width: 2, + height: 2, + colors: colors.to_vec().into(), + ..Default::default() + } + } + + #[test] + fn a_shape_sent_under_a_new_handle_keeps_its_id() { + let mut dedupe = CursorDedupe::default(); + let first = dedupe.name(&shape(1, 0, b"arrow")); + assert!(!dedupe.is_shown(first)); + dedupe.shown(first); + + let again = dedupe.name(&shape(2, 0, b"arrow")); + assert_eq!(again, first); + assert!(dedupe.is_shown(again)); + assert_eq!(dedupe.id(2), first); + assert_eq!(dedupe.id(1), first); + } + + #[test] + fn the_hotspot_is_part_of_the_shape() { + let mut dedupe = CursorDedupe::default(); + let a = dedupe.name(&shape(1, 0, b"arrow")); + let b = dedupe.name(&shape(2, 1, b"arrow")); + assert_ne!(a, b); + } + + #[test] + fn a_reused_handle_names_its_latest_shape() { + let mut dedupe = CursorDedupe::default(); + dedupe.name(&shape(1, 0, b"arrow")); + let beam = dedupe.name(&shape(1, 0, b"beam")); + assert_eq!(dedupe.id(1), beam); + } + + #[test] + fn an_id_the_peer_never_sent_is_passed_on() { + assert_eq!(CursorDedupe::default().id(7), 7); + } + + #[test] + fn every_handle_the_peer_named_stays_for_the_connection() { + let mut dedupe = CursorDedupe::default(); + let arrow = dedupe.name(&shape(0, 0, b"arrow")); + for handle in 1..100_000 { + dedupe.name(&shape(handle, 0, b"arrow")); + } + assert_eq!(dedupe.id(0), arrow); + assert_eq!(dedupe.id(99_999), arrow); + } + + #[test] + fn peers_from_1_5_0_name_cursors_by_content() { + use hbb_common::get_version_number as v; + assert!(!crate::is_peer_naming_cursors_by_content(v("1.4.9"))); + assert!(crate::is_peer_naming_cursors_by_content(v("1.5.0"))); + assert!(crate::is_peer_naming_cursors_by_content(v("1.5.1"))); + } +} diff --git a/src/common.rs b/src/common.rs index 7daee26b8..c4bc10e8a 100644 --- a/src/common.rs +++ b/src/common.rs @@ -149,6 +149,23 @@ pub fn is_support_multi_ui_session_num(ver: i64) -> bool { ver >= hbb_common::get_version_number(MIN_VER_MULTI_UI_SESSION) } +/// Peers from 1.5.0 name a cursor by `cursor_content_id`; older ones by the platform handle, +/// which apps mint anew for the same shape. +#[inline] +pub fn is_peer_naming_cursors_by_content(ver: i64) -> bool { + ver >= hbb_common::get_version_number("1.5.0") +} + +/// One id per cursor look, however many handles a platform gives it. +pub fn cursor_content_id(width: i32, height: i32, hotx: i32, hoty: i32, colors: &[u8]) -> u64 { + let mut hasher = xxhash_rust::xxh3::Xxh3::new(); + for v in [width, height, hotx, hoty] { + hasher.update(&v.to_le_bytes()); + } + hasher.update(colors); + hasher.digest() +} + #[inline] #[cfg(feature = "unix-file-copy-paste")] pub fn is_support_file_copy_paste(ver: &str) -> bool { diff --git a/src/server/drm_capturer.rs b/src/server/drm_capturer.rs index dfcb2f490..f48d8a82f 100644 --- a/src/server/drm_capturer.rs +++ b/src/server/drm_capturer.rs @@ -928,7 +928,7 @@ fn remove_drm_cursor(display: i32, epoch: u64) { /// cursor alone would stay turned and its hotspot transposed (review finding 11 on /// rustdesk#15889). The wire id hashes only the plane pixels and geometry, so a stream rebuilt /// under a new transform resends the SAME id and the client's by-id cursor cache would keep the -/// old orientation: fold the transform in (the producer's own FNV step) so id and orientation +/// old orientation: fold the transform in (with the producer's own hash) so id and orientation /// can never disagree. The hidden sentinel must survive untouched. #[allow(clippy::too_many_arguments)] fn deliver_drm_cursor( @@ -1030,7 +1030,7 @@ fn fold_cursor_id(id: u64, t: i32) -> u64 { if id == scrap::drm_reader::HIDDEN_CURSOR_ID { id } else { - (id ^ t as u32 as u64).wrapping_mul(1099511628211) + xxhash_rust::xxh3::xxh3_64_with_seed(&id.to_le_bytes(), t as u32 as u64) } } diff --git a/src/server/input_service.rs b/src/server/input_service.rs index 4eb6c7b76..1a7112a25 100644 --- a/src/server/input_service.rs +++ b/src/server/input_service.rs @@ -421,6 +421,15 @@ fn run_cursor(sp: MouseCursorService, state: &mut StateCursor) -> ResultType<()> let cache_key = served_id; #[cfg(not(all(target_os = "linux", feature = "drm")))] let cache_key = hcursor; + // Named by content, so the per-connection send below sends a shape once however + // many handles the platform gives it; see `is_peer_naming_cursors_by_content`. + data.id = crate::cursor_content_id( + data.width, + data.height, + data.hotx, + data.hoty, + &data.colors, + ); data.colors = hbb_common::compress::compress(&data.colors[..]).into(); let mut tmp = Message::new(); tmp.set_cursor_data(data);