compositor: rework xkb context/keymap/state handling

- Use smart pointers from xkbcommon_support-private.

- Avoid unnecessary strdup()/free() calls.

- Don't recreate context. And move it into qwaylandcompositor so
it can be shared between seats. It contains things like a logging
level and include paths.

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

5 files changed, 68 insertions, 52 deletions

diff --git a/src/compositor/compositor.pro b/src/compositor/compositor.pro
index 47be591d7..b887cf281 100644
--- a/src/compositor/compositor.pro
+++ b/src/compositor/compositor.pro
@@ -3,6 +3,10 @@ MODULE = waylandcompositor
 
 QT = core gui-private
 
+qtConfig(xkbcommon) {
+    QT_FOR_PRIVATE += xkbcommon_support-private
+}
+
 qtHaveModule(quick): QT += quick
 
 CONFIG -= precompile_header
diff --git a/src/compositor/compositor_api/qwaylandcompositor.cpp b/src/compositor/compositor_api/qwaylandcompositor.cpp
index e95e9a72e..856417fb4 100644
--- a/src/compositor/compositor_api/qwaylandcompositor.cpp
+++ b/src/compositor/compositor_api/qwaylandcompositor.cpp
@@ -172,6 +172,14 @@ QWaylandCompositorPrivate::QWaylandCompositorPrivate(QWaylandCompositor *composi
     timer.start();
 
     QWindowSystemInterfacePrivate::installWindowSystemEventHandler(eventHandler.data());
+
+#if QT_CONFIG(xkbcommon)
+    mXkbContext.reset(xkb_context_new(XKB_CONTEXT_NO_FLAGS));
+    if (!mXkbContext) {
+        qWarning("Failed to create a XKB context: keymap will not be supported");
+        return;
+    }
+#endif
 }
 
 void QWaylandCompositorPrivate::init()
diff --git a/src/compositor/compositor_api/qwaylandcompositor_p.h b/src/compositor/compositor_api/qwaylandcompositor_p.h
index d91977def..2c9624216 100644
--- a/src/compositor/compositor_api/qwaylandcompositor_p.h
+++ b/src/compositor/compositor_api/qwaylandcompositor_p.h
@@ -60,6 +60,10 @@
 
 #include <QtWaylandCompositor/private/qwayland-server-wayland.h>
 
+#if QT_CONFIG(xkbcommon)
+#include <QtXkbCommonSupport/private/qxkbcommon_p.h>
+#endif
+
 QT_BEGIN_NAMESPACE
 
 namespace QtWayland {
@@ -81,6 +85,10 @@ public:
     QWaylandCompositorPrivate(QWaylandCompositor *compositor);
     ~QWaylandCompositorPrivate() override;
 
+#if QT_CONFIG(xkbcommon)
+    struct xkb_context *xkbContext() const { return mXkbContext.get(); }
+#endif
+
     void preInit();
     void init();
 
@@ -169,6 +177,10 @@ protected:
     bool initialized = false;
     QList<QPointer<QObject> > polish_objects;
 
+#if QT_CONFIG(xkbcommon)
+    QXkbCommon::ScopedXKBContext mXkbContext;
+#endif
+
     Q_DECLARE_PUBLIC(QWaylandCompositor)
     Q_DISABLE_COPY(QWaylandCompositorPrivate)
 };
diff --git a/src/compositor/compositor_api/qwaylandkeyboard.cpp b/src/compositor/compositor_api/qwaylandkeyboard.cpp
index 5f3bd3d4b..c92d00ebe 100644
--- a/src/compositor/compositor_api/qwaylandkeyboard.cpp
+++ b/src/compositor/compositor_api/qwaylandkeyboard.cpp
@@ -67,12 +67,11 @@ QWaylandKeyboardPrivate::QWaylandKeyboardPrivate(QWaylandSeat *seat)
 QWaylandKeyboardPrivate::~QWaylandKeyboardPrivate()
 {
 #if QT_CONFIG(xkbcommon)
-    if (xkb_context) {
+    if (xkbContext()) {
         if (keymap_area)
             munmap(keymap_area, keymap_size);
-        close(keymap_fd);
-        xkb_context_unref(xkb_context);
-        xkb_state_unref(xkb_state);
+        if (keymap_fd >= 0)
+            close(keymap_fd);
     }
 #endif
 }
@@ -137,7 +136,7 @@ void QWaylandKeyboardPrivate::keyboard_bind_resource(wl_keyboard::Resource *reso
         send_repeat_info(resource->handle, repeatRate, repeatDelay);
 
 #if QT_CONFIG(xkbcommon)
-    if (xkb_context) {
+    if (xkbContext()) {
         send_keymap(resource->handle, WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1,
                     keymap_fd, keymap_size);
     } else
@@ -203,7 +202,7 @@ void QWaylandKeyboardPrivate::maybeUpdateXkbScanCodeTable()
     if (!scanCodesByQtKey.isEmpty() || !xkbState())
         return;
 
-    if (xkb_keymap *keymap = xkb_state_get_keymap(xkb_state)) {
+    if (xkb_keymap *keymap = xkb_state_get_keymap(xkbState())) {
         xkb_keymap_key_for_each(keymap, [](xkb_keymap *keymap, xkb_keycode_t keycode, void *d){
             auto *scanCodesByQtKey = static_cast<QMap<ScanCodeKey, uint>*>(d);
             uint numLayouts = xkb_keymap_num_layouts_for_key(keymap, keycode);
@@ -226,15 +225,15 @@ void QWaylandKeyboardPrivate::maybeUpdateXkbScanCodeTable()
 void QWaylandKeyboardPrivate::updateModifierState(uint code, uint32_t state)
 {
 #if QT_CONFIG(xkbcommon)
-    if (!xkb_context)
+    if (!xkbContext())
         return;
 
-    xkb_state_update_key(xkb_state, code, state == WL_KEYBOARD_KEY_STATE_PRESSED ? XKB_KEY_DOWN : XKB_KEY_UP);
+    xkb_state_update_key(xkbState(), code, state == WL_KEYBOARD_KEY_STATE_PRESSED ? XKB_KEY_DOWN : XKB_KEY_UP);
 
-    uint32_t modsDepressed = xkb_state_serialize_mods(xkb_state, (xkb_state_component)XKB_STATE_DEPRESSED);
-    uint32_t modsLatched = xkb_state_serialize_mods(xkb_state, (xkb_state_component)XKB_STATE_LATCHED);
-    uint32_t modsLocked = xkb_state_serialize_mods(xkb_state, (xkb_state_component)XKB_STATE_LOCKED);
-    uint32_t group = xkb_state_serialize_group(xkb_state, (xkb_state_component)XKB_STATE_EFFECTIVE);
+    uint32_t modsDepressed = xkb_state_serialize_mods(xkbState(), (xkb_state_component)XKB_STATE_DEPRESSED);
+    uint32_t modsLatched = xkb_state_serialize_mods(xkbState(), (xkb_state_component)XKB_STATE_LATCHED);
+    uint32_t modsLocked = xkb_state_serialize_mods(xkbState(), (xkb_state_component)XKB_STATE_LOCKED);
+    uint32_t group = xkb_state_serialize_group(xkbState(), (xkb_state_component)XKB_STATE_EFFECTIVE);
 
     if (this->modsDepressed == modsDepressed
             && this->modsLatched == modsLatched
@@ -266,7 +265,7 @@ void QWaylandKeyboardPrivate::maybeUpdateKeymap()
 
     pendingKeymap = false;
 #if QT_CONFIG(xkbcommon)
-    if (!xkb_context)
+    if (!xkbContext())
         return;
 
     createXKBKeymap();
@@ -274,7 +273,7 @@ void QWaylandKeyboardPrivate::maybeUpdateKeymap()
         send_keymap(res->handle, WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1, keymap_fd, keymap_size);
     }
 
-    xkb_state_update_mask(xkb_state, 0, modsLatched, modsLocked, 0, 0, 0);
+    xkb_state_update_mask(xkbState(), 0, modsLatched, modsLocked, 0, 0, 0);
     if (focusResource)
         send_modifiers(focusResource->handle,
                        compositor()->nextSerial(),
@@ -316,18 +315,6 @@ static int createAnonymousFile(size_t size)
     return fd;
 }
 
-void QWaylandKeyboardPrivate::initXKB()
-{
-    xkb_context = xkb_context_new(static_cast<xkb_context_flags>(0));
-    if (!xkb_context) {
-        qWarning("Failed to create a XKB context: keymap will not be supported");
-        return;
-    }
-
-    createXKBKeymap();
-}
-
-
 void QWaylandKeyboardPrivate::createXKBState(xkb_keymap *keymap)
 {
     char *keymap_str = xkb_keymap_get_as_string(keymap, XKB_KEYMAP_FORMAT_TEXT_V1);
@@ -356,9 +343,9 @@ void QWaylandKeyboardPrivate::createXKBState(xkb_keymap *keymap)
     strcpy(keymap_area, keymap_str);
     free(keymap_str);
 
-    if (xkb_state)
-        xkb_state_unref(xkb_state);
-    xkb_state = xkb_state_new(keymap);
+    mXkbState.reset(xkb_state_new(keymap));
+    if (!mXkbState)
+        qWarning("Failed to create XKB state");
 }
 
 uint QWaylandKeyboardPrivate::toWaylandXkbV1Key(const uint nativeScanCode)
@@ -370,32 +357,34 @@ uint QWaylandKeyboardPrivate::toWaylandXkbV1Key(const uint nativeScanCode)
 
 void QWaylandKeyboardPrivate::createXKBKeymap()
 {
-    if (!xkb_context)
+    if (!xkbContext())
         return;
 
-    auto keymap = seat->keymap();
-    struct xkb_rule_names rule_names = { strdup(qPrintable(keymap->rules())),
-                                         strdup(qPrintable(keymap->model())),
-                                         strdup(qPrintable(keymap->layout())),
-                                         strdup(qPrintable(keymap->variant())),
-                                         strdup(qPrintable(keymap->options())) };
-    struct xkb_keymap *xkbKeymap = xkb_keymap_new_from_names(xkb_context, &rule_names, static_cast<xkb_keymap_compile_flags>(0));
-
+    QWaylandKeymap *keymap = seat->keymap();
+    QByteArray rules = keymap->rules().toLocal8Bit();
+    QByteArray model = keymap->model().toLocal8Bit();
+    QByteArray layout = keymap->layout().toLocal8Bit();
+    QByteArray variant = keymap->variant().toLocal8Bit();
+    QByteArray options = keymap->options().toLocal8Bit();
+
+    struct xkb_rule_names rule_names = {
+        rules.constData(),
+        model.constData(),
+        layout.constData(),
+        variant.constData(),
+        options.constData()
+    };
+
+    QXkbCommon::ScopedXKBKeymap xkbKeymap(xkb_keymap_new_from_names(xkbContext(), &rule_names,
+                                                                    XKB_KEYMAP_COMPILE_NO_FLAGS));
     if (xkbKeymap) {
         scanCodesByQtKey.clear();
-        createXKBState(xkbKeymap);
-        xkb_keymap_unref(xkbKeymap);
+        createXKBState(xkbKeymap.get());
     } else {
         qWarning("Failed to load the '%s' XKB keymap.", qPrintable(keymap->layout()));
     }
-
-    free((char *)rule_names.rules);
-    free((char *)rule_names.model);
-    free((char *)rule_names.layout);
-    free((char *)rule_names.variant);
-    free((char *)rule_names.options);
 }
-#endif
+#endif // QT_CONFIG(xkbcommon)
 
 void QWaylandKeyboardPrivate::sendRepeatInfo()
 {
@@ -430,7 +419,7 @@ QWaylandKeyboard::QWaylandKeyboard(QWaylandSeat *seat, QObject *parent)
     connect(keymap, &QWaylandKeymap::rulesChanged, this, &QWaylandKeyboard::updateKeymap);
     connect(keymap, &QWaylandKeymap::modelChanged, this, &QWaylandKeyboard::updateKeymap);
 #if QT_CONFIG(xkbcommon)
-    d->initXKB();
+    d->createXKBKeymap();
 #endif
 }
 
diff --git a/src/compositor/compositor_api/qwaylandkeyboard_p.h b/src/compositor/compositor_api/qwaylandkeyboard_p.h
index 87e89e85e..c4d29c2d6 100644
--- a/src/compositor/compositor_api/qwaylandkeyboard_p.h
+++ b/src/compositor/compositor_api/qwaylandkeyboard_p.h
@@ -51,6 +51,7 @@
 //
 // We mean it.
 //
+#include <QtWaylandCompositor/private/qwaylandcompositor_p.h>
 
 #include <QtWaylandCompositor/private/qtwaylandcompositorglobal_p.h>
 #include <QtWaylandCompositor/qwaylandseat.h>
@@ -64,6 +65,7 @@
 
 #if QT_CONFIG(xkbcommon)
 #include <xkbcommon/xkbcommon.h>
+#include <QtXkbCommonSupport/private/qxkbcommon_p.h>
 #endif
 
 
@@ -87,7 +89,10 @@ public:
                    uint32_t mods_latched, uint32_t mods_locked, uint32_t group);
 
 #if QT_CONFIG(xkbcommon)
-    struct xkb_state *xkbState() const { return xkb_state; }
+    struct xkb_state *xkbState() const { return mXkbState.get(); }
+    struct xkb_context *xkbContext() const {
+        return QWaylandCompositorPrivate::get(seat->compositor())->xkbContext();
+    }
     uint32_t xkbModsMask() const { return modsDepressed | modsLatched | modsLocked; }
     void maybeUpdateXkbScanCodeTable();
 #endif
@@ -107,7 +112,6 @@ protected:
 
 private:
 #if QT_CONFIG(xkbcommon)
-    void initXKB();
     void createXKBKeymap();
     void createXKBState(xkb_keymap *keymap);
 #endif
@@ -134,8 +138,7 @@ private:
     char *keymap_area = nullptr;
     using ScanCodeKey = std::pair<uint,int>; // group/layout and QtKey
     QMap<ScanCodeKey, uint> scanCodesByQtKey;
-    struct xkb_context *xkb_context = nullptr;
-    struct xkb_state *xkb_state = nullptr;
+    QXkbCommon::ScopedXKBState mXkbState;
 #endif
 
     quint32 repeatRate = 40;