silx.gui.plot.PlotWidget: Allow directive zoom with shortcut ALT and SHIFT like h5web - #4631
Conversation
|
|
||
| def wheelEvent(self, event): | ||
| delta = event.angleDelta().y() | ||
| delta = event.angleDelta().x() + event.angleDelta().y() |
There was a problem hiding this comment.
when alt is pressed in opengl, event.angleDelta().y() has a value and event.angleDelta().x() is 0
There was a problem hiding this comment.
I would expect x to always be 0 except for fancy mouses no ? According to the doc
And even if x is not zero I am not sure we want to take it into account.
Discussed with @maxenceprog having
event.angleDelta().x() != 0comes from the OpenGL backend.
There was a problem hiding this comment.
sorry my comment was wrong when alt is pressend in open gl x value is set else y
that's why. DO you want me to be more specific like really use x when alt is pressed ?
There was a problem hiding this comment.
This behavior is hard-coded in qt xcb plugin: https://github.com/qt/qtbase/blob/120883a028a59864dc7691dd1efa318bd602755d/src/plugins/platforms/xcb/qxcbwindow.cpp#L1955-L1956
Rather than summing, it's also possible to use x() only when y() == 0
Since this is not documented in qt doc, it's worth a comment to explain!
There was a problem hiding this comment.
hey i'm facing a new issue actually i have the same problem with matplotlib backend but i have no clue how i solve it as when alt pressed, _onMouseWheel is just not send at all...
(you can try with current version of silx by the way and xcb plugin)
should i open an issue an matplotlib ?
or give up alt and use an other modifier ?
There was a problem hiding this comment.
I look at the problem with Loic
a solution may be to override silx.gui.plot.backends.BackendMatplotlibQt.wheelEvent
and do the same as we do for opengl
Do you want to implement that @t20100 or do you prefer to use another modifier (like ctrl) ?
There was a problem hiding this comment.
IMO best to be consistent with h5web modifier keys if possible.
Provided matplotlib/matplotlib#25671 is marked as 'won't fix', it indeed requires to override matplotlib wheelEvent.
BTW going this way, we prevent silx to support horizontal scrolling in the future without changing this (one of the point that made matplotlib not patching this Qt behavior).
What do you think?
There was a problem hiding this comment.
and do the same as we do for opengl
I thought it was a behaviour specific to matplotlib so why should something be done for opengl?
we prevent silx to support horizontal scrolling in the future
Only in the visualization area. And resizing this one does not make a scroll bar appear since the figure has no minimum size (as far as I can see).
There was a problem hiding this comment.
The behavior of using Alt modifier to change x/y of QWheelEvent.angleDelta is in Qt xcb plugin.
The OpenGL backend was doing the same as what matplotlib does.
So change to support Alt+Wheel is required in both or we have to use another key.
we prevent silx to support horizontal scrolling in the future
Only in the visualization area.
Yes.
|
@maxenceprog Thanks for the PR! Could you change the name to add the full path of |
| self.__xAxisAction = qt.QAction("X axis (alt)", parent=self) | ||
| self.__yAxisAction = qt.QAction("Y left axis (shift)", parent=self) | ||
| self.__y2AxisAction = qt.QAction("Y right axis (shift)", parent=self) |
There was a problem hiding this comment.
For me here there is clear need to clarify the policy we want to have.
I think this is not something we want to "impose" at the library side but to have at silx view side right @t20100 ?
So from my understanding either this is added at the silx view application level and/or this should be optional at the lib side.
There was a problem hiding this comment.
i think if we add this shortcut it can benefit to library too ?
There was a problem hiding this comment.
I think this is not something we want to "impose" at the library side but to have at silx view side right @t20100 ?
Agreed. It should not enabled by default in the lib but be in opt-in. And silx view would opt-in.
i think if we add this shortcut it can benefit to library too ?
Yes, it would be good to still have in the library so that other consumer apps can enable it. But not enabled by default.
There was a problem hiding this comment.
So far, plot interactions are mostly wired in PlotWidget in Plotinteraction and associated tools through PlotWidget.setInteractiveMode.
It's possible to add an API to toggle use of modifer keys, but I'm not sure it is worth it here.
I agree silx should leave it up to the application to choose it's keyboard short-cuts (unless they are standard ones). However here it is about modifier keys for the mouse wheel which is a behavior I don't think one cannot infer with from the API.
There was a problem hiding this comment.
(alt) -> (Alt+Wheel)
(shift) -> (Shift+Wheel)
There was a problem hiding this comment.
i m not sure i understand the final decision @t20100 . So i can keep thit like that and let it in silx core ?
There was a problem hiding this comment.
Unless someone disagree, I would leave it as it is in silx core.
|
|
||
| def wheelEvent(self, event): | ||
| delta = event.angleDelta().y() | ||
| delta = event.angleDelta().x() + event.angleDelta().y() |
There was a problem hiding this comment.
I would expect x to always be 0 except for fancy mouses no ? According to the doc
And even if x is not zero I am not sure we want to take it into account.
Discussed with @maxenceprog having
event.angleDelta().x() != 0comes from the OpenGL backend.
maxenceprog
changed the title
4 participants
<Module or Topic>: <Action> <Summary>(see contributing guidelines)PR summary
Add 2 short cut for directive zoom.
Alt for X and Shift for Y (like h5web)
AI Disclosure