NPE on CancelableCallback.onCancel()

[olsisaqe]

Android
Mapbox 5.2.0

Steps to trigger behavior

1. Create the map and animate the camera with a callback
2. Do animate the camera again very fast meanwhile the first animation is still in progress

Expected behavior

Should not crash but cancel correctly the first animation

Actual behavior

Crashes with

java.lang.NullPointerException: Attempt to invoke interface method 'void com.mapbox.mapboxsdk.maps.MapboxMap$CancelableCallback.onCancel()' on a null object reference at com.mapbox.mapboxsdk.maps.Transform.cancelTransitions(Transform.java:178) at com.mapbox.mapboxsdk.maps.MapGestureDetector$GestureListener.onScroll(MapGestureDetector.java:437) at android.view.GestureDetector.onTouchEvent(GestureDetector.java:622) at android.support.v4.view.GestureDetectorCompat$GestureDetectorCompatImplJellybeanMr2.onTouchEvent(GestureDetectorCompat.java:480) at android.support.v4.view.GestureDetectorCompat.onTouchEvent(GestureDetectorCompat.java:543) at com.mapbox.mapboxsdk.maps.MapGestureDetector.onTouchEvent(MapGestureDetector.java:245) at com.mapbox.mapboxsdk.maps.MapView.onTouchEvent(MapView.java:427) at android.view.View.dispatchTouchEvent(View.java:9993) at android.view.ViewGroup.dispatchTransformedTouchEvent(ViewGroup.java:2828)

[tobrun]

Thank you for reaching out and reporting your problem.

I'm not able to reproduce this one:

Code for above:

      animateButton.setOnClickListener(new View.OnClickListener() {
        @Override
        public void onClick(View view) {
          CameraPosition cameraPosition = new CameraPosition.Builder()
            .target(getNextLatLng())
            .bearing(270)
            .tilt(20)
            .build();

          MapboxMap.CancelableCallback callback = new MapboxMap.CancelableCallback() {
            @Override
            public void onCancel() {
              Timber.i("Duration onCancel Callback called.");
              Toast.makeText(
                CameraAnimationTypeActivity.this,
                "Duration onCancel Callback called.",
                Toast.LENGTH_LONG).show();
            }

            @Override
            public void onFinish() {
              Timber.i("Duration onFinish Callback called.");
              Toast.makeText(
                CameraAnimationTypeActivity.this,
                "Duration onFinish Callback called.",
                Toast.LENGTH_LONG).show();
            }
          };

          mapboxMap.animateCamera(CameraUpdateFactory.newCameraPosition(cameraPosition), 7500, callback);
        }
      });

Adding multiple animateCamera invocations doesn't show it either.
Could you share some code how you are able to make it crash?

[olsisaqe]

In our case we animate a lot more often in the range of every 100ms. Will try to create some code to reproduce the bug hopefully soon.

[tobrun]

Been doing something similar in:

No luck of reproducing this so far. I'm thinking to patch this up with a try catch.

[tobrun]

I'm thinking that positng the animation to the end of the message queue might be the source of this exception:

  public final void animateCamera(final CameraUpdate update, final int durationMs,
                                  final MapboxMap.CancelableCallback callback) {
    if (durationMs <= 0) {
      throw new IllegalArgumentException("Null duration passed into animageCamera");
    }
    new Handler().post(new Runnable() {
      @Override
      public void run() {
        transform.animateCamera(MapboxMap.this, update, durationMs, callback);
      }
    });
  }

This post wast added to allow to invoke new animateCamera animations inside of the callback, it will be cleaner doing this just before invoking the callbacks instead.

[tobrun]

@olsisaqe as an immediate workaround and since you mention wanting to animate fast, have you seen the camera animations using Android SDK animators from #10001?

[olsisaqe]

Interesting thanks. I already use a similar implementation with AnimatorSet for other purposes (like symbol layer animation) and works nicely.

[tobrun]

@olsisaqe Potential fix for this in #10664, would you be able to test that out with the 5.3.0-SNAPSHOT build once that PR is merged?

[tobrun]

landed with #10646 feel free to test this out with the snapshot build