From 04fe4a76b23aa1196ffcdb822510e0a3c84ab3b3 Mon Sep 17 00:00:00 2001 From: Yuxuan Shui Date: Mon, 6 Apr 2020 20:22:54 +0100 Subject: [PATCH] Revert previous fix for #299 Commits reverted: b652e8b58d942730f8c772f384c2d1004660b301 bdf809d0394396c058dddc54d13298c12ab15d4c e9ab9709896688d5bf54555f70aa6341a8c65c0f 481ac54f67a1ab0b5c8544b907d1491599cd118a While those commits themselves could be useful even if they don't completely fix #299, they come with the risks of introduce more unforeseen bugs, which outweigh their benefit, so revert them. A brief explanation of their problem: The fix hinges on the destroy event of the client window to work. However, the client window could be destroyed so fast, before we even handle the map, or even the create, event of the frame. And we won't be listening for substructure events on the frame at the point the client window is destroyed. Thus completely miss the client window destroy event. Because of the inherent racy nature of Xorg, this approach is really difficult to make work. Fixes #371 Signed-off-by: Yuxuan Shui --- src/event.c | 36 ++++++++++-------------------------- src/picom.c | 4 ++-- src/win.c | 8 +++----- src/win.h | 1 - 4 files changed, 15 insertions(+), 34 deletions(-) diff --git a/src/event.c b/src/event.c index 7cd50a2..87de238 100644 --- a/src/event.c +++ b/src/event.c @@ -263,21 +263,8 @@ static inline void ev_configure_notify(session_t *ps, xcb_configure_notify_event static inline void ev_destroy_notify(session_t *ps, xcb_destroy_notify_event_t *ev) { auto w = find_win(ps, ev->window); - auto mw = find_toplevel(ps, ev->window); - if (mw && mw->client_win == mw->base.id) { - // We only want _real_ frame window - assert(&mw->base == w); - mw = NULL; - } - assert(w == NULL || mw == NULL); - - if (w != NULL) { + if (w) { auto _ attr_unused = destroy_win_start(ps, w); - } else if (mw != NULL) { - win_recheck_client(ps, mw); - } else { - log_debug("Received a destroy notify from an unknown window, %#010x", - ev->window); } } @@ -349,13 +336,15 @@ static inline void ev_reparent_notify(session_t *ps, xcb_reparent_notify_event_t ps->c, ev->window, XCB_CW_EVENT_MASK, (const uint32_t[]){determine_evmask(ps, ev->window, WIN_EVMODE_UNKNOWN)}); - // Mark the window as the client window of its parent. - auto w_real_top = find_managed_window_or_parent(ps, ev->parent); - // If found, and the client window has not been determined, or its - // frame may not have a correct client, continue - if (w_real_top) { - if (!w_real_top->client_win || - w_real_top->client_win == w_real_top->base.id) { + // Check if the window is an undetected client window + // Firstly, check if it's a known client window + if (!w_top) { + // If not, look for its frame window + auto w_real_top = find_managed_window_or_parent(ps, ev->parent); + // If found, and the client window has not been determined, or its + // frame may not have a correct client, continue + if (w_real_top && (!w_real_top->client_win || + w_real_top->client_win == w_real_top->base.id)) { // If it has WM_STATE, mark it the client window if (wid_has_prop(ps, ev->window, ps->atoms->aWM_STATE)) { w_real_top->wmwin = false; @@ -370,11 +359,6 @@ static inline void ev_reparent_notify(session_t *ps, xcb_reparent_notify_event_t determine_evmask(ps, ev->window, WIN_EVMODE_UNKNOWN) | XCB_EVENT_MASK_PROPERTY_CHANGE}); } - } else { - log_warn("Window %#010x reparented to a window that " - "already has a client window, the parent is " - "%#010x (%s)", - ev->window, w_real_top->base.id, w_real_top->name); } } } diff --git a/src/picom.c b/src/picom.c index d159dff..a7bc763 100644 --- a/src/picom.c +++ b/src/picom.c @@ -303,9 +303,9 @@ uint32_t determine_evmask(session_t *ps, xcb_window_t wid, win_evmode_t mode) { struct managed_win *w = NULL; // Check if it's a mapped frame window - if (mode == WIN_EVMODE_FRAME || + if (WIN_EVMODE_FRAME == mode || ((w = find_managed_win(ps, wid)) && w->a.map_state == XCB_MAP_STATE_VIEWABLE)) { - evmask |= XCB_EVENT_MASK_PROPERTY_CHANGE | XCB_EVENT_MASK_SUBSTRUCTURE_NOTIFY; + evmask |= XCB_EVENT_MASK_PROPERTY_CHANGE; if (!ps->o.use_ewmh_active_win) { evmask |= XCB_EVENT_MASK_FOCUS_CHANGE; } diff --git a/src/win.c b/src/win.c index a9aa8ab..2da27eb 100644 --- a/src/win.c +++ b/src/win.c @@ -1037,8 +1037,6 @@ void win_mark_client(session_t *ps, struct managed_win *w, xcb_window_t client) */ void win_unmark_client(session_t *ps, struct managed_win *w) { xcb_window_t client = w->client_win; - log_debug("Detaching client window %#010x from frame %#010x (%s)", client, - w->base.id, w->name); w->client_win = XCB_NONE; @@ -1054,7 +1052,7 @@ void win_unmark_client(session_t *ps, struct managed_win *w) { * @param ps current session * @param w struct _win of the parent window */ -void win_recheck_client(session_t *ps, struct managed_win *w) { +static void win_recheck_client(session_t *ps, struct managed_win *w) { // Initialize wmwin to false w->wmwin = false; @@ -1064,14 +1062,14 @@ void win_recheck_client(session_t *ps, struct managed_win *w) { // sets override-redirect flags on all frame windows. xcb_window_t cw = find_client_win(ps, w->base.id); if (cw) { - log_debug("(%#010x): client %#010x", w->base.id, cw); + log_trace("(%#010x): client %#010x", w->base.id, cw); } // Set a window's client window to itself if we couldn't find a // client window if (!cw) { cw = w->base.id; w->wmwin = !w->a.override_redirect; - log_debug("(%#010x): client self (%s)", w->base.id, + log_trace("(%#010x): client self (%s)", w->base.id, (w->wmwin ? "wmwin" : "override-redirected")); } diff --git a/src/win.h b/src/win.h index 738529c..d3d9071 100644 --- a/src/win.h +++ b/src/win.h @@ -299,7 +299,6 @@ void win_on_win_size_change(session_t *ps, struct managed_win *w); void win_update_wintype(session_t *ps, struct managed_win *w); void win_mark_client(session_t *ps, struct managed_win *w, xcb_window_t client); void win_unmark_client(session_t *ps, struct managed_win *w); -void win_recheck_client(session_t *ps, struct managed_win *w); bool win_get_class(session_t *ps, struct managed_win *w); /**