Cancel active fling animation when appropriate

[markusressel]

added cancelFling method to ZoomApi
cancel fling in moveTo, panBy and zoomTo if necessary

fixes #84

[natario1]

Cool! Can you call this cancel() and cancel both STATE_FLINGING and STATE_ANIMATING ? Would this fit your requirements? Docs would say that this cancels any animated motion like flings or developer started animations, while of course pinch/scrolls can't be canceled because the finger is still there.

Not against a more specific API but I think that all these names (scroll/pan/pinch/zoom/fling) in API and XML attributes are already confusing enough. A simple cancel() or cancelAnimations() would be better.

[natario1]

Looking at the code now, I would also appreciate if you could simplify and remove the mClearAnimation logic and instead rely to setState().

I mean that instead of checking mClearAnimation == true, the animation APIs could check that state is still STATE_ANIMATING. If state changed, the animation must stop. It makes more sense, as we don't want a clear flag for every kind of motion. setState seems the right place to coordinate.

[markusressel]

It's funny because at first I also created a cancel flag for the fling 😄 But then I realized the state is also reflecting this and was used already to stop the fling in some situations.

[markusressel]

When I think about it, canceling ongoing animations from the update method isn't a good idea anyway because it might be possible to switch to a new animation while an old animation is still around. In that case the update method will certainly almost never reach it's if line to stop and both animations will run at the same time.

[natario1]

When I think about it, canceling ongoing animations from the update method isn't a good idea anyway because it might be possible to switch to a new animation while an old animation is still around. In that case the update method will certainly almost never reach it's if line to stop and both animations will run at the same time.

Sorry, I understand this comment now!

Need an extra check then... Maybe we could keep an animationCount integer, such that each animation that is started does val thisCount = animationCount++. Then at its own cycle we can check that state == STATE_ANIMATING && thisCount == animationCount. What do you think?

Flings should not have this issue as they are started by input events and you can't have two at the same time

[markusressel]

Need an extra check then... Maybe we could keep an animationCount integer, such that each animation that is started does val thisCount = animationCount++. Then at its own cycle we can check that state == STATE_ANIMATING && thisCount == animationCount. What do you think?

Flings should not have this issue as they are started by input events and you can't have two at the same time

I would just keep a reference of all active animators and cancel them if necessary.
Take a look at the current approach and let me know if you see problems with it.

EDIT:
Adding on the "you can't have two at the same time" thing, thats what I expected to be the case with animations too although one could argue that using pan and zoom animations at the same time can be a use case - but then again the dev would have to specify that his other animation should not get killed because it would in the state proposed by this PR 😕

[markusressel]

I created a new issue (#86) for the touch input stopping animations thing.
Let's continue the discussion over there.

[natario1]

I would say a finger fling is not code driven?

[markusressel]

Well the user isn't touching the screen anymore so... I would argue it is 😄 but I see your point.
Do you have a suggestion? Would you just write:

Cancels all currently active animations.

?

[natario1]

I would be more specific and say like, animations triggered by API calls with animate = true and fling animations triggered by touch input.

[markusressel]

@natario1 Ok with the current vesion?

[natario1]

Looks good to me! @markusressel feel free to create a new release if you think so

[natario1]

Adding on the "you can't have two at the same time" thing, thats what I expected to be the case with animations too although one could argue that using pan and zoom animations at the same time can be a use case - but then again the dev would have to specify that his other animation should not get killed because it would in the state proposed by this PR 😕

I don't have a strong position about this, I'm OK with merging your autocancel, but yes, in theory we could avoid. If you run two animations, the second should win in the end - not sure about the eye result, but I think it should run after the first at each cycle, invalidating the first. You just get a mess of listener updates. For this the dev could call cancelAnimations().

Buut again, not sure :-)

Should we add an API like allowConcurrentAnimations ?

Or, in the long run, have two booleans stopConcurrentAnimations and stopAnimationsOnTap.

[markusressel]

You just get a mess of listener updates.

You're right, this would be impossible to handle properly if the dev is allowed to start multiple animations at the same time... I don't think this is a good thing to allow then. The only drawback with limiting to a single animation concurrently is that f.ex. a moveTo call with another moveTo call in the middle will cause an abrupt stop of the animation before the new animation gets going. But first I think this is a minimal issue and second it should also be handled by the library (if ever).

Should we add an API like allowConcurrentAnimations ?

I don't think so (based on the text above).