facebookarchive / pop

An extensible iOS and OS X animation library, useful for physics-based interactions.
Other
19.66k stars 2.88k forks source link

Additive animation with no fromValue produces unexpected results #344

Open zats opened 7 years ago

zats commented 7 years ago

Conceptually, it seems that when CAAnimation.additive is set to YES, lack of fromValue should result in animation starting with 0 (whatever it means in the context of particular property. For example:

POPBasicAnimation *animation = [POPBasicAnimation animationWithPropertyNamed:kPOPViewCenter];
animation.additive = YES;
animation.toValue = [NSValue valueWithCGPoint:CGPointMake(offset, -imageView.center.y - 40)];
animation.duration = duration;

produces funky result. (I'm guessing it happens due to animation implicitly copying initial position to animation.fromValue)

Following addition fixes it

POPBasicAnimation *animation = [POPBasicAnimation animationWithPropertyNamed:kPOPViewCenter];
animation.additive = YES;
+ animation.fromValue = [NSValue valueWithCGPoint:CGPointZero];
animation.toValue = [NSValue valueWithCGPoint:CGPointMake(offset, -imageView.center.y - 40)];
animation.duration = duration;

Without knowing the underlying architecture, I'd assume that from consumer point of view, not assigning fromValue when additive animation is enabled should produce animation from the current state to toValue.

If my guess about the reason is correct, fixing this might require a somewhat large switch statement going through all possible property types and suggesting default values (e.g. @0 for numeric types, CGPointZero for CGPoint, not even sure what should it be for property of CGAffineTransform type)

But before getting into engineering aspect of it, I'd love to hear some other perspectives - am I'm the only one who has this expectation?

grp commented 7 years ago

Seems valid to me, but I can also see an argument for always starting from the property's current value being simpler. If nothing else, though, deserves to be documented that you'll want to set fromValue as well as additive.

zats commented 7 years ago

I agree with the simplicity argument, but I'd probably strive for simplicity on the consumer side (not on the inside-the-framework side).

Although if it's critical to keep it the way it is, then yes, definitely worth a @note in the documentation