Unify Key handling accross tool handlers and use GtkEventControllerKey

src/core/control/tools/BaseShapeHandler.cpp

@@ -49,31 +49,29 @@ void BaseShapeHandler::cancelStroke() {
     this->lastSnappingRange = Range();
 }
 
-auto BaseShapeHandler::onKeyEvent(GdkEventKey* event) -> bool {
-    if (event->is_modifier) {
-        GdkModifierType state;
-        if (event->keyval == GDK_KEY_Shift_L || event->keyval == GDK_KEY_Shift_R) {
-            // event->state contains the modifiers' states BEFORE the current event
-            // We need a XOR to handler both keypress and keyrelease at once
-            state = static_cast<GdkModifierType>(event->state ^ GDK_SHIFT_MASK);
-        } else if (event->keyval == GDK_KEY_Control_L || event->keyval == GDK_KEY_Control_R) {
-            state = static_cast<GdkModifierType>(event->state ^ GDK_CONTROL_MASK);
-        } else if (event->keyval == GDK_KEY_Alt_L || event->keyval == GDK_KEY_Alt_R) {
-            state = static_cast<GdkModifierType>(event->state ^ GDK_MOD1_MASK);
-        } else {
-            return false;
-        }
+auto BaseShapeHandler::onKeyEvent(const KeyEvent& event, bool pressed) -> bool {
+    bool isAltDown = event.state & GDK_MOD1_MASK;
+    bool isShiftDown = event.state & GDK_SHIFT_MASK;
+    bool isControlDown = event.state & GDK_CONTROL_MASK;
+    // event->state contains the modifiers' states BEFORE the current event
+    if (event.keyval == GDK_KEY_Shift_L || event.keyval == GDK_KEY_Shift_R) {
+        isShiftDown = pressed;
+    } else if (event.keyval == GDK_KEY_Control_L || event.keyval == GDK_KEY_Control_R) {
+        isControlDown = pressed;
+    } else if (event.keyval == GDK_KEY_Alt_L || event.keyval == GDK_KEY_Alt_R) {
+        isAltDown = pressed;
+    } else {
+        return false;
+    }
+    this->updateShape(isAltDown, isShiftDown, isControlDown);
+
+    return true;
+}
 
-        bool isAltDown = state & GDK_MOD1_MASK;
-        bool isShiftDown = state & GDK_SHIFT_MASK;
-        bool isControlDown = state & GDK_CONTROL_MASK;
+auto BaseShapeHandler::onKeyPressEvent(const KeyEvent& event) -> bool { return onKeyEvent(event, true); }
 
-        this->updateShape(isAltDown, isShiftDown, isControlDown);
+auto BaseShapeHandler::onKeyReleaseEvent(const KeyEvent& event) -> bool { return onKeyEvent(event, false); }
 
-        return true;
-    }
-    return false;
-}
 
 auto BaseShapeHandler::onMotionNotifyEvent(const PositionInputData& pos, double zoom) -> bool {
     Point newPoint(pos.x / zoom, pos.y / zoom);

src/core/control/tools/BaseShapeHandler.h

@@ -51,7 +51,8 @@ class BaseShapeHandler: public InputHandler {
     void onButtonReleaseEvent(const PositionInputData& pos, double zoom) override;
     void onButtonPressEvent(const PositionInputData& pos, double zoom) override;
     void onButtonDoublePressEvent(const PositionInputData& pos, double zoom) override;
-    bool onKeyEvent(GdkEventKey* event) override;
+    bool onKeyPressEvent(const KeyEvent& event) override;
+    bool onKeyReleaseEvent(const KeyEvent& event) override;
 
     std::unique_ptr<xoj::view::OverlayView> createView(xoj::view::Repaintable* parent) const override;
 
@@ -81,6 +82,8 @@ class BaseShapeHandler: public InputHandler {
      */
     void cancelStroke();
 
+    bool onKeyEvent(const KeyEvent& event, bool pressed);
+
 protected:
     /**
      * modifyModifiersByDrawDir

src/core/control/tools/InputHandler.h

@@ -22,6 +22,7 @@ class Control;
 class Point;
 class Stroke;
 class PositionInputData;
+struct KeyEvent;
 
 namespace xoj::view {
 class OverlayView;
@@ -55,7 +56,8 @@ class InputHandler: public OverlayBase {
      * It is used to update internal data structures and queue
      * repaints of the XojPageView if necessary.
      */
-    virtual bool onKeyEvent(GdkEventKey* event) = 0;
+    virtual bool onKeyPressEvent(const KeyEvent& event) = 0;
+    virtual bool onKeyReleaseEvent(const KeyEvent& event) = 0;
 
     /**
      * The current input device for stroken, do not react on other devices (linke mices)

src/core/control/tools/SplineHandler.cpp

@@ -16,6 +16,7 @@
 #include "control/tools/SnapToGridInputHandler.h"  // for SnapToGridInputHan...
 #include "control/zoom/ZoomControl.h"
 #include "gui/XournalppCursor.h"                 // for XournalppCursor
+#include "gui/inputdevices/InputEvents.h"        // for KeyEvent
 #include "gui/inputdevices/PositionInputData.h"  // for PositionInputData
 #include "model/Document.h"                      // for Document
 #include "model/Layer.h"                         // for Layer
@@ -48,21 +49,15 @@ constexpr double SCALE_AMOUNT = 1.05;
 constexpr double MAX_TANGENT_LENGTH = 2000.0;
 constexpr double MIN_TANGENT_LENGTH = 1.0;
 
-auto SplineHandler::onKeyEvent(GdkEventKey* event) -> bool {
-    if (!stroke || ((event->type != GDK_KEY_PRESS) ==
-                    (event->keyval != GDK_KEY_Escape))) {  // except for escape key only act on key
-                                                           // press event, not on key release event
+auto SplineHandler::onKeyPressEvent(const KeyEvent& event) -> bool {
+    if (!stroke) {
         return false;
     }
 
     xoj_assert(!this->knots.empty() && this->knots.size() == this->tangents.size());
     Range rg = this->computeLastSegmentRepaintRange();
 
-    switch (event->keyval) {
-        case GDK_KEY_Escape: {
-            this->finalizeSpline();
-            return true;
-        }
+    switch (event.keyval) {
         case GDK_KEY_BackSpace: {
             if (this->knots.size() == 1) {
                 return true;
@@ -96,7 +91,7 @@ auto SplineHandler::onKeyEvent(GdkEventKey* event) -> bool {
         }
         case GDK_KEY_r:
         case GDK_KEY_R: {  // r like "rotate"
-            double angle = (event->state & GDK_SHIFT_MASK) ? -ROTATE_AMOUNT : ROTATE_AMOUNT;
+            double angle = (event.keyval == GDK_KEY_R) ? -ROTATE_AMOUNT : ROTATE_AMOUNT;
             double xOld = this->tangents.back().x;
             double yOld = this->tangents.back().y;
             double xNew = cos(angle * M_PI / 180) * xOld + sin(angle * M_PI / 180) * yOld;
@@ -111,9 +106,11 @@ auto SplineHandler::onKeyEvent(GdkEventKey* event) -> bool {
             double yOld = this->tangents.back().y;
             double length = 2 * sqrt(pow(xOld, 2) + pow(yOld, 2));
             double factor = 1;
-            if ((event->state & GDK_SHIFT_MASK) && length >= MIN_TANGENT_LENGTH) {
-                factor = 1 / SCALE_AMOUNT;
-            } else if (!(event->state & GDK_SHIFT_MASK) && length <= MAX_TANGENT_LENGTH) {
+            if (event.keyval == GDK_KEY_S) {
+                if (length >= MIN_TANGENT_LENGTH) {
+                    factor = 1 / SCALE_AMOUNT;
+                }
+            } else if (length <= MAX_TANGENT_LENGTH) {
                 factor = SCALE_AMOUNT;
             }
             double xNew = xOld * factor;
@@ -130,6 +127,14 @@ auto SplineHandler::onKeyEvent(GdkEventKey* event) -> bool {
     return true;
 }
 
+bool SplineHandler::onKeyReleaseEvent(const KeyEvent& event) {
+    if (event.keyval == GDK_KEY_Escape) {
+        this->finalizeSpline();
+        return true;
+    }
+    return false;
+}
+
 auto SplineHandler::onMotionNotifyEvent(const PositionInputData& pos, double zoom) -> bool {
     if (!stroke) {
         return false;

src/core/control/tools/SplineHandler.h

@@ -76,7 +76,8 @@ class SplineHandler: public InputHandler, public ZoomListener {
     void onButtonReleaseEvent(const PositionInputData& pos, double zoom) override;
     void onButtonPressEvent(const PositionInputData& pos, double zoom) override;
     void onButtonDoublePressEvent(const PositionInputData& pos, double zoom) override;
-    bool onKeyEvent(GdkEventKey* event) override;
+    bool onKeyPressEvent(const KeyEvent& event) override;
+    bool onKeyReleaseEvent(const KeyEvent& event) override;
 
     void finalizeSpline();
 

src/core/control/tools/StrokeHandler.cpp

@@ -48,7 +48,8 @@ StrokeHandler::StrokeHandler(Control* control, const PageRef& page):
 
 StrokeHandler::~StrokeHandler() = default;
 
-auto StrokeHandler::onKeyEvent(GdkEventKey* event) -> bool { return false; }
+auto StrokeHandler::onKeyPressEvent(const KeyEvent&) -> bool { return false; }
+auto StrokeHandler::onKeyReleaseEvent(const KeyEvent&) -> bool { return false; }
 
 auto StrokeHandler::onMotionNotifyEvent(const PositionInputData& pos, double zoom) -> bool {
     if (!stroke) {

src/core/control/tools/StrokeHandler.h

@@ -61,7 +61,8 @@ class StrokeHandler: public InputHandler {
     void onButtonReleaseEvent(const PositionInputData& pos, double zoom) override;
     void onButtonPressEvent(const PositionInputData& pos, double zoom) override;
     void onButtonDoublePressEvent(const PositionInputData& pos, double zoom) override;
-    bool onKeyEvent(GdkEventKey* event) override;
+    bool onKeyPressEvent(const KeyEvent& event) override;
+    bool onKeyReleaseEvent(const KeyEvent& event) override;
 
     /**
      * @brief Add a straight line to the stroke (if the movement is valid).

src/core/control/tools/TextEditor.cpp

@@ -297,10 +297,11 @@ auto TextEditor::imDeleteSurroundingCallback(GtkIMContext* context, gint offset,
     return true;
 }
 
-auto TextEditor::onKeyPressEvent(GdkEventKey* event) -> bool {
+auto TextEditor::onKeyPressEvent(const KeyEvent& event) -> bool {
 
     // IME needs to handle the input first so the candidate window works correctly
-    if (gtk_im_context_filter_keypress(this->imContext.get(), event)) {
+    auto* e = (GdkEventKey*)(static_cast<GdkEvent*>(event.sourceEvent));
+    if (gtk_im_context_filter_keypress(this->imContext.get(), e)) {
         this->needImReset = true;
 
         GtkTextIter iter = getIteratorAtCursor(this->buffer.get());
@@ -314,13 +315,14 @@ auto TextEditor::onKeyPressEvent(GdkEventKey* event) -> bool {
         return true;
     }
 
-    return keyBindings.processEvent(this, (GdkEvent*)event);
+    return keyBindings.processEvent(this, event);
 }
 
-auto TextEditor::onKeyReleaseEvent(GdkEventKey* event) -> bool {
+auto TextEditor::onKeyReleaseEvent(const KeyEvent& event) -> bool {
     GtkTextIter iter = getIteratorAtCursor(this->buffer.get());
 
-    if (gtk_text_iter_can_insert(&iter, true) && gtk_im_context_filter_keypress(this->imContext.get(), event)) {
+    auto* e = (GdkEventKey*)(static_cast<GdkEvent*>(event.sourceEvent));
+    if (gtk_text_iter_can_insert(&iter, true) && gtk_im_context_filter_keypress(this->imContext.get(), e)) {
         this->needImReset = true;
         return true;
     }

src/core/control/tools/TextEditor.h

@@ -30,6 +30,7 @@ class Text;
 class XojFont;
 class Control;
 class TextEditorCallbacks;
+struct KeyEvent;
 
 namespace xoj::util {
 template <class T>
@@ -48,8 +49,8 @@ class TextEditor: public OverlayBase {
     /** Represents the different kinds of text selection */
     enum class SelectType { WORD, PARAGRAPH, ALL };
 
-    bool onKeyPressEvent(GdkEventKey* event);
-    bool onKeyReleaseEvent(GdkEventKey* event);
+    bool onKeyPressEvent(const KeyEvent& event);
+    bool onKeyReleaseEvent(const KeyEvent& event);
     void mousePressed(double x, double y);
     void mouseMoved(double x, double y);
     void mouseReleased();

src/core/control/tools/TextEditorKeyBindings.h

@@ -17,6 +17,7 @@
 #include <gdk/gdkevents.h>
 #include <gdk/gdkkeysyms.h>
 
+#include "gui/inputdevices/InputEvents.h"
 #include "util/gdk4_helper.h"
 
 #include "TextEditor.h"
@@ -35,7 +36,8 @@ constexpr PressedModifier SHIFT(GDK_SHIFT_MASK);
 
 struct TextEditor::KeyBindings {
     using hash_type = uint64_t;
-    using keyval_type = decltype(gdk_key_event_get_keyval(nullptr));
+    using keyval_type = std::invoke_result_t<decltype(gdk_key_event_get_keyval), GdkEvent*>;
+    static_assert(std::is_same<keyval_type, guint>::value);
     using mapped_type = void (*)(TextEditor*);
     using value_type = std::pair<const hash_type, mapped_type>;
     static_assert(sizeof(PressedModifier) + sizeof(keyval_type) == sizeof(hash_type));
@@ -46,15 +48,10 @@ struct TextEditor::KeyBindings {
         return (static_cast<hash_type>(keyval) | (static_cast<hash_type>(mod.mod) << 32));
     }
 
-    static hash_type hash(GdkEvent* e) {
-        GdkModifierType consumed = gdk_key_event_get_consumed_modifiers(e);
-        GdkModifierType modifiers = gtk_accelerator_get_default_mod_mask();
-        return hash(PressedModifier(GdkModifierType(gdk_event_get_modifier_state(e) & ~consumed & modifiers)),
-                    gdk_key_event_get_keyval(e));
-    }
+    static hash_type hash(const KeyEvent& e) { return hash(PressedModifier(e.state), e.keyval); }
     std::unordered_map<hash_type, mapped_type> table;
 
-    bool processEvent(TextEditor* te, GdkEvent* e) const {
+    bool processEvent(TextEditor* te, const KeyEvent& e) const {
         auto it = table.find(hash(e));
         if (it == table.end()) {
             return false;

src/core/control/tools/VerticalToolHandler.cpp

@@ -8,6 +8,7 @@
 
 #include "control/tools/SnapToGridInputHandler.h"  // for SnapToGridInputHan...
 #include "gui/LegacyRedrawable.h"                  // for Redrawable
+#include "gui/inputdevices/InputEvents.h"          // for KeyEvent
 #include "model/Element.h"                         // for Element
 #include "model/Layer.h"                           // for Layer
 #include "model/XojPage.h"                         // for XojPage
@@ -66,19 +67,17 @@ void VerticalToolHandler::currentPos(double x, double y) {
     this->viewPool->dispatch(xoj::view::VerticalToolView::SET_VERTICAL_SHIFT_REQUEST, ySnapped);
 }
 
-bool VerticalToolHandler::onKeyPressEvent(GdkEventKey* event) {
-    if ((event->keyval == GDK_KEY_Control_L || event->keyval == GDK_KEY_Control_R) &&
-        this->spacingSide == Side::Below) {
+bool VerticalToolHandler::onKeyPressEvent(const KeyEvent& event) {
+    if ((event.keyval == GDK_KEY_Control_L || event.keyval == GDK_KEY_Control_R) && this->spacingSide == Side::Below) {
         this->adoptElements(Side::Above);
         this->viewPool->dispatch(xoj::view::VerticalToolView::SWITCH_DIRECTION_REQUEST);
         return true;
     }
     return false;
 }
 
-bool VerticalToolHandler::onKeyReleaseEvent(GdkEventKey* event) {
-    if ((event->keyval == GDK_KEY_Control_L || event->keyval == GDK_KEY_Control_R) &&
-        this->spacingSide == Side::Above) {
+bool VerticalToolHandler::onKeyReleaseEvent(const KeyEvent& event) {
+    if ((event.keyval == GDK_KEY_Control_L || event.keyval == GDK_KEY_Control_R) && this->spacingSide == Side::Above) {
         this->adoptElements(Side::Below);
         this->viewPool->dispatch(xoj::view::VerticalToolView::SWITCH_DIRECTION_REQUEST);
         return true;

src/core/control/tools/VerticalToolHandler.h

@@ -14,8 +14,7 @@
 #include <memory>    // for unique_ptr
 #include <vector>    // for vector
 
-#include <cairo.h>    // for cairo_surface_t, cairo_t
-#include <gdk/gdk.h>  // for GdkEventKey, GdkWindow
+#include <cairo.h>  // for cairo_surface_t, cairo_t
 
 #include "model/ElementContainer.h"  // for ElementContainer
 #include "model/OverlayBase.h"
@@ -29,6 +28,7 @@ class Layer;
 class MoveUndoAction;
 class Settings;
 class ZoomControl;
+struct KeyEvent;
 
 using ElementPtr = std::unique_ptr<Element>;
 
@@ -59,13 +59,11 @@ class VerticalToolHandler: public ElementContainer, public OverlayBase {
     VerticalToolHandler(VerticalToolHandler&&) = delete;
     VerticalToolHandler&& operator=(VerticalToolHandler&&) = delete;
 
-    void paint(cairo_t* cr);
-
     /** Update the tool state with the new spacing position */
     void currentPos(double x, double y);
 
-    bool onKeyPressEvent(GdkEventKey* event);
-    bool onKeyReleaseEvent(GdkEventKey* event);
+    bool onKeyPressEvent(const KeyEvent& event);
+    bool onKeyReleaseEvent(const KeyEvent& event);
 
     std::unique_ptr<MoveUndoAction> finalize();
 
@@ -114,7 +112,7 @@ class VerticalToolHandler: public ElementContainer, public OverlayBase {
     Layer* layer;
     std::vector<ElementPtr> elements;
     /**
-     * @brief Stores the smallest box containing all the adopted elements. 
+     * @brief Stores the smallest box containing all the adopted elements.
      *     Used to only refresh the part of the screen that needs refreshing.
      */
     Range ownedElementsOriginalBoundingBox;

src/core/gui/PageView.cpp

@@ -738,13 +738,13 @@ auto XojPageView::onButtonReleaseEvent(const PositionInputData& pos) -> bool {
     return false;
 }
 
-auto XojPageView::onKeyPressEvent(GdkEventKey* event) -> bool {
+auto XojPageView::onKeyPressEvent(const KeyEvent& event) -> bool {
     if (this->textEditor) {
         if (this->textEditor->onKeyPressEvent(event)) {
             return true;
         }
     } else if (this->inputHandler) {
-        if (this->inputHandler->onKeyEvent(event)) {
+        if (this->inputHandler->onKeyPressEvent(event)) {
             return true;
         }
     } else if (this->verticalSpace) {
@@ -754,7 +754,7 @@ auto XojPageView::onKeyPressEvent(GdkEventKey* event) -> bool {
     }
 
     // Esc leaves text edition
-    if (event->keyval == GDK_KEY_Escape) {
+    if (event.keyval == GDK_KEY_Escape) {
         if (this->textEditor) {
             endText();
             return true;
@@ -766,12 +766,12 @@ auto XojPageView::onKeyPressEvent(GdkEventKey* event) -> bool {
     return false;
 }
 
-auto XojPageView::onKeyReleaseEvent(GdkEventKey* event) -> bool {
+auto XojPageView::onKeyReleaseEvent(const KeyEvent& event) -> bool {
     if (this->textEditor && this->textEditor->onKeyReleaseEvent(event)) {
         return true;
     }
 
-    if (this->inputHandler && this->inputHandler->onKeyEvent(event)) {
+    if (this->inputHandler && this->inputHandler->onKeyReleaseEvent(event)) {
         DrawingType drawingType = this->xournal->getControl()->getToolHandler()->getDrawingType();
         if (drawingType == DRAWING_TYPE_SPLINE) {  // Spline drawing has been finalized
             if (this->inputHandler) {