From ea79dab0621a0ef992d6289a95e72329279f6383 Mon Sep 17 00:00:00 2001 From: Campbell Barton Date: Wed, 21 Sep 2022 16:41:49 +1000 Subject: Docs: add notes about wmWindow.eventstate & modifier key checks There were undocumented limitations in the current modifier handling that came to my attention while investigating related issues. --- source/blender/makesdna/DNA_windowmanager_types.h | 17 ++++++++++++++++- .../blender/windowmanager/intern/wm_event_system.cc | 19 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) (limited to 'source') diff --git a/source/blender/makesdna/DNA_windowmanager_types.h b/source/blender/makesdna/DNA_windowmanager_types.h index 1c71129a3c7..d57cca0b055 100644 --- a/source/blender/makesdna/DNA_windowmanager_types.h +++ b/source/blender/makesdna/DNA_windowmanager_types.h @@ -307,7 +307,22 @@ typedef struct wmWindow { */ short pie_event_type_last; - /** Storage for event system. */ + /** + * Storage for event system. + * + * For the most part this is storage for `wmEvent.xy` & `wmEvent.modifiers`. + * newly added key/button events copy the cursor location and modifier state stored here. + * + * It's also convenient at times to be able to pass this as if it's a regular event. + * + * - This is not simply the current event being handled. + * The type and value is always set to the last press/release events + * otherwise cursor motion would always clear these values. + * + * - The value of `eventstate->modifiers` is set from the last pressed/released modifier key. + * This has the down side that the modifier value will be incorrect if users hold both + * left/right modifiers then release one. See note in #wm_event_add_ghostevent for details. + */ struct wmEvent *eventstate; /** Keep the last handled event in `event_queue` here (owned and must be freed). */ struct wmEvent *event_last_handled; diff --git a/source/blender/windowmanager/intern/wm_event_system.cc b/source/blender/windowmanager/intern/wm_event_system.cc index 841df253dab..c26696704dd 100644 --- a/source/blender/windowmanager/intern/wm_event_system.cc +++ b/source/blender/windowmanager/intern/wm_event_system.cc @@ -5481,6 +5481,25 @@ void wm_event_add_ghostevent(wmWindowManager *wm, wmWindow *win, int type, void } } + /* NOTE(@campbellbarton): Setting the modifier state based on press/release + * is technically incorrect. + * + * - The user might hold both left/right modifier keys, then only release one. + * + * This could be solved by storing a separate flag for the left/right modifiers, + * and combine them into `event.modifiers`. + * + * - The user might have multiple keyboards (or keyboard + NDOF device) + * where it's possible to press the same modifier key multiple times. + * + * This could be solved by tracking the number of held modifier keys, + * (this is in fact what LIBXKB does), however doing this relies on all GHOST + * back-ends properly reporting every press/release as any mismatch could result + * in modifier keys being stuck (which is very bad!). + * + * To my knowledge users never reported a bug relating to these limitations so + * it seems reasonable to keep the current logic. */ + switch (event.type) { case EVT_LEFTSHIFTKEY: case EVT_RIGHTSHIFTKEY: { -- cgit v1.2.3