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

CameraUpdates overwrite each other (no parallel camera updates) #2790

Open
westnordost opened this issue Aug 31, 2024 · 3 comments
Open

CameraUpdates overwrite each other (no parallel camera updates) #2790

westnordost opened this issue Aug 31, 2024 · 3 comments
Labels

Comments

@westnordost
Copy link
Collaborator

westnordost commented Aug 31, 2024

Note
This is probably rather a missing feature than a bug. However, the API suggests that this should be possible, but it turns out that it isn't.

Description
Any new camera update posted to MapLibreMap::easeCamera cancels the old camera animation and replaces it with a new camera animation.

To Reproduce
Calling e.g.

val zoomTo = CameraUpdateFactory.newCameraPosition(CameraPosition.Builder().zoom(18).build())
map.easeCamera(zoomTo, 1000)

delay(500)

val moveTo = CameraUpdateFactory.newCameraPosition(CameraPosition.Builder().target(12.0,15.0).build())

map.easeCamera(moveTo, 1000)

will cancel the zooming to zoom=18 after 500ms and instead (rather than additionally) move to the given target position. I.e. any new camera position update animation will overwrite the last.

Expected behavior
The animations of different properties of the camera (zoom, tilt, position, rotation, ...) should run in parallel if they don't conflict each other.

Video
In this video, we see that the position is being updated in an animation every second or so to focus on the current location. When the user taps the zoom-in button, the move-to animation is cancelled or the other way round (the zoom-in animation is cancelled), depending on the timing:

zoombounce.mp4

Important: This is not only an issue with zoom vs position. At any time, the user may also press a button to change the map's rotation (the compass button) or tilt (changing from (car-)navigation-mode to just-follow-me-mode) while the position is continuously updated. Also, it is possible for the continuously updated position to interrupt a zoom-to or zoom-back animation as seen in the video in #2789

Platform information (please complete the following information):

  • Platform: Android, any version
  • Version 11.2.0

Additional context
Originally reported in streetcomplete/StreetComplete#5860 and in streetcomplete/StreetComplete#5857 (for padding-animation)

@westnordost westnordost added the bug Something isn't working label Aug 31, 2024
@westnordost
Copy link
Collaborator Author

westnordost commented Nov 19, 2024

(Regarding the labels: This is probably rather a missing feature than abug, but an issue for cpp-core, as animations are implemented in native code.)

@westnordost westnordost added cpp-core and removed bug Something isn't working android labels Nov 19, 2024
@louwers
Copy link
Collaborator

louwers commented Nov 19, 2024

Would just including the options from the previous update in the new one be an option?

@westnordost
Copy link
Collaborator Author

You mean the previous animation target? I.e. animation A starts with zoom by +1 (zoom 17 to 18), then while animation A is still running, animation B starts with a new position?

If your suggestion is to include the zoom target of 18 in the animation B that replaces animation A: I think this behavior would be suboptimal and can lead to unexpected results that feel like bugs.
Imagine the animation A had a duration of 1s, but animation B has a duration of 4s. Then suddenly, also the zooming-in would slow down a lot. And even if the duration of animation B was just 1s, too, and animation B was started 0.5s after animation A was started, that would still result in the animation time of the zoom doubling.

The correct way to handle parallel animations, I'd say, is to animate each separate property (zoom, tilt, rotation, position, padding) separately and only replace a running animation with a newly posted animation if it animates the same property.

FYI, I wrote in the chat today, a slightly different approach:

Back when I used Tangram-ES for display of the map, I remember that that library also didn't support parallel camera animations. (Initially, they did, but it was kind of buggy so they removed it without replacement.) So, what I did was to implement this replacing the normal animations API: CameraManager.kt

In a nutshell, I re-implemented all animating of the camera with Android ObjectAnimators. They animate a set of camera properties to the target value(s), always starting from the current camera properties and cancelling any prior animations whose animated properties overlap on start. (E.g. new animation of zoom+tilt cancels prior animation of zoom+rotate but doesn't cancel prior animation of position). I think when a gesture was detected, I just called cancelAllAnimations on that class.

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

No branches or pull requests

2 participants