Event System: refactor click detection into function

Also simplify modifier & keymodifier assignment.

1 files changed, 77 insertions, 106 deletions

diff --git a/source/blender/windowmanager/intern/wm_event_system.c b/source/blender/windowmanager/intern/wm_event_system.c
index 0e262f8c388..27a7042fd62 100644
--- a/source/blender/windowmanager/intern/wm_event_system.c
+++ b/source/blender/windowmanager/intern/wm_event_system.c
@@ -4765,6 +4765,39 @@ static wmEvent *wm_event_add_trackpad(wmWindow *win, const wmEvent *event, int d
   return event_new;
 }
 
+/**
+ * Update the event-state for any kind of event that supports #KM_PRESS / #KM_RELEASE.
+ */
+static void wm_event_state_update_and_click_set(wmEvent *event, wmEvent *event_state)
+{
+  BLI_assert(ISMOUSE_BUTTON(event->type) || ISKEYBOARD(event->type));
+  BLI_assert(ELEM(event->val, KM_PRESS, KM_RELEASE));
+
+  /* Only copy these flags into the `event_state`. */
+  const eWM_EventFlag event_state_flag_mask = WM_EVENT_IS_REPEAT;
+
+  wm_event_prev_values_set(event, event_state);
+
+  /* Copy to event state. */
+  event_state->val = event->val;
+  event_state->type = event->type;
+  event_state->modifier = event->modifier;
+  event_state->flag = (event->flag & event_state_flag_mask);
+  /* NOTE: It's important that `keymodifier` is handled in the keyboard event handling logic
+   * since the `event_state` and the `event` are not kept in sync. */
+
+  /* Double click test. */
+  if (wm_event_is_double_click(event)) {
+    CLOG_INFO(WM_LOG_HANDLERS, 1, "Send double click");
+    event->val = KM_DBL_CLICK;
+  }
+  else if (event->val == KM_PRESS) {
+    if ((event->flag & WM_EVENT_IS_REPEAT) == 0) {
+      wm_event_prev_click_set(event, event_state);
+    }
+  }
+}
+
 void wm_event_add_ghostevent(wmWindowManager *wm, wmWindow *win, int type, void *customdata)
 {
   if (UNLIKELY(G.f & G_FLAG_EVENT_SIMULATE)) {
@@ -4924,20 +4957,7 @@ void wm_event_add_ghostevent(wmWindowManager *wm, wmWindow *win, int type, void
       wm_tablet_data_from_ghost(&bd->tablet, &event.tablet);
 
       wm_eventemulation(&event, false);
-      wm_event_prev_values_set(&event, event_state);
-
-      /* Copy to event state. */
-      event_state->val = event.val;
-      event_state->type = event.type;
-
-      /* Double click test. */
-      if (wm_event_is_double_click(&event)) {
-        CLOG_INFO(WM_LOG_HANDLERS, 1, "Send double click");
-        event.val = KM_DBL_CLICK;
-      }
-      if (event.val == KM_PRESS) {
-        wm_event_prev_click_set(&event, event_state);
-      }
+      wm_event_state_update_and_click_set(&event, event_state);
 
       /* Add to other window if event is there (not to both!). */
       wmWindow *win_other = wm_event_cursor_other_windows(wm, win, &event);
@@ -4966,9 +4986,6 @@ void wm_event_add_ghostevent(wmWindowManager *wm, wmWindow *win, int type, void
     case GHOST_kEventKeyDown:
     case GHOST_kEventKeyUp: {
       GHOST_TEventKeyData *kd = customdata;
-      /* Only copy these flags into the `event_state`. */
-      const eWM_EventFlag event_state_flag_mask = WM_EVENT_IS_REPEAT;
-      bool keymodifier = 0;
       event.type = convert_key(kd->key);
       event.ascii = kd->ascii;
       /* Might be not NULL terminated. */
@@ -4979,12 +4996,6 @@ void wm_event_add_ghostevent(wmWindowManager *wm, wmWindow *win, int type, void
       event.val = (type == GHOST_kEventKeyDown) ? KM_PRESS : KM_RELEASE;
 
       wm_eventemulation(&event, false);
-      wm_event_prev_values_set(&event, event_state);
-
-      /* Copy to event state. */
-      event_state->val = event.val;
-      event_state->type = event.type;
-      event_state->flag = (event.flag & event_state_flag_mask);
 
       /* Exclude arrow keys, escape, etc from text input. */
       if (type == GHOST_kEventKeyUp) {
@@ -5017,108 +5028,68 @@ void wm_event_add_ghostevent(wmWindowManager *wm, wmWindow *win, int type, void
 
       switch (event.type) {
         case EVT_LEFTSHIFTKEY:
-        case EVT_RIGHTSHIFTKEY:
-          if (event.val == KM_PRESS) {
-            keymodifier = true;
-          }
-          if (keymodifier) {
-            event.modifier |= KM_SHIFT;
-            event_state->modifier |= KM_SHIFT;
-          }
-          else {
-            event.modifier &= ~KM_SHIFT;
-            event_state->modifier &= ~KM_SHIFT;
-          }
+        case EVT_RIGHTSHIFTKEY: {
+          SET_FLAG_FROM_TEST(event.modifier, (event.val == KM_PRESS), KM_SHIFT);
           break;
+        }
         case EVT_LEFTCTRLKEY:
-        case EVT_RIGHTCTRLKEY:
-          if (event.val == KM_PRESS) {
-            keymodifier = true;
-          }
-          if (keymodifier) {
-            event.modifier |= KM_CTRL;
-            event_state->modifier |= KM_CTRL;
-          }
-          else {
-            event.modifier &= ~KM_CTRL;
-            event_state->modifier &= ~KM_CTRL;
-          }
+        case EVT_RIGHTCTRLKEY: {
+          SET_FLAG_FROM_TEST(event.modifier, (event.val == KM_PRESS), KM_CTRL);
           break;
+        }
         case EVT_LEFTALTKEY:
-        case EVT_RIGHTALTKEY:
-          if (event.val == KM_PRESS) {
-            keymodifier = true;
-          }
-          if (keymodifier) {
-            event.modifier |= KM_ALT;
-            event_state->modifier |= KM_ALT;
-          }
-          else {
-            event.modifier &= ~KM_ALT;
-            event_state->modifier &= ~KM_ALT;
-          }
+        case EVT_RIGHTALTKEY: {
+          SET_FLAG_FROM_TEST(event.modifier, (event.val == KM_PRESS), KM_ALT);
+          break;
+        }
+        case EVT_OSKEY: {
+          SET_FLAG_FROM_TEST(event.modifier, (event.val == KM_PRESS), KM_OSKEY);
           break;
-        case EVT_OSKEY:
+        }
+        default: {
           if (event.val == KM_PRESS) {
-            keymodifier = true;
-          }
-          if (keymodifier) {
-            event.modifier |= KM_OSKEY;
-            event_state->modifier |= KM_OSKEY;
+            if (event.keymodifier == 0) {
+              /* Only set in `eventstate`, for next event. */
+              event_state->keymodifier = event.type;
+            }
           }
           else {
-            event.modifier &= ~KM_OSKEY;
-            event_state->modifier &= ~KM_OSKEY;
+            BLI_assert(event.val == KM_RELEASE);
+            if (event.keymodifier == event.type) {
+              event.keymodifier = event_state->keymodifier = 0;
+            }
           }
-          break;
-        default:
-          if (event.val == KM_PRESS && event.keymodifier == 0) {
-            /* Only set in `eventstate`, for next event. */
-            event_state->keymodifier = event.type;
+
+          /* This case happens on holding a key pressed,
+           * it should not generate press events with the same key as modifier. */
+          if (event.keymodifier == event.type) {
+            event.keymodifier = 0;
           }
-          else if (event.val == KM_RELEASE && event.keymodifier == event.type) {
-            event.keymodifier = event_state->keymodifier = 0;
+          else if (event.keymodifier == EVT_UNKNOWNKEY) {
+            /* This case happens with an external number-pad, and also when using 'dead keys'
+             * (to compose complex latin characters e.g.), it's not really clear why.
+             * Since it's impossible to map a key modifier to an unknown key,
+             * it shouldn't harm to clear it. */
+            event_state->keymodifier = event.keymodifier = 0;
           }
           break;
+        }
       }
 
-      /* Double click test. */
-      /* If previous event was same type, and previous was release, and now it presses... */
-      if (wm_event_is_double_click(&event)) {
-        CLOG_INFO(WM_LOG_HANDLERS, 1, "Send double click");
-        event.val = KM_DBL_CLICK;
-      }
-
-      /* This case happens on holding a key pressed,
-       * it should not generate press events with the same key as modifier. */
-      if (event.keymodifier == event.type) {
-        event.keymodifier = 0;
-      }
-
-      /* This case happens with an external number-pad, and also when using 'dead keys'
-       * (to compose complex latin characters e.g.), it's not really clear why.
-       * Since it's impossible to map a key modifier to an unknown key,
-       * it shouldn't harm to clear it. */
-      if (event.keymodifier == EVT_UNKNOWNKEY) {
-        event_state->keymodifier = event.keymodifier = 0;
-      }
+      /* It's important `event.modifier` has been initialized first. */
+      wm_event_state_update_and_click_set(&event, event_state);
 
       /* If test_break set, it catches this. Do not set with modifier presses.
-       * XXX Keep global for now? */
-      if ((event.type == EVT_ESCKEY && event.val == KM_PRESS) &&
-          /* Check other modifiers because ms-windows uses these to bring up the task manager. */
-          ((event.modifier & (KM_SHIFT | KM_CTRL | KM_ALT)) == 0)) {
+       * Exclude modifiers because MS-Windows uses these to bring up the task manager.
+       *
+       * NOTE: in general handling events here isn't great design as
+       * event handling should be managed by the event handling loop.
+       * Make an exception for `G.is_break` as it ensures we can always cancel operations
+       * such as rendering or baking no matter which operation is currently handling events. */
+      if ((event.type == EVT_ESCKEY) && (event.val == KM_PRESS) && (event.modifier == 0)) {
         G.is_break = true;
       }
 
-      /* Double click test - only for press. */
-      if (event.val == KM_PRESS) {
-        /* Don't reset timer & location when holding the key generates repeat events. */
-        if ((event.flag & WM_EVENT_IS_REPEAT) == 0) {
-          wm_event_prev_click_set(&event, event_state);
-        }
-      }
-
       wm_event_add(win, &event);
 
       break;