Bug fix #99: Rewrite focus detection logic

- Rewrite focus detection logic. Remove w->focused_real and use
  ps->active_win to identify focused window uniformly. Use a more
  expensive way to filter FocusIn/Out events to improve reliability.
  Only limited tests are done, and bugs are likely to be introduced.
  (#99)

- Known issue: Under fvwm, compton sometimes does not consistently
  report the window input gets sent to. But there's something wrong in
  that case: XGetInputFocus() shows the root window is focused but
  another window is receiving input.

src/c2.c

@@ -1055,7 +1055,7 @@ c2_match_once_leaf(session_t *ps, win *w, const c2_l_t *pleaf,
             case C2_L_PFULLSCREEN: tgt = win_is_fullscreen(ps, w); break;
             case C2_L_POVREDIR: tgt = w->a.override_redirect;   break;
             case C2_L_PARGB:    tgt = (WMODE_ARGB == w->mode);  break;
-            case C2_L_PFOCUSED: tgt = w->focused_real;          break;
+            case C2_L_PFOCUSED: tgt = win_is_focused_real(ps, w); break;
             case C2_L_PWMWIN:   tgt = w->wmwin;                 break;
             case C2_L_PCLIENT:  tgt = w->client_win;            break;
             case C2_L_PLEADER:  tgt = w->leader;                break;

src/common.h

@@ -954,8 +954,6 @@ typedef struct _win {
   bool focused;
   /// Override value of window focus state. Set by D-Bus method calls.
   switch_t focused_force;
-  /// Whether the window is actually focused.
-  bool focused_real;
 
   // Blacklist related members
   /// Name of the window.
@@ -1695,6 +1693,14 @@ find_toplevel(session_t *ps, Window id) {
   return NULL;
 }
 
+/**
+ * Check if a window is really focused.
+ */
+static inline bool
+win_is_focused_real(session_t *ps, const win *w) {
+  return IsViewable == w->a.map_state && ps->active_win == w;
+}
+
 /**
  * Find out the currently focused window.
  *
@@ -1702,19 +1708,15 @@ find_toplevel(session_t *ps, Window id) {
  */
 static inline win *
 find_focused(session_t *ps) {
-  if (!ps->o.track_focus)
+  if (!ps->o.track_focus) return NULL;
-    return NULL;
-
-  for (win *w = ps->list; w; w = w->next) {
-    if (w->focused_real && !w->destroyed)
-      return w;
-  }
 
+  if (ps->active_win && win_is_focused_real(ps, ps->active_win))
+    return ps->active_win;
   return NULL;
 }
 
 /**
- * Copies a region
+ * Copies a region.
  */
 static inline XserverRegion
 copy_region(const session_t *ps, XserverRegion oldregion) {

src/compton.c

@@ -776,19 +776,12 @@ recheck_focus(session_t *ps) {
   // opacity on it
   Window wid = 0;
   int revert_to;
-  win *w = NULL;
 
   XGetInputFocus(ps->dpy, &wid, &revert_to);
 
-  if (!wid || PointerRoot == wid)
+  win *w = find_win_all(ps, wid);
-    return NULL;
 
-  // Fallback to the old method if find_toplevel() fails
+  // And we set the focus state here
-  if (!(w = find_toplevel(ps, wid))) {
-    w = find_toplevel2(ps, wid);
-  }
-
-  // And we set the focus state and opacity here
   if (w) {
     win_set_focused(ps, w, true);
     return w;
@@ -2066,19 +2059,12 @@ map_win(session_t *ps, Window id) {
       || IsViewable == w->a.map_state)
     return;
 
-  assert(!w->focused_real);
+  assert(!win_is_focused_real(ps, w));
 
   w->a.map_state = IsViewable;
 
   cxinerama_win_upd_scr(ps, w);
 
-  // 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));
@@ -2113,14 +2099,10 @@ map_win(session_t *ps, Window id) {
   // Detect if the window is shaped or has rounded corners
   win_update_shape_raw(ps, w);
 
-  // Occasionally compton does not seem able to get a FocusIn event from
+  // FocusIn/Out may be ignored when the window is unmapped, so we must
-  // a window just mapped. I suspect it's a timing issue again when the
+  // recheck focus here
-  // XSelectInput() is called too late. We have to recheck the focused
+  if (ps->o.track_focus)
-  // window here. It makes no sense if we are using EWMH
-  // _NET_ACTIVE_WINDOW.
-  if (ps->o.track_focus && !ps->o.use_ewmh_active_win) {
     recheck_focus(ps);
-  }
 
   // Update window focus state
   win_update_focused(ps, w);
@@ -2296,7 +2278,7 @@ calc_opacity(session_t *ps, win *w) {
     }
 
     // Respect active_opacity only when the window is physically focused
-    if (OPAQUE == opacity && ps->o.active_opacity && w->focused_real)
+    if (OPAQUE == opacity && ps->o.active_opacity && win_is_focused_real(ps, w))
       opacity = ps->o.active_opacity;
   }
 
@@ -2752,7 +2734,6 @@ add_win(session_t *ps, Window id, Window prev) {
 
     .focused = false,
     .focused_force = UNSET,
-    .focused_real = false,
 
     .name = NULL,
     .class_instance = NULL,
@@ -3270,7 +3251,7 @@ win_update_focused(session_t *ps, win *w) {
     w->focused = w->focused_force;
   }
   else {
-    w->focused = w->focused_real;
+    w->focused = win_is_focused_real(ps, w);
 
     // Use wintype_focus, and treat WM windows and override-redirected
     // windows specially
@@ -3302,51 +3283,68 @@ win_set_focused(session_t *ps, win *w, bool focused) {
   if (IsUnmapped == w->a.map_state)
     return;
 
-  if (w->focused_real != focused) {
+  if (win_is_focused_real(ps, w) == focused) return;
-    w->focused_real = focused;
 
-    // If window grouping detection is enabled
+  if (focused) {
-    if (ps->o.track_leader) {
+    if (ps->active_win)
-      Window leader = win_get_leader(ps, w);
+      win_set_focused(ps, ps->active_win, false);
+    ps->active_win = w;
+  }
+  else if (w == ps->active_win)
+    ps->active_win = NULL;
 
-      // If the window gets focused, replace the old active_leader
+  assert(win_is_focused_real(ps, w) == focused);
-      if (w->focused_real && leader != ps->active_leader) {
-        Window active_leader_old = ps->active_leader;
 
-        ps->active_leader = leader;
+  win_on_focus_change(ps, w);
+}
 
-        group_update_focused(ps, active_leader_old);
+/**
-        group_update_focused(ps, leader);
+ * Handle window focus change.
-      }
+ */
-      // If the group get unfocused, remove it from active_leader
+static void
-      else if (!w->focused_real && leader && leader == ps->active_leader
+win_on_focus_change(session_t *ps, win *w) {
-          && !group_is_focused(ps, leader)) {
+  // If window grouping detection is enabled
-        ps->active_leader = None;
+  if (ps->o.track_leader) {
-        group_update_focused(ps, leader);
+    Window leader = win_get_leader(ps, w);
-      }
 
-      // The window itself must be updated anyway
+    // If the window gets focused, replace the old active_leader
-      win_update_focused(ps, w);
+    if (win_is_focused_real(ps, w) && leader != ps->active_leader) {
+      Window active_leader_old = ps->active_leader;
+
+      ps->active_leader = leader;
+
+      group_update_focused(ps, active_leader_old);
+      group_update_focused(ps, leader);
     }
-    // Otherwise, only update the window itself
+    // If the group get unfocused, remove it from active_leader
-    else {
+    else if (!win_is_focused_real(ps, w) && leader && leader == ps->active_leader
-      win_update_focused(ps, w);
+        && !group_is_focused(ps, leader)) {
+      ps->active_leader = None;
+      group_update_focused(ps, leader);
     }
 
-    // Update everything related to conditions
+    // The window itself must be updated anyway
-    win_on_factor_change(ps, w);
+    win_update_focused(ps, w);
+  }
+  // Otherwise, only update the window itself
+  else {
+    win_update_focused(ps, w);
+  }
+
+  // Update everything related to conditions
+  win_on_factor_change(ps, w);
 
 #ifdef CONFIG_DBUS
-    // Send D-Bus signal
+  // Send D-Bus signal
-    if (ps->o.dbus) {
+  if (ps->o.dbus) {
-      if (w->focused_real)
+    if (win_is_focused_real(ps, w))
-        cdbus_ev_win_focusin(ps, w);
+      cdbus_ev_win_focusin(ps, w);
-      else
+    else
-        cdbus_ev_win_focusout(ps, w);
+      cdbus_ev_win_focusout(ps, w);
-    }
-#endif
   }
+#endif
 }
+
 /**
  * Update leader of a window.
  */
@@ -3386,7 +3384,7 @@ win_set_leader(session_t *ps, win *w, Window nleader) {
     // Update the old and new window group and active_leader if the window
     // could affect their state.
     Window cache_leader = win_get_leader(ps, w);
-    if (w->focused_real && cache_leader_old != cache_leader) {
+    if (win_is_focused_real(ps, w) && cache_leader_old != cache_leader) {
       ps->active_leader = cache_leader;
 
       group_update_focused(ps, cache_leader_old);
@@ -3808,11 +3806,10 @@ ev_focus_report(XFocusChangeEvent* ev) {
  */
 inline static bool
 ev_focus_accept(XFocusChangeEvent *ev) {
-  return ev->detail == NotifyNonlinear
+  return NotifyNormal == ev->mode || NotifyUngrab == ev->mode;
-    || ev->detail == NotifyNonlinearVirtual;
 }
 
-inline static void
+static inline void
 ev_focus_in(session_t *ps, XFocusChangeEvent *ev) {
 #ifdef DEBUG_EVENTS
   ev_focus_report(ev);
@@ -3821,12 +3818,9 @@ ev_focus_in(session_t *ps, XFocusChangeEvent *ev) {
   if (!ev_focus_accept(ev))
     return;
 
-  win *w = find_win(ps, ev->window);
+  win *w = find_win_all(ps, ev->window);
-
+  if (w)
-  // To deal with events sent from windows just destroyed
+    win_set_focused(ps, w, true);
-  if (!w) return;
-
-  win_set_focused(ps, w, true);
 }
 
 inline static void
@@ -3838,12 +3832,9 @@ ev_focus_out(session_t *ps, XFocusChangeEvent *ev) {
   if (!ev_focus_accept(ev))
     return;
 
-  win *w = find_win(ps, ev->window);
+  win *w = find_win_all(ps, ev->window);
-
+  if (w)
-  // To deal with events sent from windows just destroyed
+    win_set_focused(ps, w, false);
-  if (!w) return;
-
-  win_set_focused(ps, w, false);
 }
 
 inline static void
@@ -3965,21 +3956,11 @@ ev_expose(session_t *ps, XExposeEvent *ev) {
 static void
 update_ewmh_active_win(session_t *ps) {
   // Search for the window
-  Window wid =
+  Window wid = wid_get_prop_window(ps, ps->root, ps->atom_ewmh_active_win);
-    wid_get_prop_window(ps, ps->root, ps->atom_ewmh_active_win);
+  win *w = find_win_all(ps, wid);
-  win *w = NULL;
 
-  if (wid && !(w = find_toplevel(ps, wid)))
+  // Mark the window focused. No need to unfocus the previous one.
-    if (!(w = find_win(ps, wid)))
+  if (w) win_set_focused(ps, w, true);
-      w = find_toplevel2(ps, wid);
-
-  // Mark the window focused
-  if (w) {
-    if (ps->active_win && w != ps->active_win)
-      win_set_focused(ps, ps->active_win, false);
-    ps->active_win = w;
-    win_set_focused(ps, w, true);
-  }
 }
 
 inline static void

src/compton.h

@@ -563,6 +563,20 @@ clear_cache_win_leaders(session_t *ps) {
 static win *
 find_toplevel2(session_t *ps, Window wid);
 
+/**
+ * Find matched window.
+ */
+static inline win *
+find_win_all(session_t *ps, const Window wid) {
+  if (!wid || PointerRoot == wid || wid == ps->root || wid == ps->overlay)
+    return NULL;
+
+  win *w = find_win(ps, wid);
+  if (!w) w = find_toplevel(ps, wid);
+  if (!w) w = find_toplevel2(ps, wid);
+  return w;
+}
+
 static Window
 win_get_leader_raw(session_t *ps, win *w, int recursions);
 
@@ -589,7 +603,7 @@ group_is_focused(session_t *ps, Window leader) {
 
   for (win *w = ps->list; w; w = w->next) {
     if (win_get_leader(ps, w) == leader && !w->destroyed
-        && w->focused_real)
+        && win_is_focused_real(ps, w))
       return true;
   }
 
@@ -764,6 +778,9 @@ group_update_focused(session_t *ps, Window leader) {
 static inline void
 win_set_focused(session_t *ps, win *w, bool focused);
 
+static void
+win_on_focus_change(session_t *ps, win *w);
+
 static void
 win_determine_fade(session_t *ps, win *w);
 

src/dbus.c

@@ -722,7 +722,11 @@ cdbus_process_win_get(session_t *ps, DBusMessage *msg) {
   cdbus_m_win_get_do(window_type, cdbus_reply_enum);
   cdbus_m_win_get_do(wmwin, cdbus_reply_bool);
   cdbus_m_win_get_do(leader, cdbus_reply_wid);
-  cdbus_m_win_get_do(focused_real, cdbus_reply_bool);
+  // focused_real
+  if (!strcmp("focused_real", target)) {
+    cdbus_reply_bool(ps, msg, win_is_focused_real(ps, w));
+    return true;
+  }
   cdbus_m_win_get_do(fade_force, cdbus_reply_enum);
   cdbus_m_win_get_do(shadow_force, cdbus_reply_enum);
   cdbus_m_win_get_do(focused_force, cdbus_reply_enum);