Fix an increasing bottleneck when key press operator is too slow

The goal of this change is to fix an increasing bottleneck of the event
queue handling when there is an operator bound to a key press event and
is taking longer to finish than a key-repeat speed on the system.

Practical example of when it happens is the marker tracking operator in
a single-frame track mode. Quite often artists will hold down Alt-arrow
to track a segment of footage which seems trivial to track. The issue
arises when the Alt-arrow is released: prior to this change it is was
possible that more frames will be tracked. It also seems that redraws
are less smooth.

It is a bit hard to make easily shareable computer-independent test
case. Instead, a synthetic case can be reproduced by adding a 50 ms
sleep in the `text_move_exec()`. In such synthetic case open a long
text in the text editor and hold left/right arrow button to navigate
the cursor. The observed behavior is that seemingly redraws happen
less and less often and cursor travels longer and longer distances
between redraws. The cursor will also keep moving after the buttons
has been released.

The proposed solution is to ignore sequential key-press events from
being added to the event queue. This seems to be the least intrusive
and the most safe approach:

- If the operator is fast enough there will be no multiple press events
  in the queue in both prior and after of this change.

- If the operator is slow enough, clicking the button multiple times
  (i.e. clicking arrow button 3 times in a heavy shot will change the
  scene frame by exactly 3 frames because no events are ignored in
  this case).

- Only do it for key press events, keeping mouse and tabled behavior
  unchanged which is crucial for the paint mode.

Note that this is a bit different from the key repeat tracking and
filtering which is already implemented for keymaps as here we only want
to avoid the event queue build-up and do want to ignore all repeat
events. In other words: we do want to handle as many key presses as the
operator performance allows it without clogging anything.

A possible extension to this change could be a key press counter, so
that instead of ignoring the event we merge it into the last event in
the queue, incrementing some counter. This way if some operator really
needs to know exact number of key repeats it can still access it.

Differential Revision: https://developer.blender.org/D15444

1 files changed, 50 insertions, 1 deletions

diff --git a/source/blender/windowmanager/intern/wm_event_system.cc b/source/blender/windowmanager/intern/wm_event_system.cc
index 757c3aae00c..79b303364d8 100644
--- a/source/blender/windowmanager/intern/wm_event_system.cc
+++ b/source/blender/windowmanager/intern/wm_event_system.cc
@@ -5133,6 +5133,53 @@ static void wm_event_state_update_and_click_set(wmEvent *event,
   wm_event_state_update_and_click_set_ex(event, event_state, is_keyboard, check_double_click);
 }
 
+/* Returns true when the two events corresponds to a press of the same key with the same modifiers.
+ */
+static bool wm_event_is_same_key_press(const wmEvent &event_a, const wmEvent &event_b)
+{
+  if (event_a.val != KM_PRESS || event_b.val != KM_PRESS) {
+    return false;
+  }
+
+  if (event_a.modifier != event_b.modifier || event_a.type != event_b.type) {
+    return false;
+  }
+
+  return true;
+}
+
+/**
+ * Returns true if the event is a key press event which is to be ignored and not added to the event
+ * queue.
+ *
+ * A key press event will be ignored if there is already matched key press in the queue.
+ * This avoids the event queue "clogging" in the situations when there is an operator bound to a
+ * key press event and the execution time of the operator is longer than the key repeat.
+ */
+static bool wm_event_is_ignorable_key_press(const wmWindow *win, const wmEvent &event)
+{
+  if (BLI_listbase_is_empty(&win->event_queue)) {
+    /* If the queue is empty never ignore the event.
+     * Empty queue at this point means that the events are handled fast enough, and there is no
+     * reason to ignore anything. */
+    return false;
+  }
+
+  if ((event.flag & WM_EVENT_IS_REPEAT) == 0) {
+    /* Only ignore repeat events from the keyboard, and allow accumulation of non-repeat events.
+     *
+     * The goal of this check is to allow events coming from a keyboard macro software, which can
+     * generate events quicker than the main loop handles them. In this case we want all events to
+     * be handled (unless the keyboard macro software tags them as repeat) because otherwise it
+     * will become impossible to get reliable results of automated events testing. */
+    return false;
+  }
+
+  const wmEvent &last_event = *reinterpret_cast<const wmEvent *>(win->event_queue.last);
+
+  return wm_event_is_same_key_press(last_event, event);
+}
+
 void wm_event_add_ghostevent(wmWindowManager *wm, wmWindow *win, int type, void *customdata)
 {
   if (UNLIKELY(G.f & G_FLAG_EVENT_SIMULATE)) {
@@ -5439,7 +5486,9 @@ void wm_event_add_ghostevent(wmWindowManager *wm, wmWindow *win, int type, void
         G.is_break = true;
       }
 
-      wm_event_add(win, &event);
+      if (!wm_event_is_ignorable_key_press(win, event)) {
+        wm_event_add(win, &event);
+      }
 
       break;
     }