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

Improve fling animation #927

Closed
Helium314 opened this issue Mar 22, 2023 · 12 comments
Closed

Improve fling animation #927

Helium314 opened this issue Mar 22, 2023 · 12 comments
Labels
android enhancement New feature or request

Comments

@Helium314
Copy link
Contributor

The current fling animation feels very awkward, and is different to basically all/most other map engines and online maps.
It appears more like an easeCamera animation than a slowly decelerating continuation of the previous movement. After some testing I think the issue mainly comes from

  • the camera at some point moving faster than it started (this seems to be tunable, see below)
  • a relatively high velocity is required to start the fling animation, but if the animation starts the move is relatively far
  • the camera stopping relatively quickly

I tried using a custom move gesture detector (see below) to implement a slightly modified fling gesture. This feels much better than current behavior, but a) still a little awkward (due to the sudden stop) and b) requires re-implementing move and pinch-zoom gestures.
A first and simple step could be allowing to choose the fling velocity threshold and animation base time, but ideally the camera velocity behavior during the movement would be adjusted.

            override fun onMoveEnd(p0: MoveGestureDetector, velocityX: Float, velocityY: Float) {
                // maybe fling camera using somewhat different settings
                val minVelocity = 200 // default is 1000
                val animationVelocityFactor = 1.0 / 7 // default is 1 / 7
                val animationDurationOffset = 500 // default is 150
                // without moveFactor (or setting to 1)
                //  the move feels too far
                //  the move starts too fast (animation should not start faster than the move before lifting finger)
                val moveFactor = 0.5

                // reproduce roughly what maplibre does
                val screenDensity = mapView.pixelRatio
                val velocityXY = hypot(velocityX / screenDensity, velocityY / screenDensity)
                if (velocityXY < minVelocity) return
                val tiltFactor = 1.5 // assume tilt = 0 for simplicity
                // default way of determining animation time
                val animationTime = (velocityXY * animationVelocityFactor / tiltFactor + animationDurationOffset).toInt()
                // determine target position
                val offsetX = velocityX / tiltFactor / screenDensity
                val offsetY = velocityY / tiltFactor / screenDensity
                // below is mostly guess, but seems to fit at least roughly
                val c = mapboxMap.cameraPosition
                val p = c.target ?: return
                val metersPerPixel = mapboxMap.projection.getMetersPerPixelAtLatitude(p.latitude)
                val degreesPerMeterLat = 1 / 111225.0 // only approximate, but good enough
                val degreesPerMeterLon = degreesPerMeterLat / (cos(p.latitude * Math.PI / 180.0).coerceAtLeast(0.001))
                val moveLng = offsetX * degreesPerMeterLon * metersPerPixel * moveFactor
                val moveLat = offsetY * degreesPerMeterLat * metersPerPixel * moveFactor
                val target = LatLng(p.latitude + moveLat, p.longitude - moveLng)

                // move camera
                val update = CameraUpdateFactory.CameraPositionUpdate(c.bearing, target, c.tilt, c.zoom, c.padding)
                mapboxMap.easeCamera(update, animationTime)
            }
@louwers louwers added enhancement New feature or request android labels Mar 22, 2023
@ovivoda
Copy link
Contributor

ovivoda commented Mar 22, 2023

@Helium314 can you share a video of this?

@Helium314
Copy link
Contributor Author

Two videos using a testing version of StreetComplete for migration from Tangram to MapLibre:

In the first video you first see current behavior with Tangram (on the left), which I would like to have in MapLibre. Every move, unless very slow, leads to the map gliding a little bit.
On the right you see MapLibre with the move gesture detector above (there is some onMove code which I didn't include, otherwise scrolling wouldn't work). The deceleration is clearly less "smooth" than Tangram, but using the map still feels acceptable compared to Tangram.

ml_acceptable_cut.mp4

In the second video is MapLibre's default behavior. You can't see the taps, but basically either the move stops immediately when I take my finger off the screen, or there is this rather big "jump" with sudden acceleration and stop.
This may be related to maplibre/maplibre-gl-js#2238, which looks similar but with shorter animation.

ml_bad_cut.mp4

As far as I understand, the fling ends up calling moveBy, which sets some animation parameters that (I assume) are used for interpolation.
Then it might be enough to tune the parameters, though I'm not sure whether the starting velocity is taken into account here.

@Helium314
Copy link
Contributor Author

Helium314 commented Mar 22, 2023

Assuming the UnitBezier in animationOptions.easing.emplace(mbgl::util::UnitBezier {0.25, 0.46, 0.45, 0.94}); is the same thing as what I can play with on https://cubic-bezier.com/#.25,.46,.45,.94:

  • The initial slope of the curve is parameter2 / parameter1, which is basically the starting velocity of the camera animation. E.g. if we want to move by 100 pixels in 1 second, the starting speed of the animation would be 100 px / 1 s * 0.46 / 0.25 = 184 px/s.
    This means the start will look smooth if we have an initial velocity of 184 px/s. But actually we already have the velocity and can't change it, so we need to modify move distance or animation time (or both).
    So e.g. we could divide the distance by 1.84 (which is close to the factor 0.5 I apply in the code above) or multiply the animationTime with 1.84. Or divide the distance by sqrt(1.84) and mulitply animationTime with sqrt(1.84) or whatever.
    I tested this and all works reasonably well. [Edit: though it feels somewhat off, so I might be missing something.]
  • For a smooth stop of the animation the last parameter needs to be 1.
  • The slope should never increase above the starting slope, as this would accelerate the camera. Actually I think with the current parameters it stays nearly unchanged for too long, i.e. the first 30% of the animation.
    • Maybe something like 0.1, 0.4, 0.45, 1 would feel better (would change the factor from 1.84 to 4), but I can't really test this.

@wipfli
Copy link
Contributor

wipfli commented Mar 23, 2023

I think it would look best if the camera velocity increased linearly from zero to a maximum value and then decreased linearly to zero again. People sometimes use a quaternion slerp to interpolate the angles.

@Helium314
Copy link
Contributor Author

I think it would look best if the camera velocity increased linearly from zero to a maximum value and then decreased linearly to zero again.

Wouldn't this make the fling animation even more awkward? I can't imagine how a fling would look better if the camera started at velocity 0.

It's JS and not native, but also in ML JS the fling it not nice (imo actually worse than in native):
Compare the fling behavior of https://demotiles.maplibre.org with other maps like https://www.openstreetmap.org, https://streetcomplete.github.io/streetcomplete-mapstyle/?provider=jawg, or https://www.google.com/maps
Doing it different is not wrong by itself, but in my opinion the others do it in a better way. And in a way that is very similar, which is convenient for users who use different map sites/apps.

@ovivoda
Copy link
Contributor

ovivoda commented Mar 25, 2023

Thanks @Helium314 ! Let me ask around a bit and gather community feedback based on your proposal.

One thing that we can do is to keep old behaviour but also allow the behaviour you proposed.

Are you willing to contribute with a PR that will enable this ˆˆˆ?

@1ec5
Copy link
Contributor

1ec5 commented Mar 25, 2023

It's JS and not native, but also in ML JS the fling it not nice (imo actually worse than in native)

GL JS gestures have been primarily optimized for non-touch input. I ran into something similar way back in mapbox/mapbox-gl-native#1266. It felt like the map had a lot of “friction” because you had to fling the map very hard to get it to move much. It used mbgl’s default animation curve but with a duration that was determined by an unnecessarily complex formula. The solution on iOS was to replace it with a much more straightforward, 1-second-long animation, matching the behavior of not only MapKit but also every standard UIKit control on the system (scroll views, menus, etc.). Applying this same duration to other gestures for zooming and rotation made the map feel a lot more fluid.

The Android map SDK subsequently updated its gesture handling in mapbox/mapbox-gl-native#553, but I don’t know if it followed the iOS implementation precisely. In any case, platform parity is not as important as matching the user’s expectations based on common controls.

@Helium314
Copy link
Contributor Author

I played quite a bit with different fling parameters.
Here is what I found best:

  • offset should be proportional to animation time. This is in my opinion the most important change for a better animation.
- double offsetX = velocityX / tiltFactor / screenDensity;
- double offsetY = velocityY / tiltFactor / screenDensity;
+ double offsetX = velocityX * animationTime / 1000 * 0.28
+ double offsetY = velocityY * animationTime / 1000 * 0.28

The factor 1000 is because velocity is per second, animationTime in milliseconds. The factor 0.28 was found purely by testing what looks best, and may also depend on screenDensity and tiltFactor.
I don't have a phone where screenDensity is not 1.5, so can't test different screen densities.
For some reason I couldn't tilt at all (tilt gesure is enabled, but does nothing, and updating camera with tilt doesn't actually tilt), so this is untested as well.

  • UnitBezier parameters should be adjusted for smoother slowing down. This is in moveBy in in native_map_view.cpp. As far as I understand, moveBy is only used for the fling animation and some movement without animation, so simply adjusting parameters should not have any side effects.
- animationOptions.easing.emplace(mbgl::util::UnitBezier {0.25, 0.46, 0.45, 0.94});
+ animationOptions.easing.emplace(mbgl::util::UnitBezier {0.1, 0.4, 0.35, 1.0});
  • In general a somewhat longer animation time is good. With the changes above the animation looks nice, but the map feels "sticky". My suggestion is to increase MapboxConstants.ANIMATION_DURATION_FLING_BASE from 150 to 500 ms.
  • The value 1000 for MapboxConstants.VELOCITY_THRESHOLD_IGNORE_FLING seems much to high. I suggest to reduce it to something like 300.

Are you willing to contribute with a PR that will enable this ˆˆˆ?

I can try...
What did you have in mind?
Simply switching between the 2 animation types?
Or more flexibility, like configurable fling threshold and (base) animation time?
And what about the UnitBezier parameters? In my opinion they are more suitable and could be used for both old and new animations.

@ovivoda
Copy link
Contributor

ovivoda commented Mar 30, 2023

@Helium314 I think the more flexible solution the better. But we can do this step by step maybe?

@louwers
Copy link
Collaborator

louwers commented Apr 3, 2023

@Helium314 Can this be closed now?

@Helium314
Copy link
Contributor Author

Yes, done now.
Thanks!

@louwers
Copy link
Collaborator

louwers commented Apr 4, 2023

@Helium314 Thank YOU! 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
android enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants