From 1e4ebf25313247a2b1766faf90dd9b36005bf870 Mon Sep 17 00:00:00 2001 From: David Vacca Date: Fri, 29 Jul 2022 17:32:35 -0700 Subject: [PATCH] Disable calculation of TransformedFrames in Layoutable ShadowNodex Summary: Calculation of TransformedFrames was a method introduced by D37994809 (https://github.com/facebook/react-native/commit/64528e5faa445907b8287b412c344f30c20fca61) to fix rendering of inverted flat lists. We found that this operation is crashing in fb4a causing the UBN T127619309 This diff creates a feature flag to disable the calculation of TransformedFrames in Layoutable ShadowNodes. **The goal of this diff is to revert the behavior introduced by D37994809 (https://github.com/facebook/react-native/commit/64528e5faa445907b8287b412c344f30c20fca61)** The featureFlag is disabled in fb4a and enabled in react AR (because ReactAr apps relies on the calculation of TransformedFrames and these apps are not affected) The root cause of the bug will be fixed by D38280674 (which still requires more testing and investigation) changelog: [internal] internal Reviewed By: JoshuaGross Differential Revision: D38286857 fbshipit-source-id: 721cd0554ae6a6b369b3f8dbb584160a270d0f18 --- .../react/config/ReactFeatureFlags.java | 3 ++ .../com/facebook/react/fabric/jni/Binding.cpp | 4 +++ .../renderer/core/LayoutableShadowNode.cpp | 23 +++++++++++++-- .../react/renderer/core/ShadowNode.cpp | 4 +++ ReactCommon/react/renderer/core/ShadowNode.h | 5 ++++ .../core/tests/LayoutableShadowNodeTest.cpp | 28 +++++++++++++++++++ 6 files changed, 65 insertions(+), 2 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java index a8a5264042b..4be9e5bc100 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -83,6 +83,9 @@ public class ReactFeatureFlags { /** Enables or disables MapBuffer Serialization */ public static boolean mapBufferSerializationEnabled = false; + /** Enables or disables calculation of Transformed Frames */ + public static boolean calculateTransformedFramesEnabled = false; + /** Feature Flag to use overflowInset values provided by Yoga */ public static boolean useOverflowInset = false; diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp index be2a0f1e749..83f578955af 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/jni/Binding.cpp @@ -434,6 +434,10 @@ void Binding::installFabricUIManager( "MapBufferSerializationEnabled", getFeatureFlagValue("mapBufferSerializationEnabled")); + contextContainer->insert( + "CalculateTransformedFramesEnabled", + getFeatureFlagValue("calculateTransformedFramesEnabled")); + disablePreallocateViews_ = reactNativeConfig_->getBool( "react_fabric:disabled_view_preallocation_android"); diff --git a/ReactCommon/react/renderer/core/LayoutableShadowNode.cpp b/ReactCommon/react/renderer/core/LayoutableShadowNode.cpp index 7f1802906df..7f3c1f0382a 100644 --- a/ReactCommon/react/renderer/core/LayoutableShadowNode.cpp +++ b/ReactCommon/react/renderer/core/LayoutableShadowNode.cpp @@ -151,8 +151,21 @@ LayoutMetrics LayoutableShadowNode::computeRelativeLayoutMetrics( } // ------------------------------ + // TODO: T127619309 remove after validating that T127619309 is fixed + auto optionalCalculateTransformedFrames = + descendantNode->getContextContainer() + ? descendantNode->getContextContainer()->find( + "CalculateTransformedFramesEnabled") + : std::optional(false); - auto transformedFrames = calculateTransformedFrames(shadowNodeList, policy); + bool shouldCalculateTransformedFrames = + optionalCalculateTransformedFrames.has_value() + ? optionalCalculateTransformedFrames.value() + : false; + + auto transformedFrames = shouldCalculateTransformedFrames + ? calculateTransformedFrames(shadowNodeList, policy) + : LayoutableSmallVector(); auto layoutMetrics = descendantLayoutableNode->getLayoutMetrics(); auto &resultFrame = layoutMetrics.frame; resultFrame.origin = {0, 0}; @@ -168,7 +181,9 @@ LayoutMetrics LayoutableShadowNode::computeRelativeLayoutMetrics( return EmptyLayoutMetrics; } - auto currentFrame = transformedFrames[i]; + auto currentFrame = shouldCalculateTransformedFrames + ? transformedFrames[i] + : currentShadowNode->getLayoutMetrics().frame; if (i == size - 1) { // If it's the last element, its origin is irrelevant. currentFrame.origin = {0, 0}; @@ -185,6 +200,10 @@ LayoutMetrics LayoutableShadowNode::computeRelativeLayoutMetrics( } resultFrame.origin += currentFrame.origin; + if (!shouldCalculateTransformedFrames && i != 0 && + policy.includeTransform) { + resultFrame.origin += currentShadowNode->getContentOriginOffset(); + } } // ------------------------------ diff --git a/ReactCommon/react/renderer/core/ShadowNode.cpp b/ReactCommon/react/renderer/core/ShadowNode.cpp index 203d86e95b0..eaab746ac5c 100644 --- a/ReactCommon/react/renderer/core/ShadowNode.cpp +++ b/ReactCommon/react/renderer/core/ShadowNode.cpp @@ -121,6 +121,10 @@ ShadowNode::Unshared ShadowNode::clone( return family_->componentDescriptor_.cloneShadowNode(*this, fragment); } +ContextContainer::Shared ShadowNode::getContextContainer() const { + return family_->componentDescriptor_.getContextContainer(); +} + #pragma mark - Getters ComponentName ShadowNode::getComponentName() const { diff --git a/ReactCommon/react/renderer/core/ShadowNode.h b/ReactCommon/react/renderer/core/ShadowNode.h index 8264065ffb2..a190a0906c6 100644 --- a/ReactCommon/react/renderer/core/ShadowNode.h +++ b/ReactCommon/react/renderer/core/ShadowNode.h @@ -129,6 +129,11 @@ class ShadowNode : public Sealable, public DebugStringConvertible { */ const ComponentDescriptor &getComponentDescriptor() const; + /* + * Returns the `ContextContainer` used by this ShadowNode. + */ + ContextContainer::Shared getContextContainer() const; + /* * Returns a state associated with the particular node. */ diff --git a/ReactCommon/react/renderer/core/tests/LayoutableShadowNodeTest.cpp b/ReactCommon/react/renderer/core/tests/LayoutableShadowNodeTest.cpp index 8fde7fd7122..f8b0789e48b 100644 --- a/ReactCommon/react/renderer/core/tests/LayoutableShadowNodeTest.cpp +++ b/ReactCommon/react/renderer/core/tests/LayoutableShadowNodeTest.cpp @@ -13,6 +13,9 @@ using namespace facebook::react; +// TODO: T127619309 re-enable CalculateTransformedFrames +bool enableCalculateTransformedFrames = false; + /* * ┌────────┐ * │ │ @@ -450,6 +453,11 @@ TEST(LayoutableShadowNodeTest, invertedVerticalView) { auto builder = simpleComponentBuilder(); auto childShadowNode1 = std::shared_ptr{}; auto childShadowNode2 = std::shared_ptr{}; + + if (!enableCalculateTransformedFrames) { + return; + } + // clang-format off auto element = Element() @@ -525,6 +533,11 @@ TEST(LayoutableShadowNodeTest, nestedInvertedVerticalView) { auto builder = simpleComponentBuilder(); auto childShadowNode1 = std::shared_ptr{}; auto childShadowNode2 = std::shared_ptr{}; + + if (!enableCalculateTransformedFrames) { + return; + } + // clang-format off auto element = Element() @@ -601,6 +614,11 @@ TEST(LayoutableShadowNodeTest, invertedHorizontalView) { auto builder = simpleComponentBuilder(); auto childShadowNode1 = std::shared_ptr{}; auto childShadowNode2 = std::shared_ptr{}; + + if (!enableCalculateTransformedFrames) { + return; + } + // clang-format off auto element = Element() @@ -672,6 +690,11 @@ TEST(LayoutableShadowNodeTest, nestedInvertedHorizontalView) { auto builder = simpleComponentBuilder(); auto childShadowNode1 = std::shared_ptr{}; auto childShadowNode2 = std::shared_ptr{}; + + if (!enableCalculateTransformedFrames) { + return; + } + // clang-format off auto element = Element() @@ -738,6 +761,11 @@ TEST(LayoutableShadowNodeTest, inversedContentOriginOffset) { auto builder = simpleComponentBuilder(); auto childShadowNode = std::shared_ptr{}; + + if (!enableCalculateTransformedFrames) { + return; + } + // clang-format off auto element = Element()