mirror of
https://github.com/facebook/react-native.git
synced 2025-11-01 09:14:26 +00:00
Fix crash in view culling when view is unflattened/flattened in deep hierarchy where each node has top offset (#51639)
Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/51639 changelog: [internal] ### Fix Crash in View Culling This diff fixes a crash that occurs when a view is unflattened or flattened in a deep hierarchy where each node has a top offset. The problem is that nodes passed to *calculateShadowViewMutationsFlattener* via argument *unvisitedOtherNodes* have their positions calculated in a different coordinate space and the view culling algorithm does not have the correct data to determine visibility of a node. To fix this, we pass another argument to *calculateShadowViewMutationsFlattener* which does have original view culling context with which these nodes had their position calculated. Reviewed By: javache Differential Revision: D75455704 fbshipit-source-id: 925f14dfdc6c2b669c89e100629291921f27cd1e
This commit is contained in:
committed by
Facebook GitHub Bot
parent
a6f24a5eaa
commit
66730ae59e
Vendored
+116
@@ -1244,6 +1244,122 @@ describe('reparenting', () => {
|
||||
]);
|
||||
});
|
||||
|
||||
test('flattening grandparent ', () => {
|
||||
const root = Fantom.createRoot({viewportWidth: 100, viewportHeight: 100});
|
||||
|
||||
Fantom.runTask(() => {
|
||||
root.render(
|
||||
<ScrollView style={{height: 100, width: 100}}>
|
||||
<View // grandparent
|
||||
style={{
|
||||
marginTop: 70,
|
||||
opacity: 0, // opacity 0 - can't be flattened
|
||||
}}>
|
||||
<View // parent
|
||||
nativeID="parent"
|
||||
style={{height: 10, width: 10, marginTop: 10}}>
|
||||
<View // child
|
||||
nativeID="child"
|
||||
style={{height: 5, width: 5, marginTop: 5}}
|
||||
/>
|
||||
</View>
|
||||
</View>
|
||||
</ScrollView>,
|
||||
);
|
||||
});
|
||||
|
||||
expect(root.takeMountingManagerLogs()).toContain(
|
||||
'Insert {type: "View", parentNativeID: "parent", index: 0, nativeID: "child"}',
|
||||
);
|
||||
|
||||
// Flatten grandparent by changing opacity to default value.
|
||||
Fantom.runTask(() => {
|
||||
root.render(
|
||||
<ScrollView style={{height: 100, width: 100}}>
|
||||
<View // grandparent
|
||||
style={{
|
||||
marginTop: 70,
|
||||
}}>
|
||||
<View // parent
|
||||
nativeID="parent"
|
||||
style={{height: 10, width: 11, marginTop: 10}}>
|
||||
<View // child
|
||||
nativeID="child"
|
||||
style={{height: 5, width: 5, marginTop: 5}}
|
||||
/>
|
||||
</View>
|
||||
</View>
|
||||
</ScrollView>,
|
||||
);
|
||||
});
|
||||
|
||||
expect(root.takeMountingManagerLogs()).toEqual([
|
||||
'Update {type: "View", nativeID: "parent"}',
|
||||
'Remove {type: "View", parentNativeID: (N/A), index: 0, nativeID: "parent"}',
|
||||
'Remove {type: "View", parentNativeID: (N/A), index: 0, nativeID: (N/A)}',
|
||||
'Delete {type: "View", nativeID: (N/A)}',
|
||||
'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: "parent"}',
|
||||
]);
|
||||
});
|
||||
|
||||
test('unflattening grandparent', () => {
|
||||
const root = Fantom.createRoot({viewportWidth: 100, viewportHeight: 100});
|
||||
|
||||
Fantom.runTask(() => {
|
||||
root.render(
|
||||
<ScrollView style={{height: 100, width: 100}}>
|
||||
<View // grandparent
|
||||
style={{
|
||||
marginTop: 70,
|
||||
}}>
|
||||
<View // parent
|
||||
nativeID={'parent'}
|
||||
style={{height: 10, width: 10, marginTop: 10}}>
|
||||
<View // child
|
||||
nativeID="child"
|
||||
style={{height: 5, width: 5, marginTop: 5}}
|
||||
/>
|
||||
</View>
|
||||
</View>
|
||||
</ScrollView>,
|
||||
);
|
||||
});
|
||||
|
||||
expect(root.takeMountingManagerLogs()).toContain(
|
||||
'Insert {type: "View", parentNativeID: "parent", index: 0, nativeID: "child"}',
|
||||
);
|
||||
|
||||
// Unflatten grandparent by setting opacity to 0.
|
||||
Fantom.runTask(() => {
|
||||
root.render(
|
||||
<ScrollView style={{height: 100, width: 100}}>
|
||||
<View // grandparent
|
||||
style={{
|
||||
marginTop: 70,
|
||||
opacity: 0, // opacity 0 - can't be flattened
|
||||
}}>
|
||||
<View // parent
|
||||
nativeID={'parent'}
|
||||
style={{height: 10, width: 11, marginTop: 10}}>
|
||||
<View // child
|
||||
nativeID="child"
|
||||
style={{height: 5, width: 5, marginTop: 5}}
|
||||
/>
|
||||
</View>
|
||||
</View>
|
||||
</ScrollView>,
|
||||
);
|
||||
});
|
||||
|
||||
expect(root.takeMountingManagerLogs()).toEqual([
|
||||
'Update {type: "View", nativeID: "parent"}',
|
||||
'Remove {type: "View", parentNativeID: (N/A), index: 0, nativeID: "parent"}',
|
||||
'Create {type: "View", nativeID: (N/A)}',
|
||||
'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: (N/A)}',
|
||||
'Insert {type: "View", parentNativeID: (N/A), index: 0, nativeID: "parent"}',
|
||||
]);
|
||||
});
|
||||
|
||||
test('parent-child flattening with child culled', () => {
|
||||
const root = Fantom.createRoot({viewportWidth: 100, viewportHeight: 100});
|
||||
const nodeRef = createRef<HostInstance>();
|
||||
|
||||
@@ -165,6 +165,7 @@ static void calculateShadowViewMutationsFlattener(
|
||||
Tag parentTagForUpdate,
|
||||
TinyMap<Tag, ShadowViewNodePair*>* parentSubVisitedOtherNewNodes,
|
||||
TinyMap<Tag, ShadowViewNodePair*>* parentSubVisitedOtherOldNodes,
|
||||
const CullingContext& cullingContextForUnvisitedOtherNodes,
|
||||
const CullingContext& cullingContext);
|
||||
|
||||
/**
|
||||
@@ -221,6 +222,7 @@ static void updateMatchedPairSubtrees(
|
||||
oldPair.shadowView.tag,
|
||||
nullptr,
|
||||
nullptr,
|
||||
oldCullingContext,
|
||||
oldCullingContextCopy);
|
||||
}
|
||||
// Unflattening
|
||||
@@ -255,6 +257,7 @@ static void updateMatchedPairSubtrees(
|
||||
parentTag,
|
||||
nullptr,
|
||||
nullptr,
|
||||
newCullingContext,
|
||||
newCullingContextCopy);
|
||||
|
||||
// If old nodes were not visited, we know that we can delete
|
||||
@@ -420,6 +423,7 @@ static void calculateShadowViewMutationsFlattener(
|
||||
Tag parentTagForUpdate,
|
||||
TinyMap<Tag, ShadowViewNodePair*>* parentSubVisitedOtherNewNodes,
|
||||
TinyMap<Tag, ShadowViewNodePair*>* parentSubVisitedOtherOldNodes,
|
||||
const CullingContext& cullingContextForUnvisitedOtherNodes,
|
||||
const CullingContext& cullingContext) {
|
||||
// Step 1: iterate through entire tree
|
||||
std::vector<ShadowViewNodePair*> treeChildren =
|
||||
@@ -615,10 +619,14 @@ static void calculateShadowViewMutationsFlattener(
|
||||
parentTagForUpdate));
|
||||
}
|
||||
|
||||
auto adjustedOldCullingContext =
|
||||
cullingContext.adjustCullingContextIfNeeded(oldTreeNodePair);
|
||||
auto adjustedNewCullingContext =
|
||||
cullingContext.adjustCullingContextIfNeeded(newTreeNodePair);
|
||||
auto adjustedOldCullingContext = reparentMode == ReparentMode::Flatten
|
||||
? cullingContext.adjustCullingContextIfNeeded(oldTreeNodePair)
|
||||
: cullingContextForUnvisitedOtherNodes.adjustCullingContextIfNeeded(
|
||||
oldTreeNodePair);
|
||||
auto adjustedNewCullingContext = reparentMode == ReparentMode::Flatten
|
||||
? cullingContextForUnvisitedOtherNodes.adjustCullingContextIfNeeded(
|
||||
newTreeNodePair)
|
||||
: cullingContext.adjustCullingContextIfNeeded(newTreeNodePair);
|
||||
|
||||
// Update children if appropriate.
|
||||
if (!oldTreeNodePair.flattened && !newTreeNodePair.flattened) {
|
||||
@@ -670,6 +678,7 @@ static void calculateShadowViewMutationsFlattener(
|
||||
: parentTag),
|
||||
subVisitedNewMap,
|
||||
subVisitedOldMap,
|
||||
cullingContext,
|
||||
cullingContext.adjustCullingContextIfNeeded(treeChildPair));
|
||||
} else {
|
||||
// Get flattened nodes from either new or old tree
|
||||
@@ -720,6 +729,7 @@ static void calculateShadowViewMutationsFlattener(
|
||||
fixedParentTagForUpdate,
|
||||
subVisitedNewMap,
|
||||
subVisitedOldMap,
|
||||
adjustedNewCullingContext,
|
||||
adjustedNewCullingContext);
|
||||
} else {
|
||||
// Flatten parent, unflatten child
|
||||
@@ -740,6 +750,8 @@ static void calculateShadowViewMutationsFlattener(
|
||||
/* parentTagForUpdate */ fixedParentTagForUpdate,
|
||||
/* parentSubVisitedOtherNewNodes */ subVisitedNewMap,
|
||||
/* parentSubVisitedOtherOldNodes */ subVisitedOldMap,
|
||||
/* cullingContextForUnvisitedOtherNodes */
|
||||
adjustedOldCullingContext,
|
||||
/* cullingContext */ adjustedOldCullingContext);
|
||||
|
||||
// If old nodes were not visited, we know that we can delete them
|
||||
|
||||
Reference in New Issue
Block a user