qxt-mini: multiple Linux support fixes

- Register an event filter once and keep it there. The refcount goes out
of whack. Will further investigate.
- Use an iterator over all shortcuts registered for given key
combination
- Lessen logspam

Keyboard shortcut support on Linux looks pretty good now.

The issue that blocked keys from being further processed was running
XGrabKey multiple times with various modifiers. XGrabKey will block
unlisted modifiers from being passed to other apps. The solution was to
register once with AnyModifier and care about modifiers only later.

3 files changed, 66 insertions, 37 deletions

diff --git a/qxt-mini/qxtglobalshortcut.cpp b/qxt-mini/qxtglobalshortcut.cpp
index bf3ed33d..298472b5 100644
--- a/qxt-mini/qxtglobalshortcut.cpp
+++ b/qxt-mini/qxtglobalshortcut.cpp
@@ -30,36 +30,50 @@
 *****************************************************************************/
 
 #include "qxtglobalshortcut_p.h"
+
+#include "compat/util.hpp"
+
 #include <QAbstractEventDispatcher>
-#include <QtDebug>
 #include <QApplication>
+#include <QtDebug>
+#include <QtGlobal>
 
-#ifndef Q_OS_MAC
-int QxtGlobalShortcutPrivate::ref = 0;
-#endif // Q_OS_MAC
 QHash<QPair<quint32, quint32>, QxtGlobalShortcut*> QxtGlobalShortcutPrivate::shortcuts;
 
-QxtGlobalShortcutPrivate::QxtGlobalShortcutPrivate() : enabled(true), key(Qt::Key(0)), mods(Qt::NoModifier)
+struct QxtGlobalShortcutPrivate::event_filter_installer
+{
+    static void ensure_event_filter();
+};
+
+void QxtGlobalShortcutPrivate::event_filter_installer::ensure_event_filter()
 {
 #ifndef Q_OS_MAC
-    if (ref == 0) {
-        QAbstractEventDispatcher::instance()->installNativeEventFilter(this);
+    QAbstractEventDispatcher* instance = QAbstractEventDispatcher::instance();
+    if (instance)
+    {
+        static QxtGlobalShortcutPrivate filter(QxtGlobalShortcutPrivate::tag {});
+        static bool installed =
+            (instance->installNativeEventFilter(&filter),
+             true);
+        Q_UNUSED(installed);
     }
-    ++ref;
-#endif // Q_OS_MAC
+#endif
+}
+
+QxtGlobalShortcutPrivate::QxtGlobalShortcutPrivate(QxtGlobalShortcutPrivate::tag) :
+    enabled(false), key(Qt::Key(0)), mods(Qt::NoModifier)
+{
+    qDebug() << "qxt-mini: adding event filter";
+}
+
+QxtGlobalShortcutPrivate::QxtGlobalShortcutPrivate() :
+    enabled(true), key(Qt::Key(0)), mods(Qt::NoModifier)
+{
+    QxtGlobalShortcutPrivate::event_filter_installer::ensure_event_filter();
 }
 
 QxtGlobalShortcutPrivate::~QxtGlobalShortcutPrivate()
 {
-#ifndef Q_OS_MAC
-    --ref;
-    if (ref == 0) {
-        QAbstractEventDispatcher *ed = QAbstractEventDispatcher::instance(qApp->thread());
-        if (ed != 0) {
-            ed->removeNativeEventFilter(this);
-        }
-    }
-#endif // Q_OS_MAC
     unsetShortcut();
 }
 
@@ -76,11 +90,13 @@ bool QxtGlobalShortcutPrivate::setShortcut(const QKeySequence& shortcut)
     const quint32 nativeMods = nativeModifiers(mods);
     const bool res = registerShortcut(nativeKey, nativeMods);
     if (res)
+    {
 #ifndef Q_OS_MAC
         shortcuts.insertMulti(qMakePair(nativeKey, nativeMods), &qxt_p());
 #else
         shortcuts.insert(qMakePair(nativeKey, nativeMods), &qxt_p());
 #endif
+    }
     else
         qWarning() << "QxtGlobalShortcut failed to register:" << QKeySequence(key + mods).toString();
     return res;
@@ -102,10 +118,14 @@ bool QxtGlobalShortcutPrivate::unsetShortcut()
     else
         qWarning() << "QxtGlobalShortcut failed to unregister:" << QKeySequence(key + mods).toString();
 #else
-    auto list = shortcuts.values(qMakePair(nativeKey, nativeMods));
+    using IT = decltype(shortcuts.end());
+    const auto pair = qMakePair(nativeKey, nativeMods);
+    IT it = shortcuts.find(pair);
     bool found = false;
-    for (auto it = list.begin(); it != list.end(); it++)
+    for (; it != shortcuts.end(); it++)
     {
+        if (it.key() != pair) // DO NOT REMOVE
+            break;
         if (*it == &qxt_p())
         {
             found = true;
@@ -126,13 +146,17 @@ bool QxtGlobalShortcutPrivate::unsetShortcut()
 void QxtGlobalShortcutPrivate::activateShortcut(quint32 nativeKey, quint32 nativeMods)
 {
 #ifndef Q_OS_MAC
-    auto list = shortcuts.values(qMakePair(nativeKey, nativeMods));
+    using IT = decltype(shortcuts.end());
+    const auto pair = qMakePair(nativeKey, nativeMods);
+    IT it = shortcuts.find(pair);
 
-    for (auto it = list.begin(); it != list.end(); it++)
+    for (; it != shortcuts.end(); it++)
     {
+        if (it.key() != pair) // DO NOT REMOVE
+            break;
         auto ptr = *it;
         if (ptr->isEnabled())
-            ptr->activated();
+            emit ptr->activated();
     }
 #else
     QxtGlobalShortcut* shortcut = shortcuts.value(qMakePair(nativeKey, nativeMods));
diff --git a/qxt-mini/qxtglobalshortcut_p.h b/qxt-mini/qxtglobalshortcut_p.h
index 9eadbc7f..1835f956 100644
--- a/qxt-mini/qxtglobalshortcut_p.h
+++ b/qxt-mini/qxtglobalshortcut_p.h
@@ -53,13 +53,17 @@ public:
     bool unsetShortcut();
 
     static bool error;
-    static int ref;
     static bool eventFilter(void* message);
     bool nativeEventFilter(const QByteArray & eventType, void * message, long * result) override;
 
     static void activateShortcut(quint32 nativeKey, quint32 nativeMods);
 
 private:
+    struct event_filter_installer;
+    friend struct event_filter_installer;
+    struct tag {};
+    explicit QxtGlobalShortcutPrivate(tag);
+
     static quint32 nativeKeycode(Qt::Key keycode);
     static quint32 nativeModifiers(Qt::KeyboardModifiers modifiers);
 
@@ -67,6 +71,8 @@ private:
     static bool unregisterShortcut(quint32 nativeKey, quint32 nativeMods);
 
     static QHash<QPair<quint32, quint32>, QxtGlobalShortcut*> shortcuts;
+
+    static void ensure_event_filter();
 };
 
 #endif // QXTGLOBALSHORTCUT_P_H
diff --git a/qxt-mini/qxtglobalshortcut_x11.cpp b/qxt-mini/qxtglobalshortcut_x11.cpp
index dad12b56..e04a782a 100644
--- a/qxt-mini/qxtglobalshortcut_x11.cpp
+++ b/qxt-mini/qxtglobalshortcut_x11.cpp
@@ -42,6 +42,8 @@
 #include "qplatformnativeinterface.h"
 #include "compat/util.hpp"
 
+static constexpr quint32 AllMods = ShiftMask|LockMask|ControlMask|Mod1Mask|Mod2Mask|Mod3Mask|Mod4Mask|Mod5Mask;
+
 typedef int (*X11ErrorHandler)(Display *display, XErrorEvent *event);
 
 struct keybinding final
@@ -99,13 +101,13 @@ bool keybinding::incf(quint32 code)
 
     if (ret)
     {
-        qDebug() << "qxt-mini: registered keybinding" << code;
+        //qDebug() << "qxt-mini: registered keybinding" << code;
     }
 
     k.refcnt++;
     list.insert(code, k);
 
-    qDebug() << "qxt-mini: incf: refcount for" << code << "now" << k.refcnt;
+    //qDebug() << "qxt-mini: incf: refcount for" << code << "now" << k.refcnt;
 
     return ret;
 }
@@ -128,11 +130,11 @@ bool keybinding::decf(quint32 code)
     if (k.refcnt == 0)
     {
         list.erase(it);
-        qDebug() << "qxt-mini: removed keybinding" << code;
+        //qDebug() << "qxt-mini: removed keybinding" << code;
         return true;
     }
 
-    qDebug() << "qxt-mini: decf: refcount for" << code << "now" << k.refcnt;
+    //qDebug() << "qxt-mini: decf: refcount for" << code << "now" << k.refcnt;
 
     return false;
 }
@@ -210,7 +212,7 @@ public:
         {
             QxtX11ErrorHandler errorHandler;
 
-            XGrabKey(display(), keycode, AnyModifier, window, True,
+            XGrabKey(display(), keycode, AllMods, window, True,
                      GrabModeAsync, GrabModeAsync);
 
             if (errorHandler.error) {
@@ -227,7 +229,7 @@ public:
         if (keybinding::decf(keycode))
         {
             QxtX11ErrorHandler errorHandler;
-            XUngrabKey(display(), keycode, AnyModifier, window);
+            XUngrabKey(display(), keycode, AllMods, window);
             return !errorHandler.error;
         }
         return true;
@@ -267,9 +269,7 @@ bool QxtGlobalShortcutPrivate::nativeEventFilter(const QByteArray & eventType,
             keystate |= AltGrMask;
 #endif
 
-        activateShortcut(keycode,
-            // Mod1Mask == Alt, Mod4Mask == Meta
-            keystate & (ShiftMask | ControlMask | Mod1Mask | Mod4Mask | Mod2Mask));
+        activateShortcut(keycode, keystate);
     }
     return false;
 }
@@ -296,6 +296,8 @@ quint32 QxtGlobalShortcutPrivate::nativeModifiers(Qt::KeyboardModifiers modifier
     if (modifiers & Qt::KeypadModifier) // numlock
         native |= Mod2Mask;
 
+    native &= AllMods;
+
     return native;
 }
 
@@ -307,28 +309,25 @@ quint32 QxtGlobalShortcutPrivate::nativeKeycode(Qt::Key key)
 
     QByteArray tmp(QKeySequence(key).toString().toLatin1());
 
-
     KeySym keysym = XStringToKeysym(tmp.data());
     if (keysym == NoSymbol)
         keysym = static_cast<ushort>(key);
 
     const quint32 ret = XKeysymToKeycode(x11.display(), keysym);
 
-    qDebug() << "key is" << key << QKeySequence(key).toString(QKeySequence::PortableText) << ret;
+    //qDebug() << "key is" << key << QKeySequence(key).toString(QKeySequence::PortableText) << ret;
 
     return ret;
 }
 
 bool QxtGlobalShortcutPrivate::registerShortcut(quint32 nativeKey, unused(quint32, nativeMods))
 {
-    qDebug() << "register" << nativeKey;
     QxtX11Data x11;
     return x11.isValid() && x11.grabKey(nativeKey, x11.rootWindow());
 }
 
 bool QxtGlobalShortcutPrivate::unregisterShortcut(quint32 nativeKey, unused(quint32, nativeMods))
 {
-    qDebug() << "unregister" << nativeKey;
     QxtX11Data x11;
     return x11.isValid() && x11.ungrabKey(nativeKey, x11.rootWindow());
 }