Skip to content

Commit

Permalink
Disable calculation of TransformedFrames in Layoutable ShadowNodex
Browse files Browse the repository at this point in the history
Summary:
Calculation of TransformedFrames was a method introduced by D37994809 (facebook@64528e5) 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 (facebook@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
  • Loading branch information
mdvacca authored and roryabraham committed Aug 17, 2022
1 parent 39ca967 commit 451d13a
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,10 @@ void Binding::installFabricUIManager(
"MapBufferSerializationEnabled",
getFeatureFlagValue("mapBufferSerializationEnabled"));

contextContainer->insert(
"CalculateTransformedFramesEnabled",
getFeatureFlagValue("calculateTransformedFramesEnabled"));

disablePreallocateViews_ = reactNativeConfig_->getBool(
"react_fabric:disabled_view_preallocation_android");

Expand Down
25 changes: 22 additions & 3 deletions ReactCommon/react/renderer/core/LayoutableShadowNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,21 @@ LayoutMetrics LayoutableShadowNode::computeRelativeLayoutMetrics(
}

// ------------------------------

auto transformedFrames = calculateTransformedFrames(shadowNodeList, policy);
// TODO: T127619309 remove after validating that T127619309 is fixed
auto optionalCalculateTransformedFrames =
descendantNode->getContextContainer()
? descendantNode->getContextContainer()->find<bool>(
"CalculateTransformedFramesEnabled")
: std::optional<bool>(false);

bool shouldCalculateTransformedFrames =
optionalCalculateTransformedFrames.has_value()
? optionalCalculateTransformedFrames.value()
: false;

auto transformedFrames = shouldCalculateTransformedFrames
? calculateTransformedFrames(shadowNodeList, policy)
: LayoutableSmallVector<Rect>();
auto layoutMetrics = descendantLayoutableNode->getLayoutMetrics();
auto &resultFrame = layoutMetrics.frame;
resultFrame.origin = {0, 0};
Expand All @@ -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};
Expand All @@ -185,6 +200,10 @@ LayoutMetrics LayoutableShadowNode::computeRelativeLayoutMetrics(
}

resultFrame.origin += currentFrame.origin;
if (!shouldCalculateTransformedFrames && i != 0 &&
policy.includeTransform) {
resultFrame.origin += currentShadowNode->getContentOriginOffset();
}
}

// ------------------------------
Expand Down
4 changes: 4 additions & 0 deletions ReactCommon/react/renderer/core/ShadowNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions ReactCommon/react/renderer/core/ShadowNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
28 changes: 28 additions & 0 deletions ReactCommon/react/renderer/core/tests/LayoutableShadowNodeTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@

using namespace facebook::react;

// TODO: T127619309 re-enable CalculateTransformedFrames
bool enableCalculateTransformedFrames = false;

/*
* ┌────────┐
* │<View> │
Expand Down Expand Up @@ -450,6 +453,11 @@ TEST(LayoutableShadowNodeTest, invertedVerticalView) {
auto builder = simpleComponentBuilder();
auto childShadowNode1 = std::shared_ptr<ViewShadowNode>{};
auto childShadowNode2 = std::shared_ptr<ViewShadowNode>{};

if (!enableCalculateTransformedFrames) {
return;
}

// clang-format off
auto element =
Element<ViewShadowNode>()
Expand Down Expand Up @@ -525,6 +533,11 @@ TEST(LayoutableShadowNodeTest, nestedInvertedVerticalView) {
auto builder = simpleComponentBuilder();
auto childShadowNode1 = std::shared_ptr<ViewShadowNode>{};
auto childShadowNode2 = std::shared_ptr<ViewShadowNode>{};

if (!enableCalculateTransformedFrames) {
return;
}

// clang-format off
auto element =
Element<ViewShadowNode>()
Expand Down Expand Up @@ -601,6 +614,11 @@ TEST(LayoutableShadowNodeTest, invertedHorizontalView) {
auto builder = simpleComponentBuilder();
auto childShadowNode1 = std::shared_ptr<ViewShadowNode>{};
auto childShadowNode2 = std::shared_ptr<ViewShadowNode>{};

if (!enableCalculateTransformedFrames) {
return;
}

// clang-format off
auto element =
Element<ViewShadowNode>()
Expand Down Expand Up @@ -672,6 +690,11 @@ TEST(LayoutableShadowNodeTest, nestedInvertedHorizontalView) {
auto builder = simpleComponentBuilder();
auto childShadowNode1 = std::shared_ptr<ViewShadowNode>{};
auto childShadowNode2 = std::shared_ptr<ViewShadowNode>{};

if (!enableCalculateTransformedFrames) {
return;
}

// clang-format off
auto element =
Element<ViewShadowNode>()
Expand Down Expand Up @@ -738,6 +761,11 @@ TEST(LayoutableShadowNodeTest, inversedContentOriginOffset) {
auto builder = simpleComponentBuilder();

auto childShadowNode = std::shared_ptr<ViewShadowNode>{};

if (!enableCalculateTransformedFrames) {
return;
}

// clang-format off
auto element =
Element<ScrollViewShadowNode>()
Expand Down

0 comments on commit 451d13a

Please sign in to comment.