gui_events: get rid of the nativeButtonId callback and use MouseButtons

We had a problem where some platform (xcb, wayland) would send native
button code and (win32, cocoa) would send the converted MouseButton values.

The Mouse*Event::button getter was doing a conversion from xcb to mouseButton
and hover time a nativeButtonId() getter was added likely to work around values
not being correct on Windows/Mac.

This patch makes it so that xcb, wayland now send converted MouseButton values like
win32 and cocoa so that we can get rid of any conversion in the button() getter
and remove nativeButtonId() entirely.

The button() getter is now renamed buttons() and returns a MouseButtons flag.

Change-Id: I5c6cb8b27bfbde1e532c86230c8c792c9e22e66e
Reviewed-on: https://codereview.kdab.com/c/kdab/kdutils/+/132419
Reviewed-by: Miłosz Kosobucki <[email protected]>
Tested-by: Continuous Integration <[email protected]>

src/KDGui/abstract_platform_window.h

@@ -13,6 +13,7 @@
 
 #include <KDGui/kdgui_global.h>
 #include <KDGui/kdgui_keys.h>
+#include <KDGui/gui_events.h>
 
 #include <string>
 
@@ -68,15 +69,15 @@ class KDGUI_API AbstractPlatformWindow
     //  8 = Navigate Back, 9 = Navigate Forward
     // TODO: Wrap these up in an enum and map from the platform specific codes to our codes
     virtual void handleMousePress(
-            uint32_t timestamp, uint8_t button,
+            uint32_t timestamp, MouseButtons buttons,
             int16_t xPos, int16_t yPos) = 0;
 
     virtual void handleMouseRelease(
-            uint32_t timestamp, uint8_t button,
+            uint32_t timestamp, MouseButtons buttons,
             int16_t xPos, int16_t yPos) = 0;
 
     virtual void handleMouseMove(
-            uint32_t timestamp, uint8_t button,
+            uint32_t timestamp, MouseButtons buttons,
             int64_t xPos, int64_t yPos) = 0;
 
     virtual void handleMouseWheel(uint32_t timestamp, int32_t xDelta, int32_t yDelta) = 0;

src/KDGui/gui_events.h

@@ -31,76 +31,74 @@ using MouseButtons = uint32_t;
 class MousePressEvent : public KDFoundation::Event
 {
 public:
-    explicit MousePressEvent(uint32_t timestamp, uint8_t button,
+    explicit MousePressEvent(uint32_t timestamp, MouseButtons buttons,
                              int16_t xPos, int16_t yPos)
         : KDFoundation::Event(KDFoundation::Event::Type::MousePress)
         , m_timestamp{ timestamp }
-        , m_button{ button }
+        , m_buttons{ buttons }
         , m_xPos{ xPos }
         , m_yPos{ yPos }
     {
     }
 
     uint32_t timestamp() const { return m_timestamp; }
-    MouseButton button() const { return static_cast<MouseButton>(1 << (m_button - 1)); }
+    MouseButtons buttons() const { return m_buttons; }
     int16_t xPos() const { return m_xPos; }
     int16_t yPos() const { return m_yPos; }
 
 private:
     uint32_t m_timestamp;
-    uint8_t m_button;
+    MouseButtons m_buttons;
     int16_t m_xPos;
     int16_t m_yPos;
 };
 
 class MouseReleaseEvent : public KDFoundation::Event
 {
 public:
-    explicit MouseReleaseEvent(uint32_t timestamp, uint8_t button,
+    explicit MouseReleaseEvent(uint32_t timestamp, MouseButtons buttons,
                                int16_t xPos, int16_t yPos)
         : KDFoundation::Event(KDFoundation::Event::Type::MouseRelease)
         , m_timestamp{ timestamp }
-        , m_button{ button }
+        , m_buttons{ buttons }
         , m_xPos{ xPos }
         , m_yPos{ yPos }
     {
     }
 
     uint32_t timestamp() const { return m_timestamp; }
-    uint8_t nativeButtonId() const { return m_button; }
-    MouseButton button() const { return static_cast<MouseButton>(1 << (m_button - 1)); }
+    MouseButtons buttons() const { return m_buttons; }
     int16_t xPos() const { return m_xPos; }
     int16_t yPos() const { return m_yPos; }
 
 private:
     uint32_t m_timestamp;
-    uint8_t m_button;
+    MouseButtons m_buttons;
     int16_t m_xPos;
     int16_t m_yPos;
 };
 
 class MouseMoveEvent : public KDFoundation::Event
 {
 public:
-    explicit MouseMoveEvent(uint32_t timestamp, uint8_t button,
+    explicit MouseMoveEvent(uint32_t timestamp, MouseButtons buttons,
                             int64_t xPos, int64_t yPos)
         : KDFoundation::Event(KDFoundation::Event::Type::MouseMove)
         , m_timestamp{ timestamp }
-        , m_button{ button }
+        , m_buttons{ buttons }
         , m_xPos{ xPos }
         , m_yPos{ yPos }
     {
     }
 
     uint32_t timestamp() const { return m_timestamp; }
-    uint8_t nativeButtonId() const { return m_button; }
-    MouseButton button() const { return static_cast<MouseButton>(1 << (m_button - 1)); }
+    MouseButtons buttons() const { return m_buttons; }
     int64_t xPos() const { return m_xPos; }
     int64_t yPos() const { return m_yPos; }
 
 private:
     uint32_t m_timestamp;
-    uint8_t m_button;
+    MouseButtons m_buttons;
     int64_t m_xPos;
     int64_t m_yPos;
 };

src/KDGui/platform/cocoa/cocoa_platform_window.h

@@ -50,9 +50,9 @@ class CocoaPlatformWindow : public AbstractPlatformWindow
     void setSize(uint32_t width, uint32_t height) override;
 
     void handleResize(uint32_t width, uint32_t height) override;
-    void handleMousePress(uint32_t timestamp, uint8_t button, int16_t xPos, int16_t yPos) override;
-    void handleMouseRelease(uint32_t timestamp, uint8_t button, int16_t xPos, int16_t yPos) override;
-    void handleMouseMove(uint32_t timestamp, uint8_t button, int64_t xPos, int64_t yPos) override;
+    void handleMousePress(uint32_t timestamp, MouseButtons buttons, int16_t xPos, int16_t yPos) override;
+    void handleMouseRelease(uint32_t timestamp, MouseButtons buttons, int16_t xPos, int16_t yPos) override;
+    void handleMouseMove(uint32_t timestamp, MouseButtons buttons, int64_t xPos, int64_t yPos) override;
     void handleMouseWheel(uint32_t timestamp, int32_t xDelta, int32_t yDelta) override;
     void handleKeyPress(uint32_t timestamp, uint8_t nativeKeyCode, Key key, KeyboardModifiers modifiers) override;
     void handleKeyRelease(uint32_t timestamp, uint8_t nativeKeyCode, Key key, KeyboardModifiers modifiers) override;

src/KDGui/platform/cocoa/cocoa_platform_window.mm

@@ -505,21 +505,21 @@ - (void)keyUp:(NSEvent *)event
     CoreApplication::instance()->sendEvent(m_window, &ev);
 }
 
-void CocoaPlatformWindow::handleMousePress(uint32_t timestamp, uint8_t button, int16_t xPos, int16_t yPos)
+void CocoaPlatformWindow::handleMousePress(uint32_t timestamp, MouseButtons buttons, int16_t xPos, int16_t yPos)
 {
-    MousePressEvent ev{ timestamp, button, xPos, yPos };
+    MousePressEvent ev{ timestamp, buttons, xPos, yPos };
     CoreApplication::instance()->sendEvent(m_window, &ev);
 }
 
-void CocoaPlatformWindow::handleMouseRelease(uint32_t timestamp, uint8_t button, int16_t xPos, int16_t yPos)
+void CocoaPlatformWindow::handleMouseRelease(uint32_t timestamp, MouseButtons buttons, int16_t xPos, int16_t yPos)
 {
-    MouseReleaseEvent ev{ timestamp, button, xPos, yPos };
+    MouseReleaseEvent ev{ timestamp, buttons, xPos, yPos };
     CoreApplication::instance()->sendEvent(m_window, &ev);
 }
 
-void CocoaPlatformWindow::handleMouseMove(uint32_t timestamp, uint8_t button, int64_t xPos, int64_t yPos)
+void CocoaPlatformWindow::handleMouseMove(uint32_t timestamp, MouseButtons buttons, int64_t xPos, int64_t yPos)
 {
-    MouseMoveEvent ev{ timestamp, button, xPos, yPos };
+    MouseMoveEvent ev{ timestamp, buttons, xPos, yPos };
     CoreApplication::instance()->sendEvent(m_window, &ev);
 }
 

src/KDGui/platform/linux/wayland/linux_wayland_platform_input.cpp

@@ -15,6 +15,7 @@
 #include <KDGui/platform/linux/common/linux_xkb_keyboard_map.h>
 #include <KDGui/platform/linux/common/linux_xkb.h>
 #include <KDGui/window.h>
+#include <KDGui/gui_events.h>
 
 #include <unistd.h>
 #include <sys/mman.h>
@@ -285,24 +286,24 @@ void LinuxWaylandPlatformInput::pointerMotion(wl_pointer *pointer, uint32_t time
         m_pointer.accumulatedEvent.pos = m_pointer.pos;
         m_pointer.accumulatedEvent.time = time;
     } else {
-        m_pointer.focus->handleMouseMove(time, 0, m_pointer.pos.x, m_pointer.pos.y);
+        m_pointer.focus->handleMouseMove(time, MouseButton::NoButton, m_pointer.pos.x, m_pointer.pos.y);
     }
 }
 
 void LinuxWaylandPlatformInput::pointerButton(wl_pointer *pointer, uint32_t serial, uint32_t time, uint32_t button, uint32_t state)
 {
-    const int btn = [=]() {
+    const MouseButton btn = [=]() {
         switch (button) {
         case BTN_LEFT:
-            return 1;
+            return MouseButton::LeftButton;
         case BTN_MIDDLE:
-            return 2;
+            return MouseButton::MiddleButton;
         case BTN_RIGHT:
-            return 3;
+            return MouseButton::RightButton;
         default:
             break;
         }
-        return 0;
+        return MouseButton::NoButton;
     }();
 
     if (state == WL_POINTER_BUTTON_STATE_PRESSED) {
@@ -341,7 +342,7 @@ void LinuxWaylandPlatformInput::pointerFrame(wl_pointer *pointer)
         ev.axis = Position(0, 0);
     }
     if (ev.pos != Position(0, 0)) {
-        m_pointer.focus->handleMouseMove(ev.time, 0, ev.pos.x, ev.pos.y);
+        m_pointer.focus->handleMouseMove(ev.time, MouseButton::NoButton, ev.pos.x, ev.pos.y);
         ev.pos = Position(0, 0);
     }
     if (ev.delta != Position(0, 0)) {
@@ -510,8 +511,8 @@ void LinuxWaylandPlatformInput::touchDown(wl_touch *touch, uint32_t serial, uint
     m_touch.points.push_back(Touch::Point{ id, Position(int64_t(wl_fixed_to_double(x)), int64_t(wl_fixed_to_double(y))) });
     m_touch.time = time;
 
-    m_touch.focus->handleMouseMove(time, 1, m_touch.points[0].pos.x, m_touch.points[0].pos.y);
-    m_touch.focus->handleMousePress(time, 1, int16_t(m_touch.points[0].pos.x), int16_t(m_touch.points[0].pos.y));
+    m_touch.focus->handleMouseMove(time, MouseButton::LeftButton, m_touch.points[0].pos.x, m_touch.points[0].pos.y);
+    m_touch.focus->handleMousePress(time, MouseButton::LeftButton, int16_t(m_touch.points[0].pos.x), int16_t(m_touch.points[0].pos.y));
 }
 
 void LinuxWaylandPlatformInput::touchUp(wl_touch *touch, uint32_t serial, uint32_t time, int32_t id)
@@ -520,7 +521,7 @@ void LinuxWaylandPlatformInput::touchUp(wl_touch *touch, uint32_t serial, uint32
         return;
     }
 
-    m_touch.focus->handleMouseRelease(time, 1, int16_t(m_touch.points[0].pos.x), int16_t(m_touch.points[0].pos.y));
+    m_touch.focus->handleMouseRelease(time, MouseButton::LeftButton, int16_t(m_touch.points[0].pos.x), int16_t(m_touch.points[0].pos.y));
     m_touch.points.clear();
 }
 
@@ -530,7 +531,7 @@ void LinuxWaylandPlatformInput::touchMotion(wl_touch *touch, uint32_t time, int3
         return;
     }
 
-    m_touch.focus->handleMouseMove(time, 1, m_touch.points[0].pos.x, m_touch.points[0].pos.y);
+    m_touch.focus->handleMouseMove(time, MouseButton::LeftButton, m_touch.points[0].pos.x, m_touch.points[0].pos.y);
     m_touch.time = time;
 }
 
@@ -541,7 +542,7 @@ void LinuxWaylandPlatformInput::touchFrame(wl_touch *touch)
 void LinuxWaylandPlatformInput::touchCancel(wl_touch *touch)
 {
     if (m_touch.points.size() > 0) {
-        m_touch.focus->handleMouseRelease(m_touch.time, 1, int16_t(m_touch.points[0].pos.x), int16_t(m_touch.points[0].pos.y));
+        m_touch.focus->handleMouseRelease(m_touch.time, MouseButton::LeftButton, int16_t(m_touch.points[0].pos.x), int16_t(m_touch.points[0].pos.y));
         m_touch.points.clear();
     }
 }

src/KDGui/platform/linux/wayland/linux_wayland_platform_window.cpp

@@ -10,7 +10,6 @@
 */
 
 #include "linux_wayland_platform_window.h"
-#include <KDGui/gui_events.h>
 #include <KDGui/window.h>
 #include "linux_wayland_platform_integration.h"
 #include "linux_wayland_platform_output.h"
@@ -163,25 +162,25 @@ void LinuxWaylandPlatformWindow::handleResize(uint32_t width, uint32_t height)
     CoreApplication::instance()->sendEvent(m_window, &ev);
 }
 
-void LinuxWaylandPlatformWindow::handleMousePress(uint32_t timestamp, uint8_t button,
+void LinuxWaylandPlatformWindow::handleMousePress(uint32_t timestamp, MouseButtons buttons,
                                                   int16_t xPos, int16_t yPos)
 {
-    MousePressEvent ev{ timestamp, button, static_cast<int16_t>(scaleByFactor(xPos)), static_cast<int16_t>(scaleByFactor(yPos)) };
+    MousePressEvent ev{ timestamp, buttons, static_cast<int16_t>(scaleByFactor(xPos)), static_cast<int16_t>(scaleByFactor(yPos)) };
     CoreApplication::instance()->sendEvent(m_window, &ev);
 }
 
-void LinuxWaylandPlatformWindow::handleMouseRelease(uint32_t timestamp, uint8_t button,
+void LinuxWaylandPlatformWindow::handleMouseRelease(uint32_t timestamp, MouseButtons buttons,
                                                     int16_t xPos, int16_t yPos)
 {
-    MouseReleaseEvent ev{ timestamp, button, static_cast<int16_t>(scaleByFactor(xPos)), static_cast<int16_t>(scaleByFactor(yPos)) };
+    MouseReleaseEvent ev{ timestamp, buttons, static_cast<int16_t>(scaleByFactor(xPos)), static_cast<int16_t>(scaleByFactor(yPos)) };
     CoreApplication::instance()->sendEvent(m_window, &ev);
 }
 
-void LinuxWaylandPlatformWindow::handleMouseMove(uint32_t timestamp, uint8_t button,
+void LinuxWaylandPlatformWindow::handleMouseMove(uint32_t timestamp, MouseButtons buttons,
                                                  int64_t x, int64_t y)
 {
     if (m_cursorMode == CursorMode::Normal) {
-        MouseMoveEvent ev{ timestamp, button, scaleByFactor(x), scaleByFactor(y) };
+        MouseMoveEvent ev{ timestamp, buttons, scaleByFactor(x), scaleByFactor(y) };
         CoreApplication::instance()->sendEvent(m_window, &ev);
     }
 }
@@ -190,7 +189,7 @@ void LinuxWaylandPlatformWindow::handleMouseMoveRelative(uint32_t timestamp, int
 {
     if (m_cursorMode == CursorMode::Disabled) {
         Position pos = window()->cursorPosition.get() + Position(dx, dy);
-        MouseMoveEvent ev{ timestamp, 0, pos.x, pos.y };
+        MouseMoveEvent ev{ timestamp, MouseButton::NoButton, pos.x, pos.y };
         CoreApplication::instance()->sendEvent(m_window, &ev);
     }
 }

src/KDGui/platform/linux/wayland/linux_wayland_platform_window.h

@@ -13,7 +13,6 @@
 
 #include <KDGui/abstract_platform_window.h>
 #include <KDGui/kdgui_global.h>
-
 #include <kdbindings/signal.h>
 
 struct wl_array;
@@ -65,15 +64,15 @@ class KDGUI_API LinuxWaylandPlatformWindow : public AbstractPlatformWindow
     void handleResize(uint32_t width, uint32_t height) override;
 
     void handleMousePress(
-            uint32_t timestamp, uint8_t button,
+            uint32_t timestamp, MouseButtons buttons,
             int16_t xPos, int16_t yPos) override;
 
     void handleMouseRelease(
-            uint32_t timestamp, uint8_t button,
+            uint32_t timestamp, MouseButtons buttons,
             int16_t xPos, int16_t yPos) override;
 
     void handleMouseMove(
-            uint32_t timestamp, uint8_t button,
+            uint32_t timestamp, MouseButtons buttons,
             int64_t xPos, int64_t yPos) override;
     void handleMouseMoveRelative(uint32_t timestamp, int64_t dx, int64_t dy);
 

src/KDGui/platform/linux/xcb/linux_xcb_platform_event_loop.cpp

@@ -13,6 +13,7 @@
 #include "linux_xcb_platform_integration.h"
 #include "linux_xkb_keyboard.h"
 #include <KDGui/window.h>
+#include <KDGui/gui_events.h>
 
 #include <KDFoundation/file_descriptor_notifier.h>
 
@@ -67,6 +68,18 @@ void LinuxXcbPlatformEventLoop::waitForEvents(int timeout)
 
 void LinuxXcbPlatformEventLoop::processXcbEvents()
 {
+    auto xcbButtonToMouseButton = [](xcb_button_t b) {
+        switch (b) {
+        case 1:
+        case 2:
+        case 3:
+            return static_cast<MouseButton>(1 << (b - 1));
+        default:
+            break;
+        }
+        return MouseButton::NoButton;
+    };
+
     const auto connection = m_platformIntegration->connection();
     while (auto xcbEvent = xcb_poll_for_event(connection)) {
         // We don't care where the event came from (server or client), so remove that
@@ -130,14 +143,16 @@ void LinuxXcbPlatformEventLoop::processXcbEvents()
             case 2:
             case 3:
             default: {
+                const MouseButton mouseButton = xcbButtonToMouseButton(button);
+
                 SPDLOG_LOGGER_DEBUG(m_logger,
                                     "Mouse press event for button {} at pos ({}, {})",
                                     button,
                                     buttonEvent->event_x,
                                     buttonEvent->event_y);
                 window->handleMousePress(
                         buttonEvent->time,
-                        button,
+                        mouseButton,
                         buttonEvent->event_x,
                         buttonEvent->event_y);
                 break;
@@ -209,7 +224,7 @@ void LinuxXcbPlatformEventLoop::processXcbEvents()
                                 buttonEvent->event_y);
             window->handleMouseRelease(
                     buttonEvent->time,
-                    buttonEvent->detail,
+                    xcbButtonToMouseButton(buttonEvent->detail),
                     buttonEvent->event_x,
                     buttonEvent->event_y);
             break;
@@ -226,7 +241,7 @@ void LinuxXcbPlatformEventLoop::processXcbEvents()
                                 mouseMoveEvent->event_y);
             window->handleMouseMove(
                     mouseMoveEvent->time,
-                    mouseMoveEvent->detail,
+                    xcbButtonToMouseButton(mouseMoveEvent->detail),
                     static_cast<int64_t>(mouseMoveEvent->event_x),
                     static_cast<int64_t>(mouseMoveEvent->event_y));
             break;