There was a tweet a couple of days ago that resulted in a discussion/question about what it wrong with the usage of removedOnCompletion = NO in the SparkRecordingCircle code for that Subjective-C post.
We all kept saying that the explanation doesn't fit in a tweet, so here is my rough explanation about the issues.
But, let me first say that I think that the Subjective-C articles are reallt cool and useful to learn from. This is trying to reason about the general usage of removedOnCompletion = NO, using that code as an example, since that was what was discussed on twitter.
The root problem, as Nacho Soto pointed out, is that removedOnCompletion = NO in combination with fillMode = kCAFillModeForwards is almost always used when you are not updating the model layer. This means that after the animation has finished, what you see on screen is not reflected in the property of the layer. The biggest issue that this can cause is that all the mechanisms that rely on the model value (for example: hit-testing, KVO, etc.) won't work correctly if the model value doesn't change. That is not the case in this very example. But I see this mistake being made
a lot by people who are new to Core Animation who just want the animation to persist.
In my opinion, it boils down to what Core Animation was built to do and how the animation part of it's meant to work (or at least according to my understanding of how it's meant to work). Animations in Core Animation are designed to be decoupled from the model value:
The data and state information of a layer object is decoupled from the visual presentation of that layer’s content onscreen.
They can however be triggered by a change in the model, which is the case with implicit animations. Animations can also be created and added explicitly to a layer without there being a property change. This is the very, very common case where we create our own CABasicAnimation and add it to the layer, just as it was done in this example. The important thing to note is that: since the animation is only changing how things appear on screen and since that is decoupled from the state information, adding an animation to a layer without changing the model is only doing half of the necessary work.
But, as R. Peres pointed out, in this example, the model value is updated in the removeAnimations method. And the animations are removed from the layer, so at the end of it all there is no inconsistency between the model and the presentation. That is completely true. In this example, everything works out in the end. The author writes:
You may have noticed in the updateAnimations method we set both the stokeEnd and strokeStart animations to not be removed on completion. Since we’re going to be doing some fancy footwork by copying the state of the layer when we stop the animations, we will remove the animations ourselves.
but I, personally, feel that if you are going to use a technique which is so often misused, it is important to say so and to explain why this may be a good use for that technique. This is a popular site and people look up to such sites and accept the coding practices that are used as best practices. This makes a lot of sense and the writers out there also generally write really good code. I just think that there is a responsibility to push yourself to a keep really high standard when you publish code on the web. But that is a completely separate topic, I won't say more about it.
OK, so you might say: "but the author updates the model and manually removes the animations. so what is all this fuss about?"
I, again personally, don't think that this was a good use of not removing the animation because it made the animation part of the logic. But, what not removing the animation upon completion really means in this case is that if the user holds their finger down until the the full circle is filled, the animations will persist until the user lifts their finger.
This case could very easylly have been handeled by instead updating the model value before adding the animation. This means setting the toValue explicitly on the model.
Update:
Actually, as was pointed out in the comments, the animation is removed upon completion but was done so manually in the delgate callback. If that is all that the delegate callback was used for then it could be removed completely (along with setting the delegate). This would mean that the code that ecplicitly sets the model value can be made slighly clearer, since there is one less method involved to deal with the animation.
An example of this can be seen in my modified code below (I've commented out the lines that I removed and put a comment on the lines that I added or modified):
- (void)updateAnimations
{
CGFloat duration = self.duration * (1.f - [[self.progressLayers firstObject] strokeEnd]);
CGFloat strokeEndFinal = 1.f;
for (CAShapeLayer *progressLayer in self.progressLayers)
{
CABasicAnimation *strokeEndAnimation = [CABasicAnimation animationWithKeyPath:@"strokeEnd"];
strokeEndAnimation.duration = duration;
strokeEndAnimation.fromValue = @(progressLayer.strokeEnd);
strokeEndAnimation.toValue = @(strokeEndFinal);
strokeEndAnimation.autoreverses = NO;
strokeEndAnimation.repeatCount = 0.f;
// strokeEndAnimation.fillMode = kCAFillModeForwards; // REMOVED
// strokeEndAnimation.removedOnCompletion = NO; // REMOVED
// strokeEndAnimation.delegate = self; // REMOVED (optionally, after an update)
CGFloat oldStrokeEnd = progressLayer.strokeEnd; // ADDED
progressLayer.strokeEnd = strokeEndFinal; // ADDED
[progressLayer addAnimation:strokeEndAnimation forKey:@"strokeEndAnimation"];
strokeEndFinal -= (oldStrokeEnd - progressLayer.strokeStart); // MODIFIED
if (progressLayer != self.currentProgressLayer)
{
CABasicAnimation *strokeStartAnimation = [CABasicAnimation animationWithKeyPath:@"strokeStart"];
strokeStartAnimation.duration = duration;
strokeStartAnimation.fromValue = @(progressLayer.strokeStart);
strokeStartAnimation.toValue = @(strokeEndFinal);
strokeStartAnimation.autoreverses = NO;
strokeStartAnimation.repeatCount = 0.f;
// strokeStartAnimation.fillMode = kCAFillModeForwards; // REMOVED
// strokeStartAnimation.removedOnCompletion = NO; // REMOVED
progressLayer.strokeStart = strokeEndFinal; // ADDED
[progressLayer addAnimation:strokeStartAnimation forKey:@"strokeStartAnimation"];
}
}
CABasicAnimation *backgroundLayerAnimation = [CABasicAnimation animationWithKeyPath:@"strokeStart"];
backgroundLayerAnimation.duration = duration;
backgroundLayerAnimation.fromValue = @(self.backgroundLayer.strokeStart);
backgroundLayerAnimation.toValue = @(1.f);
backgroundLayerAnimation.autoreverses = NO;
backgroundLayerAnimation.repeatCount = 0.f;
// backgroundLayerAnimation.fillMode = kCAFillModeForwards; // REMOVED
// backgroundLayerAnimation.removedOnCompletion = NO; // REMOVED
// backgroundLayerAnimation.delegate = self; // REMOVED (optionally, after an update)
self.backgroundLayer.strokeStart = 1.0; // ADDED
[self.backgroundLayer addAnimation:backgroundLayerAnimation forKey:@"strokeStartAnimation"];
}
I don't think that is any more complex than the original code. And to the point about "copying the state of the layer": if the animation has already completed then it will be at it's correct state and we don't need to do any work at all. If that has not happened, the animation is still running which means that the current values can be read from the presentation layer, even though the animation wasn't configured to keep the animation around. Doing the animations like this, in my opinion, is closer to what regular usage of Core Animation looks like (add the explicit animation and update the model).
Thx for this. Doing research into CADisplayLink. It will eventually be posted on www.stylekit.org