Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Commit

Permalink
[android] Fix tracking mode + camera race condition (#9133)
Browse files Browse the repository at this point in the history
* [android] fix tracking mode + camera race condition

* fix resetTrackingModesIfRequired bug (comparing current camera position with target camera position
  • Loading branch information
Guardiola31337 committed May 31, 2017
1 parent 04c5399 commit 8935845
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 95 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ public class MapboxConstants {
public static final String STATE_MY_BEARING_TRACKING_MODE = "mapbox_myBearingTracking";
public static final String STATE_MY_LOCATION_TRACKING_DISMISS = "mapbox_myLocationTrackingDismiss";
public static final String STATE_MY_BEARING_TRACKING_DISMISS = "mapbox_myBearingTrackingDismiss";
public static final String STATE_MY_TRACKING_MODE_DISMISS_FOR_CAMERA = "mapbox_myBearingTrackingDismiss";
public static final String STATE_COMPASS_ENABLED = "mapbox_compassEnabled";
public static final String STATE_COMPASS_GRAVITY = "mapbox_compassGravity";
public static final String STATE_COMPASS_MARGIN_LEFT = "mapbox_compassMarginLeft";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ public boolean onFling(MotionEvent e1, MotionEvent e2, float velocityX, float ve
return false;
}

trackingSettings.resetTrackingModesIfRequired(true, false);
trackingSettings.resetTrackingModesIfRequired(true, false, false);

// cancel any animation
transform.cancelTransitions();
Expand Down Expand Up @@ -390,7 +390,7 @@ public boolean onScroll(MotionEvent e1, MotionEvent e2, float distanceX, float d
}

// reset tracking if needed
trackingSettings.resetTrackingModesIfRequired(true, false);
trackingSettings.resetTrackingModesIfRequired(true, false, false);
// Cancel any animation
transform.cancelTransitions();

Expand Down Expand Up @@ -475,7 +475,7 @@ public boolean onScale(ScaleGestureDetector detector) {
// to be in the center of the map. Therefore the zoom will translate the map center, so tracking
// should be disabled.

trackingSettings.resetTrackingModesIfRequired(!quickZoom, false);
trackingSettings.resetTrackingModesIfRequired(!quickZoom, false, false);
// Scale the map
if (focalPoint != null) {
// arround user provided focal point
Expand Down Expand Up @@ -560,7 +560,7 @@ public boolean onRotate(RotateGestureDetector detector) {
// rotation constitutes translation of anything except the center of
// rotation, so cancel both location and bearing tracking if required

trackingSettings.resetTrackingModesIfRequired(true, true);
trackingSettings.resetTrackingModesIfRequired(true, true, false);

// Get rotate value
double bearing = transform.getRawBearing();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -729,10 +729,38 @@ public final void easeCamera(CameraUpdate update, int durationMs, boolean easing
@UiThread
public final void easeCamera(final CameraUpdate update, final int durationMs, final boolean easingInterpolator,
final MapboxMap.CancelableCallback callback) {
easeCamera(update, durationMs, easingInterpolator, callback, false);
}

/**
* Gradually move the camera by a specified duration in milliseconds, zoom will not be affected
* unless specified within {@link CameraUpdate}. A callback can be used to be notified when
* easing the camera stops. If {@link #getCameraPosition()} is called during the animation, it
* will return the current location of the camera in flight.
* <p>
* Note that this will cancel location tracking mode if enabled. You can change this behaviour by calling
* {@link TrackingSettings#setDismissTrackingModeForCameraPositionChange(boolean)} with false before invoking this
* method and calling it with true in the {@link CancelableCallback#onFinish()}.
* </p>
*
* @param update The change that should be applied to the camera.
* @param durationMs The duration of the animation in milliseconds. This must be strictly
* positive, otherwise an IllegalArgumentException will be thrown.
* @param easingInterpolator True for easing interpolator, false for linear.
* @param callback An optional callback to be notified from the main thread when the animation
* stops. If the animation stops due to its natural completion, the callback
* will be notified with onFinish(). If the animation stops due to interruption
* by a later camera movement or a user gesture, onCancel() will be called.
* Do not update or ease the camera from within onCancel().
* @param isDismissable true will allow animated camera changes dismiss a tracking mode.
*/
@UiThread
public final void easeCamera(final CameraUpdate update, final int durationMs, final boolean easingInterpolator,
final MapboxMap.CancelableCallback callback, final boolean isDismissable) {
new Handler().post(new Runnable() {
@Override
public void run() {
transform.easeCamera(MapboxMap.this, update, durationMs, easingInterpolator, callback);
transform.easeCamera(MapboxMap.this, update, durationMs, easingInterpolator, callback, isDismissable);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ public final class TrackingSettings {
private boolean myLocationEnabled;
private boolean dismissLocationTrackingOnGesture = true;
private boolean dismissBearingTrackingOnGesture = true;
private boolean isResetTrackingWithCameraPositionChange = true;

private MapboxMap.OnMyLocationTrackingModeChangeListener onMyLocationTrackingModeChangeListener;
private MapboxMap.OnMyBearingTrackingModeChangeListener onMyBearingTrackingModeChangeListener;
Expand All @@ -57,8 +56,6 @@ void onSaveInstanceState(Bundle outState) {
outState.putBoolean(MapboxConstants.STATE_MY_LOCATION_TRACKING_DISMISS, isDismissLocationTrackingOnGesture());
outState.putBoolean(MapboxConstants.STATE_MY_BEARING_TRACKING_DISMISS, isDismissBearingTrackingOnGesture());
outState.putBoolean(MapboxConstants.STATE_MY_LOCATION_ENABLED, isMyLocationEnabled());
outState.putBoolean(MapboxConstants.STATE_MY_TRACKING_MODE_DISMISS_FOR_CAMERA,
isDismissTrackingModesForCameraPositionChange());
}

void onRestoreInstanceState(Bundle savedInstanceState) {
Expand All @@ -77,8 +74,6 @@ void onRestoreInstanceState(Bundle savedInstanceState) {
MapboxConstants.STATE_MY_LOCATION_TRACKING_DISMISS, true));
setDismissBearingTrackingOnGesture(savedInstanceState.getBoolean(
MapboxConstants.STATE_MY_BEARING_TRACKING_DISMISS, true));
setDismissTrackingModeForCameraPositionChange(savedInstanceState.getBoolean(
MapboxConstants.STATE_MY_TRACKING_MODE_DISMISS_FOR_CAMERA, true));
}

/**
Expand Down Expand Up @@ -259,15 +254,16 @@ public boolean isScrollGestureCurrentlyEnabled() {
}

/**
* Reset the tracking modes as necessary. Location tracking is reset if the map center is changed,
* bearing tracking if there is a rotation.
* Reset the tracking modes as necessary. Location tracking is reset if the map center is changed and not from
* location, bearing tracking if there is a rotation.
*
* @param translate true if translation
* @param rotate true if rotation
* @param translate true if translation
* @param rotate true if rotation
* @param isFromLocation true if from location
*/
void resetTrackingModesIfRequired(boolean translate, boolean rotate) {
void resetTrackingModesIfRequired(boolean translate, boolean rotate, boolean isFromLocation) {
// if tracking is on, and we should dismiss tracking with gestures, and this is a scroll action, turn tracking off
if (translate && !isLocationTrackingDisabled() && isDismissLocationTrackingOnGesture()) {
if (translate && !isLocationTrackingDisabled() && isDismissLocationTrackingOnGesture() && !isFromLocation) {
setMyLocationTrackingMode(MyLocationTracking.TRACKING_NONE);
}

Expand All @@ -280,36 +276,14 @@ void resetTrackingModesIfRequired(boolean translate, boolean rotate) {
/**
* Reset the tracking modes as necessary. Animated camera position changes can reset the underlying tracking modes.
*
* @param cameraPosition the changed camera position
* @param currentCameraPosition the current camera position
* @param targetCameraPosition the changed camera position
* @param isFromLocation true if from location
*/
void resetTrackingModesIfRequired(CameraPosition cameraPosition) {
if (isDismissTrackingModesForCameraPositionChange()) {
resetTrackingModesIfRequired(cameraPosition.target != null, false);
}
}

/**
* Returns if a animation allows to dismiss a tracking mode.
* <p>
* By default this is set to true.
* </p>
*
* @return True if camera animations will allow to dismiss a tracking mode
*/
public boolean isDismissTrackingModesForCameraPositionChange() {
return isResetTrackingWithCameraPositionChange;
}

/**
* Sets a flag to allow animated camera position changes to dismiss a tracking mode.
* <p>
* <p>
* </p>
*
* @param willAllowToDismiss True will allow animated camera changes dismiss a trackig mode
*/
public void setDismissTrackingModeForCameraPositionChange(boolean willAllowToDismiss) {
isResetTrackingWithCameraPositionChange = willAllowToDismiss;
void resetTrackingModesIfRequired(CameraPosition currentCameraPosition, CameraPosition targetCameraPosition,
boolean isFromLocation) {
resetTrackingModesIfRequired(!currentCameraPosition.target.equals(targetCameraPosition.target), false,
isFromLocation);
}

Location getMyLocation() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public void onMapChanged(@MapView.MapChange int change) {
final void moveCamera(MapboxMap mapboxMap, CameraUpdate update, MapboxMap.CancelableCallback callback) {
CameraPosition cameraPosition = update.getCameraPosition(mapboxMap);
if (!cameraPosition.equals(this.cameraPosition)) {
trackingSettings.resetTrackingModesIfRequired(cameraPosition);
trackingSettings.resetTrackingModesIfRequired(this.cameraPosition, cameraPosition, false);
cancelTransitions();
mapView.jumpTo(cameraPosition.bearing, cameraPosition.target, cameraPosition.tilt, cameraPosition.zoom);
if (callback != null) {
Expand All @@ -98,16 +98,15 @@ final void moveCamera(MapboxMap mapboxMap, CameraUpdate update, MapboxMap.Cancel

@UiThread
final void easeCamera(MapboxMap mapboxMap, CameraUpdate update, int durationMs, boolean easingInterpolator,
final MapboxMap.CancelableCallback callback) {
final MapboxMap.CancelableCallback callback, boolean isDismissable) {
CameraPosition cameraPosition = update.getCameraPosition(mapboxMap);
if (!cameraPosition.equals(this.cameraPosition)) {
trackingSettings.resetTrackingModesIfRequired(cameraPosition);
trackingSettings.resetTrackingModesIfRequired(this.cameraPosition, cameraPosition, isDismissable);
cancelTransitions();
if (callback != null) {
cameraCancelableCallback = callback;
mapView.addOnMapChangedListener(this);
}

mapView.easeTo(cameraPosition.bearing, cameraPosition.target, durationMs, cameraPosition.tilt,
cameraPosition.zoom, easingInterpolator);
}
Expand All @@ -118,7 +117,7 @@ final void animateCamera(MapboxMap mapboxMap, CameraUpdate update, int durationM
final MapboxMap.CancelableCallback callback) {
CameraPosition cameraPosition = update.getCameraPosition(mapboxMap);
if (!cameraPosition.equals(this.cameraPosition)) {
trackingSettings.resetTrackingModesIfRequired(cameraPosition);
trackingSettings.resetTrackingModesIfRequired(this.cameraPosition, cameraPosition, false);

cancelTransitions();
if (callback != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -484,19 +484,8 @@ public void setMyLocationTrackingMode(@MyLocationTracking.Mode int myLocationTra
if (location != null) {
if (myLocationTrackingMode == MyLocationTracking.TRACKING_FOLLOW) {
// center map directly
mapboxMap.getTrackingSettings().setDismissTrackingModeForCameraPositionChange(false);
mapboxMap.easeCamera(CameraUpdateFactory.newLatLng(new LatLng(location)), 0, false /*linear interpolator*/,
new MapboxMap.CancelableCallback() {
@Override
public void onCancel() {

}

@Override
public void onFinish() {
mapboxMap.getTrackingSettings().setDismissTrackingModeForCameraPositionChange(true);
}
});
null, true);
} else {
// do not use interpolated location from tracking mode
latLng = null;
Expand Down Expand Up @@ -662,19 +651,8 @@ public void onSensorChanged(SensorEvent event) {
private void rotateCamera(float rotation) {
CameraPosition.Builder builder = new CameraPosition.Builder();
builder.bearing(rotation);
mapboxMap.getTrackingSettings().setDismissTrackingModeForCameraPositionChange(false);
mapboxMap.easeCamera(CameraUpdateFactory.newCameraPosition(builder.build()), COMPASS_UPDATE_RATE_MS,
false /*linear interpolator*/, new MapboxMap.CancelableCallback() {
@Override
public void onCancel() {

}

@Override
public void onFinish() {
mapboxMap.getTrackingSettings().setDismissTrackingModeForCameraPositionChange(true);
}
});
false /*linear interpolator*/, null);
}

@Override
Expand Down Expand Up @@ -749,7 +727,7 @@ void updateAccuracy(@NonNull Location location) {
abstract void invalidate();
}

private class MyLocationTrackingBehavior extends MyLocationBehavior implements MapboxMap.CancelableCallback {
private class MyLocationTrackingBehavior extends MyLocationBehavior {

@Override
void updateLatLng(@NonNull Location location) {
Expand Down Expand Up @@ -788,10 +766,9 @@ void updateLatLng(@NonNull Location location) {
// accuracy
updateAccuracy(location);

// disable dismiss of tracking settings, enabled in #onFinish
mapboxMap.getTrackingSettings().setDismissTrackingModeForCameraPositionChange(false);
// ease to new camera position with a linear interpolator
mapboxMap.easeCamera(CameraUpdateFactory.newCameraPosition(builder.build()), animationDuration, false, this);
mapboxMap.easeCamera(CameraUpdateFactory.newCameraPosition(builder.build()), animationDuration, false, null,
true);
}

@Override
Expand All @@ -802,22 +779,6 @@ void invalidate() {
screenLocation = new PointF(x, y);
MyLocationView.this.invalidate();
}

@Override
public void onCancel() {
//no op
}

@Override
public void onFinish() {
// Posting to end message queue to avoid race condition #8560
post(new Runnable() {
@Override
public void run() {
mapboxMap.getTrackingSettings().setDismissTrackingModeForCameraPositionChange(true);
}
});
}
}

private class MyLocationShowBehavior extends MyLocationBehavior {
Expand Down

0 comments on commit 8935845

Please sign in to comment.