mapbox / mapbox-maps-android

Interactive, thoroughly customizable maps in native Android powered by vector tiles and OpenGL.
https://www.mapbox.com/mobile-maps-sdk
Other
477 stars 134 forks source link

Unexpected Camera Shift After Disabling Location Tracking #2513

Open Copatych opened 1 week ago

Copatych commented 1 week ago

Environment

Observed behavior and steps to reproduce

There is an unexpected camera behavior when using location tracking

val followPuckViewportState = viewport.makeFollowPuckViewportState(
    FollowPuckViewportStateOptions.Builder()
        .pitch(45.0)
        .bearing(FollowPuckViewportStateBearing.SyncWithLocationPuck)
        .zoom(17.0)
        .build()
)

viewport.transitionTo(
    followPuckViewportState,
    viewport.makeDefaultViewportTransition(
        DefaultViewportTransitionOptions.Builder().maxDurationMs(500).build()
    )
)

Steps to reproduce:

  1. Enable location tracking with the code above (camera will move to user's last coordinate).
  2. Do not receive any coordinate updates.
  3. Disable location tracking by manually moving the camera.
  4. Get new coordinate.

Observed behavior: The camera returns to the previous location as if location tracking was still active (but with the previous coordinate, not the new coordinate). After this, if we receive new coordinates, then everything is as expected: location tracking is disabled.

Expected behavior

If we disable location tracking (either by moving the camera or by calling viewport.idle()), then it no longer affects the camera in any way, all internal deferred actions are cancelled in ViewportPlugin.

Notes / preliminary analysis

In the debug I saw that FollowPuckViewportStateImpl.stopUpdatingCamera() is called, in which removeIndicatorListenerIfNeeded() is called. In removeIndicatorListenerIfNeeded() there is such code

private fun removeIndicatorListenerIfNeeded() {
  if (isObservingLocationUpdates && dataSourceUpdateObservers.isEmpty() && !isFollowingStateRunning) {
    locationComponent.removeOnIndicatorPositionChangedListener(indicatorPositionChangedListener)
    locationComponent.removeOnIndicatorBearingChangedListener(indicatorBearingChangedListener)
    isObservingLocationUpdates = false
    // when unsubscribed from the location updates, we don't want to keep an outdated location, so
    // when user transition to the FollowPuckViewportState, there wouldn't be any unintentional jump.
    lastBearing = null
    lastLocation = null
  }
}

dataSourceUpdateObservers is not empty.

If replace

viewport.makeDefaultViewportTransition(
    DefaultViewportTransitionOptions.Builder().maxDurationMs(500).build()
)

with

viewport.makeImmediateViewportTransition()

then there is no such problem. And dataSourceUpdateObservers is empty on stopUpdatingCamera().

flasher297 commented 1 week ago

@Copatych thank you for the detailed report. Unfortunately, I can't reproduce the issue. Maybe you forgot to provide some mapView.location settings that cause such behavior. I would appreciate it if you provide a reproducible example.

Copatych commented 5 days ago

Location settings

location.enabled = true
location.puckBearingEnabled = true
location.accuracyRingColor = Color.parseColor("#331D9ED8")
location.showAccuracyRing = true
location.locationPuck = LocationPuck2D(...

We have implemented a custom LocationProvider that retrieves location data either from the Android LocationManager or from mock locations created by tapping on the map. We are also utilizing the LocationCompassEngine, which we've adapted from Mapbox version 11.2.1 (According to Mapbox's response https://github.com/mapbox/mapbox-maps-android/issues/2064, they do not intend to make the API public).

The issue is most easily observed when moving over significant distances. This is primarily due to the camera movements becoming more noticeable. On certain smartphones, the compass can introduce additional complications. This is because we treat any rotation of the smartphone as "coordinate updates" (accordingly, point 2 of "Steps to reproduce" may be violated). Some devices trigger such events even without actual physical rotation, for instance, when lying flat. To improve the consistency of reproduction, it's advisable to disable events about changes in bearing. This helps reduce interference from the compass on smartphones that report changes even when not in motion.