Fresco: AnimatedZoomableController->setTransformAnimated does nothing

[karaarda]

In zoomable sample this method does nothing. Only setTransformImmediate works. Please fix.

[karaarda]

I found out that stopAnimation is called after setTransformAnimated on onGestureBegin method of TransformGestureDetector

[oprisnik]

Can you give us more details what the issue you're facing is (maybe with a video / gif of the issue), how to repro and what the desired behavior would be?

[karaarda]

When I use the "zoomToPoint" method as followed:
zoomToPoint(getMaxScaleFactor(), imagePoint, viewPoint, LIMIT_ALL, 400, null);
inAbstractAnimatedZoomableController->setTransform method it calls setTransformAnimated method according to this code:

if (durationMs <= 0) {
      setTransformImmediate(newTransform);
    } else {
      setTransformAnimated(newTransform, durationMs, onAnimationComplete);
    }

and the method is as followed:

public void setTransformAnimated(
      final Matrix newTransform,
      long durationMs,
      @Nullable final Runnable onAnimationComplete) {
    FLog.v(getLogTag(), "setTransformAnimated: duration %d ms", durationMs);
    stopAnimation();
    Preconditions.checkArgument(durationMs > 0);
    Preconditions.checkState(!isAnimating());
    setAnimating(true);
    mValueAnimator.setDuration(durationMs);
    getTransform().getValues(getStartValues());
    newTransform.getValues(getStopValues());
    mValueAnimator.addUpdateListener(new ValueAnimator.AnimatorUpdateListener() {
      @Override
      public void onAnimationUpdate(ValueAnimator valueAnimator) {
        calculateInterpolation(getWorkingTransform(), (float) valueAnimator.getAnimatedValue());
        AnimatedZoomableController.super.setTransform(getWorkingTransform());
      }
    });
    mValueAnimator.addListener(new AnimatorListenerAdapter() {
      @Override
      public void onAnimationCancel(Animator animation) {
        FLog.v(getLogTag(), "setTransformAnimated: animation cancelled");
        onAnimationStopped();
      }
      @Override
      public void onAnimationEnd(Animator animation) {
        FLog.v(getLogTag(), "setTransformAnimated: animation finished");
        onAnimationStopped();
      }
      private void onAnimationStopped() {
        if (onAnimationComplete != null) {
          onAnimationComplete.run();
        }
        setAnimating(false);
        getDetector().restartGesture();
      }
    });
    mValueAnimator.start();
  }

but after that call nothing happens because this code is called immediatly after starting animation:

  public void stopAnimation() {
      /*
      if (!isAnimating()) {
        return;
      }
      FLog.v(getLogTag(), "stopAnimation");
      mValueAnimator.end();
      mValueAnimator.removeAllUpdateListeners();
      mValueAnimator.removeAllListeners(); */
  }

and where it is called is this method:

  @Override
  public void onGestureBegin(TransformGestureDetector detector) {
    FLog.v(getLogTag(), "onGestureBegin");
    stopAnimation();
    super.onGestureBegin(detector);
  }

In the conclusion when you set a duration for the animation it doesnt animate, it does nothing, in fact it shoud zoom with an animation to the desired scale.

How to reproduce:
call this meethod and see it does nothing:
zoomToPoint(getMaxScaleFactor(), imagePoint, viewPoint, LIMIT_ALL, 400, null)

I hope I answered all your questions. Thanks for your interest.

[oprisnik]

Thank you very much for the detailed description @senhor .
That sounds like a bug indeed. If you have time, feel free to send a pull request that fixes this. Otherwise we'll take a look at this at some point.

[karaarda]

I could just come up with an idea of commenting out the stopAnimation method but it is hackish and contains some bugs. It would be a nice approach to cancel the gesture event instead of the animation but it is hackish also I think. I will share it with you if I can come up with a better idea.

[oprisnik]

Thank you. Maybe we also have some spare time soon to take a look at this.

[foghina]

I can't repro on the latest version. I added your exact zoomToPoint call to the zoomable sample app and it works:

https://gfycat.com/EnormousExcitableAngelwingmussel

Closing the issue for now. Feel free to reopen if you can supply a valid repro (e.g. as a sample app).

[karaarda]

As I mentioned what prevents animation from starting is onGestureBegin method which is so:

@Override
  public void onGestureBegin(TransformGestureDetector detector) {
    FLog.v(getLogTag(), "onGestureBegin");
    stopAnimation();
    super.onGestureBegin(detector);
  }

But you are not using any gesture to make zoom. It works fine whit your method but I want to zoom into a point when double tapped. Try to do it that way.

How I achieved Zoom on double tap:

GestureListenerWrapper.java -> onDoubleTap

@Override
  public boolean onDoubleTap(MotionEvent e) {
      mDraweeView.onDoubleTap(e);
    return mDelegate.onDoubleTap(e);
  }

ZoomableDraweeView.java -> onDoubleTap

public void onDoubleTap(MotionEvent e) {
        float screenX = e.getX();
        float screenY = e.getY();
        float viewX = screenX - getLeft();
        float viewY = screenY - getTop();

        PointF viewPoint = new PointF(viewX, viewY);

        ((AnimatedZoomableController)mZoomableController).zoom(viewPoint);
    }

AnimatedZoomableController.java -> zoom

  public void zoom(PointF viewPoint) {

      PointF imagePoint = mapViewToImage(viewPoint);

      if(getScaleFactor() < getMaxScaleFactor())
          zoomToPoint(getMaxScaleFactor(), imagePoint, viewPoint, LIMIT_ALL, 400, null);
      else
          zoomToPoint(getMinScaleFactor(), imagePoint, viewPoint, LIMIT_ALL, 400, null);
  }

Sorry that I don't have time to create a samle app nowadays. If you can't repro or implement the double tap future I can try my best next week.

[karaarda]

Who is gonna reopen this issue? I cannot because of you closed the issue as a Collaborator. Please read the problem carefully next time and wait for an answer for the parts you didn't understand especially when there is another contributor who says it is a bug.

[foghina]

@senhor I've added double tap to the zoomableapp sample in 6309246. Check it out!