From 812ba93516a114472de7f0be94266f8004b68f7e Mon Sep 17 00:00:00 2001 From: Richard Grenville Date: Fri, 7 Dec 2012 22:38:10 +0800 Subject: [PATCH] Bug fix #68: Possible fix for failure in client window detection - Note I'm not in the best state today (bad cold & sleep-deprived). This commit is likely to introduce bugs. - Attempt to fix client window detection failures happening when compton searches for client window before it's ready. - Fix build failure with role is not freed on window destruction. --- src/compton.c | 205 +++++++++++++++++++++++++++++++++++++------------- src/compton.h | 29 +++++-- 2 files changed, 176 insertions(+), 58 deletions(-) diff --git a/src/compton.c b/src/compton.c index 5817cb4..d06c0d7 100644 --- a/src/compton.c +++ b/src/compton.c @@ -1143,7 +1143,7 @@ border_size(session_t *ps, win *w) { */ static Window find_client_win(session_t *ps, Window w) { - if (wid_has_attr(ps, w, ps->atom_client)) { + if (wid_has_prop(ps, w, ps->atom_client)) { return w; } @@ -1799,15 +1799,23 @@ map_win(session_t *ps, Window id) { win *w = find_win(ps, id); // Don't care about window mapping if it's an InputOnly window - if (!w || InputOnly == w->a.class) return; + // Try avoiding mapping a window twice + if (!w || InputOnly == w->a.class + || IsViewable == w->a.map_state) + return; - w->focused_real = false; - if (ps->o.track_focus && ps->o.use_ewmh_active_win - && w == ps->active_win) - w->focused_real = true; + assert(!w->focused_real); w->a.map_state = IsViewable; + // Set focused to false + bool focused_real = false; + if (ps->o.track_focus && ps->o.use_ewmh_active_win + && w == ps->active_win) + focused_real = true; + win_set_focused(ps, w, focused_real); + + // Call XSelectInput() before reading properties so that no property // changes are lost XSelectInput(ps->dpy, id, determine_evmask(ps, id, WIN_EVMODE_FRAME)); @@ -1823,31 +1831,11 @@ map_win(session_t *ps, Window id) { // Detect client window here instead of in add_win() as the client // window should have been prepared at this point if (!w->client_win) { - w->wmwin = false; - - Window cw = None; - // Always recursively look for a window with WM_STATE, as Fluxbox - // sets override-redirect flags on all frame windows. - cw = find_client_win(ps, w->id); -#ifdef DEBUG_CLIENTWIN - if (cw) - printf("find_client_win(%#010lx): client %#010lx\n", w->id, cw); -#endif - // Set a window's client window to itself if we couldn't find a - // client window - if (!cw) { - cw = w->id; - w->wmwin = !w->a.override_redirect; -#ifdef DEBUG_CLIENTWIN - printf("find_client_win(%#010lx): client self (%s)\n", w->id, - (w->wmwin ? "wmwin": "override-redirected")); -#endif - } - mark_client_win(ps, w, cw); + win_recheck_client(ps, w); } else { // Re-mark client window here - mark_client_win(ps, w, w->client_win); + win_mark_client(ps, w, w->client_win); } assert(w->client_win); @@ -1947,11 +1935,11 @@ unmap_win(session_t *ps, Window id) { if (!w || IsUnmapped == w->a.map_state) return; - w->a.map_state = IsUnmapped; - // Set focus out win_set_focused(ps, w, false); + w->a.map_state = IsUnmapped; + // Fading out w->flags |= WFLAG_OPCT_CHANGE; set_fade_callback(ps, w, unmap_callback, false); @@ -2207,18 +2195,20 @@ calc_shadow_geometry(session_t *ps, win *w) { /** * Mark a window as the client window of another. * - * @param dpy display to use + * @param ps current session * @param w struct _win of the parent window * @param client window ID of the client window */ static void -mark_client_win(session_t *ps, win *w, Window client) { +win_mark_client(session_t *ps, win *w, Window client) { w->client_win = client; XSelectInput(ps->dpy, client, determine_evmask(ps, client, WIN_EVMODE_CLIENT)); - // Get the frame width and monitor further frame width changes on client - // window if necessary + // Make sure the XSelectInput() requests are sent + XSync(ps->dpy, False); + + // Get frame widths if needed if (ps->o.frame_opacity) { get_frame_extents(ps, w, client); } @@ -2232,11 +2222,69 @@ mark_client_win(session_t *ps, win *w, Window client) { // _NET_WM_WINDOW_TYPE_NORMAL, otherwise as _NET_WM_WINDOW_TYPE_DIALOG. if (WINTYPE_UNKNOWN == w->window_type) { if (w->a.override_redirect - || !wid_has_attr(ps, client, ps->atom_transient)) + || !wid_has_prop(ps, client, ps->atom_transient)) w->window_type = WINTYPE_NORMAL; else w->window_type = WINTYPE_DIALOG; } + + win_update_focused(ps, w); +} + +/** + * Unmark current client window of a window. + * + * @param ps current session + * @param w struct _win of the parent window + */ +static void +win_unmark_client(session_t *ps, win *w) { + Window client = w->client_win; + + w->client_win = None; + + // Recheck event mask + XSelectInput(ps->dpy, client, + determine_evmask(ps, client, WIN_EVMODE_UNKNOWN)); +} + +/** + * Recheck client window of a window. + * + * @param ps current session + * @param w struct _win of the parent window + */ +static void +win_recheck_client(session_t *ps, win *w) { + // Initialize wmwin to false + w->wmwin = false; + + // Look for the client window + + // Always recursively look for a window with WM_STATE, as Fluxbox + // sets override-redirect flags on all frame windows. + Window cw = find_client_win(ps, w->id); +#ifdef DEBUG_CLIENTWIN + if (cw) + printf_dbgf("(%#010lx): client %#010lx\n", w->id, cw); +#endif + // Set a window's client window to itself if we couldn't find a + // client window + if (!cw) { + cw = w->id; + w->wmwin = !w->a.override_redirect; +#ifdef DEBUG_CLIENTWIN + printf_dbgf("(%#010lx): client self (%s)\n", w->id, + (w->wmwin ? "wmwin": "override-redirected")); +#endif + } + + // Unmark the old one + if (w->client_win && w->client_win != cw) + win_unmark_client(ps, w); + + // Mark the new one + win_mark_client(ps, w, cw); } static void @@ -2268,6 +2316,11 @@ add_win(session_t *ps, Window id, Window prev) { return; } + // Delay window mapping + int map_state = new->a.map_state; + assert(IsViewable == map_state || IsUnmapped == map_state); + new->a.map_state = IsUnmapped; + new->damaged = 0; new->to_paint = false; new->pixmap = None; @@ -2342,7 +2395,7 @@ add_win(session_t *ps, Window id, Window prev) { new->next = *p; *p = new; - if (new->a.map_state == IsViewable) { + if (map_state == IsViewable) { map_win(ps, id); } } @@ -2655,6 +2708,27 @@ expose_root(session_t *ps, XRectangle *rects, int nrects) { add_damage(ps, region); } +/** + * Get the value of a type-Window property of a window. + * + * @return the value if successful, 0 otherwise + */ +static Window +wid_get_prop_window(session_t *ps, Window wid, Atom aprop) { + // Get the attribute + Window p = None; + winprop_t prop = wid_get_prop(ps, wid, aprop, 1L, XA_WINDOW, 32); + + // Return it + if (prop.nitems) { + p = *prop.data.p32; + } + + free_winprop(&prop); + + return p; +} + /** * Get the value of a text property of a window. */ @@ -3006,15 +3080,32 @@ ev_reparent_notify(session_t *ps, XReparentEvent *ev) { add_win(ps, ev->window, 0); } else { destroy_win(ps, ev->window); + // Reset event mask in case something wrong happens XSelectInput(ps->dpy, ev->window, determine_evmask(ps, ev->window, WIN_EVMODE_UNKNOWN)); - /* - // Check if the window is a client window of another - win *w_top = find_toplevel2(ps, ev->window); - if (w_top && !(w_top->client_win)) { - mark_client_win(ps, w_top, ev->window); - } */ + + // Check if the window is an undetected client window + // Firstly, check if it's a known client window + if (!find_toplevel(ps, ev->window)) { + // If not, look for its frame window + win *w_top = find_toplevel2(ps, ev->parent); + // If found, and its frame may not have a correct client, continue + if (w_top && w_top->client_win == w_top->id) { + // If it has WM_STATE, mark it the client window + if (wid_has_prop(ps, ev->window, ps->atom_client)) { + w_top->wmwin = false; + win_unmark_client(ps, w_top); + win_mark_client(ps, w_top, ev->window); + } + // Otherwise, watch for WM_STATE on it + else { + XSelectInput(ps->dpy, ev->window, + determine_evmask(ps, ev->window, WIN_EVMODE_UNKNOWN) + | PropertyChangeMask); + } + } + } } } @@ -3059,18 +3150,10 @@ ev_expose(session_t *ps, XExposeEvent *ev) { */ static void update_ewmh_active_win(session_t *ps) { - // Get the attribute firstly - winprop_t prop = wid_get_prop(ps, ps->root, ps->atom_ewmh_active_win, - 1L, XA_WINDOW, 32); - if (!prop.nitems) { - free_winprop(&prop); - return; - } - // Search for the window - Window wid = *prop.data.p32; + Window wid = + wid_get_prop_window(ps, ps->root, ps->atom_ewmh_active_win); win *w = NULL; - free_winprop(&prop); if (wid && !(w = find_toplevel(ps, wid))) if (!(w = find_win(ps, wid))) @@ -3107,6 +3190,24 @@ ev_property_notify(session_t *ps, XPropertyEvent *ev) { return; } + // If WM_STATE changes + if (ev->atom == ps->atom_client) { + // Check whether it could be a client window + if (!find_toplevel(ps, ev->window)) { + // Reset event mask anyway + XSelectInput(ps->dpy, ev->window, + determine_evmask(ps, ev->window, WIN_EVMODE_UNKNOWN)); + + win *w_top = find_toplevel2(ps, ev->window); + if (w_top && w_top->client_win == w_top->id + && wid_has_prop(ps, ev->window, ps->atom_client)) { + w_top->wmwin = false; + win_unmark_client(ps, w_top); + win_mark_client(ps, w_top, ev->window); + } + } + } + // If _NET_WM_OPACITY changes if (ev->atom == ps->atom_opacity) { win *w = NULL; diff --git a/src/compton.h b/src/compton.h index 9bc5bd0..6b4eed2 100644 --- a/src/compton.h +++ b/src/compton.h @@ -58,7 +58,10 @@ // For compatiblity with pattern); #ifdef CONFIG_REGEX_PCRE if (cond->regex_pcre_extra) - pcre_free_study(cond->regex_pcre_extra); + LPCRE_FREE_STUDY(cond->regex_pcre_extra); if (cond->regex_pcre) pcre_free(cond->regex_pcre); #endif @@ -1077,6 +1080,7 @@ free_win_res(session_t *ps, win *w) { free(w->name); free(w->class_instance); free(w->class_general); + free(w->role); } /** @@ -1149,15 +1153,15 @@ static inline bool is_normal_win(const win *w) { } /** - * Determine if a window has a specific attribute. + * Determine if a window has a specific property. * * @param session_t current session * @param w window to check - * @param atom atom of attribute to check + * @param atom atom of property to check * @return 1 if it has the attribute, 0 otherwise */ static inline bool -wid_has_attr(const session_t *ps, Window w, Atom atom) { +wid_has_prop(const session_t *ps, Window w, Atom atom) { Atom type = None; int format; unsigned long nitems, after; @@ -1451,6 +1455,10 @@ win_update_focused(session_t *ps, win *w) { */ static inline void win_set_focused(session_t *ps, win *w, bool focused) { + // Unmapped windows will have their focused state reset on map + if (IsUnmapped == w->a.map_state) + return; + w->focused_real = focused; win_update_focused(ps, w); } @@ -1480,7 +1488,13 @@ static void calc_shadow_geometry(session_t *ps, win *w); static void -mark_client_win(session_t *ps, win *w, Window client); +win_mark_client(session_t *ps, win *w, Window client); + +static void +win_unmark_client(session_t *ps, win *w); + +static void +win_recheck_client(session_t *ps, win *w); static void add_win(session_t *ps, Window id, Window prev); @@ -1516,6 +1530,9 @@ static bool wid_get_text_prop(session_t *ps, Window wid, Atom prop, char ***pstrlst, int *pnstr); +static Window +wid_get_prop_window(session_t *ps, Window wid, Atom aprop); + static bool wid_get_name(session_t *ps, Window w, char **name);