diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b3d69a3..7038b3e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/common.h b/src/common.h index 7892d645..635fb973 100644 --- a/src/common.h +++ b/src/common.h @@ -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. diff --git a/src/event.c b/src/event.c index 67cdaaaf..b2e9558e 100644 --- a/src/event.c +++ b/src/event.c @@ -190,16 +190,128 @@ static inline const char *attr_pure ev_focus_detail_name(xcb_focus_in_event_t *e #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, + xcb_raw_generic_event_t *reply_or_error) { + auto ps = ((struct ev_ewmh_active_win_request *)req_base)->ps; + free(req_base); + + if (reply_or_error->response_type == 0) { + log_error("Failed to get _NET_ACTIVE_WINDOW: %s", + x_strerror(((xcb_generic_error_t *)reply_or_error))); + return; + } + + // 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; + } + + auto wid = *(xcb_window_t *)xcb_get_property_value(reply); + log_debug("EWMH _NET_ACTIVE_WINDOW is %#010x", wid); + + auto cursor = wm_find_by_client(ps->wm, wid); + auto w = cursor ? wm_ref_deref(cursor) : NULL; + + // Mark the window focused. No need to unfocus the previous one. + if (w) { + win_set_focused(ps, w); + } +} + +struct ev_recheck_focus_request { + struct x_async_request_base base; + session_t *ps; +}; + +/** + * Recheck currently focused window and set its w->focused + * 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; + } + + 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); + } + return; + } + + cursor = wm_ref_toplevel_of(ps->wm, cursor); + if (cursor == NULL) { + assert(!wm_is_consistent(ps->wm)); + return; + } + + // 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; + } + + 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) { @@ -293,7 +405,7 @@ static inline void ev_configure_notify(session_t *ps, xcb_configure_notify_event } if (ev->window == ps->c.screen_info->root) { - set_root_flags(ps, ROOT_FLAGS_CONFIGURED); + configure_root(ps); } else { configure_win(ps, ev); } @@ -425,6 +537,8 @@ static inline void ev_expose(session_t *ps, xcb_expose_event_t *ev) { } 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( @@ -442,8 +556,16 @@ static inline void ev_property_notify(session_t *ps, xcb_property_notify_event_t 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, + 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"); } else { // Destroy the root "image" if the wallpaper probably changed if (x_is_root_back_pixmap_atom(ps->atoms, ev->atom)) { @@ -712,9 +834,9 @@ void ev_handle(session_t *ps, xcb_generic_event_t *ev) { 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); break; } if (ps->damage_event + XCB_DAMAGE_NOTIFY == ev->response_type) { diff --git a/src/picom.c b/src/picom.c index 14689bc8..a49c9129 100644 --- a/src/picom.c +++ b/src/picom.c @@ -125,12 +125,6 @@ const char *const BACKEND_STRS[] = {[BKEND_XRENDER] = "xrender", /// XXX Limit what xerror can access by not having this pointer session_t *ps_g = NULL; -void set_root_flags(session_t *ps, uint64_t flags) { - log_debug("Setting root flags: %" PRIu64, flags); - ps->root_flags |= flags; - ps->pending_updates = true; -} - void quit(session_t *ps) { ps->quit = true; ev_break(ps->loop, EVBREAK_ALL); @@ -449,81 +443,6 @@ void add_damage(session_t *ps, const region_t *damage) { // === Windows === -/** - * 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. - */ -void update_ewmh_active_win(session_t *ps) { - // Search for the window - bool exists; - xcb_window_t wid = wid_get_prop_window(&ps->c, ps->c.screen_info->root, - ps->atoms->a_NET_ACTIVE_WINDOW, &exists); - - auto cursor = wm_find_by_client(ps->wm, wid); - auto w = cursor ? wm_ref_deref(cursor) : NULL; - - // Mark the window focused. No need to unfocus the previous one. - if (w) { - win_set_focused(ps, w); - } -} - -/** - * Recheck currently focused window and set its w->focused - * to true. - * - * @param ps current session - * @return struct _win of currently focused window, NULL if not found - */ -static void recheck_focus(session_t *ps) { - // Use EWMH _NET_ACTIVE_WINDOW if enabled - if (ps->o.use_ewmh_active_win) { - update_ewmh_active_win(ps); - return; - } - - // Determine the currently focused window so we can apply appropriate - // opacity on it - xcb_generic_error_t *e = NULL; - auto reply = xcb_get_input_focus_reply(ps->c.c, xcb_get_input_focus(ps->c.c), &e); - if (reply == NULL) { - // Not able to get input focus means very not good things... - log_error_x_error(e, "Failed to get focused window."); - free(e); - return; - } - xcb_window_t wid = reply->focus; - free(reply); - - 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) { - log_error("Window %#010x not found in window tree.", wid); - return; - } - - cursor = wm_ref_toplevel_of(ps->wm, cursor); - if (cursor == NULL) { - assert(!wm_is_consistent(ps->wm)); - return; - } - - // And we set the focus state here - auto w = wm_ref_deref(cursor); - if (w) { - log_debug("%#010" PRIx32 " (%#010" PRIx32 " \"%s\") focused.", wid, - win_id(w), w->name); - win_set_focused(ps, w); - } -} - /** * Rebuild cached screen_reg. */ @@ -727,7 +646,7 @@ static inline void invalidate_reg_ignore(session_t *ps) { } /// Handle configure event of the root window -static void configure_root(session_t *ps) { +void configure_root(session_t *ps) { // TODO(yshui) re-initializing backend should be done outside of the // critical section. Probably set a flag and do it in draw_callback_impl. auto r = XCB_AWAIT(xcb_get_geometry, ps->c.c, ps->c.screen_info->root); @@ -808,20 +727,6 @@ static void configure_root(session_t *ps) { } } -static void handle_root_flags(session_t *ps) { - if ((ps->root_flags & ROOT_FLAGS_SCREEN_CHANGE) != 0) { - if (ps->o.crop_shadow_to_monitor && ps->randr_exists) { - x_update_monitors(&ps->c, &ps->monitors); - } - ps->root_flags &= ~(uint64_t)ROOT_FLAGS_SCREEN_CHANGE; - } - - if ((ps->root_flags & ROOT_FLAGS_CONFIGURED) != 0) { - configure_root(ps); - ps->root_flags &= ~(uint64_t)ROOT_FLAGS_CONFIGURED; - } -} - /** * Go through the window stack and calculate some parameters for rendering. * @@ -941,7 +846,7 @@ static bool paint_preprocess(session_t *ps, bool *animation, struct win **out_bo } else if (w->paint_excluded) { log_trace("|- is excluded from painting"); to_paint = false; - } else if (unlikely((w->flags & WIN_FLAGS_IMAGE_ERROR) != 0)) { + } else if (unlikely((w->flags & WIN_FLAGS_PIXMAP_ERROR) != 0)) { log_trace("|- has image errors"); to_paint = false; } @@ -1541,6 +1446,10 @@ static void unredirect(session_t *ps) { /// keeps an internal queue of events, so we have to be 100% sure no events are /// left in that queue before we go to sleep. static void handle_x_events(struct session *ps) { + if (ps->vblank_scheduler) { + vblank_handle_x_events(ps->vblank_scheduler); + } + // Flush because if we go into sleep when there is still requests in the // outgoing buffer, they will not be sent for an indefinite amount of // time. Use XFlush here too, we might still use some Xlib functions @@ -1557,10 +1466,6 @@ static void handle_x_events(struct session *ps) { XFlush(ps->c.dpy); xcb_flush(ps->c.c); - if (ps->vblank_scheduler) { - vblank_handle_x_events(ps->vblank_scheduler); - } - xcb_generic_event_t *ev; while ((ev = x_poll_for_event(&ps->c))) { ev_handle(ps, (xcb_generic_event_t *)ev); @@ -1578,6 +1483,66 @@ static void handle_x_events_ev(EV_P attr_unused, ev_prepare *w, int revents attr handle_x_events(ps); } +struct new_window_attributes_request { + struct x_async_request_base base; + struct session *ps; + xcb_window_t wid; +}; + +static void handle_new_window_attributes_reply(struct x_connection * /*c*/, + struct x_async_request_base *req_base, + xcb_raw_generic_event_t *reply_or_error) { + auto req = (struct new_window_attributes_request *)req_base; + auto ps = req->ps; + auto wid = req->wid; + auto new_window = wm_find(ps->wm, wid); + free(req); + + if (reply_or_error->response_type == 0) { + log_debug("Failed to get window attributes for newly created window " + "%#010x", + wid); + return; + } + + if (new_window == NULL) { + // Highly unlikely. This window is destroyed, then another window is + // created with the same window ID before this request completed, and the + // latter window isn't in our tree yet. + if (wm_is_consistent(ps->wm)) { + log_error("Newly created window %#010x is not in the window tree", wid); + assert(false); + } + return; + } + + auto toplevel = wm_ref_toplevel_of(ps->wm, new_window); + if (toplevel != new_window) { + log_debug("Newly created window %#010x was moved away from toplevel " + "while we were waiting for its attributes", + wid); + return; + } + if (wm_ref_deref(toplevel) != NULL) { + // This is possible if a toplevel is reparented away, then reparented to + // root so it became a toplevel again. If the GetWindowAttributes request + // sent for the first time it became a toplevel wasn't completed for this + // whole duration, it will create a managed window object for the + // toplevel. But there is another get attributes request sent the + // second time it became a toplevel. When we get the reply for the second + // request, we will reach here. + log_debug("Newly created window %#010x is already managed", wid); + return; + } + + auto w = win_maybe_allocate(ps, toplevel, + (xcb_get_window_attributes_reply_t *)reply_or_error); + if (w != NULL && w->a.map_state == XCB_MAP_STATE_VIEWABLE) { + win_map_start(w); + ps->pending_updates = true; + } +} + static void handle_new_windows(session_t *ps) { // Check tree changes first, because later property updates need accurate // client window information @@ -1588,17 +1553,22 @@ static void handle_new_windows(session_t *ps) { break; } switch (wm_change.type) { - case WM_TREE_CHANGE_TOPLEVEL_NEW: - w = win_maybe_allocate(ps, wm_change.toplevel); - if (w != NULL && w->a.map_state == XCB_MAP_STATE_VIEWABLE) { - win_map_start(w); - } + case WM_TREE_CHANGE_TOPLEVEL_NEW:; + auto req = ccalloc(1, struct new_window_attributes_request); + // We don't directly record the toplevel wm_ref here, because any + // number of things could happen before we get the reply. The + // window can be reparented, destroyed, then get its window ID + // reused, etc. + req->wid = wm_ref_win_id(wm_change.toplevel); + req->ps = ps; + req->base.callback = handle_new_window_attributes_reply, + req->base.sequence = + xcb_get_window_attributes(ps->c.c, req->wid).sequence; + x_await_request(&ps->c, &req->base); break; case WM_TREE_CHANGE_TOPLEVEL_KILLED: w = wm_ref_deref(wm_change.toplevel); if (w != NULL) { - // Pointing the window tree_ref to the zombie. - w->tree_ref = wm_change.toplevel; win_destroy_start(ps, w); } else { // This window is not managed, no point keeping the zombie @@ -1655,56 +1625,21 @@ static void tmout_unredir_callback(EV_P attr_unused, ev_timer *w, int revents at } static void handle_pending_updates(struct session *ps, double delta_t) { - log_trace("Delayed handling of events, entering critical section"); - auto e = xcb_request_check(ps->c.c, xcb_grab_server_checked(ps->c.c)); - if (e) { - log_fatal_x_error(e, "failed to grab x server"); - free(e); - return quit(ps); - } + // Process new windows, and maybe allocate struct managed_win for them + handle_new_windows(ps); - ps->server_grabbed = true; + if (ps->pending_updates) { + log_debug("Delayed handling of events"); - // Catching up with X server - // Handle all events X server has sent us so far, so that our internal state will - // catch up with the X server's state. It only makes sense to call this function - // in the X critical section, otherwise we will be chasing a moving goal post. - // - // (Disappointingly, grabbing the X server doesn't actually prevent the X server - // state from changing. It should, but in practice we have observed window still - // being destroyed while we have the server grabbed. This is very disappointing - // and forces us to use some hacky code to recover when we discover we are - // out-of-sync.) - handle_x_events(ps); - if (ps->pending_updates || wm_has_tree_changes(ps->wm)) { - log_debug("Delayed handling of events, entering critical section"); - // Process new windows, and maybe allocate struct managed_win for them - handle_new_windows(ps); - - // Handle screen changes - // This HAS TO be called before refresh_windows, as handle_root_flags - // could call configure_root, which will release images and mark them - // stale. - handle_root_flags(ps); - - // Process window flags + // Process window flags. This needs to happen before `refresh_images` + // because this might set the pixmap stale window flag. refresh_windows(ps); - - recheck_focus(ps); - - // Process window flags (stale images) - refresh_images(ps); - } - e = xcb_request_check(ps->c.c, xcb_ungrab_server_checked(ps->c.c)); - if (e) { - log_fatal_x_error(e, "failed to ungrab x server"); - free(e); - return quit(ps); } - ps->server_grabbed = false; + // Process window flags (stale images) + refresh_images(ps); + ps->pending_updates = false; - log_trace("Exited critical section"); wm_stack_foreach_safe(ps->wm, cursor, tmp) { auto w = wm_ref_deref(cursor); @@ -1975,13 +1910,9 @@ static void draw_callback(EV_P_ ev_timer *w, int revents) { } } -static void x_event_callback(EV_P attr_unused, ev_io *w, int revents attr_unused) { - session_t *ps = (session_t *)w; - xcb_generic_event_t *ev = x_poll_for_event(&ps->c); - if (ev) { - ev_handle(ps, ev); - free(ev); - } +static void x_event_callback(EV_P attr_unused, ev_io * /*w*/, int revents attr_unused) { + // This function is intentionally left blank, events are actually read and handled + // in the ev_prepare listener. } static void config_file_change_cb(void *_ps) { @@ -2408,7 +2339,7 @@ static session_t *session_init(int argc, char **argv, Display *dpy, if (ps->randr_exists && ps->o.crop_shadow_to_monitor) { xcb_randr_select_input(ps->c.c, ps->c.screen_info->root, XCB_RANDR_NOTIFY_MASK_SCREEN_CHANGE); - x_update_monitors(&ps->c, &ps->monitors); + x_update_monitors_async(&ps->c, &ps->monitors); } { diff --git a/src/picom.h b/src/picom.h index f6f7b309..73a501c6 100644 --- a/src/picom.h +++ b/src/picom.h @@ -24,12 +24,6 @@ #include "wm/win.h" #include "x.h" -enum root_flags { - ROOT_FLAGS_SCREEN_CHANGE = 1, // Received RandR screen change notify, we - // use this to track refresh rate changes - ROOT_FLAGS_CONFIGURED = 2 // Received configure notify on the root window -}; - // == Functions == // TODO(yshui) move static inline functions that are only used in picom.c, into picom.c @@ -43,7 +37,7 @@ void queue_redraw(session_t *ps); void discard_pending(session_t *ps, uint32_t sequence); -void set_root_flags(session_t *ps, uint64_t flags); +void configure_root(session_t *ps); void quit(session_t *ps); diff --git a/src/renderer/layout.c b/src/renderer/layout.c index f4095312..b34cdab0 100644 --- a/src/renderer/layout.c +++ b/src/renderer/layout.c @@ -44,6 +44,9 @@ static bool layer_from_window(struct layer *out_layer, struct win *w, ivec2 size if (!w->ever_damaged || w->paint_excluded) { goto out; } + if (w->win_image == NULL) { + goto out; + } out_layer->scale = (vec2){ .x = win_animatable_get(w, WIN_SCRIPT_SCALE_X), diff --git a/src/vblank.c b/src/vblank.c index 270f93e2..b7281360 100644 --- a/src/vblank.c +++ b/src/vblank.c @@ -401,6 +401,10 @@ static void handle_present_complete_notify(struct present_vblank_scheduler *self assert(self->base.vblank_event_requested); + struct timespec now; + clock_gettime(CLOCK_MONOTONIC, &now); + auto now_us = (unsigned long)(now.tv_sec * 1000000L + now.tv_nsec / 1000); + // X sometimes sends duplicate/bogus MSC events, when screen has just been turned // off. Don't use the msc value in these events. We treat this as not receiving a // vblank event at all, and try to get a new one. @@ -409,18 +413,15 @@ static void handle_present_complete_notify(struct present_vblank_scheduler *self // https://gitlab.freedesktop.org/xorg/xserver/-/issues/1418 bool event_is_invalid = cne->msc <= self->last_msc || cne->ust == 0; if (event_is_invalid) { - log_debug("Invalid PresentCompleteNotify event, %" PRIu64 " %" PRIu64, + log_debug("Invalid PresentCompleteNotify event, %" PRIu64 " %" PRIu64 + ". Trying to recover, reporting a fake vblank.", cne->msc, cne->ust); - x_request_vblank_event(self->base.c, cne->window, self->last_msc + 1); - return; + self->last_ust = now_us; + self->last_msc += 1; + } else { + self->last_ust = cne->ust; + self->last_msc = cne->msc; } - - self->last_ust = cne->ust; - self->last_msc = cne->msc; - - struct timespec now; - clock_gettime(CLOCK_MONOTONIC, &now); - auto now_us = (unsigned long)(now.tv_sec * 1000000L + now.tv_nsec / 1000); double delay_sec = 0.0; if (now_us < cne->ust) { log_trace("The end of this vblank is %" PRIu64 " us into the " diff --git a/src/wm/defs.h b/src/wm/defs.h index cc2abb1d..801aea6e 100644 --- a/src/wm/defs.h +++ b/src/wm/defs.h @@ -50,10 +50,8 @@ enum win_flags { /// pixmap is out of date, will be update in win_process_flags WIN_FLAGS_PIXMAP_STALE = 1, - /// window does not have pixmap bound - WIN_FLAGS_PIXMAP_NONE = 2, - /// there was an error trying to bind the images - WIN_FLAGS_IMAGE_ERROR = 4, + /// there was an error binding the window pixmap + WIN_FLAGS_PIXMAP_ERROR = 4, /// the client window needs to be updated WIN_FLAGS_CLIENT_STALE = 32, /// the window is mapped by X, we need to call map_win_start for it diff --git a/src/wm/tree.c b/src/wm/tree.c index 99e195d3..bbd452ae 100644 --- a/src/wm/tree.c +++ b/src/wm/tree.c @@ -34,78 +34,8 @@ void wm_tree_reap_zombie(struct wm_tree_node *zombie) { free(zombie); } +/// Enqueue a tree change. static void wm_tree_enqueue_change(struct wm_tree *tree, struct wm_tree_change change) { - if (change.type == WM_TREE_CHANGE_TOPLEVEL_KILLED) { - // A gone toplevel will cancel out a previous - // `WM_TREE_CHANGE_TOPLEVEL_NEW` change in the queue. - bool found = false; - list_foreach_safe(struct wm_tree_change_list, i, &tree->changes, siblings) { - if (!wm_treeid_eq(i->item.toplevel, change.toplevel)) { - continue; - } - if (i->item.type == WM_TREE_CHANGE_TOPLEVEL_NEW) { - list_remove(&i->siblings); - list_insert_after(&tree->free_changes, &i->siblings); - found = true; - } else if (found) { - // We also need to delete all other changes - // related to this toplevel in between the new and - // gone changes. - list_remove(&i->siblings); - list_insert_after(&tree->free_changes, &i->siblings); - } else if (i->item.type == WM_TREE_CHANGE_CLIENT) { - // Need to update client changes, so they points to the - // zombie instead of the old toplevel node, since the old - // toplevel node could be freed before tree changes are - // processed. - i->item.client.toplevel = change.killed; - } - } - if (found) { - wm_tree_reap_zombie(change.killed); - return; - } - } else if (change.type == WM_TREE_CHANGE_CLIENT) { - // A client change can coalesce with a previous client change. - list_foreach_safe(struct wm_tree_change_list, i, &tree->changes, siblings) { - if (!wm_treeid_eq(i->item.toplevel, change.toplevel) || - i->item.type != WM_TREE_CHANGE_CLIENT) { - continue; - } - - if (!wm_treeid_eq(i->item.client.new_, change.client.old)) { - log_warn("Inconsistent client change for toplevel " - "%#010x. Missing changes from %#010x to %#010x. " - "Possible bug.", - change.toplevel.x, i->item.client.new_.x, - change.client.old.x); - } - - i->item.client.new_ = change.client.new_; - if (wm_treeid_eq(i->item.client.old, change.client.new_)) { - list_remove(&i->siblings); - list_insert_after(&tree->free_changes, &i->siblings); - } - return; - } - } else if (change.type == WM_TREE_CHANGE_TOPLEVEL_RESTACKED) { - list_foreach(struct wm_tree_change_list, i, &tree->changes, siblings) { - if (i->item.type == WM_TREE_CHANGE_TOPLEVEL_RESTACKED || - i->item.type == WM_TREE_CHANGE_TOPLEVEL_NEW || - i->item.type == WM_TREE_CHANGE_TOPLEVEL_KILLED) { - // Only need to keep one - // `WM_TREE_CHANGE_TOPLEVEL_RESTACKED` change, and order - // doesn't matter. - return; - } - } - } - - // We don't let a `WM_TREE_CHANGE_TOPLEVEL_NEW` cancel out a previous - // `WM_TREE_CHANGE_TOPLEVEL_GONE`, because the new toplevel would be a different - // window reusing the same ID. So we need to go through the proper destruction - // process for the previous toplevel. Changes are not commutative (naturally). - struct wm_tree_change_list *change_list; if (!list_is_empty(&tree->free_changes)) { change_list = list_entry(tree->free_changes.next, @@ -119,6 +49,114 @@ static void wm_tree_enqueue_change(struct wm_tree *tree, struct wm_tree_change c list_insert_before(&tree->changes, &change_list->siblings); } +/// Enqueue a `WM_TREE_CHANGE_TOPLEVEL_KILLED` change for a toplevel window. If there are +/// any `WM_TREE_CHANGE_TOPLEVEL_NEW` changes in the queue for the same toplevel, they +/// will be cancelled out. +/// +/// @return true if this change is cancelled out by a previous change, false otherwise. +static bool wm_tree_enqueue_toplevel_killed(struct wm_tree *tree, wm_treeid toplevel, + struct wm_tree_node *zombie) { + // A gone toplevel will cancel out a previous + // `WM_TREE_CHANGE_TOPLEVEL_NEW` change in the queue. + bool found = false; + list_foreach_safe(struct wm_tree_change_list, i, &tree->changes, siblings) { + if (!wm_treeid_eq(i->item.toplevel, toplevel)) { + continue; + } + if (i->item.type == WM_TREE_CHANGE_TOPLEVEL_NEW) { + list_remove(&i->siblings); + list_insert_after(&tree->free_changes, &i->siblings); + found = true; + } else if (found) { + // We also need to delete all other changes + // related to this toplevel in between the new and + // gone changes. + list_remove(&i->siblings); + list_insert_after(&tree->free_changes, &i->siblings); + } else if (i->item.type == WM_TREE_CHANGE_CLIENT) { + // Need to update client changes, so they points to the + // zombie instead of the old toplevel node, since the old + // toplevel node could be freed before tree changes are + // processed. + i->item.client.toplevel = zombie; + } + } + if (found) { + wm_tree_reap_zombie(zombie); + return true; + } + + wm_tree_enqueue_change(tree, (struct wm_tree_change){ + .toplevel = toplevel, + .type = WM_TREE_CHANGE_TOPLEVEL_KILLED, + .killed = zombie, + }); + return false; +} + +static void wm_tree_enqueue_client_change(struct wm_tree *tree, struct wm_tree_node *toplevel, + wm_treeid old_client, wm_treeid new_client) { + // A client change can coalesce with a previous client change. + list_foreach_safe(struct wm_tree_change_list, i, &tree->changes, siblings) { + if (!wm_treeid_eq(i->item.toplevel, toplevel->id) || + i->item.type != WM_TREE_CHANGE_CLIENT) { + continue; + } + + if (!wm_treeid_eq(i->item.client.new_, old_client)) { + log_warn("Inconsistent client change for toplevel " + "%#010x. Missing changes from %#010x to %#010x. " + "Possible bug.", + toplevel->id.x, i->item.client.new_.x, old_client.x); + } + + i->item.client.new_ = new_client; + if (wm_treeid_eq(i->item.client.old, new_client)) { + list_remove(&i->siblings); + list_insert_after(&tree->free_changes, &i->siblings); + } + return; + } + wm_tree_enqueue_change(tree, (struct wm_tree_change){ + .toplevel = toplevel->id, + .type = WM_TREE_CHANGE_CLIENT, + .client = + { + .toplevel = toplevel, + .old = old_client, + .new_ = new_client, + }, + }); +} + +static void wm_tree_enqueue_toplevel_new(struct wm_tree *tree, struct wm_tree_node *toplevel) { + // We don't let a `WM_TREE_CHANGE_TOPLEVEL_NEW` cancel out a previous + // `WM_TREE_CHANGE_TOPLEVEL_KILLED`, because the new toplevel would be a different + // window reusing the same ID. So we need to go through the proper destruction + // process for the previous toplevel. Changes are not commutative (naturally). + wm_tree_enqueue_change(tree, (struct wm_tree_change){ + .toplevel = toplevel->id, + .type = WM_TREE_CHANGE_TOPLEVEL_NEW, + .new_ = toplevel, + }); +} + +static void wm_tree_enqueue_toplevel_restacked(struct wm_tree *tree) { + list_foreach(struct wm_tree_change_list, i, &tree->changes, siblings) { + if (i->item.type == WM_TREE_CHANGE_TOPLEVEL_RESTACKED || + i->item.type == WM_TREE_CHANGE_TOPLEVEL_NEW || + i->item.type == WM_TREE_CHANGE_TOPLEVEL_KILLED) { + // Only need to keep one + // `WM_TREE_CHANGE_TOPLEVEL_RESTACKED` change, and order + // doesn't matter. + return; + } + } + wm_tree_enqueue_change(tree, (struct wm_tree_change){ + .type = WM_TREE_CHANGE_TOPLEVEL_RESTACKED, + }); +} + /// Dequeue the oldest change from the change queue. If the queue is empty, a change with /// `toplevel` set to `XCB_NONE` will be returned. struct wm_tree_change wm_tree_dequeue_change(struct wm_tree *tree) { @@ -222,22 +260,15 @@ void wm_tree_set_wm_state(struct wm_tree *tree, struct wm_tree_node *node, bool return; } - struct wm_tree_change change = { - .toplevel = toplevel->id, - .type = WM_TREE_CHANGE_CLIENT, - .client = {.toplevel = toplevel}, - }; if (!has_wm_state && toplevel->client_window == node) { auto new_client = wm_tree_find_client(toplevel); toplevel->client_window = new_client; - change.client.old = node->id; - change.client.new_ = new_client != NULL ? new_client->id : WM_TREEID_NONE; - wm_tree_enqueue_change(tree, change); + wm_tree_enqueue_client_change( + tree, toplevel, node->id, + new_client != NULL ? new_client->id : WM_TREEID_NONE); } else if (has_wm_state && toplevel->client_window == NULL) { toplevel->client_window = node; - change.client.old = WM_TREEID_NONE; - change.client.new_ = node->id; - wm_tree_enqueue_change(tree, change); + wm_tree_enqueue_client_change(tree, toplevel, WM_TREEID_NONE, node->id); } else if (has_wm_state) { // If the toplevel window already has a client window, we won't // try to usurp it. @@ -268,27 +299,24 @@ wm_tree_refresh_client_and_queue_change(struct wm_tree *tree, struct wm_tree_nod BUG_ON(toplevel->parent->parent != NULL); auto new_client = wm_tree_find_client(toplevel); if (new_client != toplevel->client_window) { - struct wm_tree_change change = {.toplevel = toplevel->id, - .type = WM_TREE_CHANGE_CLIENT, - .client = {.toplevel = toplevel, - .old = WM_TREEID_NONE, - .new_ = WM_TREEID_NONE}}; + wm_treeid old_client_id = WM_TREEID_NONE, new_client_id = WM_TREEID_NONE; if (toplevel->client_window != NULL) { - change.client.old = toplevel->client_window->id; + old_client_id = toplevel->client_window->id; } if (new_client != NULL) { - change.client.new_ = new_client->id; + new_client_id = new_client->id; } toplevel->client_window = new_client; - wm_tree_enqueue_change(tree, change); + wm_tree_enqueue_client_change(tree, toplevel, old_client_id, new_client_id); } } -void wm_tree_detach(struct wm_tree *tree, struct wm_tree_node *subroot) { +struct wm_tree_node *wm_tree_detach(struct wm_tree *tree, struct wm_tree_node *subroot) { BUG_ON(subroot == NULL); BUG_ON(subroot->parent == NULL); // Trying to detach the root window?! auto toplevel = wm_tree_find_toplevel_for(tree, subroot); + struct wm_tree_node *zombie = NULL; if (toplevel != subroot) { list_remove(&subroot->siblings); if (toplevel != NULL) { @@ -296,21 +324,18 @@ void wm_tree_detach(struct wm_tree *tree, struct wm_tree_node *subroot) { } } else { // Detached a toplevel, create a zombie for it - auto zombie = ccalloc(1, struct wm_tree_node); + zombie = ccalloc(1, struct wm_tree_node); zombie->parent = subroot->parent; zombie->id = subroot->id; zombie->is_zombie = true; - zombie->win = subroot->win; - subroot->win = NULL; list_init_head(&zombie->children); list_replace(&subroot->siblings, &zombie->siblings); - wm_tree_enqueue_change(tree, (struct wm_tree_change){ - .toplevel = subroot->id, - .type = WM_TREE_CHANGE_TOPLEVEL_KILLED, - .killed = zombie, - }); + if (wm_tree_enqueue_toplevel_killed(tree, subroot->id, zombie)) { + zombie = NULL; + } } subroot->parent = NULL; + return zombie; } void wm_tree_attach(struct wm_tree *tree, struct wm_tree_node *child, @@ -329,11 +354,7 @@ void wm_tree_attach(struct wm_tree *tree, struct wm_tree_node *child, auto toplevel = wm_tree_find_toplevel_for(tree, child); if (child == toplevel) { - wm_tree_enqueue_change(tree, (struct wm_tree_change){ - .toplevel = child->id, - .type = WM_TREE_CHANGE_TOPLEVEL_NEW, - .new_ = child, - }); + wm_tree_enqueue_toplevel_new(tree, child); } else if (toplevel != NULL) { wm_tree_refresh_client_and_queue_change(tree, toplevel); } @@ -355,9 +376,7 @@ void wm_tree_move_to_end(struct wm_tree *tree, struct wm_tree_node *node, bool t list_insert_after(&node->parent->children, &node->siblings); } if (node->parent == tree->root) { - wm_tree_enqueue_change(tree, (struct wm_tree_change){ - .type = WM_TREE_CHANGE_TOPLEVEL_RESTACKED, - }); + wm_tree_enqueue_toplevel_restacked(tree); } } @@ -377,9 +396,7 @@ void wm_tree_move_to_above(struct wm_tree *tree, struct wm_tree_node *node, list_remove(&node->siblings); list_insert_before(&other->siblings, &node->siblings); if (node->parent == tree->root) { - wm_tree_enqueue_change(tree, (struct wm_tree_change){ - .type = WM_TREE_CHANGE_TOPLEVEL_RESTACKED, - }); + wm_tree_enqueue_toplevel_restacked(tree); } } @@ -433,7 +450,7 @@ TEST_CASE(tree_manipulation) { assert(change.toplevel.x == 3); assert(change.type == WM_TREE_CHANGE_TOPLEVEL_NEW); - wm_tree_detach(&tree, node2); + auto zombie = wm_tree_detach(&tree, node2); wm_tree_attach(&tree, node2, node3); assert(node2->parent == node3); assert(node3->children.next == &node2->siblings); @@ -442,6 +459,7 @@ TEST_CASE(tree_manipulation) { change = wm_tree_dequeue_change(&tree); assert(change.toplevel.x == 2); assert(change.type == WM_TREE_CHANGE_TOPLEVEL_KILLED); + TEST_EQUAL(change.killed, zombie); wm_tree_reap_zombie(change.killed); wm_tree_set_wm_state(&tree, node2, true); @@ -462,7 +480,7 @@ TEST_CASE(tree_manipulation) { // node3 already has node2 as its client window, so the new one should be ignored. assert(change.type == WM_TREE_CHANGE_NONE); - wm_tree_detach(&tree, node2); + TEST_EQUAL(wm_tree_detach(&tree, node2), NULL); HASH_DEL(tree.nodes, node2); free(node2); change = wm_tree_dequeue_change(&tree); @@ -472,7 +490,7 @@ TEST_CASE(tree_manipulation) { assert(change.client.new_.x == 4); // Test window ID reuse - wm_tree_detach(&tree, node4); + TEST_EQUAL(wm_tree_detach(&tree, node4), NULL); HASH_DEL(tree.nodes, node4); free(node4); node4 = wm_tree_new_window(&tree, 4); @@ -489,7 +507,7 @@ TEST_CASE(tree_manipulation) { auto node5 = wm_tree_new_window(&tree, 5); wm_tree_add_window(&tree, node5); wm_tree_attach(&tree, node5, root); - wm_tree_detach(&tree, node5); + TEST_EQUAL(wm_tree_detach(&tree, node5), NULL); HASH_DEL(tree.nodes, node5); free(node5); change = wm_tree_dequeue_change(&tree); diff --git a/src/wm/win.c b/src/wm/win.c index eef92171..02e6bcf3 100644 --- a/src/wm/win.c +++ b/src/wm/win.c @@ -310,13 +310,10 @@ void add_damage_from_win(session_t *ps, const struct win *w) { /// Release the images attached to this window static inline void win_release_pixmap(backend_t *base, struct win *w) { log_debug("Releasing pixmap of window %#010x (%s)", win_id(w), w->name); - assert(w->win_image); if (w->win_image) { xcb_pixmap_t pixmap = XCB_NONE; pixmap = base->ops.release_image(base, w->win_image); w->win_image = NULL; - // Bypassing win_set_flags, because `w` might have been destroyed - w->flags |= WIN_FLAGS_PIXMAP_NONE; if (pixmap != XCB_NONE) { xcb_free_pixmap(base->c->c, pixmap); } @@ -346,40 +343,13 @@ static inline void win_release_mask(backend_t *base, struct win *w) { } } -static inline bool win_bind_pixmap(struct backend_base *b, struct win *w) { - assert(!w->win_image); - auto pixmap = x_new_id(b->c); - auto e = xcb_request_check( - b->c->c, xcb_composite_name_window_pixmap_checked(b->c->c, win_id(w), pixmap)); - if (e) { - log_error("Failed to get named pixmap for window %#010x(%s)", win_id(w), - w->name); - free(e); - return false; - } - log_debug("New named pixmap for %#010x (%s) : %#010x", win_id(w), w->name, pixmap); - w->win_image = b->ops.bind_pixmap(b, pixmap, x_get_visual_info(b->c, w->a.visual)); - if (!w->win_image) { - log_error("Failed to bind pixmap"); - xcb_free_pixmap(b->c->c, pixmap); - win_set_flags(w, WIN_FLAGS_IMAGE_ERROR); - return false; - } - - win_clear_flags(w, WIN_FLAGS_PIXMAP_NONE); - return true; -} - void win_release_images(struct backend_base *backend, struct win *w) { // We don't want to decide what we should do if the image we want to // release is stale (do we clear the stale flags or not?) But if we are // not releasing any images anyway, we don't care about the stale flags. + assert(w->win_image == NULL || !win_check_flags_all(w, WIN_FLAGS_PIXMAP_STALE)); - if (!win_check_flags_all(w, WIN_FLAGS_PIXMAP_NONE)) { - assert(!win_check_flags_all(w, WIN_FLAGS_PIXMAP_STALE)); - win_release_pixmap(backend, w); - } - + win_release_pixmap(backend, w); win_release_shadow(backend, w); win_release_mask(backend, w); } @@ -568,35 +538,41 @@ void win_process_image_flags(session_t *ps, struct win *w) { return; } - // Not a loop - while (win_check_flags_any(w, WIN_FLAGS_PIXMAP_STALE) && - !win_check_flags_all(w, WIN_FLAGS_IMAGE_ERROR)) { - // Image needs to be updated, update it. - if (!ps->backend_data) { - // We are using legacy backend, nothing to do here. - break; - } - - if (win_check_flags_all(w, WIN_FLAGS_PIXMAP_STALE)) { - // Check to make sure the window is still mapped, - // otherwise we won't be able to rebind pixmap after - // releasing it, yet we might still need the pixmap for - // rendering. - if (!win_check_flags_all(w, WIN_FLAGS_PIXMAP_NONE)) { - // Must release images first, otherwise breaks - // NVIDIA driver - win_release_pixmap(ps->backend_data, w); - } - win_bind_pixmap(ps->backend_data, w); - } - - // break here, loop always run only once - break; + if (!win_check_flags_any(w, WIN_FLAGS_PIXMAP_STALE) || + win_check_flags_all(w, WIN_FLAGS_PIXMAP_ERROR) || + // We don't need to do anything here for legacy backends + ps->backend_data == NULL) { + win_clear_flags(w, WIN_FLAGS_PIXMAP_STALE); + return; } - // Clear stale image flags - if (win_check_flags_any(w, WIN_FLAGS_PIXMAP_STALE)) { - win_clear_flags(w, WIN_FLAGS_PIXMAP_STALE); + // Image needs to be updated, update it. + win_clear_flags(w, WIN_FLAGS_PIXMAP_STALE); + + // Check to make sure the window is still mapped, otherwise we won't be able to + // rebind pixmap after releasing it, yet we might still need the pixmap for + // rendering. + auto pixmap = x_new_id(&ps->c); + auto e = xcb_request_check( + ps->c.c, xcb_composite_name_window_pixmap_checked(ps->c.c, win_id(w), pixmap)); + if (e != NULL) { + log_debug("Failed to get named pixmap for window %#010x(%s): %s. " + "Retaining its current window image", + win_id(w), w->name, x_strerror(e)); + free(e); + return; + } + + log_debug("New named pixmap for %#010x (%s) : %#010x", win_id(w), w->name, pixmap); + + // Must release images first, otherwise breaks NVIDIA driver + win_release_pixmap(ps->backend_data, w); + w->win_image = ps->backend_data->ops.bind_pixmap( + ps->backend_data, pixmap, x_get_visual_info(&ps->c, w->a.visual)); + if (!w->win_image) { + log_error("Failed to bind pixmap"); + xcb_free_pixmap(ps->c.c, pixmap); + win_set_flags(w, WIN_FLAGS_PIXMAP_ERROR); } } @@ -845,9 +821,7 @@ void unmap_win_finish(session_t *ps, struct win *w) { if (ps->backend_data) { // Only the pixmap needs to be freed and reacquired when mapping. // Shadow image can be preserved. - if (!win_check_flags_all(w, WIN_FLAGS_PIXMAP_NONE)) { - win_release_pixmap(ps->backend_data, w); - } + win_release_pixmap(ps->backend_data, w); } else { assert(!w->win_image); assert(!w->shadow_image); @@ -858,7 +832,7 @@ void unmap_win_finish(session_t *ps, struct win *w) { // Try again at binding images when the window is mapped next time if (w->state != WSTATE_DESTROYED) { - win_clear_flags(w, WIN_FLAGS_IMAGE_ERROR); + win_clear_flags(w, WIN_FLAGS_PIXMAP_ERROR); } assert(w->running_animation == NULL); } @@ -1384,14 +1358,15 @@ void free_win_res(session_t *ps, struct win *w) { /// Query the Xorg for information about window `win`, and assign a window to `cursor` if /// this window should be managed. -struct win *win_maybe_allocate(session_t *ps, struct wm_ref *cursor) { +struct win *win_maybe_allocate(session_t *ps, struct wm_ref *cursor, + xcb_get_window_attributes_reply_t *attrs) { static const struct win win_def = { .frame_opacity = 1.0, .in_openclose = true, // set to false after first map is done, // true here because window is just created - .flags = WIN_FLAGS_PIXMAP_NONE, // updated by - // property/attributes/etc - // change + .flags = 0, // updated by + // property/attributes/etc + // change // Runtime variables, updated by dbus .fade_force = UNSET, @@ -1421,23 +1396,18 @@ struct win *win_maybe_allocate(session_t *ps, struct wm_ref *cursor) { xcb_window_t wid = wm_ref_win_id(cursor); log_debug("Managing window %#010x", wid); - xcb_get_window_attributes_cookie_t acookie = xcb_get_window_attributes(ps->c.c, wid); - xcb_get_window_attributes_reply_t *a = - xcb_get_window_attributes_reply(ps->c.c, acookie, NULL); - if (!a || a->map_state == XCB_MAP_STATE_UNVIEWABLE) { + if (attrs->map_state == XCB_MAP_STATE_UNVIEWABLE) { // Failed to get window attributes or geometry probably means // the window is gone already. Unviewable means the window is // already reparented elsewhere. // BTW, we don't care about Input Only windows, except for // stacking proposes, so we need to keep track of them still. - free(a); return NULL; } - if (a->_class == XCB_WINDOW_CLASS_INPUT_ONLY) { + if (attrs->_class == XCB_WINDOW_CLASS_INPUT_ONLY) { // No need to manage this window, but we still keep it on the // window stack - free(a); return NULL; } @@ -1448,16 +1418,14 @@ struct win *win_maybe_allocate(session_t *ps, struct wm_ref *cursor) { // We only need to initialize the part that are not initialized // by map_win *new = win_def; - new->a = *a; + new->a = *attrs; new->shadow_opacity = ps->o.shadow_opacity; pixman_region32_init(&new->bounding_shape); - free(a); - xcb_generic_error_t *e; auto g = xcb_get_geometry_reply(ps->c.c, xcb_get_geometry(ps->c.c, wid), &e); if (!g) { - log_error_x_error(e, "Failed to get geometry of window %#010x", wid); + log_debug("Failed to get geometry of window %#010x: %s", wid, x_strerror(e)); free(e); free(new); return NULL; @@ -1478,7 +1446,7 @@ struct win *win_maybe_allocate(session_t *ps, struct wm_ref *cursor) { ps->c.c, xcb_damage_create_checked(ps->c.c, new->damage, wid, XCB_DAMAGE_REPORT_LEVEL_NON_EMPTY)); if (e) { - log_error_x_error(e, "Failed to create damage"); + log_debug("Failed to create damage for window %#010x: %s", wid, x_strerror(e)); free(e); free(new); return NULL; @@ -1490,12 +1458,13 @@ struct win *win_maybe_allocate(session_t *ps, struct wm_ref *cursor) { if (!ps->o.use_ewmh_active_win) { frame_event_mask |= XCB_EVENT_MASK_FOCUS_CHANGE; } - xcb_change_window_attributes(ps->c.c, wid, XCB_CW_EVENT_MASK, - (const uint32_t[]){frame_event_mask}); + x_set_error_action_ignore( + &ps->c, xcb_change_window_attributes(ps->c.c, wid, XCB_CW_EVENT_MASK, + (const uint32_t[]){frame_event_mask})); // Get notification when the shape of a window changes if (ps->shape_exists) { - xcb_shape_select_input(ps->c.c, wid, 1); + x_set_error_action_ignore(&ps->c, xcb_shape_select_input(ps->c.c, wid, 1)); } new->pictfmt = x_get_pictform_for_visual(&ps->c, new->a.visual); @@ -1688,17 +1657,17 @@ void win_set_focused(session_t *ps, struct win *w) { } auto old_active_win = wm_active_win(ps->wm); - if (w->is_ewmh_focused) { + if (w->is_focused) { assert(old_active_win == w); return; } wm_set_active_win(ps->wm, w); - w->is_ewmh_focused = true; + w->is_focused = true; if (old_active_win) { - assert(old_active_win->is_ewmh_focused); - old_active_win->is_ewmh_focused = false; + assert(old_active_win->is_focused); + old_active_win->is_focused = false; win_on_focus_change(ps, old_active_win); } win_on_focus_change(ps, w); @@ -2029,10 +1998,17 @@ double win_animatable_get(const struct win *w, enum win_script_output output) { bool win_process_animation_and_state_change(struct session *ps, struct win *w, double delta_t) { // If the window hasn't ever been damaged yet, it won't be rendered in this frame. - // And if it is also unmapped/destroyed, it won't receive damage events. In this - // case we can skip its animation. For mapped windows, we need to provisionally - // start animation, because its first damage event might come a bit late. - bool will_never_render = !w->ever_damaged && w->state != WSTATE_MAPPED; + // Or if it doesn't have a image bound, it won't be rendered either. (This can + // happen is a window is destroyed during a backend reset. Backend resets releases + // all images, and if a window is freed during that, its image cannot be + // reacquired.) + // + // If the window won't be rendered, and it is also unmapped/destroyed, it won't + // receive damage events or reacquire an image. In this case we can skip its + // animation. For mapped windows, we need to provisionally start animation, + // because its first damage event might come a bit late. + bool will_never_render = + (!w->ever_damaged || w->win_image == NULL) && w->state != WSTATE_MAPPED; if (!ps->redirected || will_never_render) { // This window won't be rendered, so we don't need to run the animations. bool state_changed = w->previous.state != w->state; @@ -2332,5 +2308,5 @@ bool win_is_bypassing_compositor(const session_t *ps, const struct win *w) { * settings */ bool win_is_focused_raw(const struct win *w) { - return w->a.map_state == XCB_MAP_STATE_VIEWABLE && w->is_ewmh_focused; + return w->a.map_state == XCB_MAP_STATE_VIEWABLE && w->is_focused; } diff --git a/src/wm/win.h b/src/wm/win.h index 9e498e35..9935b11f 100644 --- a/src/wm/win.h +++ b/src/wm/win.h @@ -195,8 +195,8 @@ struct win { /// `is_ewmh_fullscreen`, or the windows spatial relation with the /// root window. Which one is used is determined by user configuration. bool is_fullscreen; - /// Whether the window is the EWMH active window. - bool is_ewmh_focused; + /// Whether the window is the active window. + bool is_focused; // Opacity-related members /// Window opacity @@ -353,7 +353,7 @@ void win_destroy_start(session_t *ps, struct win *w); void win_map_start(struct win *w); /// Release images bound with a window, set the *_NONE flags on the window. Only to be /// used when de-initializing the backend outside of win.c -void win_release_images(struct backend_base *base, struct win *w); +void win_release_images(struct backend_base *backend, struct win *w); winmode_t attr_pure win_calc_mode_raw(const struct win *w); // TODO(yshui) `win_calc_mode` is only used by legacy backends winmode_t attr_pure win_calc_mode(const struct win *w); @@ -408,7 +408,8 @@ void win_get_region_frame_local(const struct win *w, region_t *res); region_t win_get_region_frame_local_by_val(const struct win *w); /// Query the Xorg for information about window `win` /// `win` pointer might become invalid after this function returns -struct win *win_maybe_allocate(session_t *ps, struct wm_ref *cursor); +struct win *win_maybe_allocate(session_t *ps, struct wm_ref *cursor, + xcb_get_window_attributes_reply_t *attrs); /** * Release a destroyed window that is no longer needed. diff --git a/src/wm/wm.c b/src/wm/wm.c index 58e75c5e..1403eb83 100644 --- a/src/wm/wm.c +++ b/src/wm/wm.c @@ -265,6 +265,15 @@ static void wm_reap_orphans(struct wm *wm) { } } +/// Move `from->win` to `to->win`, update `win->tree_ref`. +static void wm_move_win(struct wm_tree_node *from, struct wm_tree_node *to) { + if (from->win != NULL) { + from->win->tree_ref = (struct wm_ref *)&to->siblings; + } + to->win = from->win; + from->win = NULL; +} + void wm_destroy(struct wm *wm, xcb_window_t wid) { struct wm_tree_node *node = wm_tree_find(&wm->tree, wid); if (!node) { @@ -279,7 +288,11 @@ void wm_destroy(struct wm *wm, xcb_window_t wid) { if (!list_is_empty(&node->children)) { log_error("Window %#010x is destroyed but it still has children", wid); } - wm_tree_detach(&wm->tree, node); + auto zombie = wm_tree_detach(&wm->tree, node); + assert(zombie != NULL || node->win == NULL); + if (zombie != NULL) { + wm_move_win(node, zombie); + } // There could be an in-flight query tree request for this window, we orphan it. // It will be reaped when all query tree requests are completed. (Or right now if // the tree is already consistent.) @@ -315,7 +328,11 @@ void wm_reparent(struct wm *wm, xcb_window_t wid, xcb_window_t parent) { return; } - wm_tree_detach(&wm->tree, window); + auto zombie = wm_tree_detach(&wm->tree, window); + assert(zombie != NULL || window->win == NULL); + if (zombie != NULL) { + wm_move_win(window, zombie); + } // Attaching `window` to `new_parent` will change the children list of // `new_parent`, if there is a pending query tree request for `new_parent`, doing @@ -390,7 +407,13 @@ wm_handle_query_tree_reply(struct x_connection *c, struct x_async_request_base * // If child node exists, it must be a previously orphaned node. assert(child_node->parent == &wm->orphan_root); - wm_tree_detach(&wm->tree, child_node); + auto zombie = wm_tree_detach(&wm->tree, child_node); + if (zombie != NULL) { + // This only happens if `child_node` is not orphaned, which means + // things are already going wrong. (the assert above would fail + // too). + wm_tree_reap_zombie(zombie); + } wm_tree_attach(&wm->tree, child_node, node); } @@ -463,7 +486,12 @@ static void wm_import_start_no_flush(struct wm *wm, struct x_connection *c, stru assert(false); } - wm_tree_detach(&wm->tree, new); + auto zombie = wm_tree_detach(&wm->tree, new); + if (zombie != NULL) { + // This only happens if `new` is not orphaned, which means things + // are already going wrong. + wm_tree_reap_zombie(zombie); + } // Need to bump the generation number, as `new` is actually an entirely // new window, just reusing the same window ID. new->id.gen = wm->tree.gen++; diff --git a/src/wm/wm_internal.h b/src/wm/wm_internal.h index f9a8c9ff..e48ec16e 100644 --- a/src/wm/wm_internal.h +++ b/src/wm/wm_internal.h @@ -94,7 +94,9 @@ void wm_tree_destroy_window(struct wm_tree *tree, struct wm_tree_node *node); /// Detach the subtree rooted at `subroot` from `tree`. The subtree root is removed from /// its parent, and the disconnected tree nodes won't be able to be found via /// `wm_tree_find`. Relevant events will be generated. -void wm_tree_detach(struct wm_tree *tree, struct wm_tree_node *subroot); +/// +/// Returns the zombie tree node if one is created, or NULL. +struct wm_tree_node *must_use wm_tree_detach(struct wm_tree *tree, struct wm_tree_node *subroot); /// Attach `node` to `parent`. `node` becomes the topmost child of `parent`. If `parent` /// is NULL, `node` becomes the root window. void wm_tree_attach(struct wm_tree *tree, struct wm_tree_node *child, diff --git a/src/x.c b/src/x.c index 62913069..ae0e00fe 100644 --- a/src/x.c +++ b/src/x.c @@ -334,39 +334,35 @@ xcb_window_t wid_get_prop_window(struct x_connection *c, xcb_window_t wid, */ bool wid_get_text_prop(struct x_connection *c, struct atom *atoms, xcb_window_t wid, xcb_atom_t prop, char ***pstrlst, int *pnstr) { - auto prop_info = x_get_prop_info(c, wid, prop); - auto type = prop_info.type; - auto format = prop_info.format; - auto length = prop_info.length; - - if (type == XCB_ATOM_NONE) { - return false; - } - - if (!x_is_type_string(atoms, type)) { - log_warn("Text property %d of window %#010x has unsupported type: %d", - prop, wid, type); - return false; - } - - if (format != 8) { - log_warn("Text property %d of window %#010x has unexpected format: %d", - prop, wid, format); - return false; - } - xcb_generic_error_t *e = NULL; - auto word_count = (length + 4 - 1) / 4; auto r = xcb_get_property_reply( - c->c, xcb_get_property(c->c, 0, wid, prop, type, 0, word_count), &e); + c->c, xcb_get_property(c->c, 0, wid, prop, XCB_ATOM_ANY, 0, UINT_MAX), &e); if (!r) { log_debug_x_error(e, "Failed to get window property for %#010x", wid); free(e); return false; } - assert(length == (uint32_t)xcb_get_property_value_length(r)); + if (r->type == XCB_ATOM_NONE) { + free(r); + return false; + } + if (!x_is_type_string(atoms, r->type)) { + log_warn("Text property %d of window %#010x has unsupported type: %d", + prop, wid, r->type); + free(r); + return false; + } + + if (r->format != 8) { + log_warn("Text property %d of window %#010x has unexpected format: %d", + prop, wid, r->format); + free(r); + return false; + } + + uint32_t length = to_u32_checked(xcb_get_property_value_length(r)); void *data = xcb_get_property_value(r); unsigned int nstr = 0; uint32_t current_offset = 0; @@ -904,25 +900,43 @@ struct xvisual_info x_get_visual_info(struct x_connection *c, xcb_visualid_t vis }; } -void x_update_monitors(struct x_connection *c, struct x_monitors *m) { - x_free_monitor_info(m); +struct x_update_monitors_request { + struct x_async_request_base base; + struct x_monitors *monitors; +}; - xcb_randr_get_monitors_reply_t *r = xcb_randr_get_monitors_reply( - c->c, xcb_randr_get_monitors(c->c, c->screen_info->root, true), NULL); - if (!r) { +static void x_handle_update_monitors_reply(struct x_connection * /*c*/, + struct x_async_request_base *req_base, + xcb_raw_generic_event_t *reply_or_error) { + auto m = ((struct x_update_monitors_request *)req_base)->monitors; + free(req_base); + + if (reply_or_error->response_type == 0) { + log_warn("Failed to get monitor information using RandR: %s", + x_strerror((xcb_generic_error_t *)reply_or_error)); return; } - m->count = xcb_randr_get_monitors_monitors_length(r); + x_free_monitor_info(m); + + auto reply = (xcb_randr_get_monitors_reply_t *)reply_or_error; + + m->count = xcb_randr_get_monitors_monitors_length(reply); m->regions = ccalloc(m->count, region_t); xcb_randr_monitor_info_iterator_t monitor_info_it = - xcb_randr_get_monitors_monitors_iterator(r); + xcb_randr_get_monitors_monitors_iterator(reply); for (int i = 0; monitor_info_it.rem; xcb_randr_monitor_info_next(&monitor_info_it)) { xcb_randr_monitor_info_t *mi = monitor_info_it.data; pixman_region32_init_rect(&m->regions[i++], mi->x, mi->y, mi->width, mi->height); } +} - free(r); +void x_update_monitors_async(struct x_connection *c, struct x_monitors *m) { + auto req = ccalloc(1, struct x_update_monitors_request); + req->base.callback = x_handle_update_monitors_reply; + req->base.sequence = xcb_randr_get_monitors(c->c, c->screen_info->root, 1).sequence; + req->monitors = m; + x_await_request(c, &req->base); } void x_free_monitor_info(struct x_monitors *m) { diff --git a/src/x.h b/src/x.h index 6afc5ddc..68bedccb 100644 --- a/src/x.h +++ b/src/x.h @@ -419,8 +419,8 @@ xcb_visualid_t x_get_visual_for_depth(xcb_screen_t *screen, uint8_t depth); xcb_render_pictformat_t x_get_pictfmt_for_standard(struct x_connection *c, xcb_pict_standard_t std); -/// Populates a `struct x_monitors` with the current monitor configuration. -void x_update_monitors(struct x_connection *, struct x_monitors *); +/// Populates a `struct x_monitors` with the current monitor configuration asynchronously. +void x_update_monitors_async(struct x_connection *, struct x_monitors *); /// Free memory allocated for a `struct x_monitors`. void x_free_monitor_info(struct x_monitors *);