LayoutAnimations: ensure that all mutations retain the correct "old" node

Summary:
The current implementation of LayoutAnimations assumed that the "previous/old" ShadowView passed into the diff mutation didn't matter except for purposes of diffing.

As it turns out, iOS components could possibly use the "old" version of props, state, etc - so we should try to keep track of the current value in the tree as much as possible.

This diff accomplishes that by keeping track of the "previous" view, which the AnimationDriver will update over time. This also allows us to simplify logic around conflicting animations.

I'm also adding a few additional asserts to assist in debugging.

This doesn't totally eliminate all asserts hit on iOS, yet, but it does reduce the number of times the asserts are hit in StubViewTree.

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D25048644

fbshipit-source-id: d00aeece5af04624d8193063be453c7ce4a6e565
This commit is contained in:
Joshua Gross
2020-11-19 14:50:25 -08:00
committed by Facebook GitHub Bot
parent 6d2a527984
commit 5bd5fcdd38
5 changed files with 84 additions and 171 deletions
@@ -29,96 +29,6 @@
namespace facebook {
namespace react {
static double
getProgressFromValues(double start, double end, double currentValue) {
auto opacityMinmax = std::minmax({start, end});
auto min = opacityMinmax.first;
auto max = opacityMinmax.second;
return (
currentValue < min
? 0
: (currentValue > max ? 0 : ((max - currentValue) / (max - min))));
}
/**
* Given an animation and a ShadowView with properties set on it, detect how
* far through the animation the ShadowView has progressed.
*
* @param mutationsList
* @param now
*/
double LayoutAnimationDriver::getProgressThroughAnimation(
AnimationKeyFrame const &keyFrame,
LayoutAnimation const *layoutAnimation,
ShadowView const &animationStateView) const {
auto layoutAnimationConfig = layoutAnimation->layoutAnimationConfig;
auto const mutationConfig =
*(keyFrame.type == AnimationConfigurationType::Delete
? layoutAnimationConfig.deleteConfig
: (keyFrame.type == AnimationConfigurationType::Create
? layoutAnimationConfig.createConfig
: layoutAnimationConfig.updateConfig));
auto initialProps = keyFrame.viewStart.props;
auto finalProps = keyFrame.viewEnd.props;
if (mutationConfig.animationProperty == AnimationProperty::Opacity) {
// Detect progress through opacity animation.
const auto &oldViewProps =
dynamic_cast<const ViewProps *>(initialProps.get());
const auto &newViewProps =
dynamic_cast<const ViewProps *>(finalProps.get());
const auto &animationStateViewProps =
dynamic_cast<const ViewProps *>(animationStateView.props.get());
if (oldViewProps != nullptr && newViewProps != nullptr &&
animationStateViewProps != nullptr) {
return getProgressFromValues(
oldViewProps->opacity,
newViewProps->opacity,
animationStateViewProps->opacity);
}
} else if (
mutationConfig.animationProperty != AnimationProperty::NotApplicable) {
// Detect progress through layout animation.
LayoutMetrics const &finalLayoutMetrics = keyFrame.viewEnd.layoutMetrics;
LayoutMetrics const &baselineLayoutMetrics =
keyFrame.viewStart.layoutMetrics;
LayoutMetrics const &animationStateLayoutMetrics =
animationStateView.layoutMetrics;
if (baselineLayoutMetrics.frame.size.height !=
finalLayoutMetrics.frame.size.height) {
return getProgressFromValues(
baselineLayoutMetrics.frame.size.height,
finalLayoutMetrics.frame.size.height,
animationStateLayoutMetrics.frame.size.height);
}
if (baselineLayoutMetrics.frame.size.width !=
finalLayoutMetrics.frame.size.width) {
return getProgressFromValues(
baselineLayoutMetrics.frame.size.width,
finalLayoutMetrics.frame.size.width,
animationStateLayoutMetrics.frame.size.width);
}
if (baselineLayoutMetrics.frame.origin.x !=
finalLayoutMetrics.frame.origin.x) {
return getProgressFromValues(
baselineLayoutMetrics.frame.origin.x,
finalLayoutMetrics.frame.origin.x,
animationStateLayoutMetrics.frame.origin.x);
}
if (baselineLayoutMetrics.frame.origin.y !=
finalLayoutMetrics.frame.origin.y) {
return getProgressFromValues(
baselineLayoutMetrics.frame.origin.y,
finalLayoutMetrics.frame.origin.y,
animationStateLayoutMetrics.frame.origin.y);
}
}
return 0;
}
void LayoutAnimationDriver::animationMutationsForFrame(
SurfaceId surfaceId,
ShadowViewMutation::List &mutationsList,
@@ -132,7 +42,7 @@ void LayoutAnimationDriver::animationMutationsForFrame(
}
int incompleteAnimations = 0;
for (const auto &keyframe : animation.keyFrames) {
for (auto &keyframe : animation.keyFrames) {
if (keyframe.type == AnimationConfigurationType::Noop) {
continue;
}
@@ -164,10 +74,19 @@ void LayoutAnimationDriver::animationMutationsForFrame(
// Create the mutation instruction
auto updateMutation = ShadowViewMutation::UpdateMutation(
keyframe.parentView, baselineShadowView, mutatedShadowView, -1);
keyframe.parentView, keyframe.viewPrev, mutatedShadowView, -1);
// All generated Update mutations must have an "old" and "new"
// ShadowView. Checking for nonzero tag doesn't guarantee that the views
// are valid/correct, just that something is there.
assert(updateMutation.oldChildShadowView.tag != 0);
assert(updateMutation.newChildShadowView.tag != 0);
mutationsList.push_back(updateMutation);
PrintMutationInstruction("Animation Progress:", updateMutation);
keyframe.viewPrev = mutatedShadowView;
if (animationTimeProgressLinear < 1) {
incompleteAnimations++;
}
@@ -200,23 +119,31 @@ void LayoutAnimationDriver::animationMutationsForFrame(
// Copy so that if something else mutates the inflight animations, it
// won't change this mutation after this point.
mutationsList.push_back(
auto mutation =
ShadowViewMutation{finalMutationForKeyFrame.type,
finalMutationForKeyFrame.parentShadowView,
finalMutationForKeyFrame.oldChildShadowView,
keyframe.viewPrev,
finalMutationForKeyFrame.newChildShadowView,
finalMutationForKeyFrame.index});
finalMutationForKeyFrame.index};
assert(mutation.oldChildShadowView.tag != 0);
assert(
mutation.newChildShadowView.tag != 0 ||
finalMutationForKeyFrame.type == ShadowViewMutation::Remove ||
finalMutationForKeyFrame.type == ShadowViewMutation::Delete);
mutationsList.push_back(mutation);
} else {
// Issue a final UPDATE so that the final props object sent to the
// mounting layer is the same as the one on the ShadowTree. This is
// mostly to make the MountingCoordinator StubViewTree assertions
// pass.
mutationsList.push_back(
ShadowViewMutation{ShadowViewMutation::Type::Update,
keyframe.parentView,
{},
keyframe.viewEnd,
-1});
auto mutation = ShadowViewMutation{ShadowViewMutation::Type::Update,
keyframe.parentView,
keyframe.viewPrev,
keyframe.viewEnd,
-1};
assert(mutation.oldChildShadowView.tag != 0);
assert(mutation.newChildShadowView.tag != 0);
mutationsList.push_back(mutation);
}
}
@@ -35,10 +35,6 @@ class LayoutAnimationDriver : public LayoutAnimationKeyFrameManager {
SurfaceId surfaceId,
ShadowViewMutation::List &mutationsList,
uint64_t now) const override;
virtual double getProgressThroughAnimation(
AnimationKeyFrame const &keyFrame,
LayoutAnimation const *layoutAnimation,
ShadowView const &animationStateView) const override;
};
} // namespace react
@@ -667,9 +667,7 @@ LayoutAnimationKeyFrameManager::getAndEraseConflictingAnimations(
bool conflicting = animatedKeyFrame.tag == baselineShadowView.tag ||
((mutation.type == ShadowViewMutation::Type::Delete ||
mutation.type == ShadowViewMutation::Type::Create) &&
animatedKeyFrame.parentView.tag == baselineShadowView.tag) /* ||
finalMutationTag == baselineShadowView.tag*/
;
animatedKeyFrame.parentView.tag == baselineShadowView.tag);
// Conflicting animation detected: if we're mutating a tag under
// animation, or deleting the parent of a tag under animation, or
@@ -888,7 +886,8 @@ LayoutAnimationKeyFrameManager::pullTransaction(
for (auto const &mutation : mutations) {
ShadowView baselineShadowView =
(mutation.type == ShadowViewMutation::Type::Delete ||
mutation.type == ShadowViewMutation::Type::Remove
mutation.type == ShadowViewMutation::Type::Remove ||
mutation.type == ShadowViewMutation::Type::Update
? mutation.oldChildShadowView
: mutation.newChildShadowView);
bool haveComponentDescriptor =
@@ -949,7 +948,9 @@ LayoutAnimationKeyFrameManager::pullTransaction(
mutation.newChildShadowView.tag,
mutation.parentShadowView,
movedIt->second.oldChildShadowView,
mutation.newChildShadowView});
mutation.newChildShadowView,
movedIt->second.oldChildShadowView,
0});
}
}
@@ -1017,6 +1018,7 @@ LayoutAnimationKeyFrameManager::pullTransaction(
parent,
viewStart,
viewFinal,
baselineShadowView,
0};
} else if (mutation.type == ShadowViewMutation::Type::Delete) {
if (mutationConfig->animationProperty ==
@@ -1058,6 +1060,7 @@ LayoutAnimationKeyFrameManager::pullTransaction(
parent,
viewStart,
viewFinal,
baselineShadowView,
0};
} else if (mutation.type == ShadowViewMutation::Type::Update) {
viewFinal = ShadowView(mutation.newChildShadowView);
@@ -1069,6 +1072,7 @@ LayoutAnimationKeyFrameManager::pullTransaction(
parent,
viewStart,
viewFinal,
baselineShadowView,
0};
} else {
// This should just be "Remove" instructions that are not animated
@@ -1092,8 +1096,9 @@ LayoutAnimationKeyFrameManager::pullTransaction(
AnimationConfigurationType::Noop,
tag,
parent,
{},
{},
mutation.oldChildShadowView,
mutation.oldChildShadowView,
mutation.oldChildShadowView,
0};
} else {
PrintMutationInstruction(
@@ -1112,30 +1117,13 @@ LayoutAnimationKeyFrameManager::pullTransaction(
// We've found a conflict.
if (conflictingMutationBaselineShadowView.tag == tag) {
// What's the progress of this ongoing animation?
double conflictingAnimationProgress =
calculateAnimationProgress(
now,
*std::get<2>(conflictingKeyframeTuple),
std::get<1>(conflictingKeyframeTuple))
.first;
// Get a baseline ShadowView at the current progress of the
// inflight animation. TODO: handle multiple properties being
// animated separately?
auto interpolatedInflightShadowView =
createInterpolatedShadowView(
conflictingAnimationProgress,
conflictingKeyFrame.viewStart,
conflictingKeyFrame.viewEnd);
// Pick a Prop or layout property, depending on the current
// animation configuration. Figure out how much progress we've
// already made in the current animation, and start the animation
// from this point.
keyFrame.viewStart = interpolatedInflightShadowView;
keyFrame.initialProgress = getProgressThroughAnimation(
keyFrame, &animation, interpolatedInflightShadowView);
keyFrame.viewStart = conflictingKeyFrame.viewPrev;
assert(keyFrame.viewStart.tag != 0);
keyFrame.initialProgress = 0;
// We're guaranteed that a tag only has one animation associated
// with it, so we can break here. If we support multiple
@@ -1145,6 +1133,9 @@ LayoutAnimationKeyFrameManager::pullTransaction(
}
}
assert(keyFrame.viewStart.tag != 0);
assert(keyFrame.viewEnd.tag != 0);
assert(keyFrame.viewPrev.tag != 0);
keyFramesToAnimate.push_back(keyFrame);
}
@@ -1185,36 +1176,32 @@ LayoutAnimationKeyFrameManager::pullTransaction(
for (auto const &conflictingKeyframeTuple : conflictingAnimations) {
auto &keyFrame = std::get<0>(conflictingKeyframeTuple);
if (keyFrame.finalMutationForKeyFrame.hasValue()) {
auto &mutation = *keyFrame.finalMutationForKeyFrame;
if (mutation.type == ShadowViewMutation::Type::Update) {
auto mutationInstruction = ShadowViewMutation::UpdateMutation(
mutation.parentShadowView,
{},
mutation.newChildShadowView,
mutation.index);
PrintMutationInstruction(
"Queueing up final mutation instruction - update:",
mutationInstruction);
finalConflictingMutations.push_back(std::move(mutationInstruction));
} else {
PrintMutationInstruction(
"Queueing up final mutation instruction - non-update",
mutation);
finalConflictingMutations.push_back(mutation);
}
auto &finalMutation = *keyFrame.finalMutationForKeyFrame;
auto mutationInstruction =
ShadowViewMutation{finalMutation.type,
finalMutation.parentShadowView,
keyFrame.viewPrev,
finalMutation.newChildShadowView,
finalMutation.index};
PrintMutationInstruction(
"Queueing up final mutation instruction - update:",
mutationInstruction);
assert(mutationInstruction.oldChildShadowView.tag != 0);
assert(
mutationInstruction.newChildShadowView.tag != 0 ||
mutationInstruction.type == ShadowViewMutation::Delete ||
mutationInstruction.type == ShadowViewMutation::Remove);
finalConflictingMutations.push_back(mutationInstruction);
} else {
// If there's no final mutation associated, create a mutation that
// corresponds to the animation being 100% complete. This is important
// for, for example, INSERT mutations being animated from opacity 0
// to 1. If the animation is interrupted we must force the View to be
// at opacity 1.
auto mutatedShadowView = createInterpolatedShadowView(
1, keyFrame.viewStart, keyFrame.viewEnd);
auto generatedMutation = ShadowViewMutation::UpdateMutation(
keyFrame.parentView,
keyFrame.viewStart,
std::move(mutatedShadowView),
-1);
keyFrame.parentView, keyFrame.viewPrev, keyFrame.viewEnd, -1);
assert(generatedMutation.oldChildShadowView.tag != 0);
assert(generatedMutation.newChildShadowView.tag != 0);
PrintMutationInstruction(
"Queueing up final mutation instruction - synthetic",
generatedMutation);
@@ -1361,28 +1348,30 @@ LayoutAnimationKeyFrameManager::pullTransaction(
for (auto const &conflictingKeyframeTuple : conflictingAnimations) {
auto &keyFrame = std::get<0>(conflictingKeyframeTuple);
if (keyFrame.finalMutationForKeyFrame.hasValue()) {
auto &finalMutation = (*keyFrame.finalMutationForKeyFrame);
auto mutation = ShadowViewMutation{finalMutation.type,
finalMutation.parentShadowView,
keyFrame.viewPrev,
finalMutation.newChildShadowView,
finalMutation.index};
PrintMutationInstruction(
"No Animation: Queueing final mutation instruction",
*keyFrame.finalMutationForKeyFrame);
finalMutationsForConflictingAnimations.push_back(
*keyFrame.finalMutationForKeyFrame);
"No Animation: Queueing up final conflicting mutation instruction",
mutation);
finalMutationsForConflictingAnimations.push_back(mutation);
} else {
// If there's no final mutation associated, create a mutation that
// corresponds to the animation being 100% complete. This is important
// for, for example, INSERT mutations being animated from opacity 0
// to 1. If the animation is interrupted we must force the View to be
// at opacity 1.
auto mutatedShadowView = createInterpolatedShadowView(
1, keyFrame.viewStart, keyFrame.viewEnd);
auto generatedMutation = ShadowViewMutation::UpdateMutation(
keyFrame.parentView,
keyFrame.viewStart,
std::move(mutatedShadowView),
-1);
keyFrame.parentView, keyFrame.viewPrev, keyFrame.viewEnd, -1);
assert(generatedMutation.oldChildShadowView.tag != 0);
assert(generatedMutation.newChildShadowView.tag != 0);
PrintMutationInstruction(
"Queueing up final mutation instruction - synthetic",
generatedMutation);
// Create the mutation instruction
// Create the mutation instructions
finalMutationsForConflictingAnimations.push_back(generatedMutation);
}
}
@@ -103,6 +103,9 @@ struct AnimationKeyFrame {
ShadowView viewStart;
ShadowView viewEnd;
// ShadowView representing the previous frame of the animation.
ShadowView viewPrev;
// If an animation interrupts an existing one, the starting state may actually
// be halfway through the intended transition.
double initialProgress;
@@ -252,11 +255,6 @@ class LayoutAnimationKeyFrameManager : public UIManagerAnimationDelegate,
ShadowViewMutation::List &mutationsList,
uint64_t now) const = 0;
virtual double getProgressThroughAnimation(
AnimationKeyFrame const &keyFrame,
LayoutAnimation const *layoutAnimation,
ShadowView const &animationStateView) const = 0;
SharedComponentDescriptorRegistry componentDescriptorRegistry_;
mutable better::optional<LayoutAnimation> currentAnimation_{};
mutable std::mutex currentAnimationMutex_;
@@ -129,12 +129,15 @@ void StubViewTree::mutate(ShadowViewMutationList const &mutations) {
STUB_VIEW_LOG({
LOG(ERROR) << "StubView: Update: " << mutation.newChildShadowView.tag;
});
STUB_VIEW_ASSERT(mutation.oldChildShadowView.tag != 0);
STUB_VIEW_ASSERT(mutation.newChildShadowView.tag != 0);
STUB_VIEW_ASSERT(mutation.newChildShadowView.props);
STUB_VIEW_ASSERT(
mutation.newChildShadowView.tag == mutation.oldChildShadowView.tag);
STUB_VIEW_ASSERT(
registry.find(mutation.newChildShadowView.tag) != registry.end());
auto oldStubView = registry[mutation.newChildShadowView.tag];
STUB_VIEW_ASSERT(oldStubView->tag != 0);
STUB_VIEW_ASSERT(
(ShadowView)(*oldStubView) == mutation.oldChildShadowView);
oldStubView->update(mutation.newChildShadowView);