Skip to content
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

Remove uses of GrabServer #1283

Merged
merged 18 commits into from
Jun 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@
* `xcb-dpms` is not needed anymore.
* `libXext` is not needed anymore.

## Behind the scene changes

* The X critical section is removed, picom no longer grabs the server to fetch updates. Hopefully, if everything works, this change is unnoticeable. Minor responsiveness improvements could be possible, but I won't bet on it. The main point of this change is this makes debugging much less painful. Previously if you breaks inside the X critical section, the whole X server will lock up, and you would have to connect to the computer remotely to recover. Now there is no longer such worries. This also avoids a bug inside Xorg that makes server grabbing unreliable.

# v11.2 (2024-Feb-13)

## Build changes
Expand Down
2 changes: 0 additions & 2 deletions src/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,6 @@ typedef struct session {
// === Display related ===
/// X connection
struct x_connection c;
/// Whether the X server is grabbed by us
bool server_grabbed;
/// Width of root window.
int root_width;
/// Height of root window.
Expand Down
140 changes: 131 additions & 9 deletions src/event.c
Original file line number Diff line number Diff line change
Expand Up @@ -190,16 +190,128 @@

#undef CASESTRRET

struct ev_ewmh_active_win_request {
struct x_async_request_base base;
session_t *ps;
};

/// Update current active window based on EWMH _NET_ACTIVE_WIN.
///
/// Does not change anything if we fail to get the attribute or the window
/// returned could not be found.
static void
update_ewmh_active_win(struct x_connection * /*c*/, struct x_async_request_base *req_base,

Check warning on line 203 in src/event.c

View check run for this annotation

Codecov / codecov/patch

src/event.c#L203

Added line #L203 was not covered by tests
xcb_raw_generic_event_t *reply_or_error) {
auto ps = ((struct ev_ewmh_active_win_request *)req_base)->ps;
free(req_base);

Check warning on line 206 in src/event.c

View check run for this annotation

Codecov / codecov/patch

src/event.c#L205-L206

Added lines #L205 - L206 were not covered by tests

if (reply_or_error->response_type == 0) {
log_error("Failed to get _NET_ACTIVE_WINDOW: %s",

Check warning on line 209 in src/event.c

View check run for this annotation

Codecov / codecov/patch

src/event.c#L208-L209

Added lines #L208 - L209 were not covered by tests
x_strerror(((xcb_generic_error_t *)reply_or_error)));
return;

Check warning on line 211 in src/event.c

View check run for this annotation

Codecov / codecov/patch

src/event.c#L211

Added line #L211 was not covered by tests
}

// Search for the window
auto reply = (xcb_get_property_reply_t *)reply_or_error;
if (reply->type == XCB_NONE || xcb_get_property_value_length(reply) < 4) {
log_debug("EWMH _NET_ACTIVE_WINDOW not set.");
return;

Check warning on line 218 in src/event.c

View check run for this annotation

Codecov / codecov/patch

src/event.c#L215-L218

Added lines #L215 - L218 were not covered by tests
}

auto wid = *(xcb_window_t *)xcb_get_property_value(reply);
log_debug("EWMH _NET_ACTIVE_WINDOW is %#010x", wid);

Check warning on line 222 in src/event.c

View check run for this annotation

Codecov / codecov/patch

src/event.c#L221-L222

Added lines #L221 - L222 were not covered by tests

auto cursor = wm_find_by_client(ps->wm, wid);
auto w = cursor ? wm_ref_deref(cursor) : NULL;

Check warning on line 225 in src/event.c

View check run for this annotation

Codecov / codecov/patch

src/event.c#L224-L225

Added lines #L224 - L225 were not covered by tests

// Mark the window focused. No need to unfocus the previous one.
if (w) {
win_set_focused(ps, w);

Check warning on line 229 in src/event.c

View check run for this annotation

Codecov / codecov/patch

src/event.c#L228-L229

Added lines #L228 - L229 were not covered by tests
}
}

struct ev_recheck_focus_request {
struct x_async_request_base base;
session_t *ps;
};

/**
* Recheck currently focused window and set its <code>w->focused</code>
* to true.
*
* @param ps current session
* @return struct _win of currently focused window, NULL if not found
*/
static void recheck_focus(struct x_connection * /*c*/, struct x_async_request_base *req_base,
xcb_raw_generic_event_t *reply_or_error) {
auto ps = ((struct ev_ewmh_active_win_request *)req_base)->ps;
free(req_base);

// Determine the currently focused window so we can apply appropriate
// opacity on it
if (reply_or_error->response_type == 0) {
// Not able to get input focus means very not good things...
auto e = (xcb_generic_error_t *)reply_or_error;
log_error_x_error(e, "Failed to get focused window.");
return;

Check warning on line 256 in src/event.c

View check run for this annotation

Codecov / codecov/patch

src/event.c#L254-L256

Added lines #L254 - L256 were not covered by tests
}

auto reply = (xcb_get_input_focus_reply_t *)reply_or_error;
xcb_window_t wid = reply->focus;
log_debug("Current focused window is %#010x", wid);
if (wid == XCB_NONE || wid == XCB_INPUT_FOCUS_POINTER_ROOT ||
wid == ps->c.screen_info->root) {
// Focus is not on a toplevel.
return;
}

auto cursor = wm_find(ps->wm, wid);
if (cursor == NULL) {
if (wm_is_consistent(ps->wm)) {
log_error("Window %#010x not found in window tree.", wid);
assert(false);

Check warning on line 272 in src/event.c

View check run for this annotation

Codecov / codecov/patch

src/event.c#L270-L272

Added lines #L270 - L272 were not covered by tests
}
return;

Check warning on line 274 in src/event.c

View check run for this annotation

Codecov / codecov/patch

src/event.c#L274

Added line #L274 was not covered by tests
}

cursor = wm_ref_toplevel_of(ps->wm, cursor);
if (cursor == NULL) {
assert(!wm_is_consistent(ps->wm));
return;

Check warning on line 280 in src/event.c

View check run for this annotation

Codecov / codecov/patch

src/event.c#L279-L280

Added lines #L279 - L280 were not covered by tests
}

// And we set the focus state here
auto w = wm_ref_deref(cursor);
if (w) {
log_debug("%#010x (%s) focused.", wid, w->name);
win_set_focused(ps, w);
}
}

static inline void ev_focus_change(session_t *ps) {
if (ps->o.use_ewmh_active_win) {
// Not using focus_in/focus_out events.
return;

Check warning on line 294 in src/event.c

View check run for this annotation

Codecov / codecov/patch

src/event.c#L294

Added line #L294 was not covered by tests
}

auto req = ccalloc(1, struct ev_recheck_focus_request);
req->base.sequence = xcb_get_input_focus(ps->c.c).sequence;
req->base.callback = recheck_focus;
req->ps = ps;
x_await_request(&ps->c, &req->base);
log_debug("Started async request to recheck focus");
}

static inline void ev_focus_in(session_t *ps, xcb_focus_in_event_t *ev) {
log_debug("{ mode: %s, detail: %s }\n", ev_focus_mode_name(ev),
log_debug("{ mode: %s, detail: %s }", ev_focus_mode_name(ev),
ev_focus_detail_name(ev));
ps->pending_updates = true;
ev_focus_change(ps);
}

static inline void ev_focus_out(session_t *ps, xcb_focus_out_event_t *ev) {
log_debug("{ mode: %s, detail: %s }\n", ev_focus_mode_name(ev),
log_debug("{ mode: %s, detail: %s }", ev_focus_mode_name(ev),
ev_focus_detail_name(ev));
ps->pending_updates = true;
ev_focus_change(ps);
}

static inline void ev_create_notify(session_t *ps, xcb_create_notify_event_t *ev) {
Expand Down Expand Up @@ -293,7 +405,7 @@
}

if (ev->window == ps->c.screen_info->root) {
set_root_flags(ps, ROOT_FLAGS_CONFIGURED);
configure_root(ps);
} else {
configure_win(ps, ev);
}
Expand Down Expand Up @@ -425,6 +537,8 @@
}

static inline void ev_property_notify(session_t *ps, xcb_property_notify_event_t *ev) {
log_debug("{ atom = %#010x, window = %#010x, state = %d }", ev->atom, ev->window,
ev->state);
if (unlikely(log_get_level_tls() <= LOG_LEVEL_TRACE)) {
// Print out changed atom
xcb_get_atom_name_reply_t *reply = xcb_get_atom_name_reply(
Expand All @@ -442,8 +556,16 @@

if (ps->c.screen_info->root == ev->window) {
if (ps->o.use_ewmh_active_win && ps->atoms->a_NET_ACTIVE_WINDOW == ev->atom) {
// to update focus
ps->pending_updates = true;
auto req = ccalloc(1, struct ev_ewmh_active_win_request);
req->base.sequence =
xcb_get_property(ps->c.c, 0, ps->c.screen_info->root,
ps->atoms->a_NET_ACTIVE_WINDOW,

Check warning on line 562 in src/event.c

View check run for this annotation

Codecov / codecov/patch

src/event.c#L559-L562

Added lines #L559 - L562 were not covered by tests
XCB_ATOM_WINDOW, 0, 1)
.sequence;
req->base.callback = update_ewmh_active_win;
req->ps = ps;
x_await_request(&ps->c, &req->base);
log_debug("Started async request to get _NET_ACTIVE_WINDOW");

Check warning on line 568 in src/event.c

View check run for this annotation

Codecov / codecov/patch

src/event.c#L564-L568

Added lines #L564 - L568 were not covered by tests
} else {
// Destroy the root "image" if the wallpaper probably changed
if (x_is_root_back_pixmap_atom(ps->atoms, ev->atom)) {
Expand Down Expand Up @@ -712,9 +834,9 @@
ev_shape_notify(ps, (xcb_shape_notify_event_t *)ev);
break;
}
if (ps->randr_exists &&
if (ps->randr_exists && ps->o.crop_shadow_to_monitor &&
ev->response_type == (ps->randr_event + XCB_RANDR_SCREEN_CHANGE_NOTIFY)) {
set_root_flags(ps, ROOT_FLAGS_SCREEN_CHANGE);
x_update_monitors_async(&ps->c, &ps->monitors);

Check warning on line 839 in src/event.c

View check run for this annotation

Codecov / codecov/patch

src/event.c#L839

Added line #L839 was not covered by tests
break;
}
if (ps->damage_event + XCB_DAMAGE_NOTIFY == ev->response_type) {
Expand Down
Loading