Properly blend delayed animations about android_additive_animations HOT 5 CLOSED

Hi @ryanmoelter,
nice to hear you are enjoying the library! 🎉

You are right, this behaviour might seem unexpected, but it's a design decision I made when writing the library.

The then()-delayed animations are (more or less) treated as if you had created them after the delay.
If you don't do this, you might find yourself with even stranger behaviour, where views have intermediate states you could never get to with the target animation values.
Consider this example:

view.setX(0);
AdditiveAnimator.animate(view).x(100).then().x(0).start();
AdditiveAnimator.animate(view).x(50).then().x(100).start();

In this case, because of the additive nature of the animations the view would actually go past the maximum set value of 100 while it was animating to 50, before settling at 100!
The relative changes are:

0 to 100 = +100  100 to 0  = -100
0 to 50  = +50   50 to 100 = +50

But the order in which the animations would be played is be this:

+100 +50 -100 +50

Notice that x shortly goes to 150 in this version!

The way the library works right now would make the changes in this order:

0 to 100 = +100 100 to 50 = -50
50 to 0  = -50  0 to 100  = +100
=>
+100 -50 -50 +100

This implementation makes sure that the animation never produces intermediate values that were not enqueued, but suffers from the problem that you described in your ticket.

Changing this behaviour itself would not require a lot of code (1 line, if I'm not mistaken (famous last words)), but I'm hesitant to make this change because of the implications it would have on existing projects.

Maybe a flag to explicitly enable this behaviour would make sense?

In the meantime, you could opt out of the additive behaviour by calling cancelAnimations() on the views in question for this specific case, or use AnimationEndListeners instead of then()-chaining, where you can check for some condition before animating to alpha = 1.

Cheers,
David

from android_additive_animations.

Another idea I had was to add an optional parameter to the then() method family: a condition to be evaluated before running the next animation block.

In you example, the code could look like this:

animator.animate(view)
    .alpha(0f)
    .thenIf(() -> mIsViewHidden)
    .target(otherView)
    .alpha(1f)
    .start();

In this case, otherView would not be animated to alpha = 1f if the instance variable mIsViewHidden is false.
This condition would be propagated to all subsequently enqueued animations in the same animation chain, but you could always override it manually at any point to skip only some animations.

What would you think of this solution? It also has some pitfalls, but solves a common problem of statefulness in animation chaining.

from android_additive_animations.

Wow, this ended up at the bottom of my inbox, sorry for the late reply!

What I'm trying to achieve with the proposed style of blending is to make it easy for my teammates (who may not understand the library as well as you or I do) to get the right behavior, defined mostly by not ending in an unexpected state.

I see the tradeoff you made, and it makes sense. You want to keep the animated value within the expected range, i.e. within the bounds of the values passed into the animations.

I think adding a flag would work fine! I understand not wanting to change the behavior for existing codebases.

Unfortunately, the other two options won't work for me. Calling cancelAnimations() defeats the purpose of the library by preventing the blending of animations. And the thenIf(...), while potentially useful, it requires users to assert that the animation finished, which is extra and obscure work and very easy to accidentally leave out.

Overall, I'd love if we could add a flag for the style of blending I'm asking for!

P.S. I understand not wanting to have animations go outside of the expected bounds, but you can already do that. Consider this:

view.setX(100);
AdditiveAnimator.animate(view).setDuration(200).x(0).start();
AdditiveAnimator.animate(view).setDuration(50).x(100).start();

This will do the following if I understand how animations get added together correctly:

So, the existing framework allows for out-of-bounds animations, as long as they're overlapping. That doesn't address the people who are relying on the library, so I still don't think we should change the default implementation without a major version bump, but I do still think it makes more sense.

from android_additive_animations.

Hi Ryan,

I understand your needs in this case, but I'm still not quite convinced the proposed change is a good one - it breaks basically all of the then()-chaining demos quite badly.
Some exhibit the 'out of bounds' problem where views drift off-screen, others break down completely with views jumping from place to place.

It's not obvious exactly when and how an animation will break (except for repeating and reversing animations, which are always busted).
I couldn't predict exactly what would happen in each of the demos by looking at the code, so it's probably incomprehensible to someone who is not familiar with the library works...

I can push the change to a branch if you'd like to try it out yourself?

Regarding the thenIf() idea - I'm not really convinced by the argument that you would need to additionally keep track of the state of your views, you need to do that anyway in my opinion.
You are probably going to have a state flag of some sort in your ViewModel (if you use the MVVM architecture) to preserve the state for interface rotation etc, which you could simply pass to the thenIf() lambda.
It's true that it might be easy to forget about this, or miss it completely as a newcomer, but I would argue the same holds true for selecting the proper blend-mode for each animation (which also requires more knowledge of the inner workings of the library)?

You're right about the possibility of going out-of-bounds by starting two animations with different durations, I had not thought about that!
I don't think I can do anything about the specific case you mentioned though :/

from android_additive_animations.

Hi Ryan,

I'm happy to tell you that I've made some progress on this issue, by rethinking what the user actually wants to do in cases such as your initial example.

I think the best way of abstracting the expected behaviour is to think about it as the state of the view, so that is the basis of a new approach I'm trying: explicitly animate between predefined States as opposed to animating specific parameters which coincide with the expected UI state.

One of the new constructs is an AnimationState object, which has a list of animations it wants to run, an ID, and optional start/end actions.
The animations are ONLY run if the animated object is still in the state that enqueued them.

Using the new API looks like this:

AdditiveAnimator.animate(view1)
    .state(AnimationStates.HIDDEN)
    .thenWithDelay(150)
    .target(view2)
    .state(AnimationStates.VISIBLE)
    .start();

… and behaves exactly like you want, even when quickly triggering between the HIDDEN and VISIBLE states!

(The AnimationStates class just holds some predefined AnimationState values for better readability.)

I'm interested in hearing what you think about it, you can check out the code and play around with the demo in the animation_states branch!

from android_additive_animations.