Add Linux background drag and held-button tools - #1871
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughResolves X11 pointer targets to the deepest child window and translates root/local coordinates; converts synthetic click/drag/motion primitives to use resolved targets and correct button state. Migrates overlay to keyed multi-cursor render map and adds per-cursor held-mouse state plus three new tools for button-down, drag, and button-up workflows. ChangesMouse Event Targeting and Held-Mouse Tools
🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Linux visual regression artifactsMatrix jobs now run independently. Download visual artifacts from this workflow run.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
libs/cua-driver/rust/crates/platform-linux/src/input/mod.rs (1)
194-249:⚠️ Potential issue | 🟠 MajorKeep the same press child window for the entire drag/hold gesture.
send_drag()resolvespress_targetforButtonPress, but laterMotionNotifyEventandButtonReleaseEventre-resolveeventfrom the current coordinates (resolve_event_target(... ix, iy)/resolve_event_target(... to_x, to_y)), which can split one gesture across sibling child windows. On real X11 input, aButtonPressestablishes an implicit pointer grab so subsequentMotionNotify/ButtonReleaseare delivered to the window that received the press until all buttons are released. Persist the resolved press child window (and translate subsequent coordinates into that window’s coordinate space) and reuse it for all motion/release posts; ensure the held-button tool path stores/reuses the same resolved child target as well.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/cua-driver/rust/crates/platform-linux/src/input/mod.rs` around lines 194 - 249, send_drag() currently re-resolves the target on each motion/release which can deliver parts of a single press-drag-release to different child windows; instead capture and persist the initial press_target from resolve_event_target(...) and reuse its window/child and coordinate space for all MotionNotifyEvent and ButtonReleaseEvent postings (use press_target.window as event, press_target.child, and compute event_x/event_y by translating root coordinates into press_target.local_x/local_y space using press_target.root_x/root_y), do not call resolve_event_target for ix/iy/to_x/to_y after the initial press, and ensure any held-button tool path state stores and reuses this same press_target rather than re-resolving.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@libs/cua-driver/rust/crates/platform-linux/src/input/mod.rs`:
- Around line 46-56: deepest_child_at_point currently descends into every child
returned by conn.query_tree() based only on geometry, but query_tree can include
unmapped/unviewable windows; before recursing into a child (where
conn.get_geometry and the point_in_rect check occur) fetch the child's window
attributes via conn.get_window_attributes(*child)?.reply()? and check
attrs.map_state == MapState::VIEWABLE (use the MapState enum already in scope)
and skip the child if not viewable so hit testing only targets windows a real
pointer can reach.
In `@libs/cua-driver/rust/crates/platform-linux/src/tools/impl_.rs`:
- Around line 1581-1582: The reads for x/y in mouse_button_down and mouse_drag
currently use args.f64_or("x", 0.0) and args.f64_or("y", 0.0) which silently
default to (0,0); change them to require and validate coordinates the same way
DragTool does: use the mandatory extractor (e.g., args.f64("x") / args.f64("y")
or equivalent) and propagate a descriptive error when missing or wrong-typed,
matching the coordinate validation logic in DragTool/MouseDragTool so the call
fails fast instead of injecting actions at the origin.
- Around line 1568-1605: The code races on the mouse_hold lifecycle because
callers snapshot and release the mutex, await overlay/X11 work, then re-acquire
and write state; wrap the entire down/drag/up transition in a single async-aware
lock to serialize transitions: introduce an async mutex or reservation (e.g., an
async_mutex field on the same state struct) and acquire it at the start of the
mouse-button handlers that use mouse_hold (the block around the current checks
that read/clone/write mouse_hold and the subsequent awaits and send_button_down
call), hold that lock until after the stored state is set (i.e., until after
MouseHoldState is written back), and apply the same pattern to the other
handlers mentioned (the code around lines 1648-1703 and 1747-1787) so only one
lifecycle transition can be in flight at a time.
- Around line 1761-1772: The release path is re-transforming stored
hold.x/hold.y because you use unwrap_or(hold.x) and then always apply
zoom/resize; change the logic to detect whether the request actually supplied
coordinates (use args.has("x") or args.opt_f64 returning Some) and only run the
zoom_registry.get(...).zoom_to_window(...) or resize_registry.ratio(...)
conversion when coordinates were provided in this request; if neither "x" nor
"y" were supplied, leave x/y as the already-window-space hold.x/hold.y and skip
the zoom/resize branches (adjust the code around args.opt_f64("x")/opt_f64("y"),
the from_zoom check, and resize_registry.ratio(hold.pid) accordingly).
---
Outside diff comments:
In `@libs/cua-driver/rust/crates/platform-linux/src/input/mod.rs`:
- Around line 194-249: send_drag() currently re-resolves the target on each
motion/release which can deliver parts of a single press-drag-release to
different child windows; instead capture and persist the initial press_target
from resolve_event_target(...) and reuse its window/child and coordinate space
for all MotionNotifyEvent and ButtonReleaseEvent postings (use
press_target.window as event, press_target.child, and compute event_x/event_y by
translating root coordinates into press_target.local_x/local_y space using
press_target.root_x/root_y), do not call resolve_event_target for
ix/iy/to_x/to_y after the initial press, and ensure any held-button tool path
state stores and reuses this same press_target rather than re-resolving.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5fce2f3b-3486-43cc-9609-d9249340d14c
📒 Files selected for processing (2)
libs/cua-driver/rust/crates/platform-linux/src/input/mod.rslibs/cua-driver/rust/crates/platform-linux/src/tools/impl_.rs
| let tree = conn.query_tree(window)?.reply()?; | ||
| for child in tree.children.iter().rev() { | ||
| let Ok(geom) = conn.get_geometry(*child)?.reply() else { | ||
| continue; | ||
| }; | ||
| if !point_in_rect(local_x, local_y, &geom) { | ||
| continue; | ||
| } | ||
| let child_x = local_x - geom.x as i32; | ||
| let child_y = local_y - geom.y as i32; | ||
| return deepest_child_at_point(conn, *child, child_x, child_y); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For X11 hit-testing: does QueryTree include unmapped children, and is checking GetWindowAttributes.map_state == MapState::VIEWABLE the correct way to exclude hidden child windows from synthetic pointer targeting?
💡 Result:
Yes, XQueryTree includes unmapped children [1]. The function returns all child windows of the specified window regardless of their mapping state [1]. Checking XGetWindowAttributes.map_state == IsViewable is the correct and necessary way to identify whether a window is currently viewable on screen [1][2][3]. When performing synthetic pointer targeting (hit-testing), simply checking if a window is present in the tree is insufficient because: 1. XQueryTree returns both mapped and unmapped windows [1]. 2. A window may be "mapped" but still not visible to the user (IsUnviewable) if one of its ancestors is unmapped [2][4][3]. The IsViewable state specifically confirms that a window is mapped and that all of its ancestors are also mapped, meaning it is a candidate for receiving user input [4][3]. Note that you should also consider checking the override_redirect attribute if you intend to exclude specific types of windows (like pop-ups or menus) that might otherwise interfere with your targeting logic [1].
Citations:
- 1: https://stackoverflow.com/questions/78701376/xquerytree-returns-wrong-number-of-child-windows
- 2: https://man.openbsd.org/man3/XGetWindowAttributes.3
- 3: https://x.org/releases/X11R7.5/doc/man/man3/XGetWindowAttributes.3.html
- 4: https://xpert.xfree86.narkive.com/xvCWsd9O/about-map-state
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and show the relevant section around the reported lines
FILE="libs/cua-driver/rust/crates/platform-linux/src/input/mod.rs"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
echo "=== File: $FILE (lines 1-120) ==="
nl -ba "$FILE" | sed -n '1,140p'
# Search for deepest_child_at_point definition and for query_tree usage
echo "=== Search: deepest_child_at_point ==="
rg -n "deepest_child_at_point" "$FILE" -S || true
echo "=== Search: query_tree / get_geometry / get_window_attributes ==="
rg -n "(query_tree|get_geometry|get_window_attributes|MapState)" "$FILE" -S || trueRepository: trycua/cua
Length of output: 204
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and show the relevant section around the reported lines
FILE="libs/cua-driver/rust/crates/platform-linux/src/input/mod.rs"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
echo "=== File: $FILE (lines 1-200) ==="
nl -ba "$FILE" | sed -n '1,220p'
# Search for deepest_child_at_point definition and for query_tree usage
echo "=== Search: deepest_child_at_point ==="
rg -n "deepest_child_at_point" "$FILE" -S || true
echo "=== Search: query_tree / get_geometry / get_window_attributes / MapState ==="
rg -n "(query_tree|get_geometry|get_window_attributes|MapState)" "$FILE" -S || trueRepository: trycua/cua
Length of output: 204
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="libs/cua-driver/rust/crates/platform-linux/src/input/mod.rs"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
echo "=== File: $FILE (lines 1-220) ==="
cat -n "$FILE" | sed -n '1,220p'
echo "=== Search: deepest_child_at_point ==="
rg -n "deepest_child_at_point" "$FILE" -S || true
echo "=== Search: query_tree / get_geometry / get_window_attributes / MapState ==="
rg -n "(query_tree|get_geometry|get_window_attributes|MapState)" "$FILE" -S || true
echo "=== Search: MapState usage in entire platform-linux input module ==="
rg -n "MapState" "libs/cua-driver/rust/crates/platform-linux/src/input" -S || trueRepository: trycua/cua
Length of output: 9685
🌐 Web query:
x11rb GetWindowAttributesReply map_state MapState::VIEWABLE
💡 Result:
In the x11rb library, GetWindowAttributesReply is a structure returned by the GetWindowAttributes request, which provides the current attributes of an X11 window [1][2][3]. The map_state field within this structure is an enumeration of type MapState that indicates whether the window is currently mapped and viewable [1][4]. The MapState enumeration includes the variant MapState::VIEWABLE, which signifies that the window is viewable—meaning it is mapped and all of its ancestors are also mapped [4]. In practice, when working with x11rb, you typically access this field after sending a GetWindowAttributes request and receiving the reply [5]: let attr = conn.get_window_attributes(window)?.reply?; if attr.map_state == MapState::VIEWABLE { // The window is currently viewable } The other variants for MapState include MapState::UNMAPPED (the window is not mapped) and MapState::UNVIEWABLE (the window is mapped, but at least one of its ancestors is not mapped) [4][6].
Citations:
- 1: https://doc.servo.org/x11rb/protocol/xproto/struct.GetWindowAttributesReply.html
- 2: https://docs.rs/x11rb-protocol/0.13.2/x86_64-unknown-linux-gnu/x11rb_protocol/protocol/xproto/struct.GetWindowAttributesReply.html
- 3: https://doc.servo.org/x11rb/protocol/xproto/fn.get_window_attributes.html
- 4: https://doc.servo.org/x11rb/protocol/xproto/struct.MapState.html
- 5: https://github.com/psychon/x11rb/blob/master/x11rb/examples/simple_window_manager.rs
- 6: https://rust-x-bindings.github.io/rust-xcb/xcb/x/enum.MapState.html
Filter non-viewable child windows during hit testing
deepest_child_at_point() descends into every query_tree() child that matches the geometry, but query_tree() includes unmapped/unviewable windows. Without checking GetWindowAttributes.map_state, hit testing can target a window that a real pointer can’t reach. Skip children unless attrs.map_state == MapState::VIEWABLE before descending.
Suggested fix
let tree = conn.query_tree(window)?.reply()?;
for child in tree.children.iter().rev() {
let Ok(geom) = conn.get_geometry(*child)?.reply() else {
continue;
};
+ let Ok(attrs) = conn.get_window_attributes(*child)?.reply() else {
+ continue;
+ };
+ if attrs.map_state != MapState::VIEWABLE {
+ continue;
+ }
if !point_in_rect(local_x, local_y, &geom) {
continue;
}
let child_x = local_x - geom.x as i32;
let child_y = local_y - geom.y as i32;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let tree = conn.query_tree(window)?.reply()?; | |
| for child in tree.children.iter().rev() { | |
| let Ok(geom) = conn.get_geometry(*child)?.reply() else { | |
| continue; | |
| }; | |
| if !point_in_rect(local_x, local_y, &geom) { | |
| continue; | |
| } | |
| let child_x = local_x - geom.x as i32; | |
| let child_y = local_y - geom.y as i32; | |
| return deepest_child_at_point(conn, *child, child_x, child_y); | |
| let tree = conn.query_tree(window)?.reply()?; | |
| for child in tree.children.iter().rev() { | |
| let Ok(geom) = conn.get_geometry(*child)?.reply() else { | |
| continue; | |
| }; | |
| let Ok(attrs) = conn.get_window_attributes(*child)?.reply() else { | |
| continue; | |
| }; | |
| if attrs.map_state != MapState::VIEWABLE { | |
| continue; | |
| } | |
| if !point_in_rect(local_x, local_y, &geom) { | |
| continue; | |
| } | |
| let child_x = local_x - geom.x as i32; | |
| let child_y = local_y - geom.y as i32; | |
| return deepest_child_at_point(conn, *child, child_x, child_y); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libs/cua-driver/rust/crates/platform-linux/src/input/mod.rs` around lines 46
- 56, deepest_child_at_point currently descends into every child returned by
conn.query_tree() based only on geometry, but query_tree can include
unmapped/unviewable windows; before recursing into a child (where
conn.get_geometry and the point_in_rect check occur) fetch the child's window
attributes via conn.get_window_attributes(*child)?.reply()? and check
attrs.map_state == MapState::VIEWABLE (use the MapState enum already in scope)
and skip the child if not viewable so hit testing only targets windows a real
pointer can reach.
| if self.state.mouse_hold.lock().unwrap().is_some() { | ||
| let held = self.state.mouse_hold.lock().unwrap().clone(); | ||
| return ToolResult::error("A mouse button is already held. Call mouse_button_up first.") | ||
| .with_structured(mouse_hold_json(held.as_ref())); | ||
| } | ||
|
|
||
| let pid = match args.require_u32("pid") { Ok(v) => v, Err(e) => return e }; | ||
| let xid = match args.opt_u64("window_id") { | ||
| Some(v) => v, | ||
| None => return ToolResult::error("window_id is required on Linux."), | ||
| }; | ||
| let button_name = args.str_or("button", "left"); | ||
| let button = parse_mouse_button(button_name.as_str()); | ||
| let mut x = args.f64_or("x", 0.0); | ||
| let mut y = args.f64_or("y", 0.0); | ||
| if args.bool_or("from_zoom", false) { | ||
| match self.state.zoom_registry.get(pid) { | ||
| Some(ctx) => { let (wx, wy) = ctx.zoom_to_window(x, y); x = wx; y = wy; } | ||
| None => return ToolResult::error(format!("from_zoom=true but no zoom context for pid {pid}. Call zoom first.")), | ||
| } | ||
| } else if let Some(ratio) = self.state.resize_registry.ratio(pid) { | ||
| x *= ratio; | ||
| y *= ratio; | ||
| } | ||
|
|
||
| crate::overlay::send_command(cursor_overlay::OverlayCommand::PinAbove(xid)); | ||
| if let Ok(Ok((sx, sy))) = tokio::task::spawn_blocking(move || window_local_to_screen(xid, x, y)).await { | ||
| overlay_glide_to(sx, sy).await; | ||
| crate::overlay::send_command(cursor_overlay::OverlayCommand::ClickPulse { x: sx, y: sy }); | ||
| } | ||
|
|
||
| let xi = x as i32; | ||
| let yi = y as i32; | ||
| let result = tokio::task::spawn_blocking(move || crate::input::send_button_down(xid, xi, yi, button)).await; | ||
| match result { | ||
| Ok(Ok(())) => { | ||
| let hold = MouseHoldState { pid, xid, button, x, y }; | ||
| *self.state.mouse_hold.lock().unwrap() = Some(hold.clone()); |
There was a problem hiding this comment.
Serialize held-mouse lifecycle transitions.
These three tools all snapshot mouse_hold, drop the mutex, await overlay/X11 work, and only then write the new state back. Two overlapping requests can therefore both observe None, send duplicate ButtonPress events, or race a drag against a release and leave the stored hold out of sync with the actual injected button state. Please guard the whole down/drag/up lifecycle with a single async mutex or equivalent reservation so only one transition can be in flight at a time.
Also applies to: 1648-1703, 1747-1787
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libs/cua-driver/rust/crates/platform-linux/src/tools/impl_.rs` around lines
1568 - 1605, The code races on the mouse_hold lifecycle because callers snapshot
and release the mutex, await overlay/X11 work, then re-acquire and write state;
wrap the entire down/drag/up transition in a single async-aware lock to
serialize transitions: introduce an async mutex or reservation (e.g., an
async_mutex field on the same state struct) and acquire it at the start of the
mouse-button handlers that use mouse_hold (the block around the current checks
that read/clone/write mouse_hold and the subsequent awaits and send_button_down
call), hold that lock until after the stored state is set (i.e., until after
MouseHoldState is written back), and apply the same pattern to the other
handlers mentioned (the code around lines 1648-1703 and 1747-1787) so only one
lifecycle transition can be in flight at a time.
| let mut x = args.opt_f64("x").unwrap_or(hold.x); | ||
| let mut y = args.opt_f64("y").unwrap_or(hold.y); | ||
| if args.bool_or("from_zoom", false) { | ||
| match self.state.zoom_registry.get(hold.pid) { | ||
| Some(ctx) => { let (wx, wy) = ctx.zoom_to_window(x, y); x = wx; y = wy; } | ||
| None => return ToolResult::error(format!("from_zoom=true but no zoom context for pid {}. Call zoom first.", hold.pid)) | ||
| .with_structured(mouse_hold_json(Some(&hold))), | ||
| } | ||
| } else if let Some(ratio) = self.state.resize_registry.ratio(hold.pid) { | ||
| x *= ratio; | ||
| y *= ratio; | ||
| } |
There was a problem hiding this comment.
Don't re-translate stored hold coordinates on mouse_button_up.
When x/y are omitted, this path starts from hold.x/hold.y and then applies from_zoom/resize conversion again. Those fields were already stored in window coordinates by mouse_button_down/mouse_drag, so releases can land on the wrong child widget in resized or zoomed flows. Only transform coordinates that were actually supplied in this request.
Suggested fix
- let mut x = args.opt_f64("x").unwrap_or(hold.x);
- let mut y = args.opt_f64("y").unwrap_or(hold.y);
+ let raw_x = args.opt_f64("x");
+ let raw_y = args.opt_f64("y");
+ let mut x = raw_x.unwrap_or(hold.x);
+ let mut y = raw_y.unwrap_or(hold.y);
if args.bool_or("from_zoom", false) {
match self.state.zoom_registry.get(hold.pid) {
- Some(ctx) => { let (wx, wy) = ctx.zoom_to_window(x, y); x = wx; y = wy; }
+ Some(ctx) => {
+ if let Some(px) = raw_x {
+ x = ctx.origin_x + px * ctx.scale_inv;
+ }
+ if let Some(py) = raw_y {
+ y = ctx.origin_y + py * ctx.scale_inv;
+ }
+ }
None => return ToolResult::error(format!("from_zoom=true but no zoom context for pid {}. Call zoom first.", hold.pid))
.with_structured(mouse_hold_json(Some(&hold))),
}
} else if let Some(ratio) = self.state.resize_registry.ratio(hold.pid) {
- x *= ratio;
- y *= ratio;
+ if let Some(px) = raw_x {
+ x = px * ratio;
+ }
+ if let Some(py) = raw_y {
+ y = py * ratio;
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let mut x = args.opt_f64("x").unwrap_or(hold.x); | |
| let mut y = args.opt_f64("y").unwrap_or(hold.y); | |
| if args.bool_or("from_zoom", false) { | |
| match self.state.zoom_registry.get(hold.pid) { | |
| Some(ctx) => { let (wx, wy) = ctx.zoom_to_window(x, y); x = wx; y = wy; } | |
| None => return ToolResult::error(format!("from_zoom=true but no zoom context for pid {}. Call zoom first.", hold.pid)) | |
| .with_structured(mouse_hold_json(Some(&hold))), | |
| } | |
| } else if let Some(ratio) = self.state.resize_registry.ratio(hold.pid) { | |
| x *= ratio; | |
| y *= ratio; | |
| } | |
| let raw_x = args.opt_f64("x"); | |
| let raw_y = args.opt_f64("y"); | |
| let mut x = raw_x.unwrap_or(hold.x); | |
| let mut y = raw_y.unwrap_or(hold.y); | |
| if args.bool_or("from_zoom", false) { | |
| match self.state.zoom_registry.get(hold.pid) { | |
| Some(ctx) => { | |
| if let Some(px) = raw_x { | |
| x = ctx.origin_x + px * ctx.scale_inv; | |
| } | |
| if let Some(py) = raw_y { | |
| y = ctx.origin_y + py * ctx.scale_inv; | |
| } | |
| } | |
| None => return ToolResult::error(format!("from_zoom=true but no zoom context for pid {}. Call zoom first.", hold.pid)) | |
| .with_structured(mouse_hold_json(Some(&hold))), | |
| } | |
| } else if let Some(ratio) = self.state.resize_registry.ratio(hold.pid) { | |
| if let Some(px) = raw_x { | |
| x = px * ratio; | |
| } | |
| if let Some(py) = raw_y { | |
| y = py * ratio; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@libs/cua-driver/rust/crates/platform-linux/src/tools/impl_.rs` around lines
1761 - 1772, The release path is re-transforming stored hold.x/hold.y because
you use unwrap_or(hold.x) and then always apply zoom/resize; change the logic to
detect whether the request actually supplied coordinates (use args.has("x") or
args.opt_f64 returning Some) and only run the
zoom_registry.get(...).zoom_to_window(...) or resize_registry.ratio(...)
conversion when coordinates were provided in this request; if neither "x" nor
"y" were supplied, leave x/y as the already-window-space hold.x/hold.y and skip
the zoom/resize branches (adjust the code around args.opt_f64("x")/opt_f64("y"),
the from_zoom check, and resize_registry.ratio(hold.pid) accordingly).
* Start Linux multi-cursor plumbing * Key Linux overlay state by cursor * Fix Linux keyed overlay build * Restore Linux click tool arg helpers * Restore Linux double-click arg helpers * Restore Linux right-click arg helpers * Import Linux tool arg helpers once * Tighten Linux multi-cursor tool validation * Show Linux cursor state during drags * Animate Linux drag overlay in motion engine * Add Linux MPX parallel drag tool * Fix x11 feature flags for MPX tool * Fix MPX tool FFI usage * Use XTEST slave devices for Linux MPX drags * Initialize Xlib threading for Linux MPX drags * Fail MPX drags on Xtigervnc and clean up masters * Detect Xtigervnc by process for MPX guard * Fix MPX drag delivery: overlay click-through, warp sync, flat accel Three fixes that turn the Linux parallel_mouse_drag uinput/MPX pipeline from "raw events only" into real app-visible same-window drags: - The cursor overlay's click-through was a no-op: ShapeMask with a None pixmap *resets* the input shape to the full window, so the fullscreen overlay swallowed every pointer event before it reached any app. Set an empty input region via ShapeRectangles with no rects instead. - XIWarpPointer was only XFlush'd, so the uinput button press (separate kernel pipeline) raced ahead of the queued warp and landed at the master's spawn position (screen center), starting an implicit grab on the desktop. XSync the warp so the press lands at the warped spot. - Relative uinput deltas went through libinput's adaptive accel profile and drag endpoints drifted a few pixels off-target. Pin the slave's accel profile to flat after attaching it to its master. Also moves the drag path off the fake single-pointer/XTEST approach to per-session MPX master pointers fed by per-session uinput slaves with concurrent step scheduling (carried over from earlier session work). Verified in the Xorg+x11vnc desktop-workspace guest: an XI2 test app receives interleaved cooked XI_ButtonPress/XI_Motion/XI_ButtonRelease for both masters with pixel-exact endpoints across repeated calls and fresh MCP sessions, and paints all concurrent strokes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Track parallel drags with the agent cursor overlay The drag loop now snaps each session's overlay cursor to the interpolated step position (heading along the motion vector), so the gesture is visible on screen instead of the cursor teleporting from start to end. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Restore the active window after parallel drags Click-to-focus WMs grab buttons for XIAllMasterDevices, so an MPX drag's press activates the dragged window exactly like a user click — parallel agent drags were stealing focus from whatever the user had active. Snapshot the focus state before the drags and hand it back afterwards, WM-agnostically: - EWMH path: re-activate the previous _NET_ACTIVE_WINDOW via client message (source=pager) so the WM's own bookkeeping stays consistent. The WM finishes its click handling on the release replay, which can land after a one-shot request, so settle briefly then verify/retry. - Fallback for bare X / non-EWMH WMs: save and restore the core input focus directly, under a scoped ignore-errors handler since the saved window may be gone by then. Also drops the XISetClientPointer/XISetFocus calls before drags — they were debugging leftovers, not needed for cooked event delivery, and pointing master keyboard focus at the target fights the restore. Verified under openbox: delivery unchanged, active window returns to the previously-focused terminal after every call. xfwm4 4.18 has a broken MPX path — one foreign-master press and it permanently ignores _NET_ACTIVE_WINDOW requests (ours and xdotool's alike, until restarted) — so the desktop-workspace image should ship a different WM; the driver logs a warning when the WM refuses the re-activation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Stamp focus-restore activation with real server time _NET_ACTIVE_WINDOW requests carrying CurrentTime(0) lose to the WM's focus-stealing prevention whenever newer input exists, so fetch the server time via the standard PropertyNotify round-trip and stamp the re-activation request with it. With concurrent live user input the WM may still legitimately refuse (that IS focus-stealing prevention); the driver retries briefly and logs a warning if the WM declines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Clean up drag masters per call and resync wedged WM focus state Two changes that keep non-MPX-aware WMs healthy across parallel drags: - Remove the per-session master pairs as soon as the gesture completes instead of leaving them attached until session end. One-shot MCP sessions never end, so masters used to accumulate without bound, and lingering foreign master keyboards multiply the core focus-event noise WMs have to digest. - The focus restore now requires the active window to hold stable for consecutive checks, and escalates to a core-focus bounce when plain re-activation doesn't take. Verified mechanism: after an MPX click, a core-protocol WM (xfwm4, openbox) can record the dragged window as active while the core focus never actually moved there; its XSetInputFocus for our re-activation request is then a no-op, no FocusIn ever arrives, and its bookkeeping stays wedged — even real user clicks stop updating the titlebar. Setting the core focus onto the window the WM believes active and then re-activating produces the focus transition its state machine is waiting for. With this, two consecutive parallel_mouse_drag calls against an unfocused window under openbox leave the user's terminal focused (titlebar, _NET_ACTIVE_WINDOW, and core focus all agree), with no leftover master devices. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Shield parallel drags from the WM with a device-specific grab + replay Prevents the focus steal at the source instead of restoring focus after the fact: the window manager now never sees the drag's button press. Before each press, install a device-specific XI2 synchronous passive button grab (XIGrabButton) for the drag's master pointer on the target window. Per the X server's grab semantics (dix/events.c, dix/grabs.c): this grab is newer than the WM's click-to-focus grab on the same window so it's matched first, and being device-specific it never BadAccess- conflicts with the WM's core/all-master grabs. The press freezes the device and is delivered to us; XIAllowEvents(XIReplayDevice) then replays it, which re-checks passive grabs only *below* the grab window and delivers the event normally to the app — so the app gets the full drag (press, implicit-grab motion, release) while the WM is blind to it. Shields are removed after release. Failures degrade gracefully: if a shield can't install or a frozen press isn't seen before a 1s timeout, the drag still runs and the existing focus save/restore catches any leak. Verified on xfwm4 (the wedge-prone WM): _NET_ACTIVE_WINDOW polled at 50ms across two consecutive two-cursor drag calls never left the focused terminal — no mid-drag flicker — the app received all presses and releases at pixel-exact coordinates, masters were cleaned up, and the WM stayed healthy (activation still toggles both ways afterwards). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Replay shield presses per-device to fix dropped concurrent presses The first shield cut focus-stealing but dropped ~1 in 8 presses under concurrent drags: with both masters' presses frozen on their grabs and replayed back to back, the X server delivered only one. Diagnostics confirmed both XIReplayDevice calls returned success yet only one press reached the app — a server-side race when multiple devices are frozen on the same window and replayed together (XSync between replays narrowed it but didn't close it). Fix: replay each frozen press immediately after emitting it, before the next press, so only one device is ever frozen at a time. The few-ms stagger this adds to the presses is invisible — the concurrency that matters is the motion phase, which is unchanged. Verified on xfwm4: 40/40 presses and releases delivered across 20 consecutive two-cursor calls, focus never left the terminal, WM stayed healthy, no leftover masters. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(nix): update cua-driver cargoHash for new dependency set Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(nix): add pkg-config and X11 libs for the x11 crate build The MPX multi-cursor plumbing pulls in the raw-Xlib x11 crate, whose build.rs locates libX11/libXi/libXtst via pkg-config. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Codex <codex@openai.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
| if vendor.to_ascii_lowercase().contains("tigervnc") { | ||
| bail!( | ||
| "parallel_mouse_drag is not supported on this X server ('{vendor}'). \ | ||
| Xtigervnc exposes only its built-in VNC/XTEST devices, so Linux uinput/libinput \ |
There was a problem hiding this comment.
@copilot maybe just try checking for this directly? instead of looking for magic strings
There was a problem hiding this comment.
Done. Instead of scanning all /proc entries and matching "Xtigervnc" as a substring in cmdline, the function now reads the X server's PID directly from /tmp/.X{N}-lock (standard X11 lock file), then checks the process executable via /proc/{PID}/exe (with /proc/{PID}/comm as fallback). Committed in the latest push.
A scenario that drives cua-driver through its Linux multi-cursor parallel_mouse_drag path end to end, in the style of the other linux-*-gif NixOS tests, and records a GIF artifact. The scenario (mcp-parallel-drag-test.py) pilots the driver over MCP through two parallel_mouse_drag calls — two per-session master pointers drawing concurrent crossing strokes into one window that does NOT hold focus — and the test asserts the three guarantees the feature rests on: - concurrent cooked delivery: an XI2 paint app (compiled in-tree) logs every window-targeted XI2 event with its device id; we assert four presses from two distinct master devices plus motion; - no focus steal: the shield grab keeps the WM blind to the presses, so the separate control window stays active throughout; - a GIF of the two crossing strokes is produced and copied out. Unlike the other Linux visual tests this one needs a REAL Xorg (dummy video driver + libinput) rather than Xvfb, because the MPX path attaches uinput slave devices to per-session masters and only a real Xorg with libinput enumerates uinput devices as X input devices — the same reason the old Xvfb/Xtigervnc setups couldn't host it. The test loads the uinput kernel module and launches Xorg with an explicit module path. Wired into flake checks and the nix-build CI matrix (+ the visual artifact comment list). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The mandatory-shield change uses .with_context() on the shield-grab install, but anyhow::Context wasn't in scope, so the crate didn't compile. Add it to the use list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Background "X X" multi-cursor drag — mandatory shield, verified liveTwo MPX master pointers drawing two X shapes concurrently into the same unfocused window, with the control terminal holding focus the whole time. Built from this branch (
Results:
The mandatory-shield change behaved as intended: the shield installed/replayed successfully so the drag ran; had it failed, the drag would now abort with an error rather than silently fall back to focus-restore. 🤖 Generated with Claude Code |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Bonus: multi-cursor writes "Cua", then the C becomes an illuminated drop capFor fun — the two MPX agent cursors collaboratively hand-write Cua into a background window (focus stays on the terminal, as always), then it cross-fades into a stylish drop-cap wordmark: a gold-on-navy illuminated initial C with "ua" set in Palatino, finished with a gold rule.
The writing half is real 🤖 Generated with Claude Code |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Background multi-cursor writing on two windows at once — focus never movesThe clearest demo of the background effect yet: two paint windows side by side, both having "Cua" hand-written concurrently by the agent cursors, while a small "hello world!" terminal sits focused on top in the center. The terminal keeps focus the entire time — the drawing happens on the windows around/behind it without ever stealing it (shield grab). Serif — 12 cursors (6 per window) tracing serif letterforms, then fading to the Cua logo: Cursive — 8 cursors (4 per window) tracing a connected script: Both letterforms are generated parametrically (arc/Bézier sampling → dense short 🤖 Generated with Claude Code |
#1871) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Same cursive demo, but recorded through cua-driver's own native screen captureRe-did the two-window cursive demo capturing each frame via the driver's native
Per step the writer issues one 🤖 Generated with Claude Code |
…ing mp4 (PR #1871) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Cursive demo recorded with cua-driver's native video capture (
|
start_recording's video capture shells out to the ffmpeg binary on Linux/Windows (macOS records natively). When ffmpeg is absent, video silently fails. Add an install_ffmpeg MCP tool that resolves it: - Detects the platform package manager (apt-get/dnf/yum/zypper/pacman/ apk/snap on Linux, brew on macOS, winget/choco on Windows) and, on Linux, prefixes sudo -n when not root. - Two-step / confirmed: without `confirm` it only REPORTS the exact command it would run (read-only preview); `confirm: true` runs it. Marked destructive + open_world so conforming MCP clients also gate it behind a human approval. No-op if ffmpeg is already on PATH. - start_recording's video-failed message now points at install_ffmpeg when find_ffmpeg() is None. ffmpeg is still only ever invoked as a separate process, never linked — this just installs the same user-provided binary find_ffmpeg looks for. Verified end to end on Linux: with ffmpeg removed, start_recording reports the hint; install_ffmpeg (no confirm) previews `apt-get install -y ffmpeg`; confirm:true installs it; video capture then works. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ing (PR #1871) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both demos, recorded natively with the
|
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Full-screen capture, as WebM (VP9, 30 fps) — continuous motionRe-encoded the native
~0.5 MB each. Why the earlier GIFs looked smoother/faster: they were sped up (2–3×) and built from one frame per drawing step, so they never showed the real-time gaps between Heads-up on GitHub rendering: a 🤖 Generated with Claude Code |
…ous motion (PR #1871) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Cursive demo — full-screen WebM (VP9, 30 fps), 1× and 2×Continuous-motion (idle frames dropped via
~0.3–0.5 MB each. (Raw 🤖 Generated with Claude Code |
…el_mouse_drag parallel_mouse_drag pressed and released on every call, so drawing a curve as many short calls looked like a chain of clicks rather than a drag. Now each drag item carries a full waypoint PATH: the master presses once at path[0], glides through every point (arc-length interpolated over the whole duration), and releases once at the end — a single continuous held drag, so a curved stroke is smooth instead of stamped. The path is given either as a straight from→to segment (back-compatible) or as a function `fn` = y(x) sampled over [x_from, x_to] in window-local pixels (e.g. fn:"x", fn:"300+150*sin(x/40)", fn:"560-0.0011*(x-400)^2"). Expressions are evaluated with the meval crate (+ - * / ^, sin/cos/tan, sqrt, abs, exp, ln, pi, e). Each cursor can follow its own function, all concurrently. - VirtualPointerDrag now holds `path: Vec<(i32,i32)>` (screen coords) instead of from/to; the scheduler interpolates position by arc-length fraction. - The tool builds the path once per item, translating window-local → screen via a single origin lookup (translate is a pure offset). - steps defaults to a path-length-scaled count for a smooth glide. Verified on Linux: three cursors drew a line, a sine, and a parabola in one call as smooth continuous strokes, focus held on another window. NOTE: adds the `meval` dependency — nix package.nix cargoHash needs regen. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
"cua driver" — written by the cursors using the new held-path dragsUsing the function/path drag feature (744cd2f): each pen presses once, glides through its whole stroke, and releases once — so the letters are smooth continuous strokes, not the chain-of-clicks from before. Multiple cursors write concurrently into a background window while the https://raw.githubusercontent.com/trycua/cua/r33d/assets-linux-multi-cursor/cua-driver-write.webm Each lowercase letter is a few stroke polylines fed as 🤖 Generated with Claude Code |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Just "cua" — smooth multi-cursor held-path dragsBack to the short wordmark: 4 cursors write "cua" concurrently into a background window (focus stays on the https://raw.githubusercontent.com/trycua/cua/r33d/assets-linux-multi-cursor/cua-write.webm ~0.26 MB. (raw link → click to play, or drag into a comment to embed.) 🤖 Generated with Claude Code |
…#1871) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Cursive "Cua" — original letterforms, both windows, fixed transitionAddresses the three issues:
Native https://raw.githubusercontent.com/trycua/cua/r33d/assets-linux-multi-cursor/cua-cursive-2win.webm ~0.33 MB. (raw link → click to play, or drag into a comment to embed.) 🤖 Generated with Claude Code |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pushing it: cursive "Cua" on 16 windows at once16 paint windows in a 4×4 grid, each getting the full connected cursive "Cua" written by its own cursor — 16 MPX master pointers drawing concurrently — while the https://raw.githubusercontent.com/trycua/cua/r33d/assets-linux-multi-cursor/cua-16windows.webm Plus the earlier ask — a plain real-time, no-fade "cua": https://raw.githubusercontent.com/trycua/cua/r33d/assets-linux-multi-cursor/cua-cursive-realtime.webm Notes: 16 cursors = 16 masters + 16 uinput slaves created in the one call (~7s of that is device setup, trimmed from the clip; the draw itself is real-time). One cursor per window here (the whole cursive as a single held stroke); 4-per-window × 16 = 64 masters would exceed X's device limits, so per-window is the sweet spot for going wide. 🤖 Generated with Claude Code |
…argoHash - Add path_tests: path_cumulative / point_on_path (arc-length glide) and sample_function (y=f(x) sampling: linear, affine, trig, invalid-expr, non-finite drop). Factor the fn sampling out of the tool into input::sample_function so it's unit-testable without X. - Reset nix cargoHash to "" so CI prints the correct got: hash for the new meval/nom/fnv dependency set (will be filled in next commit). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The function-path drag feature added the meval crate (+ nom, fnv) to Cargo.lock, which changed the vendored dependency set; the nix cua-driver build (and every nix integration job that depends on it) failed with a cargoHash mismatch. Set the new fetchCargoVendor hash. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ulated VM) The cargoHash fix turned the nix suite green except this one test, which fails because a hand-launched real Xorg (dummy+libinput) — required so the MPX uinput slaves enumerate as X input devices — does not start within the timeout in the emulated GHA nixos-test VM (uinput loads fine; the 'Start a real Xorg' subtest times out). The parallel_mouse_drag / held-path / fn feature is covered by unit tests (platform-linux/src/input/mod.rs). Remove this scenario from the flake checks + CI matrix + artifact list; keep the .nix file (documented) for local / real-X manual runs. Can be brought back to CI via services.xserver if a reliable real-X env is wired up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rver
Replaces the hand-launched-Xorg linux-parallel-drag-gif.nix (which timed
out because a self-launched Xorg can't get a VT/seat in the emulated
nixos-test VM) with a version that lets NixOS bring Xorg up properly:
- services.xserver with the `dummy` video driver + a 1280x1024 virtual
screen, libinput input backend, and an icewm window manager
- lightdm + services.displayManager.autoLogin a normal user into the
icewm session; the session runs `xhost +local:` so the root-run
cua-driver / test clients can connect to :0
- boot.kernelModules uinput + udev rule so MPX slaves enumerate as X
input devices under libinput
Same proof as before: two per-session master pointers draw concurrent,
window-targeted XI2 events into one unfocused window (assert 2 distinct
devices + >=4 motions), the shield grab keeps focus on a separate control
window, and a GIF artifact is produced. First-run diagnostics dump the
display-manager journal / Xorg log if X fails to come up.
Wired into flake checks and the nix-build matrix (+ visual-artifact list).
The old GIF scenario file is kept for local/real-X manual runs.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The first CI run booted fine — wait_for_x() confirmed /tmp/.X11-unix/X0 and the graphical target, lightdm autologged-in the user, and the session's `xhost +local:` ran — but root's xdpyinfo on :0 still timed out for 60s. The server's auth ACL only lists the autologin user, and xhost +local: did not reliably grant the other uid. Disable access control outright with the X `-ac` flag (fine for a throwaway single-user test VM); keep xhost as a fallback. Also extend the bring-up diagnostics to cover the xdpyinfo step. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Both prior CI runs failed at the same line — the X bring-up subtest's `DISPLAY=:0 xdpyinfo` probe timed out — not because root couldn't reach :0 but because xdpyinfo was never in systemPackages, so the command was not-found and thus always nonzero. The dumped Xorg log shows X itself is healthy (XINPUT enumerates devices). Add xorg.xdpyinfo so the probe runs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Patch: Linux background drag + held-button tools with MPX parallel drags, function-path held glides, focus-shield grab, and install_ffmpeg (#1871); Linux agent cursor + typing in background terminals, XTEST keyboard injection (#1789). Changelog gains 0.5.3 and backfills the missing 0.5.2 entry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The MPX multi-cursor work (#1871) pulled in the raw-Xlib x11 crate, whose build.rs locates libX11/libXi/libXtst via pkg-config. The nix build got these inputs in #1871, but the CD workflow's linux-x86_64 job had no system deps step, so the v0.5.3 release build failed on the x11 crate's build script. Install the same dev libs via apt. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This pull request adds support for a "pressed" (held-button) visual state to the cursor overlay system, and refactors the Linux overlay backend to support multiple simultaneous overlays (multi-cursor) via keying. The changes include new overlay commands, new rendering logic for the pressed state, and significant refactoring to manage multiple cursors in the Linux backend.
New cursor pressed state and visual feedback:
pressedfield toRenderStateCoreand a newSetPressedcommand toOverlayCommand, allowing the overlay to visually indicate when a button is held. The rendering code now draws an enlarged bloom and ring effect when pressed. [1] [2] [3] [4] [5] [6]New overlay commands and improvements:
SnapTocommand toOverlayCommandfor immediately moving the cursor to a given position, optionally updating heading. [1] [2]Linux overlay backend refactor for multi-cursor:
RenderMapkeyed byCursorKey, supporting per-cursor state, commands, and removal. Functions likesend_command,is_enabled, andcurrent_positionnow have per-cursor variants. [1] [2] [3] [4] [5] [6] [7]X11 overlay improvements:
shape_rectangleswith an empty rectangle list instead of an empty pixmap, ensuring proper click-through behavior.Dependency updates:
x11andevdevcrates to Linux overlay dependencies for future input and X11 integration. [1] [2]Summary by CodeRabbit
New Features
Bug Fixes
Infrastructure