Separately track modifier keys for mouse events. by anntzer · Pull Request #23473 · matplotlib/matplotlib · GitHub

PR Summary

Ensure that modifier keys are correctly reported for mouse events even if the last key press occurred out of the canvas.

Whether the event modifiers are directly available on enter/leave events
depends on the backend, but all are handled here (except possibly for
macos, which I haven't checked).

There's a first separate commit for the macosx backend for some preliminary work with ObjC.

Followup to #16931 (it's the reason for that big PR...). Supersedes #6159. See also the side note at #23436 (comment).

PR Checklist

Tests and Styling

Has pytest style unit tests (and pytest passes).
Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

New features are documented, with examples if plot related.
New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
Documentation is sphinx and numpydoc compliant (the docs should build without error).

I haven't had a chance to look at the details here, but I just pulled and tested quick on macosx and hit some errors that you can probably fix pretty quick:

Clicking anywhere:

TypeError: MouseEvent.__init__() got an unexpected keyword argument 'num'

ctrl + clicking anywhere:

SystemError: NULL object passed to Py_BuildValue

Oops, thanks for trying. Hopefully fixed now?

Looks like the failures with macosx are real and a PROCESS_EVENT snuck in after your initial PR.

Oops, probably a careless rebase, thanks for the ping.

tk seems to be double registering the modifiers in key_press now. (I see shift+shift and maybe more illuminating is ctrl+control rather than control+control although I only expect a single control).

The modifiers appear with the mouse presses as expected in macosx, qt, tk. However, I think it'd be nice to also add these to the keypress events because currently it is only an empty frozenset on those events.

Test script I was using:

import matplotlib.pyplot as plt
fig, ax = plt.subplots(1,1)
fig.canvas.mpl_connect('key_press_event', lambda key_event: print(f'press {key_event.key!r}'))
fig.canvas.mpl_connect('key_release_event', lambda key_event: print(f'release {key_event.key!r}'))
fig.canvas.mpl_connect('key_press_event', lambda key_event: print(f'modifiers press {key_event.modifiers!r}'))
fig.canvas.mpl_connect('button_press_event', lambda key_event: print(f'modifiers press {key_event.modifiers!r}'))
plt.show()

Ah, I had forgotten to implement the exclude kwarg to _mpl_modifiers for some backends, thanks for catching that, the doubling of modifiers should now be fixed.
Can I ask for a joker and defer adding modifiers to key events to another issue?

Fine to save some work for follow-ups and break this up. This is a good start!

Works as expected for me now, but there are some differences between backends which is unrelated to this PR, but it might be nice to fix up the inconsistencies later. It is likely related to mac's keyboards.

macosx -> good
qt5 -> good
tk -> Pressing command, I get a KeyPress "super+meta" (KeyRelease is only "meta" which is odd), but only "super" in the modifiers. I would expect "cmd" in the modifiers.
gtk3 -> Pressing command, I get a KeyPress "meta" but nothing in the modifiers when I expect a "cmd" in the modifiers

I suspect it's just a matter of updating (if needed) the table in _mpl_modifiers (previously _get_key) (perhaps by manually inspecting event.state in various cases); as you have access to a mac, can you give it a try (perhaps both with a laptop keyboard and with an independent one just to be sure)?

I tested qt + tk on linux and osx on osx.

Also saw something funny going on with 'meta' on tk. I saw it in the out put from the snippet Greg posted, but I could not sort out which keys I hit to get it.

Anyone can merge on green (again, excluding the patch coverage).

Whether the event modifiers are directly available on enter/leave events depends on the backend, but all are handled here (except possibly for macos, which I haven't checked).

Thank you @anntzer !

@greglucas I got my hands on a mac :) and had a look at #23473 (review).
I think the relevant patch to fix the super+meta issue on tk is basically

diff --git i/lib/matplotlib/backends/_backend_tk.py w/lib/matplotlib/backends/_backend_tk.py
index 7aa24e06ff..6893120ace 100644
--- i/lib/matplotlib/backends/_backend_tk.py
+++ w/lib/matplotlib/backends/_backend_tk.py
@@ -354,7 +354,7 @@ class FigureCanvasTk(FigureCanvasBase):
             ("ctrl", 1 << 2, "control"),
             ("alt", 1 << 4, "alt"),
             ("shift", 1 << 0, "shift"),
-            ("super", 1 << 3, "super"),
+            ("foobar", 1 << 3, "meta"),
         ] if sys.platform == "darwin" else [
             ("ctrl", 1 << 2, "control"),
             ("alt", 1 << 3, "alt"),

where the last tuple item must be "meta" (to match tk reporting the cmd key with event.keysym = "Meta_L" (or "Meta_R")) and the first entry is how you think the modifier should actually be called (I guess "cmd", or perhaps "meta", I don't really have a good intuition here)? Can you decide on that last point and pick up the patch?

anntzer force-pushed the mouse-modifiers branch 3 times, most recently from d625e11 to 38c9039 Compare July 22, 2022 19:51

anntzer mentioned this pull request Jul 26, 2022

[ENH]: Add buttons (plural) attribute to mouseevents #23490

Closed

anntzer force-pushed the mouse-modifiers branch from 38c9039 to 9957c82 Compare July 29, 2022 15:49

anntzer force-pushed the mouse-modifiers branch from 9957c82 to 1cddbef Compare October 22, 2022 16:43

anntzer force-pushed the mouse-modifiers branch from 1cddbef to cd9185f Compare December 8, 2022 20:25

Construct events with kwargs in macosx.

49724bf

anntzer force-pushed the mouse-modifiers branch from cd9185f to 794c623 Compare December 13, 2022 10:43

greglucas reviewed Dec 13, 2022

View reviewed changes

anntzer force-pushed the mouse-modifiers branch from 794c623 to ecc7f36 Compare December 13, 2022 21:34

greglucas approved these changes Dec 13, 2022

View reviewed changes

tacaswell added this to the v3.7.0 milestone Dec 15, 2022

tacaswell reviewed Dec 15, 2022

View reviewed changes

Comment thread lib/matplotlib/backends/backend_qt.py Outdated

tacaswell approved these changes Dec 15, 2022

View reviewed changes

Separately track modifier keys for mouse events.

b4e9e31

Whether the event modifiers are directly available on enter/leave events depends on the backend, but all are handled here (except possibly for macos, which I haven't checked).

anntzer force-pushed the mouse-modifiers branch from 5929f98 to b4e9e31 Compare December 16, 2022 07:04

tacaswell merged commit eb92923 into matplotlib:main Dec 16, 2022

anntzer deleted the mouse-modifiers branch December 16, 2022 17:47

anntzer mentioned this pull request Dec 16, 2022

added modifier key tracking in MouseEvents #6159

Closed

greglucas mentioned this pull request Feb 2, 2023

FIX: Correctly report command keypress on mac for Tk + Gtk #25131

Merged

ianhi mentioned this pull request Feb 15, 2023

zoom and move buttons broken for mpl 3.7.0 matplotlib/ipympl#516

Closed

This was referenced Feb 15, 2023

IPyWidgets do not renderer due to a 401 error in webview microsoft/vscode-jupyter#12843

Closed

Using %matplotlib widget results in `KeyError: 'modifiers'. microsoft/vscode-jupyter#12888

Closed

ksunden mentioned this pull request Aug 24, 2023

[Bug]: MacOS screen capture causes wx backend exception #26587

Closed

anntzer mentioned this pull request Aug 24, 2023

[ENH]: Properly set .modifiers field on key events #26589

Open

anntzer mentioned this pull request Jun 25, 2024

Stop relying on dead-reckoning mouse buttons for motion_notify_event. #28453

Merged

5 tasks

Separately track modifier keys for mouse events. by anntzer · Pull Request #23473 · matplotlib/matplotlib · GitHub

Repository navigation

Separately track modifier keys for mouse events. - #23473

Separately track modifier keys for mouse events.#23473tacaswell merged 2 commits intomatplotlib:mainmatplotlib/matplotlib:mainfrom anntzer:mouse-modifiersanntzer/matplotlib:mouse-modifiersCopy head branch name to clipboard

Conversation

anntzer commented Jul 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Checklist

Uh oh!

greglucas commented Jul 29, 2022

Uh oh!

anntzer commented Jul 29, 2022

Uh oh!

greglucas commented Dec 12, 2022

Uh oh!

anntzer commented Dec 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greglucas left a comment

Choose a reason for hiding this comment

Uh oh!

anntzer commented Dec 13, 2022

Uh oh!

greglucas left a comment

Choose a reason for hiding this comment

Uh oh!

anntzer commented Dec 14, 2022

Uh oh!

Uh oh!

tacaswell commented Dec 15, 2022

Uh oh!

tacaswell commented Dec 15, 2022

Uh oh!

tacaswell commented Dec 16, 2022

Uh oh!

anntzer commented Jan 31, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Separately track modifier keys for mouse events.#23473
tacaswell merged 2 commits into
matplotlib:mainfrom
anntzer:mouse-modifiers

anntzer commented Jul 22, 2022 •
edited
Loading

anntzer commented Dec 13, 2022 •
edited
Loading