-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix (x11): replace xfixes with x11rb in set_hittest #4416
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Fixes rust-windowing#4120 This commit is fundamentally a workaround for an underlying bug in either the xfixes extension or how (recent versions of) DEs/window managers interact with it. However it also removes the unnecessary abstraction (and risks that come with it) that `xfixes` brings, so I think is worthwhile regardless. It brings us closer to the core X libraries and reduces dependencies. On many platforms xfixes was simply not changing the rectangle at all. This can be verified with the following test program; on platforms with the issue, this will create a window that can be clicked through and sets the correct rectangles, but if switched to using xfixes the window will intercept mouse clicks and the new rectangle will not be set. The size of the newly-set window input rectangle will be printed to the console as verification of the change. ```rust use x11rb::connection::Connection; use x11rb::protocol::shape::{ConnectionExt as ShapeExt, SK, SO}; use x11rb::protocol::xproto::{ ConnectionExt as XProtoExt, CreateWindowAux, Rectangle, WindowClass, }; use x11rb::rust_connection::RustConnection; use x11rb::protocol::xfixes::{set_window_shape_region, ConnectionExt}; use winit::platform::x11; use x11rb::protocol::xproto::ClipOrdering; fn main() -> Result<(), Box<dyn std::error::Error>> { let (conn, screen_num) = RustConnection::connect(None)?; let screen = &conn.setup().roots[screen_num]; let root = screen.root; // Create a managed window (override_redirect = false) let win = conn.generate_id()?; let aux = CreateWindowAux::new().background_pixel(screen.white_pixel).override_redirect(1); conn.create_window( screen.root_depth, win, root, 100, 100, 200, 200, 0, WindowClass::INPUT_OUTPUT, 0, &aux, )?; conn.map_window(win)?; println!("Window created (id: {:#x}). override_redirect=true", win); use x11rb::protocol::shape::SK; // Generate an empty region with xfixes - not working let region = conn.generate_id()?; let empty_rect = Rectangle { x: 0, y: 0, width: 1, height: 1 }; conn.xfixes_create_region(region, &[empty_rect])?; conn.xfixes_set_region(region, &[empty_rect])?; // // Apply it to the window’s input shape // set_window_shape_region(&conn, win, SK::INPUT, 0, 0, region)?; // conn.flush()?; // Working - directly use the underlying shape rectangles api conn.shape_rectangles( SO::SET, SK::INPUT, ClipOrdering::UNSORTED, win, 0, 0, &[], // empty list = empty input shape )?; conn.flush()?; // --- Verify with shape_get_rectangles --- let reply = conn.shape_get_rectangles(win, SK::INPUT)?.reply()?; if reply.rectangles.is_empty() { println!("Input shape is empty → window is click-through"); } else { println!("Input shape has {} rectangles:", reply.rectangles.len()); for (i, r) in reply.rectangles.iter().enumerate() { println!(" rect[{i}]: x={} y={} w={} h={}", r.x, r.y, r.width, r.height); } } loop { conn.flush()?; std::thread::sleep(std::time::Duration::from_millis(100)); } } ```
|
Can you link the relevant code in xfixes for the reference? |
|
You mean the xfixes code for setting the region rectangles? Sure, I'll dig that up. |
|
This was the most authoritative documentation I could find around the shape system in general. If you search for region in this document, it covers using region to set the shape rectangles. I think this also verifies that using xfixes for this task is overkill, because it looks like the purpose of the xfixes extension is to combine multiple rectangles easily, whereas we only care about setting one rectangle here. I couldn't find better docs for the functions themselves than the automatically created ones, which aren't particularly illuminating around behaviour or intended usage. https://www.x.org/releases/current/doc/man/man3/xcb_shape_rectangles.3.xhtml https://www.x.org/releases/current/doc/man/man3/xcb_xfixes_set_window_shape_region.3.xhtml |
|
I was mostly asking for code in system library(libXfixes?) that is in question, x11 docs are not clear I think. |
|
https://salsa.debian.org/xorg-team/lib/libxfixes/-/blame/debian-unstable/src/Region.c#L390 void
XFixesSetWindowShapeRegion (Display *dpy, Window win, int shape_kind,
int x_off, int y_off, XserverRegion region)
{
XFixesExtDisplayInfo *info = XFixesFindDisplay (dpy);
xXFixesSetWindowShapeRegionReq *req;
XFixesSimpleCheckExtension (dpy, info);
LockDisplay (dpy);
GetReq (XFixesSetWindowShapeRegion, req);
req->reqType = info->codes->major_opcode;
req->xfixesReqType = X_XFixesSetWindowShapeRegion;
req->dest = win;
req->destKind = shape_kind;
req->xOff = x_off;
req->yOff = y_off;
req->region = region;
UnlockDisplay (dpy);
SyncHandle();
} |
|
can't find another def of Region |
Fixes #4120
I originally developed this fix for talon's fork in talonvoice#1 - there is some more detail and discussion there, as well as in #4409. That has been tested by at least four people on the talon side now, who have all confirmed it fixed the issue they were having.
This PR is code-equivalent to to #4415, but with more verbose documentation (and from the person who investigated and developed the fix). It will really just be a maintainer repo style preference that determines which gets merged.
This commit is fundamentally a workaround for an underlying bug in either the xfixes extension or how (recent versions of) DEs/window managers interact with it. However it also removes the unnecessary abstraction (and risks that come with it) that
xfixesbrings, so I think is worthwhile regardless. It brings us closer to the core X libraries and reduces dependencies.On many platforms xfixes was simply not changing the rectangle at all. This can be verified with the following test program; on platforms with the issue, this will create a window that can be clicked through and sets the correct rectangles, but if switched to using xfixes the window will intercept mouse clicks and the rectangles will be unchanged. The size of the newly-set window input rectangle will be printed to the console as verification of the change
This test program demos the library differences, the test program below this one demonstrates the code path via winit.
And with winit:
changelogmodule if knowledge of this change could be valuable to users