Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add keyModifier to buttonPressEvent and buttonReleaseEvent #1274

Open
dov opened this issue Sep 4, 2024 · 8 comments
Open

Add keyModifier to buttonPressEvent and buttonReleaseEvent #1274

dov opened this issue Sep 4, 2024 · 8 comments

Comments

@dov
Copy link

dov commented Sep 4, 2024

Is your feature request related to a problem? Please describe.

I'm displaying a scene to the user with multiple objects, where one or several objects can be selected. Selection occurs by a LMB click. If control is pressed, the clicked object is added to the selection, otherwise it replaces it.

The problem is that vsg::ButtonPressEvent does not contain a keyModifier field. This means that I have to cache it myself by storing it in the KeyPressEvent and KeyReleaseEvent callbacks. However, this is not foolproof, because if the Control key is pressed outside the vsg Window,, and subsequently when the object is clicked, the stored modifier mask is wrong, even though at the time of the LMB mouse press event, Control is being pressed.

Describe the solution you'd like

I'd like the keyModifier field to be added to the vsg::ButtonPressEvent and vsg::ButtonReleaseEvent classes.

Describe alternatives you've considered

As I described above, I tried caching the state on my own, however that is not foolproof.

Additional context

@robertosfield
Copy link
Collaborator

An event for one device is not the place to maintain state of another device. I don't think we should conflate functionality across devices as this could lead to an opened push to complicate event classes.

I wrote the vsg::Keyboard class for tracking keyboard state based in handling the keyboard events, perhaps this can be used for your work.

@dov
Copy link
Author

dov commented Sep 4, 2024

The question is whether the keyboard modifiers are really a state of "another device", or perhaps a global state of the system?

E.g. in qt, all input events have access to the keyboard modifiers:

class Q_GUI_EXPORT QInputEvent : public QEvent
{
public:
    explicit QInputEvent(Type type, Qt::KeyboardModifiers modifiers = Qt::NoModifier);
:

If there in vsg was a method of querying the current modifier state, then that would be ok as well. Otherwise it is not clear to me how to implement Control-Mouse click in a robust way.

@dov
Copy link
Author

dov commented Oct 6, 2024

Meanwhile I worked around this issue by querying qt about the modifier state:

Qt::KeyboardModifiers qtmodifiers = QGuiApplication::queryKeyboardModifiers();

But I still think there should be supported by vsg itself. In my opinion, just like in Qt, the modifiers are not "another device" w.r.t. mouse button presses, but a system wide state.

The problem with tracking the keyboard state through keyboard events is that the shift might have been pressed outside of the application.

E.g. tracking vsg::KeyboardEvents fail in the following scenario:

  1. Defocus the VSG application window
  2. Press Shift and hold it down
  3. Focus the VSG application window
  4. Press Mouse Button 1 and catch the vsg::ButtonPressEvent.
  5. The application has no way of knowing that shift is currently pressed

@robertosfield
Copy link
Collaborator

I still believe a subset of device state isn't something that should cached on events for a different device. If we want the device state to be querriable like key modifiers then vsg::Keyboard is appropriate tool, and if there are cases where it's current implementation can break then perhaps implementing direct support for vsg::Keyboard in the vsg::Window subclasses handling of evens would be appropriate.

@dov
Copy link
Author

dov commented Oct 6, 2024

Isn't the global keyboard modifier state accessible from all major OS's? A quick search turned up the following:

So there doesn't seem to be a reason that vsg needs to cache it, but can query it from the OS. So perhaps vsg::Keyboard should be made an adapter to underlying modifier state.

It is also noteworthy that Qt makes a distinction between the modifier state at time of an event, and the current keyboard modifiers. In practice it shouldn't matter.

@robertosfield
Copy link
Collaborator

Thanks for the links. I'm inclined towards having a scheme for vsg::Keyboard being available via the either the window and/or events themselves. I can't tackle this right away but I've put it on my TODO list.

@dov
Copy link
Author

dov commented Oct 27, 2024

Here is another relevant piece of information, that I think is relevant to the discussion.

I mapped the zoom/pan/rotate behavior of a common commercial program that I would like to replicate, and it is using the modifiers to change the behavior of the same mouse button. E.g. RMB rotates, but Ctrl+RMB pans, and Shift+RMB zooms. If I want to replicate this behavior in VSG for trackball, I currently have to override the button press event, move event, and button release event in a new class.

Ideally, I'd like to have ButtonMasks that also includes the modifier keys. I could then do panButtonMask = BUTTON_MASK_3 | BUTTON_CONTROL_MASK; zoomButtonMask = BUTTON_MASK_3 | BUTTON_SHIFT|MASK, etc. Further, as I would like to be able to have multiple binding to the same behavior, I'd prefer to have set<uint16_t> zoomButtonMasks and then add all combinations that should zoom to it.

@robertosfield
Copy link
Collaborator

I really don't like trying to stuff disparate functionality together in this way. I am happy to see means of tracking modifier and button mask state in a coherent and convenient way but it's got to keep logical groups to the associated hardware.

I have a lot of other to work get on with so really not in a position to take a detour into refactor or writing another part of the VSG, so for now I can provide some high level direction, but not put any development work time into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants