Repository navigation
Conversation
54d7cdb to
281374d
Compare
#Conflicts: # winit-core/src/event.rs # winit-core/src/event_loop/mod.rs # winit-wayland/src/seat/touch/mod.rs
ogoffart
left a comment
There was a problem hiding this comment.
(i only looked at the API so far)
| /// Should be emitted regardless of window focus. | ||
| PointerMoved { | ||
| device_id: Option<DeviceId>, | ||
| event_time: Option<Duration>, |
There was a problem hiding this comment.
Shouldn't that be an Instant?
(and all other event_time)
Also there should be more documentation that explain what these event_time are.
There was a problem hiding this comment.
It is a duration against an unknown clock reference we don't know (On wayland it is not defined, so the idea is to use the first receiving event to calibrate the offset)
There was a problem hiding this comment.
ok, then this needs to be properly documented.
An alternative is to make a newtype struct EventTime(/*priv*/ Duration) with function such as Sub to obtain a duration.
But if that is documented properly, then we can go with that.
There was a problem hiding this comment.
I used a type EventTime = Duration So all common functionality like wrapped sub and so on must not be implemented manually.
|
Also this adds an event_time to the mouse event, but not to the keyboad event. We might also need them there for completeness. Or? |
Android and ios have them also in the event it self, so you can easily pass the event around and don't have to think about to pass also the event time around. I will add to the keyboard as well. |
|
Review from Claude: Bugs
API and design feedback
Housekeeping
Overall I'd request changes for bugs 1–3, and agree on how history works across platforms (especially Web vs. mobile) before this lands. |
coalescedTouchesForTouch returns separate UITouch objects, so deriving the FingerId from their address gave every history entry a different finger ID than the PointerMoved event it belongs to. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The array returned by coalescedTouchesForTouch ends with the sample of the main touch, so the last history entry duplicated the position and time of the PointerMoved event. Drop it, matching Android where the history only contains earlier samples, and document that the current sample is not part of the history. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The extender lived in WindowData, so after the 49.7-day tick wrap a newly created window started from the raw (small) tick value while older windows reported extended values above 2^32, making event times from different windows incomparable. Keep a single extender on the EventLoopRunner, like the X11 and Wayland backends do. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Key repeats are synthesized on the client with times extrapolated as press + delay + n * gap, so the real time of the following compositor event (e.g. the release) can be earlier than the last synthesized repeat. Clamp key event times to the last emitted one, and document that repeat times on Wayland are approximate. Co-Authored-By: Claude Opus 5.5 <[email protected]>
It describes a pointer sample, so it belongs next to PointerSource and EventTime rather than in event_loop. Order its fields like the arguments of new() and like WindowEvent::PointerMoved, and document position and source. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Web split getCoalescedEvents() into one PointerMoved event per sample, each with an empty history, while Android and iOS emit a single PointerMoved and put the earlier samples into history. Emit one PointerMoved for the dispatched pointermove event on Web as well and put the coalesced samples, without the last one (the dispatched event itself), into history. DeviceEvent::PointerMotion is still emitted once per coalesced sample. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Thanks I fixed the issues in the last commits.
|
#Conflicts: # winit-appkit/src/view.rs # winit-common/src/lib.rs # winit-core/src/event.rs # winit-wayland/src/seat/pointer/mod.rs # winit-win32/src/event_loop/runner.rs # winit-x11/src/event_processor.rs # winit/src/changelog/unreleased.md
…racted from another Duration
changelogmodule if knowledge of this change could be valuable to usersAndroid and Ios do not publish touch events for every single touch movement, but collect them and publish them lower frequent. Using only the lower frequent events, velocity estimation is not appropriate (For a 50ms gesture we would get with 60Hz only 3 events).
Android and Apple expose additional touch event history which can be used.
Implemented for:
@kchibisov We could further discuss if we wanna collect all events and send it out whenever the frame event occured? https://wayland-book.com/seat/touch.html (Clients should accumulate all wl_touch events as they're received, then process pending inputs as a single touch event when the "frame" event is received.)
Currently
client-tool-kitjust sends out the data when receiving theFrameevent (https://github.com/Smithay/client-toolkit/blob/master/src/seat/touch.rs#L137-L205)