Refactor the input module

[maxammann]

The input module, which handles key presses and other inputs is quite cluttered. That means that, the logic in order to update the libraries state is quite complicated. The update_state function does the actual updates, while the inputs are recorded through input listeners.

• Take into account #130 in order to support multi-touch.
• Take into account #28

🤔 Expected Behavior

We need a clear abstraction above the recorded inputs and the state update.

😯 Current Behavior

In the event handlers data is recorded. Based on this data, the update_state function is updating the state.

💁 Possible Solution

Maybe reactive programming could be a good abstraction.

🔦 Context

💻 Code Sample

[maxammann]

Related issue: https://github.com/maplibre/maplibre-rs/blob/main/maplibre/src/input/tilt_handler.rs#L49

Pressing a key longer, causes the tilt to change more. The expected behavior is that the tilt changes only once per key press.

[Drabble]

I have found 2 additional potential issues within the input module:

1. In the Winit event doc, nothing seems to indicate that multiple events for a single input can be sent by the OS in the same event loop iteration. In the case of a mouse press and release, it could cause the mouse click to be undetected with the current clicked boolean value that is processed inside of the RedrawRequested event.
2. We call self.window().request_redraw(); every time in the Event::MainEventsCleared event. It could be an optimisation to only call it when an actual change has happened to the map. This would reduce the CPU/GPU usage significantly when the map is idle and this is probably important, especially on the web.

[DerKarlos]

(This is not an extra github issue, yet)

Pressing a key longer, causes the tilt to change more.

ElementState::Pressed is used. Will the key repeat apply? In general, I would use ::Pressed/Released to set/reset a bool. No further Pressed action if the bool is set. Another way to control a camera value would be to integrate in UpdateState as long as the bool is set.

[DerKarlos]

We have 3 inputs from keys, mouse and touch (joystick, game controllers and VR-controllers may follow) We have 3 camera set values for move/pan(x, y), zoom and rotate/tilt(pitch, yaw, no roll) So we could have at last 3x3 separate input handler sources.

Maplibre JS: uses sources for each input type only. An input event causes a delta and a new camera set value. I would call this the reactive part. The set values are processed by the map source, either directly or by integrating to the new set value(s). (The map code extends the camera "class")

Maplibre RS: has different sources for input types and for set values too. The delta and integration is done inside the different sources!

If we put the map and the controller in different Rust crates, RS could use the JS concept.

[maxammann]

What do you mean with a "map source"/"sources"?

For the input module refactoring we first need to introduce an maplibre-rs event loop. The only way to update the camera is through the event loop. Because of the ownership model it is not possible to update for example yaw/pitch or the position of the camera at an arbitrary point in time.

So an RFC for the refactoring of the input module must first design an event system & and event loop.

[maxammann]

@DerKarlos I started a proposal for the RFC process: https://github.com/maplibre/maplibre-rs/pull/188

[DerKarlos]

RFC: no Issue needed ✓ #188: We/I will add comments. You edit/add into the text. ✓

"map source"?: In MJS the map-class source-code has a function to set new positions for the map view. The map will interpolate from the actual to the newly set position. That sounds like your event loop, hm? The map-render needs to run cyclically anyway, so it can change i.E. the pitch value frame by frame. May be in Rust, a function is not possible and an event is needed. I am not that good at this things

[maxammann]

Yes that is correct,.the event loop will interpolate. That is definitely planned.

Im still a bit unsure what you mean with source.

I hope we can pass functions/closures. But maybe we will need events and event handlers.

[maxammann]

RFC: no Issue needed ✓ #188: We/I will add comments. You edit/add into the text. ✓

We should create a separate PR for the input module RFC. My PR is just for the setup.

It would be great if you start the PR based on mine.

[DerKarlos]

"source-code" "source-file"? The many files with the Rust code we edit.

#188: Setup of the input module? No, setup/Bootstrap the RFC process. I will wait until you added/showed, how to handle RFC (copy template from..., first draft, ... )

Then I will start a new RFC for - what - all this?:

• The variations of winit, we need? (#none)
• The event loop (this #91?)
• Splitting the input controller functions in functions and modules/source-codes (not #91?)
• Define the function of the default input-controller (#186)
• Put the input-controller in an extra Rust crate (#185)

[maxammann]

On android this library is used for maplibre/mapbox: https://github.com/mapbox/mapbox-gestures-android