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

Animator Updates #349

Merged
merged 25 commits into from
Mar 28, 2018
Merged

Animator Updates #349

merged 25 commits into from
Mar 28, 2018

Conversation

danesfeder
Copy link
Contributor

@danesfeder danesfeder commented Mar 7, 2018

Chop for the compass animation updates was coming from how we were choosing the previous target.
- I opted to cache the previous values for bearing and latlng in the LocationLayerAnimator rather than the LocationLayerPlugin. To me, this just seemed easier to keep track of.

Now fixed:
ezgif com-video-to-gif

Right now, the animator between camera modes is instant and does not interpolate. I see this as something that we should consider fixing because we allow devs to change this setting on-the-fly and it ends up being a jarring UX to have the map jump like this:

ezgif com-video-to-gif

In order to smooth the transitions though, we will need the current map camera position to use as the "previous" bearing or LatLng. I also think this may mean we need separate animators for the camera / location layer? Adding animators would be more code, but we'd still be able to get the same effect (one animator driving both the layer / camera) if we ensure they both have the same duration and are played together with AnimatorSet

Still TODO:

  • Optimize time of animations / interpolators
  • Find a way to not cancel transition animations when switching between camera modes
  • Reset camera when camera mode changes
  • GPS North camera mode

danesfeder and others added 14 commits February 19, 2018 17:38
* Add style / tracking modes for camera integration

* Fix broken tests

* API tweaks and example update
* Update with render / camera modes

* Update camera with mode manager and cancel animation check

* Update method compass name
* initial LocationLayerPlugin cleanup

* added the animator class

* animator implementation in the LocationLayer

* reworked location layer camera

* added Animator to LocationLayerPlugin

* adjusted location activities

* animator order fix

* last values ordering fix

* string fixes

* Fix GPS layer

* using previous animated value when starting a new one

* small tweaks

* setting accuracy ring only if using the right mode
* Clean up and javadoc

* Updates per review
* [location-layer-plugin] - improve setting location engine

* [location-layer-plugin] - attaching location engine listener when resetting the engine
* Clean up and javadoc

* Add long click listener

* Add max / min zoom and padding APIs

* Fix javadoc

* Add default values to options
* [location-layer-plugin] - bumped maps SDK to 6.x

* [location-layer-plugin] - style options initialization fixes

* [location-layer-plugin] - added gestures handling implementation
@danesfeder danesfeder self-assigned this Mar 7, 2018
@danesfeder danesfeder mentioned this pull request Mar 7, 2018
latLngAnimator = new LatLngAnimator(previousLatLng, newLatLng, 1000);
gpsBearingAnimator = new BearingAnimator(previousBearing, newLocation.getBearing(), 1000);
// FIXME: 22/02/2018 evaluate duration of animation better
latLngAnimator = new LatLngAnimator(previousLatLng, targetLatLng, 1000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we replace the constant 1000 with the following logic for now. This should make the animation a bit more continuous.

compassBearingAnimator = new BearingAnimator(previousCompassBearing, targetCompassBearing, 1000);
// FIXME: 22/02/2018 evaluate duration of animation better
compassBearingAnimator = new BearingAnimator(previousBearing, targetCompassBearing, 1000);
compassBearingAnimator.setInterpolator(new LinearInterpolator());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to set LinearInterpolator specifically, this is the default.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah nice 👍


private float getPreviousGpsBearing() {
float previousBearing;
if (gpsBearingAnimator != null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the animator is never set to null? those this need to happen as part of cancel?


private float getPreviousCompassBearing() {
float previousBearing;
if (compassBearingAnimator != null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

// locationChangeAnimator.setInterpolator(new LinearInterpolator());
// } else {
// locationChangeAnimator.setInterpolator(new AccelerateDecelerateInterpolator());
// }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove commented code?

@tobrun
Copy link
Member

tobrun commented Mar 8, 2018

@danesfeder

Right now, the animator between camera modes is instant and does not interpolate. I see this as something that we should consider fixing because we allow devs to change this setting on-the-fly and it ends up being a jarring UX to have the map jump like this:

The issue is not the interpolator, it's the set duration. When animating from one location to another using linear is the most logical one and should be default imo. The only cases where another interpolator could be used to finetune the experience would be with when speed from previous update = 0 and new speed > 0 (AccelerateInterpolator) or when speed from new update = 0 and previous speed > 0 (DecelerateInterpolator)

cc @LukasPaczos

@cammace cammace force-pushed the map-location-animators branch from 94520b5 to bfe40e9 Compare March 12, 2018 16:24
@danesfeder danesfeder force-pushed the dan-animator-updates branch from f8a9f4c to 45dad00 Compare March 21, 2018 15:31
@danesfeder
Copy link
Contributor Author

@cammace @tobrun @LukasPaczos

Added a method to reset the camera animators when switching between camera modes (interpolates between current camera position and current target). Also added the gps north work:

ezgif com-video-to-gif 1

Only two remaining tasks now:

  1. The duration of the animation is still set to 1000 millis -- something weird is going on with the location updates. I'm seeing 2 - 3 updates from the listener for each mock location from pathfinder. This looks like an issue originating from the new location library in maps 6.0

  2. When we transition between camera modes, the animation transitioning to the new camera mode is cancelled and interrupted until finally finishes (you can see this in the gif above). We need to come up with some sort of system to basically say "okay, we are done transitioning now, you can cancel")

@LukasPaczos
Copy link
Contributor

LukasPaczos commented Mar 21, 2018

@danesfeder great work so far!
Just wanted to add that I feel like 2. will be much less of an issue when we resolve 1.

@danesfeder danesfeder merged commit e001cf4 into map-location-animators Mar 28, 2018
@danesfeder danesfeder deleted the dan-animator-updates branch March 28, 2018 13:01
danesfeder added a commit that referenced this pull request Mar 28, 2018
* Add initial animators

* Add Camera and Tracking Modes (#294)

* Add style / tracking modes for camera integration

* Fix broken tests

* API tweaks and example update

* Update with Render / Camera Modes (#297)

* Update with render / camera modes

* Update camera with mode manager and cancel animation check

* Update method compass name

* Add animator class (#302)

* initial LocationLayerPlugin cleanup

* added the animator class

* animator implementation in the LocationLayer

* reworked location layer camera

* added Animator to LocationLayerPlugin

* adjusted location activities

* animator order fix

* last values ordering fix

* string fixes

* Fix GPS layer

* using previous animated value when starting a new one

* small tweaks

* setting accuracy ring only if using the right mode

* Cleanup stale runnable (#304)

* only update the location layer accuracy when not in RenderMode.GPS (#306)

* Improve enabling/disabling location layer plugin (#308)

* LocationLayerPlugin Javadoc (#309)

* Clean up and javadoc

* Updates per review

* LocationEngine listening to updates after resetting (#307)

* [location-layer-plugin] - improve setting location engine

* [location-layer-plugin] - attaching location engine listener when resetting the engine

* Add max / min zoom and padding APIs  (#313)

* Clean up and javadoc

* Add long click listener

* Add max / min zoom and padding APIs

* Fix javadoc

* Add default values to options

* Gestures logic for camera tracking, new telemetry library (#327)

* [location-layer-plugin] - bumped maps SDK to 6.x

* [location-layer-plugin] - style options initialization fixes

* [location-layer-plugin] - added gestures handling implementation

* Update dependencies

* Add missing long click listener

* [location-layer] - fix crash on startup

* Add initial animators

* Add Camera and Tracking Modes (#294)

* Add style / tracking modes for camera integration

* Fix broken tests

* API tweaks and example update

* Update with Render / Camera Modes (#297)

* Update with render / camera modes

* Update camera with mode manager and cancel animation check

* Update method compass name

* Add animator class (#302)

* initial LocationLayerPlugin cleanup

* added the animator class

* animator implementation in the LocationLayer

* reworked location layer camera

* added Animator to LocationLayerPlugin

* adjusted location activities

* animator order fix

* last values ordering fix

* string fixes

* Fix GPS layer

* using previous animated value when starting a new one

* small tweaks

* setting accuracy ring only if using the right mode

* Cleanup stale runnable (#304)

* only update the location layer accuracy when not in RenderMode.GPS (#306)

* Improve enabling/disabling location layer plugin (#308)

* LocationLayerPlugin Javadoc (#309)

* Clean up and javadoc

* Updates per review

* LocationEngine listening to updates after resetting (#307)

* [location-layer-plugin] - improve setting location engine

* [location-layer-plugin] - attaching location engine listener when resetting the engine

* Add max / min zoom and padding APIs  (#313)

* Clean up and javadoc

* Add long click listener

* Add max / min zoom and padding APIs

* Fix javadoc

* Add default values to options

* Gestures logic for camera tracking, new telemetry library (#327)

* [location-layer-plugin] - bumped maps SDK to 6.x

* [location-layer-plugin] - style options initialization fixes

* [location-layer-plugin] - added gestures handling implementation

* Update dependencies

* Add missing long click listener

* [location-layer] - fix crash on startup

* updated branch to latest on master

* Fixed checkstyle error

* fixed up ui test

* use old mas lib for geojson plugin

* Animator Updates (#349)

* Add initial animators

* Add Camera and Tracking Modes (#294)

* Add style / tracking modes for camera integration

* Fix broken tests

* API tweaks and example update

* Update with Render / Camera Modes (#297)

* Update with render / camera modes

* Update camera with mode manager and cancel animation check

* Update method compass name

* Add animator class (#302)

* initial LocationLayerPlugin cleanup

* added the animator class

* animator implementation in the LocationLayer

* reworked location layer camera

* added Animator to LocationLayerPlugin

* adjusted location activities

* animator order fix

* last values ordering fix

* string fixes

* Fix GPS layer

* using previous animated value when starting a new one

* small tweaks

* setting accuracy ring only if using the right mode

* Cleanup stale runnable (#304)

* only update the location layer accuracy when not in RenderMode.GPS (#306)

* Improve enabling/disabling location layer plugin (#308)

* LocationLayerPlugin Javadoc (#309)

* Clean up and javadoc

* Updates per review

* LocationEngine listening to updates after resetting (#307)

* [location-layer-plugin] - improve setting location engine

* [location-layer-plugin] - attaching location engine listener when resetting the engine

* Add max / min zoom and padding APIs  (#313)

* Clean up and javadoc

* Add long click listener

* Add max / min zoom and padding APIs

* Fix javadoc

* Add default values to options

* Gestures logic for camera tracking, new telemetry library (#327)

* [location-layer-plugin] - bumped maps SDK to 6.x

* [location-layer-plugin] - style options initialization fixes

* [location-layer-plugin] - added gestures handling implementation

* Update dependencies

* Add missing long click listener

* [location-layer] - fix crash on startup

* Smooth bearing animation

* Add separate camera animators

* Fix camera transitions between animators

* Add reset function for switching camera modes

* Add gps north functionality to camera animator

* Camera transition updates

* [location-layer-plugin] - fixed camera tracking callbacks

* [location-layer-plugin] - decreased default max zoom

* [location-layer-plugin] - saving state in LocationLayerModesActivity

* update maps sdk to latest beta (#384)

* update maps sdk to latest beta

* fixed checkstyle

* Gestures thresholds adjustments (#386)

* [location-layer-plugin] - fixed gestures threshold

* [location-layer-plugin] - layer drawable updates adjustments

* Fix order of interpolator expression (#388)

* [android] - fix order of interpolator

* [location-layer-plugin] - fine tuned scale values

* Update feature import

* fixup geojson

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

Successfully merging this pull request may close these issues.

3 participants