Fix cursor_over_conky and override atoms

WIP commit - relative calculation is still off.

Calculate relative valuators only once per event and cache results.

Switch to finding top-level window before root. This made me reuse
virtual root code so I refactored it and made it apply automatically in
place of X11 macros. This means that all code in x11.cc will use virtual
roots if they exist.

Fixed query_x11_windows fallback to exclude windows without WM hints. I
believe this is the correct way to ignore windows specific to some
special WM functionality.

Removed InputEvent and xev_as_input_event as they were being used in
only a single place.

Removed lua::state from x11_init_window arguments and replaced it with a
struct containing needed data:
- Makes a lot of code in the function look neater.
- Removes weird class_name reference because lua setting value now stays
  allocated (has same lifetime) as the init function.
- Reduces cost of accessing same settings (own_window_type) multiple times.
- Helps decouple lua from x11 sources

Signed-off-by: Tin Švagelj <tin.svagelj@live.com>

src/display-x11.cc

@@ -71,6 +71,12 @@
 #include "logging.h"
 #include "x11.h"
 
+// Use display, screen and window globals from x11.h instead
+// This is here to avoid issues with virtual roots
+#undef RootWindowOfScreen
+#undef RootWindow
+#undef DefaultRootWindow
+
 // TODO: cleanup externs (move to conky.h ?)
 #ifdef OWN_WINDOW
 extern int fixed_size, fixed_pos;
@@ -477,22 +483,22 @@ bool handle_event<x_event_handler::MOUSE_INPUT>(
   }
   device_info = device_info::from_xi_id(data->deviceid, data->display);
 
-  Window event_window;
-  modifier_state_t mods;
-  if (data->evtype == XI_Motion || data->evtype == XI_ButtonPress ||
-      data->evtype == XI_ButtonRelease) {
-    event_window = query_x11_window_at_pos(display, data->root_x, data->root_y);
-    // query_result is not window.window in some cases.
-    event_window = query_x11_last_descendant(display, event_window);
-    mods = x11_modifier_state(data->mods.effective);
+  if (!(data->evtype == XI_Motion || data->evtype == XI_ButtonPress ||
+        data->evtype == XI_ButtonRelease)) {
+    return true;
   }
 
-  bool cursor_over_conky =
-      (event_window == window.window ||
-       window.window == 0L &&
-           (event_window == window.root || event_window == window.desktop)) &&
-      (data->root_x >= window.x && data->root_x < (window.x + window.width) &&
-       data->root_y >= window.y && data->root_y < (window.y + window.height));
+  Window event_window =
+      query_x11_window_at_pos(display, data->root_x, data->root_y);
+  // query_result is not window.window in some cases.
+  modifier_state_t mods = x11_modifier_state(data->mods.effective);
+
+  bool same_window = query_x11_top_level(display, event_window) ==
+                     query_x11_top_level(display, window.window);
+  bool cursor_over_conky = same_window && data->root_x >= window.x &&
+                           data->root_x < (window.x + window.width) &&
+                           data->root_y >= window.y &&
+                           data->root_y < (window.y + window.height);
 
   // XInput reports events twice on some hardware (even by 'xinput --test-xi2')
   auto hash = std::make_tuple(data->serial, data->evtype, data->event);

src/gui.cc

@@ -52,11 +52,6 @@ int workarea[4];
 /* Window stuff */
 char window_created = 0;
 
-/* local prototypes */
-#ifdef BUILD_X11
-void x11_init_window(lua::state &l, bool own);
-#endif /*BUILD_X11*/
-
 /********************* <SETTINGS> ************************/
 
 bool out_to_gui(lua::state &l) {
@@ -88,7 +83,24 @@ void own_window_setting::lua_setter(lua::state &l, bool init) {
 
     if (out_to_gui(l)) {
 #ifdef BUILD_X11
-      x11_init_window(l, do_convert(l, -1).first);
+      x11_window_options options;
+
+#ifdef OWN_WINDOW
+      options.use_own_window = do_convert(l, -1).first;
+      options.window_ty = own_window_type.get(l);
+      options.window_hints = own_window_hints.get(l);
+      options.window_title = own_window_title.get(l);
+      options.window_class = own_window_class.get(l);
+
+      options.inner_margin = border_inner_margin.get(l);
+      options.border_thickness = border_width.get(l);
+      options.outer_margin = border_outer_margin.get(l);
+#ifdef BUILD_ARGB
+      options.argb_visual = use_argb_visual.get(l);
+#endif /* BUILD_ARGB */
+#endif /* OWN_WINDOW */
+
+      x11_init_window(options);
 #endif /*BUILD_X11*/
     } else {
       // own_window makes no sense when not drawing to X

src/mouse-events.cc

@@ -248,10 +248,10 @@ device_info *device_info::from_xi_id(xi_device_id device_id, Display *display) {
 }
 
 void handle_xi_device_change(const XIHierarchyEvent *event) {
-  if (event->flags & XISlaveRemoved != 0) {
+  if ((event->flags & XISlaveRemoved) != 0) {
     for (int i = 0; i < event->num_info; i++) {
       auto info = event->info[i];
-      if (info.flags & XISlaveRemoved != 0 &&
+      if ((info.flags & XISlaveRemoved) != 0 &&
           xi_id_mapping.count(info.deviceid) > 0) {
         size_t id = xi_id_mapping[info.deviceid];
         xi_id_mapping.erase(info.deviceid);
@@ -275,22 +275,25 @@ size_t fixed_valuator_index(Display *display, XIDeviceInfo *device,
   int format_return;
   unsigned long num_items;
   unsigned long bytes_after;
-  if (XIGetProperty(display, device->deviceid, override_atom, 0, 1, False,
-                    XA_INTEGER, &type_return, &format_return, &num_items,
-                    &bytes_after,
-                    reinterpret_cast<unsigned char **>(&value)) == Success) {
-    if (type_return != XA_INTEGER || num_items > 1) {
-      NORM_ERR(
-          "invalid '%s' option value, expected a single integer; value will be "
-          "ignored",
-          atom_names[valuator]);
+  do {
+    if (XIGetProperty(display, device->deviceid, override_atom, 0, 1, False,
+                      XA_INTEGER, &type_return, &format_return, &num_items,
+                      &bytes_after,
+                      reinterpret_cast<unsigned char **>(&value)) == Success) {
+      if (num_items == 0) break;
+      if (type_return != XA_INTEGER) {
+        NORM_ERR(
+            "invalid '%s' option value, expected a single integer; value will "
+            "be ignored",
+            atom_names[valuator]);
+        XFree(value);
+        break;
+      }
+      uint32_t result = *reinterpret_cast<uint32_t *>(value);
       XFree(value);
-      return valuator;
+      return static_cast<size_t>(result);
     }
-    uint32_t result = *reinterpret_cast<uint32_t *>(value);
-    XFree(value);
-    return static_cast<size_t>(result);
-  }
+  } while (true);
   return valuator;
 }
 
@@ -300,46 +303,55 @@ bool fixed_valuator_relative(Display *display, XIDeviceInfo *device,
                              valuator_t valuator,
                              XIValuatorClassInfo *class_info) {
   const std::array<const char *, 2> atom_names = {
-      "ConkyValuatorMoveType",
-      "ConkyValuatorScrollType",
+      "ConkyValuatorMoveMode",
+      "ConkyValuatorScrollMode",
   };
-  Atom override_atom = XInternAtom(display, atom_names[valuator >> 1], False);
-  unsigned char *value;
+
+  Atom override_atom = XInternAtom(display, atom_names[valuator >> 1], True);
+  unsigned char *value_return;
   Atom type_return;
   int format_return;
   unsigned long num_items;
   unsigned long bytes_after;
-  if (XIGetProperty(display, device->deviceid, override_atom, 0, 9, False,
-                    XA_STRING, &type_return, &format_return, &num_items,
-                    &bytes_after,
-                    reinterpret_cast<unsigned char **>(&value)) == Success) {
-    if (type_return != XA_STRING) {
-      NORM_ERR(
-          "invalid '%s' option value, expected a string; value will be "
-          "ignored",
-          atom_names[valuator >> 1]);
-      XFree(value);
-      return class_info->type == XIModeRelative;
-    }
 
-    // lowercase value
-    for (auto c = value; *c; ++c) *c = tolower(*c);
-
-    bool relative = false;
-    if (strcmp(reinterpret_cast<char *>(value), "relative") == 0) {
-      relative = true;
-    } else if (strcmp(reinterpret_cast<char *>(value), "absolute") != 0) {
-      NORM_ERR(
-          "unknown '%s' option value: '%s', expected 'absolute' or 'relative'; "
-          "value will be ignored",
-          atom_names[static_cast<size_t>(valuator) >> 1]);
+  do {
+    if (XIGetProperty(
+            display, device->deviceid, override_atom, 0, 9, False, XA_ATOM,
+            &type_return, &format_return, &num_items, &bytes_after,
+            reinterpret_cast<unsigned char **>(&value_return)) == Success) {
+      if (num_items == 0) break;
+      if (type_return != XA_ATOM) {
+        NORM_ERR(
+            "invalid '%s' option value, expected an atom (string); value will "
+            "be ignored",
+            atom_names[valuator >> 1]);
+        XFree(value_return);
+        break;
+      }
+      Atom return_atom = *reinterpret_cast<Atom *>(value_return);
+      XFree(value_return);
+      char *value = XGetAtomName(display, return_atom);
+
+      // lowercase value
+      for (auto c = value; *c; ++c) *c = tolower(*c);
+
+      bool relative = false;
+      if (strcmp(reinterpret_cast<char *>(value), "relative") == 0) {
+        relative = true;
+      } else if (strcmp(reinterpret_cast<char *>(value), "absolute") != 0) {
+        NORM_ERR(
+            "unknown '%s' option value: '%s', expected 'absolute' or "
+            "'relative'; "
+            "value will be ignored",
+            atom_names[valuator >> 1]);
+        XFree(value);
+        break;
+      }
       XFree(value);
-      return class_info->type == XIModeRelative;
+      return relative;
     }
-    XFree(value);
-    return relative;
-  }
-  return class_info->type == XIModeRelative;
+  } while (true);
+  return class_info->mode == XIModeRelative;
 }
 
 void device_info::init_xi_device(
@@ -386,7 +398,6 @@ void device_info::init_xi_device(
     };
 
     this->valuators[valuator] = info;
-    DBGP2("SToRING: %s %d", name.c_str(), info.index);
   }
 
   if (std::holds_alternative<xi_device_id>(source)) {
@@ -440,9 +451,31 @@ xi_event_data *xi_event_data::read_cookie(Display *display,
       .valuators = valuators,
       .mods = source->mods,
       .group = source->group,
+      .valuators_relative = {0.0, 0.0, 0.0, 0.0},
   };
   XFreeEventData(display, cookie);
 
+  // Precompute relative values if they're absolute
+  auto device = device_info::from_xi_id(result->deviceid, result->display);
+  if (device == nullptr) return result;  // shouldn't happen
+  for (size_t v = 0; v < valuator_t::VALUATOR_COUNT; v++) {
+    valuator_t valuator = static_cast<valuator_t>(v);
+    auto valuator_info = device->valuator(valuator);
+
+    if (result->valuators.count(valuator_info.index) == 0) { continue; }
+    auto current = result->valuators[valuator_info.index];
+
+    if (valuator_info.relative) {
+      result->valuators_relative[v] = current;
+    } else {
+      // XXX these doubles come from int values and might wrap around though
+      // it's hard to tell what int type is the source as it depends on the
+      // device/driver.
+      result->valuators_relative[v] = current - valuator_info.value;
+    }
+    valuator_info.value = current;
+  }
+
   return result;
 }
 
@@ -466,20 +499,7 @@ std::optional<double> xi_event_data::valuator_value(valuator_t valuator) const {
 
 std::optional<double> xi_event_data::valuator_relative_value(
     valuator_t valuator) const {
-  auto current = this->valuator_value(valuator);
-  if (!current.has_value()) return std::nullopt;
-
-  auto valuator_info = this->valuator_info(valuator);
-  if (valuator_info == nullptr) return std::nullopt;
-
-  if (valuator_info->relative) {
-    return current.value();
-  } else {
-    // XXX these doubles come from int values and might wrap around though it's
-    // hard to tell what int type is the source as it depends on the
-    // device/driver.
-    return current.value() - valuator_info->value;
-  }
+  return this->valuators_relative.at(valuator);
 }
 
 std::vector<std::tuple<int, XEvent *>> xi_event_data::generate_events(

src/mouse-events.h

@@ -241,7 +241,6 @@ struct mouse_crossing_event : public mouse_positioned_event {
 };
 
 #ifdef BUILD_XINPUT
-
 typedef int xi_device_id;
 typedef int xi_event_type;
 
@@ -282,8 +281,6 @@ struct xi_event_data {
   Time time;
   xi_device_id deviceid;
   int sourceid;
-  /// Primary event detail. Meaning depends on `evtype` value:
-  /// XI_ButtonPress - Mouse button
   int detail;
   Window root;
   Window event;
@@ -299,6 +296,11 @@ struct xi_event_data {
   XIModifierState mods;
   XIGroupState group;
 
+  // Extra data
+
+  /// Precomputed relative values
+  std::array<double, valuator_t::VALUATOR_COUNT> valuators_relative;
+
   static xi_event_data *read_cookie(Display *display,
                                     XGenericEventCookie *cookie);