-
Notifications
You must be signed in to change notification settings - Fork 928
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
MouseInput should have a position #883
Comments
It may be important to keep in mind that there are literally edge cases to take into account: when the cursor reaches the border of the monitor, More generally, there is the question of the behavior of such a field when the cursor is outside of the window, as there will be platform dependent quirks. |
Good points. Another problem I thought of is since it uses the location from the previous event poll, if your program is lagging badly, than the mouse will have moved a big distance relative to the previous poll, so it will seem like your input is lagging even more. Not sure if that's how it works, but it seems logical. |
This would convolute the the There is already You would need to pick one of these to attach to |
This takes like 25 lines of code for the user to implement and doesn't fall within the scope of things that winit sets out to do. This feels opinionated; it's really up to the user to decide if they want to use the location data when the mouse is clicked. The API, as it is now, reflects that. |
I do not believe it convolutes MouseInput, in my opinion it is a fairly basic task and it would help for winit to at least show the user how to track MouseInput locations. If that's in the form of an example instead of a MouseInput field, that's alright, but I think winit would benefit from adding a field. The fact that there are edge cases that need to be accounted for just gives weight to including it in winit and preventing the user from messing it up. In terms of technical feasibility and api choices:
@vberger what do you mean by "MouseInput events can keep being generated while the actual location of the cursor does not change."? I assume MouseInput's are generated due to clicks, in which case if the position dosen't change, that dosen't matter, then you just have many clicks in the same place. |
Sorry, I was not formulating this correctly, forget about this part. The thing is, currently winit strives to have a scope small enough to be reasonably maintainable. As such, anything that can be easily implemented outside of winit will, in general not be implemented in winit unless it can be justified that the implementation would be significantly simpler if it lived in winit. This really does not seem to be the case here. However I agree it'd be worth adding an example showcasing the proper way to track cursor location. |
I agree with the original argument that it is quite rare to want to get a mouse input event without having the position associated with it. Same with I'll make a demo PR for discussion if people want. |
@icefoxen Eh not really, take most games, they just want the delta without the position. Should they pay the calculation cost every time? (I'd probably honestly say yes but I can see a reason for no) |
I've yet to see a game that wanted the delta on click, not the position. Plenty want the delta on motion, but you can perfectly calculate delta from position and vice versa, and the performance difference is negligible -- a single vector subtraction. For the sake of argument I've implemented this for linux: icefoxen@0ff0b8e . It is indeed somewhat narsty, I won't lie. Would be better if there were a layer that could be inserted between |
Even the delta on click, take an FPS, it wants the location to 'finish' moving before applying the click (shot) itself. But yeah, devil's advocate, I'm for including the calculated position anyway if it's not an exclusive mouse. ^.^ If it is exclusive, then there is no absolute position and that should never be sent. |
Where did the 25 lines number come from? Isn't this just adding a match on the At what point do you draw the line? The API as it exists provides everything needed to do this right now. |
It came from cedric-h:
Besides, why not put it in? Isn't this just adding a match on the
|
I think the main issue with this idea is that it forces users to deal with multiple sources of truth. If we are already tracking the mouse position, and then we get a mouse input event... Which position do we trust? Is it a guarantee that the position in the mouse input event will be the same as the one in the last cursor moved event? Should this even be something we should be worrying about? The worst parts are that it introduces a guarantee that the API has to satisfy at runtime, makes things more stateful, and opens a door for bugs that aren't possible with the current design. |
I think we're making this too complicated. Looking at the various platform impl's, the only one that doesn't already include a position along with a mouse button event is Wayland, and Wayland is deliberately pretty heckin' low level. It also defines explicitly how to get the position of the button event. So if the goal is to expose the platform API in a portable manner with no fluff, I'd say that that's what we should do. |
@icefoxen Does that include the mouse cursor in exclusive mode? Having an absolute position doesn't make sense for exclusive mode. Things like many games as one example will absolutely require an exclusive mouse lock, which doesn't have absolute coordinates, the program only gets delta's and absolute doesn't make sense (since it of course may not be constrained to the screen, could be rotating around a 3D camera for example). I'm now leaning to absolute should not 'always' exist. |
@OvermindDL1 It looks like the way you get mouse's motion delta is via |
I agree with this sentiment. In my mind, there are two ways Winit can expose device input state:
Either of those options are acceptable. However, not committing to either option in particular and internally keeping track of some device state for some events is far, far worse than going all-in on either option, since that makes the API harder to predict. It forces users to reference the documentation to check and see if a value is exposed in an event, rather than assuming that a particular event will only store information related to that event. As such, I'd prefer not to expose mouse position through (On that note, that's also the main reason I think we should remove the |
I agree that we want a single source of truth and that exposing things through events is nicer. I also agree that we want to do it once, right. But in this case SOMEONE is going to need to keep track of this state no matter what, and in almost all backends the OS already does it for you. So why not expose the information they give you instead of forcing the user to re-implement their own state? There's also the fact that, you know, if you ever want to actually USE |
@icefoxen wouldn't any actual user of Winit already be keeping track of the mouse position externally to Winit anyway? How would adding the mouse position to
That's a better argument for a general-purpose input state querying API rather than exposing mouse position in |
...the OS tracks the state for you, and gives it to you in the form of a position for a button event. So you don't NEED to store it anywhere, because you already have it when you need it. Please, please just actually look at the PR I made as an example? |
I'm somewhat curious about this, since its something that came up in discussion. If i'm not mistaken, the deltas reported by |
Deltas aren't reported by If we want more events to include delta information I think we should discuss that in a different issue. |
I see this is four years old now. What happens with DeviceId when there are two input devices? Suppose you have a laptop with both a touchpad or touchscreen, and a mouse plugged in.
I'm not sure if this is an argument for separating the two issues or combining them. |
Not accounting for any backend specific weirdness:
That is what should happen.
You get a
No. I'm interested in why there would be any confusion around this. And how does any of this relate to this issue? Apologies if I missed anything. |
OK. This make sense only if each input device has its own cursor. If there are two input devices, are there two cursors? |
I see the issue now! I'm unsure though how this would exactly relate to Winit, I'm only familiar in detail with the Web backend, but I would assume that all backends just faithfully report what their windowing system tells them. So it really entirely depends on the OS (or windowing system in the case of Linux). |
Linux supports both multiple devices for the same cursor and multiple on-screen cursors are possible. That's not the default, though. The default is multiple devices feeding into one cursor. Here's what multiple mice feeding one cursor looks like.
The XTEST device is for playback of recorded sessions. Plugging in a USB joystick did not result in a mouse-type device appearing. At least not by default. Windows supports touchscreen and mouse input on the same machine, but the last one used apparently becomes primary and gets cursor control. So that's different than UNIX. There are third-party multi-cursor programs for Windows, but that's considered unusual. So input devices and cursors are different. (Do they have different DeviceId values?) Not returning cursor position on mouse events is correct. Programs should probably check if more than one cursor is being reported and complain. Otherwise, the last cursor position applies to all pointing devices. So, not asking for any change to Winit. Some of this might be explained in the documentation, though. |
I see now why this was relevant to the issue here. Unfortunately I have no clue even how this exactly plays out in the Web backend. I believe any discussion around this should live in a separate issue, at least until we figure out all the details. The outcome might be that this just needs better documentation, which is a very valid request as well. |
See #795 |
- Rename `CursorMoved` to `PointerMoved`. - Rename `CursorEntered` to `PointerEntered`. - Rename `CursorLeft` to `PointerLeft`. - Rename `MouseInput` to `PointerButton`. - Add `position` to every `PointerEvent`. - Remove `Touch`, which is folded into the `Pointer*` events. - New `PointerType` added to `PointerEntered` and `PointerLeft`, signifying which pointer type is the source of this event. - New `PointerSource` added to `PointerMoved`, similar to `PointerType` but holding additional data. - New `ButtonSource` added to `PointerButton`, similar to `PointerType` but holding pointer type specific buttons. Use `ButtonSource::mouse_button()` to easily normalize any pointer button type to a generic mouse button. - In the same spirit rename `DeviceEvent::MouseMotion` to `PointerMotion`. - Remove `Force::Calibrated::altitude_angle`. Fixes rust-windowing#3833. Fixes rust-windowing#883. Fixes rust-windowing#336.
- Rename `CursorMoved` to `PointerMoved`. - Rename `CursorEntered` to `PointerEntered`. - Rename `CursorLeft` to `PointerLeft`. - Rename `MouseInput` to `PointerButton`. - Add `position` to every `PointerEvent`. - Remove `Touch`, which is folded into the `Pointer*` events. - New `PointerType` added to `PointerEntered` and `PointerLeft`, signifying which pointer type is the source of this event. - New `PointerSource` added to `PointerMoved`, similar to `PointerType` but holding additional data. - New `ButtonSource` added to `PointerButton`, similar to `PointerType` but holding pointer type specific buttons. Use `ButtonSource::mouse_button()` to easily normalize any pointer button type to a generic mouse button. - In the same spirit rename `DeviceEvent::MouseMotion` to `PointerMotion`. - Remove `Force::Calibrated::altitude_angle`. Fixes rust-windowing#3833. Fixes rust-windowing#883. Fixes rust-windowing#336. Co-authored-by: Kirill Chibisov <[email protected]>
A common case is to get the location on screen a user clicks. The way to do that currently is to create a variable that stores the cursor location, which is updated whenever CursorMoved is received. Then, when a MouseInput event is received, you use whatever that variable is currently set to. I think it would be useful for winit to do this internally, so that the MouseInput event could store a position, making it easier for the user. winit should also update the variable whenever Window::set_cursor_position() is called.
The text was updated successfully, but these errors were encountered: