Page MenuHome

When "Pie Menu on Drag" is enabled in user prefs, Ctrl + Z has a tendency to open the shading menu by accident
Open, Confirmed, MediumPublic

Description

version: 2.80 (sub 74), branch: master, commit date: 2019-07-05 21:52
build date: 2019-07-06, 01:03:01
platform: Linux

This may be more of a design issue than a bug, but it's happened to me about 100 times now and it's quite irritating.

To reproduce:

  1. enable User Preferences > Keymap > Preferences > Pie Menu on Drag.
  2. open a complex scene (one where the undo operator will have a significant lag)
  3. fill the undo history with events, as though you've been working for a while
  4. Press Ctrl + Z a couple times while moving the mouse a bit, rolling your hand so that Ctrl is released just before Z (in the manner of a busy artist)

If the user happens to release Ctrl earlier than Z while moving the mouse, this can bring up the shading menu, interrupting the user's workflow. This may sound like an edge case, but when working quickly, it's actually really frequent and annoying! It seems to especially effect large scenes where there is a lag in the effect of the undo operator.

Ideally, the "Pie On Drag" feature should not listen for key presses if those key presses were part of a different combination that has yet to be completely released. What do you think?

Details

Type
Bug

Event Timeline

Key repeat is conflicting w/ click detection when modifiers are pressed,
ideally we could detect repeat events to avoid this (see D5153).

This works around the problem w/ minimal changes to event handling logic.

diff --git a/source/blender/windowmanager/WM_types.h b/source/blender/windowmanager/WM_types.h
index a0c7e8ef752..9a47d2d970f 100644
--- a/source/blender/windowmanager/WM_types.h
+++ b/source/blender/windowmanager/WM_types.h
@@ -540,6 +540,9 @@ typedef struct wmEvent {
   double prevclicktime;
   int prevclickx, prevclicky;
 
+  /** Don't use this event type for click events. */
+  short prevtype_dont_click;
+
   /** Modifier states. */
   /** 'oskey' is apple or windows-key, value denotes order of pressed. */
   short shift, ctrl, alt, oskey;
diff --git a/source/blender/windowmanager/intern/wm_event_system.c b/source/blender/windowmanager/intern/wm_event_system.c
index 76cb78b1e04..5a27373ff06 100644
--- a/source/blender/windowmanager/intern/wm_event_system.c
+++ b/source/blender/windowmanager/intern/wm_event_system.c
@@ -3007,7 +3007,8 @@ static int wm_handlers_do(bContext *C, wmEvent *event, ListBase *handlers)
         }
       }
 
-      if (win && win->eventstate->prevtype == event->type) {
+      if (win && (win->eventstate->prevtype == event->type) &&
+          (win->eventstate->prevtype_dont_click != event->type)) {
 
         if ((event->val == KM_RELEASE) && (win->eventstate->prevval == KM_PRESS) &&
             (win->eventstate->check_click == true)) {
@@ -4564,6 +4565,33 @@ void wm_event_add_ghostevent(wmWindowManager *wm, wmWindow *win, int type, void
       evt->val = event.val;
       evt->type = event.type;
 
+      /* Begin 'prevtype_dont_click'.
+       *
+       * Don't allow keyboard click events to occur when modifiers have been touched.
+       * Needed until we can properly detect repeat events: D5153. */
+
+      /* Modifiers wont prevent key-repeat, however after pressing modifiers we shouldn't click. */
+      if (ISKEYMODIFIER(evt->type)) {
+        if (!ISKEYMODIFIER(evt->prevtype)) {
+          evt->prevtype_dont_click = evt->prevtype;
+        }
+      }
+      else {
+        if (evt->val == KM_PRESS) {
+          /* When pressing a new non-modifier, clear previous don't click state. */
+          if (evt->type != evt->prevtype_dont_click) {
+            evt->prevtype_dont_click = EVENT_NONE;
+          }
+        }
+        if (evt->prevval == KM_RELEASE) {
+          /* Use the previous so this event wont become a click when it shouldn't. */
+          if (evt->prevtype == evt->prevtype_dont_click) {
+            evt->prevtype_dont_click = EVENT_NONE;
+          }
+        }
+      }
+      /* End 'prevtype_dont_click' workaround. */
+
       /* exclude arrow keys, esc, etc from text input */
       if (type == GHOST_kEventKeyUp) {
         event.ascii = '\0';
Philipp Oeser (lichtwerk) lowered the priority of this task from Needs Triage by Developer to Confirmed, Medium.

@Campbell Barton (campbellbarton) : I hope you dont mind if this gets put on your desk for now? (Since you seem to have a clear idea of the direction this should take...)