client: rework xkb context/keymap/state handling

- Use smart pointers from xkbcommon_support-private.

- Remove needless strdup() calls.

- Don't recreate context. And move it into qwaylanddisplay so it can
be shared in future between several keyboards. It contains things like
a logging level and include paths.

Change-Id: I5d1f667e710046e6b62aa2caf82fdb2decc24520
Reviewed-by: Johan Helsing <johan.helsing@qt.io>

src/plugins/platforms/wayland/client.pro

@@ -16,7 +16,7 @@ CONFIG -= precompile_header
 CONFIG += link_pkgconfig wayland-scanner
 
 qtConfig(xkbcommon) {
-    QT_PRIVATE += xkbcommon_support-private
+    QT_FOR_PRIVATE += xkbcommon_support-private
 }
 
 qtHaveModule(linuxaccessibility_support_private): \

src/plugins/platforms/wayland/qwaylanddisplay.cpp

@@ -142,6 +142,12 @@ QWaylandDisplay::QWaylandDisplay(QWaylandIntegration *waylandIntegration)
 
     mWindowManagerIntegration.reset(new QWaylandWindowManagerIntegration(this));
 
+#if QT_CONFIG(xkbcommon)
+    mXkbContext.reset(xkb_context_new(XKB_CONTEXT_NO_FLAGS));
+    if (!mXkbContext)
+        qCWarning(lcQpaWayland, "failed to create xkb context");
+#endif
+
     forceRoundTrip();
 }
 

src/plugins/platforms/wayland/qwaylanddisplay_p.h

@@ -65,6 +65,10 @@
 
 #include <qpa/qplatforminputcontextfactory_p.h>
 
+#if QT_CONFIG(xkbcommon)
+#include <QtXkbCommonSupport/private/qxkbcommon_p.h>
+#endif
+
 struct wl_cursor_image;
 
 QT_BEGIN_NAMESPACE
@@ -111,6 +115,10 @@ public:
     QWaylandDisplay(QWaylandIntegration *waylandIntegration);
     ~QWaylandDisplay(void) override;
 
+#if QT_CONFIG(xkbcommon)
+    struct xkb_context *xkbContext() const { return mXkbContext.get(); }
+#endif
+
     QList<QWaylandScreen *> screens() const { return mScreens; }
 
     QWaylandScreen *screenForOutput(struct wl_output *output) const;
@@ -246,6 +254,10 @@ private:
     void registry_global(uint32_t id, const QString &interface, uint32_t version) override;
     void registry_global_remove(uint32_t id) override;
 
+#if QT_CONFIG(xkbcommon)
+    QXkbCommon::ScopedXKBContext mXkbContext;
+#endif
+
     friend class QWaylandIntegration;
 };
 

src/plugins/platforms/wayland/qwaylandinputdevice.cpp

@@ -70,10 +70,6 @@
 
 #include <QtGui/QGuiApplication>
 
-#if QT_CONFIG(xkbcommon)
-#include <xkbcommon/xkbcommon.h>
-#endif
-
 QT_BEGIN_NAMESPACE
 
 namespace QtWaylandClient {
@@ -85,51 +81,34 @@ QWaylandInputDevice::Keyboard::Keyboard(QWaylandInputDevice *p)
 }
 
 #if QT_CONFIG(xkbcommon)
-bool QWaylandInputDevice::Keyboard::createDefaultKeyMap()
+bool QWaylandInputDevice::Keyboard::createDefaultKeymap()
 {
-    if (mXkbContext && mXkbMap && mXkbState) {
+    struct xkb_context *ctx = mParent->mQDisplay->xkbContext();
-        return true;
+    if (!ctx)
-    }
+        return false;
 
-    xkb_rule_names names;
+    struct xkb_rule_names names;
-    names.rules = strdup("evdev");
+    names.rules = "evdev";
-    names.model = strdup("pc105");
+    names.model = "pc105";
-    names.layout = strdup("us");
+    names.layout = "us";
-    names.variant = strdup("");
+    names.variant = "";
-    names.options = strdup("");
+    names.options = "";
 
-    mXkbContext = xkb_context_new(xkb_context_flags(0));
+    mXkbKeymap.reset(xkb_keymap_new_from_names(ctx, &names, XKB_KEYMAP_COMPILE_NO_FLAGS));
-    if (mXkbContext) {
+    if (mXkbKeymap)
-        mXkbMap = xkb_map_new_from_names(mXkbContext, &names, xkb_map_compile_flags(0));
+        mXkbState.reset(xkb_state_new(mXkbKeymap.get()));
-        if (mXkbMap) {
-            mXkbState = xkb_state_new(mXkbMap);
-        }
-    }
 
-    if (!mXkbContext || !mXkbMap || !mXkbState) {
+    if (!mXkbKeymap || !mXkbState) {
-        qWarning() << "xkb_map_new_from_names failed, no key input";
+        qCWarning(lcQpaWayland, "failed to create default keymap");
         return false;
     }
 
     return true;
 }
-
-void QWaylandInputDevice::Keyboard::releaseKeyMap()
-{
-    if (mXkbState)
-        xkb_state_unref(mXkbState);
-    if (mXkbMap)
-        xkb_map_unref(mXkbMap);
-    if (mXkbContext)
-        xkb_context_unref(mXkbContext);
-}
 #endif
 
 QWaylandInputDevice::Keyboard::~Keyboard()
 {
-#if QT_CONFIG(xkbcommon)
-    releaseKeyMap();
-#endif
     if (mFocus)
         QWindowSystemInterface::handleWindowActivated(nullptr);
     if (mParent->mVersion >= 3)
@@ -501,7 +480,7 @@ Qt::KeyboardModifiers QWaylandInputDevice::Keyboard::modifiers() const
     if (!mXkbState)
         return ret;
 
-    ret = QWaylandXkb::modifiers(mXkbState);
+    ret = QWaylandXkb::modifiers(mXkbState.get());
 #endif
 
     return ret;
@@ -761,16 +740,16 @@ void QWaylandInputDevice::Keyboard::keyboard_keymap(uint32_t format, int32_t fd,
         return;
     }
 
-    // Release the old keymap resources in the case they were already created in
+    mXkbKeymap.reset(xkb_keymap_new_from_string(mParent->mQDisplay->xkbContext(), map_str,
-    // the key event or when the compositor issues a new map
+                                                XKB_KEYMAP_FORMAT_TEXT_V1,
-    releaseKeyMap();
+                                                XKB_KEYMAP_COMPILE_NO_FLAGS));
-
-    mXkbContext = xkb_context_new(XKB_CONTEXT_NO_FLAGS);
-    mXkbMap = xkb_map_new_from_string(mXkbContext, map_str, XKB_KEYMAP_FORMAT_TEXT_V1, XKB_KEYMAP_COMPILE_NO_FLAGS);
     munmap(map_str, size);
     close(fd);
 
-    mXkbState = xkb_state_new(mXkbMap);
+    if (mXkbKeymap)
+        mXkbState.reset(xkb_state_new(mXkbKeymap.get()));
+    else
+        mXkbState.reset(nullptr);
 #else
     Q_UNUSED(format);
     Q_UNUSED(fd);
@@ -860,11 +839,10 @@ void QWaylandInputDevice::Keyboard::keyboard_key(uint32_t serial, uint32_t time,
         mParent->mQDisplay->setLastInputDevice(mParent, serial, window);
 
 #if QT_CONFIG(xkbcommon)
-    if (!createDefaultKeyMap()) {
+    if ((!mXkbKeymap || !mXkbState) && !createDefaultKeymap())
         return;
-    }
 
-    xkb_keysym_t sym = xkb_state_key_get_one_sym(mXkbState, code);
+    xkb_keysym_t sym = xkb_state_key_get_one_sym(mXkbState.get(), code);
 
     Qt::KeyboardModifiers modifiers = mParent->modifiers();
 
@@ -878,7 +856,7 @@ void QWaylandInputDevice::Keyboard::keyboard_key(uint32_t serial, uint32_t time,
 
     if (state == WL_KEYBOARD_KEY_STATE_PRESSED
 #if QT_CONFIG(xkbcommon)
-        && xkb_keymap_key_repeats(mXkbMap, code)
+        && xkb_keymap_key_repeats(mXkbKeymap.get(), code)
 #endif
         ) {
         mRepeatKey = qtkey;
@@ -952,7 +930,7 @@ void QWaylandInputDevice::Keyboard::keyboard_modifiers(uint32_t serial,
     Q_UNUSED(serial);
 #if QT_CONFIG(xkbcommon)
     if (mXkbState)
-        xkb_state_update_mask(mXkbState,
+        xkb_state_update_mask(mXkbState.get(),
                               mods_depressed, mods_latched, mods_locked,
                               0, 0, group);
     mNativeModifiers = mods_depressed | mods_latched | mods_locked;

src/plugins/platforms/wayland/qwaylandinputdevice_p.h

@@ -65,8 +65,7 @@
 #include <QtWaylandClient/private/qwayland-wayland.h>
 
 #if QT_CONFIG(xkbcommon)
-#include <xkbcommon/xkbcommon.h>
+#include <QtXkbCommonSupport/private/qxkbcommon_p.h>
-#include <xkbcommon/xkbcommon-keysyms.h>
 #endif
 
 #include <QtCore/QDebug>
@@ -209,11 +208,7 @@ public:
 
     QWaylandInputDevice *mParent = nullptr;
     ::wl_surface *mFocus = nullptr;
-#if QT_CONFIG(xkbcommon)
+
-    xkb_context *mXkbContext = nullptr;
-    xkb_keymap *mXkbMap = nullptr;
-    xkb_state *mXkbState = nullptr;
-#endif
     uint32_t mNativeModifiers = 0;
 
     int mRepeatKey;
@@ -222,9 +217,6 @@ public:
     int mRepeatRate = 25;
     int mRepeatDelay = 400;
     QString mRepeatText;
-#if QT_CONFIG(xkbcommon)
-    xkb_keysym_t mRepeatSym;
-#endif
     QTimer mRepeatTimer;
 
     Qt::KeyboardModifiers modifiers() const;
@@ -236,12 +228,17 @@ private slots:
 
 private:
 #if QT_CONFIG(xkbcommon)
-    bool createDefaultKeyMap();
+    bool createDefaultKeymap();
-    void releaseKeyMap();
 #endif
     void sendKey(QWindow *tlw, ulong timestamp, QEvent::Type type, int key, Qt::KeyboardModifiers modifiers,
                  quint32 nativeScanCode, quint32 nativeVirtualKey, quint32 nativeModifiers,
                  const QString& text = QString(), bool autorep = false, ushort count = 1);
+
+#if QT_CONFIG(xkbcommon)
+    xkb_keysym_t mRepeatSym = XKB_KEY_NoSymbol;
+    QXkbCommon::ScopedXKBKeymap mXkbKeymap;
+    QXkbCommon::ScopedXKBState mXkbState;
+#endif
 };
 
 class Q_WAYLAND_CLIENT_EXPORT QWaylandInputDevice::Pointer : public QObject, public QtWayland::wl_pointer

src/plugins/platforms/wayland/qwaylandintegration.cpp

@@ -479,7 +479,7 @@ void QWaylandIntegration::reconfigureInputContext()
         mInputContext.reset(QPlatformInputContextFactory::create(defaultInputContext));
 
 #if QT_CONFIG(xkbcommon)
-    QXkbCommon::setXkbContext(mInputContext.data(), xkb_context_new(XKB_CONTEXT_NO_FLAGS));
+    QXkbCommon::setXkbContext(mInputContext.data(), mDisplay->xkbContext());
 #endif
 
     // Even if compositor-side input context handling has been requested, we fallback to