Skip to content

Commit

Permalink
Fix handling of layout animations coinciding with view flattening (#6460
Browse files Browse the repository at this point in the history
)

## Summary

This PR fixes the way a reparenting of views is handled in Layout
Animations. It can so happen that a view is being removed and in the
same transaction its parent is being moved (due to view flattening). The
current implementation in this scenario would move the parent correctly
and remove the `MutationNode` from our registry. However, the pointer to
this `MutationNode` was still stored as a parent by the child node. This
would lead to us trying to remove the previously moved node when the
child animation ended. This would lead to a crash, since we tried to
remove the view from its old parent.

## Test plan

Check the `[LA] View flattening` example, and other examples for
regressions.
  • Loading branch information
bartlomiejbloniarz authored Sep 16, 2024
1 parent a3de2b2 commit 0744e73
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 6 deletions.
64 changes: 64 additions & 0 deletions apps/common-app/src/examples/LayoutAnimations/ViewFlattening.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { StyleSheet, View, Button } from 'react-native';
import Animated, { FadeOut } from 'react-native-reanimated';

import React from 'react';

export default function ViewFlatteningExample() {
const [visible, setVisible] = React.useState(true);

return (
<View style={styles.container}>
<Button title="Toggle" onPress={() => setVisible(!visible)} />
<View style={styles.purpleBox} collapsable={!visible}>
<View style={styles.redBox} collapsable={visible}>
<View style={styles.greenBox} collapsable={false}>
{visible && (
<Animated.View
style={styles.blueBox}
exiting={FadeOut.duration(2000)}
/>
)}
</View>
</View>
<View style={styles.redBox} collapsable={!visible}>
<View style={styles.greenBox} collapsable={false}>
{visible && (
<Animated.View
style={styles.blueBox}
exiting={FadeOut.duration(2000)}
/>
)}
</View>
</View>
</View>
</View>
);
}

const styles = StyleSheet.create({
container: {
flex: 1,
alignItems: 'center',
justifyContent: 'center',
},
purpleBox: {
width: 200,
height: 200,
backgroundColor: 'purple',
},
redBox: {
width: 100,
height: 100,
backgroundColor: 'red',
},
greenBox: {
width: 50,
height: 50,
backgroundColor: 'green',
},
blueBox: {
width: 25,
height: 25,
backgroundColor: 'blue',
},
});
5 changes: 5 additions & 0 deletions apps/common-app/src/examples/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ import TabNavigatorExample from './SharedElementTransitions/TabNavigatorExample'
import StrictDOMExample from './StrictDOMExample';
import BottomTabsExample from './LayoutAnimations/BottomTabs';
import ListItemLayoutAnimation from './LayoutAnimations/ListItemLayoutAnimation';
import ViewFlatteningExample from './LayoutAnimations/ViewFlattening';
import InvalidValueAccessExample from './InvalidValueAccessExample';

interface Example {
Expand Down Expand Up @@ -717,6 +718,10 @@ export const EXAMPLES: Record<string, Example> = {
title: '[LA] Bottom Tabs',
screen: BottomTabsExample,
},
ViewFlattening: {
title: '[LA] View Flattening',
screen: ViewFlatteningExample,
},

// Shared Element Transitions

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ std::optional<MountingTransaction> LayoutAnimationsProxy::pullTransaction(
const TransactionTelemetry &telemetry,
ShadowViewMutationList mutations) const {
#ifdef LAYOUT_ANIMATIONS_LOGS
LOG(INFO) << "\npullTransaction " << std::this_thread::get_id() << " "
LOG(INFO) << std::endl;
LOG(INFO) << "pullTransaction " << std::this_thread::get_id() << " "
<< surfaceId << std::endl;
#endif
auto lock = std::unique_lock<std::recursive_mutex>(mutex);
Expand Down Expand Up @@ -240,15 +241,15 @@ void LayoutAnimationsProxy::handleRemovals(
if (!startAnimationsRecursively(
node, true, true, false, filteredMutations)) {
filteredMutations.push_back(node->mutation);
nodeForTag_.erase(node->tag);
node->unflattenedParent->removeChildFromUnflattenedTree(node); //???
#ifdef LAYOUT_ANIMATIONS_LOGS
LOG(INFO) << "delete " << node->tag << std::endl;
#endif
if (node->state != MOVED) {
maybeCancelAnimation(node->tag);
filteredMutations.push_back(ShadowViewMutation::DeleteMutation(
node->mutation.oldChildShadowView));
nodeForTag_.erase(node->tag);
#ifdef LAYOUT_ANIMATIONS_LOGS
LOG(INFO) << "delete " << node->tag << std::endl;
#endif
}
}
}
Expand Down Expand Up @@ -486,7 +487,7 @@ bool LayoutAnimationsProxy::startAnimationsRecursively(
hasAnimatedChildren = true;
} else if (subNode->state == MOVED) {
mutations.push_back(subNode->mutation);
nodeForTag_.erase(subNode->tag);
toBeRemoved.push_back(subNode);
} else if (shouldRemoveSubviewsWithoutAnimations) {
maybeCancelAnimation(subNode->tag);
mutations.push_back(subNode->mutation);
Expand All @@ -508,6 +509,14 @@ bool LayoutAnimationsProxy::startAnimationsRecursively(
}

if (node->state == MOVED) {
auto replacement = std::make_shared<Node>(*node);
for (auto subNode : node->children) {
subNode->parent = replacement;
}
for (auto subNode : node->unflattenedChildren) {
subNode->unflattenedParent = replacement;
}
nodeForTag_[replacement->tag] = replacement;
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ struct Node {
: children(std::move(node.children)),
unflattenedChildren(std::move(node.unflattenedChildren)),
tag(node.tag) {}
Node(Node &node)
: children(node.children),
unflattenedChildren(node.unflattenedChildren),
tag(node.tag) {}
virtual ~Node() = default;
};

Expand Down

0 comments on commit 0744e73

Please sign in to comment.