From cf91e0cd2484488611657d5ca01ab28ec2cbf606 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Wed, 28 May 2025 03:19:43 -0700 Subject: [PATCH] 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 --- .../__tests__/ScrollView-viewCulling-itest.js | 116 ++++++++++++++++++ .../renderer/mounting/Differentiator.cpp | 20 ++- 2 files changed, 132 insertions(+), 4 deletions(-) diff --git a/packages/react-native/Libraries/Components/ScrollView/__tests__/ScrollView-viewCulling-itest.js b/packages/react-native/Libraries/Components/ScrollView/__tests__/ScrollView-viewCulling-itest.js index 6bfbe37cc2527c..640dd5eee11483 100644 --- a/packages/react-native/Libraries/Components/ScrollView/__tests__/ScrollView-viewCulling-itest.js +++ b/packages/react-native/Libraries/Components/ScrollView/__tests__/ScrollView-viewCulling-itest.js @@ -1244,6 +1244,122 @@ describe('reparenting', () => { ]); }); + test('flattening grandparent ', () => { + const root = Fantom.createRoot({viewportWidth: 100, viewportHeight: 100}); + + Fantom.runTask(() => { + root.render( + + + + + + + , + ); + }); + + 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( + + + + + + + , + ); + }); + + 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( + + + + + + + , + ); + }); + + expect(root.takeMountingManagerLogs()).toContain( + 'Insert {type: "View", parentNativeID: "parent", index: 0, nativeID: "child"}', + ); + + // Unflatten grandparent by setting opacity to 0. + Fantom.runTask(() => { + root.render( + + + + + + + , + ); + }); + + 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(); diff --git a/packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp b/packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp index acd3c7073c3d19..6aa22194ad166c 100644 --- a/packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp +++ b/packages/react-native/ReactCommon/react/renderer/mounting/Differentiator.cpp @@ -165,6 +165,7 @@ static void calculateShadowViewMutationsFlattener( Tag parentTagForUpdate, TinyMap* parentSubVisitedOtherNewNodes, TinyMap* 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* parentSubVisitedOtherNewNodes, TinyMap* parentSubVisitedOtherOldNodes, + const CullingContext& cullingContextForUnvisitedOtherNodes, const CullingContext& cullingContext) { // Step 1: iterate through entire tree std::vector 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