From facf5db260410eb635b466bb9c30158aaa4473f1 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Fri, 31 Jan 2020 21:35:41 -0800 Subject: [PATCH] Fabric: Test for reseting (and preserving) Yoga dirty flag during ShadowNode cloning Summary: This is a quite fragile and important part of the render engine. We need to dirty Yoga node only in cases where a change affects layout. In the case of over-dirtying, we can kill performance. In the case of under-dirtying, we can produce an incorrect layout. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D19596279 fbshipit-source-id: 9f2ac67c44cb35c8ba44be1025b94b7921b74e17 --- ReactCommon/fabric/components/view/BUCK | 5 +- .../fabric/components/view/tests/ViewTest.cpp | 167 +++++++++++++++++- ReactCommon/fabric/element/testUtils.h | 2 +- 3 files changed, 169 insertions(+), 5 deletions(-) diff --git a/ReactCommon/fabric/components/view/BUCK b/ReactCommon/fabric/components/view/BUCK index 8bf109b4da..86105da371 100644 --- a/ReactCommon/fabric/components/view/BUCK +++ b/ReactCommon/fabric/components/view/BUCK @@ -80,8 +80,9 @@ fb_xplat_cxx_test( contacts = ["oncall+react_native@xmail.facebook.com"], platforms = (ANDROID, APPLE, CXX), deps = [ - "fbsource//xplat/folly:molly", "fbsource//xplat/third-party/gmock:gtest", - ":view", + react_native_xplat_target("fabric/element:element"), + react_native_xplat_target("fabric/components/root:root"), + react_native_xplat_target("fabric/components/view:view"), ], ) diff --git a/ReactCommon/fabric/components/view/tests/ViewTest.cpp b/ReactCommon/fabric/components/view/tests/ViewTest.cpp index bb33c9fe10..1efa4f4478 100644 --- a/ReactCommon/fabric/components/view/tests/ViewTest.cpp +++ b/ReactCommon/fabric/components/view/tests/ViewTest.cpp @@ -5,13 +5,176 @@ * LICENSE file in the root directory of this source tree. */ +#include #include #include + +#include #include +#include +#include +#include +#include using namespace facebook::react; -TEST(ViewTest, testSomething) { - // TODO +TEST(ElementTest, testYogaDirtyFlag) { + auto builder = simpleComponentBuilder(); + + auto rootShadowNode = std::shared_ptr{}; + auto innerShadowNode = std::shared_ptr{}; + + // clang-format off + auto element = + Element() + .reference(rootShadowNode) + .tag(1) + .children({ + Element() + .tag(2), + Element() + .tag(3) + .reference(innerShadowNode) + .children({ + Element() + .tag(4) + .props([] { + /* + * Some non-default props. + */ + auto mutableViewProps = std::make_shared(); + auto &props = *mutableViewProps; + props.nativeId = "native Id"; + props.opacity = 0.5; + props.yogaStyle.alignContent() = YGAlignBaseline; + props.yogaStyle.flexDirection() = YGFlexDirectionRowReverse; + return mutableViewProps; + }), + Element() + .tag(5), + Element() + .tag(6) + }) + }); + // clang-format on + + builder.build(element); + + /* + * Yoga nodes are dirty right after creation. + */ + EXPECT_TRUE(rootShadowNode->layoutIfNeeded()); + + /* + * Yoga nodes are clean (not dirty) right after layout pass. + */ + EXPECT_FALSE(rootShadowNode->layoutIfNeeded()); + + { + /* + * Cloning props without changing them must *not* dirty Yoga nodes. + */ + auto newRootShadowNode = rootShadowNode->clone( + innerShadowNode->getFamily(), [](ShadowNode const &oldShadowNode) { + auto &componentDescriptor = oldShadowNode.getComponentDescriptor(); + auto props = componentDescriptor.cloneProps( + oldShadowNode.getProps(), RawProps()); + return oldShadowNode.clone(ShadowNodeFragment{props}); + }); + + EXPECT_FALSE(newRootShadowNode->layoutIfNeeded()); + } + + { + /* + * Changing *non-layout* sub-props must *not* dirty Yoga nodes. + */ + auto newRootShadowNode = rootShadowNode->clone( + innerShadowNode->getFamily(), [](ShadowNode const &oldShadowNode) { + auto viewProps = std::make_shared(); + auto &props = *viewProps; + + props.nativeId = "some new native Id"; + props.foregroundColor = whiteColor(); + props.backgroundColor = blackColor(); + props.opacity = props.opacity + 0.042; + props.zIndex = props.zIndex + 42; + props.shouldRasterize = !props.shouldRasterize; + props.collapsable = !props.collapsable; + + return oldShadowNode.clone(ShadowNodeFragment{viewProps}); + }); + + EXPECT_FALSE(newRootShadowNode->layoutIfNeeded()); + } + + { + /* + * Changing *layout* sub-props *must* dirty Yoga nodes. + */ + auto newRootShadowNode = rootShadowNode->clone( + innerShadowNode->getFamily(), [](ShadowNode const &oldShadowNode) { + auto viewProps = std::make_shared(); + auto &props = *viewProps; + + props.yogaStyle.alignContent() = YGAlignBaseline; + props.yogaStyle.display() = YGDisplayNone; + + return oldShadowNode.clone(ShadowNodeFragment{viewProps}); + }); + + EXPECT_TRUE(newRootShadowNode->layoutIfNeeded()); + } + + { + /* + * Removing all children *must* dirty Yoga nodes. + */ + auto newRootShadowNode = rootShadowNode->clone( + innerShadowNode->getFamily(), [](ShadowNode const &oldShadowNode) { + return oldShadowNode.clone( + {ShadowNodeFragment::propsPlaceholder(), + ShadowNode::emptySharedShadowNodeSharedList()}); + }); + + EXPECT_TRUE(newRootShadowNode->layoutIfNeeded()); + } + + { + /* + * Removing the last child *must* dirty Yoga nodes. + */ + auto newRootShadowNode = rootShadowNode->clone( + innerShadowNode->getFamily(), [](ShadowNode const &oldShadowNode) { + auto children = oldShadowNode.getChildren(); + children.pop_back(); + + std::reverse(children.begin(), children.end()); + + return oldShadowNode.clone( + {ShadowNodeFragment::propsPlaceholder(), + std::make_shared(children)}); + }); + + EXPECT_TRUE(newRootShadowNode->layoutIfNeeded()); + } + + { + /* + * Reversing a list of children *must* dirty Yoga nodes. + */ + auto newRootShadowNode = rootShadowNode->clone( + innerShadowNode->getFamily(), [](ShadowNode const &oldShadowNode) { + auto children = oldShadowNode.getChildren(); + + std::reverse(children.begin(), children.end()); + + return oldShadowNode.clone( + {ShadowNodeFragment::propsPlaceholder(), + std::make_shared(children)}); + }); + + EXPECT_TRUE(newRootShadowNode->layoutIfNeeded()); + } } diff --git a/ReactCommon/fabric/element/testUtils.h b/ReactCommon/fabric/element/testUtils.h index cb29e91afb..0d65c15cd8 100644 --- a/ReactCommon/fabric/element/testUtils.h +++ b/ReactCommon/fabric/element/testUtils.h @@ -15,7 +15,7 @@ namespace facebook { namespace react { -extern ComponentBuilder simpleComponentBuilder() { +inline ComponentBuilder simpleComponentBuilder() { ComponentDescriptorProviderRegistry componentDescriptorProviderRegistry{}; auto eventDispatcher = EventDispatcher::Shared{}; auto componentDescriptorRegistry =