From 0e5d54a8ee8f43cf39c3ee9b47acdcb933a762ab Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Mon, 24 Apr 2023 18:48:48 -0700 Subject: [PATCH] Cleanup YGNode for explicit per-node config Summary: Cleans up some of the changes to UseWebDefaults that were made in the interest of moving it outside of YGConfig. It still exists in YGConfig, but also exists on the node. We also assert on null config, or when someone tries to change UseWebDefaults after creating a node (since right now YGStyle does not know the difference between unset vs set explicitly to what would normally be default). Removes a peculiar constructor which was added to avoid config setting. Reviewed By: rshest Differential Revision: D45133644 fbshipit-source-id: 2b5e2baeb826653133df9b1175cf5c194e342e3e --- .../view/YogaLayoutableShadowNode.cpp | 7 ++- .../ReactCommon/yoga/yoga/YGNode.cpp | 49 +++++++++++++------ .../ReactCommon/yoga/yoga/YGNode.h | 19 +++---- 3 files changed, 42 insertions(+), 33 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp index 8e20136627a6a1..85b8fa8a1039f6 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp @@ -103,10 +103,8 @@ YogaLayoutableShadowNode::YogaLayoutableShadowNode( ShadowNodeFragment const &fragment) : LayoutableShadowNode(sourceShadowNode, fragment), yogaConfig_(FabricDefaultYogaLog), - yogaNode_( - static_cast(sourceShadowNode) - .yogaNode_, - &initializeYogaConfig(yogaConfig_)) { + yogaNode_(static_cast(sourceShadowNode) + .yogaNode_) { // Note, cloned `YGNode` instance (copied using copy-constructor) inherits // dirty flag, measure function, and other properties being set originally in // the `YogaLayoutableShadowNode` constructor above. @@ -124,6 +122,7 @@ YogaLayoutableShadowNode::YogaLayoutableShadowNode( yogaNode_.setContext(this); yogaNode_.setOwner(nullptr); + yogaNode_.setConfig(&initializeYogaConfig(yogaConfig_)); updateYogaChildrenOwnersIfNeeded(); // This is the only legit place where we can dirty cloned Yoga node. diff --git a/packages/react-native/ReactCommon/yoga/yoga/YGNode.cpp b/packages/react-native/ReactCommon/yoga/yoga/YGNode.cpp index 0bddb89b6da372..90e13fadceb0d0 100644 --- a/packages/react-native/ReactCommon/yoga/yoga/YGNode.cpp +++ b/packages/react-native/ReactCommon/yoga/yoga/YGNode.cpp @@ -13,6 +13,27 @@ using namespace facebook; using facebook::yoga::detail::CompactValue; +YGNode::YGNode(const YGConfigRef config) : config_{config} { + YGAssert( + config != nullptr, "Attempting to construct YGNode with null config"); + + flags_.hasNewLayout = true; + if (config->useWebDefaults) { + useWebDefaults(); + } +}; + +YGNode::YGNode(const YGNode& node, YGConfigRef config) : YGNode{node} { + YGAssert( + config != nullptr, "Attempting to construct YGNode with null config"); + + config_ = config; + flags_.hasNewLayout = true; + if (config->useWebDefaults) { + useWebDefaults(); + } +} + YGNode::YGNode(YGNode&& node) { context_ = node.context_; flags_ = node.flags_; @@ -32,13 +53,6 @@ YGNode::YGNode(YGNode&& node) { } } -YGNode::YGNode(const YGNode& node, YGConfigRef config) : YGNode{node} { - config_ = config; - if (config->useWebDefaults) { - useWebDefaults(); - } -} - void YGNode::print(void* printContext) { if (print_.noContext != nullptr) { if (flags_.printUsesContext) { @@ -260,6 +274,15 @@ void YGNode::insertChild(YGNodeRef child, uint32_t index) { children_.insert(children_.begin() + index, child); } +void YGNode::setConfig(YGConfigRef config) { + YGAssert(config != nullptr, "Attempting to set a null config on a YGNode"); + YGAssertWithConfig( + config, + config->useWebDefaults == config_->useWebDefaults, + "UseWebDefaults may not be changed after constructing a YGNode"); + config_ = config; +} + void YGNode::setDirty(bool isDirty) { if (isDirty == flags_.isDirty) { return; @@ -407,7 +430,7 @@ YGValue YGNode::resolveFlexBasisPtr() const { return flexBasis; } if (!style_.flex().isUndefined() && style_.flex().unwrap() > 0.0f) { - return flags_.useWebDefaults ? YGValueAuto : YGValueZero; + return config_->useWebDefaults ? YGValueAuto : YGValueZero; } return YGValueAuto; } @@ -483,11 +506,11 @@ float YGNode::resolveFlexShrink() const { if (!style_.flexShrink().isUndefined()) { return style_.flexShrink().unwrap(); } - if (!flags_.useWebDefaults && !style_.flex().isUndefined() && + if (!config_->useWebDefaults && !style_.flex().isUndefined() && style_.flex().unwrap() < 0.0f) { return -style_.flex().unwrap(); } - return flags_.useWebDefaults ? kWebDefaultFlexShrink : kDefaultFlexShrink; + return config_->useWebDefaults ? kWebDefaultFlexShrink : kDefaultFlexShrink; } bool YGNode::isNodeFlexible() { @@ -563,11 +586,5 @@ void YGNode::reset() { YGAssertWithNode( this, owner_ == nullptr, "Cannot reset a node still attached to a owner"); - clearChildren(); - - auto webDefaults = flags_.useWebDefaults; *this = YGNode{getConfig()}; - if (webDefaults) { - useWebDefaults(); - } } diff --git a/packages/react-native/ReactCommon/yoga/yoga/YGNode.h b/packages/react-native/ReactCommon/yoga/yoga/YGNode.h index 098c1b5ea22ad1..141223426bc135 100644 --- a/packages/react-native/ReactCommon/yoga/yoga/YGNode.h +++ b/packages/react-native/ReactCommon/yoga/yoga/YGNode.h @@ -29,7 +29,6 @@ struct YGNodeFlags { bool measureUsesContext : 1; bool baselineUsesContext : 1; bool printUsesContext : 1; - bool useWebDefaults : 1; }; #pragma pack(pop) @@ -72,7 +71,6 @@ struct YOGA_EXPORT YGNode { void setBaselineFunc(decltype(baseline_)); void useWebDefaults() { - flags_.useWebDefaults = true; style_.flexDirection() = YGFlexDirectionRow; style_.alignContent() = YGAlignStretch; } @@ -87,14 +85,8 @@ struct YOGA_EXPORT YGNode { using CompactValue = facebook::yoga::detail::CompactValue; public: - YGNode() : YGNode{YGConfigGetDefault()} {} - explicit YGNode(const YGConfigRef config) : config_{config} { - flags_.hasNewLayout = true; - - if (config->useWebDefaults) { - useWebDefaults(); - } - }; + YGNode() : YGNode{YGConfigGetDefault()} { flags_.hasNewLayout = true; } + explicit YGNode(const YGConfigRef config); ~YGNode() = default; // cleanup of owner/children relationships in YGNodeFree YGNode(YGNode&&); @@ -103,8 +95,9 @@ struct YOGA_EXPORT YGNode { // Should we remove this? YGNode(const YGNode& node) = default; - // for RB fabric - YGNode(const YGNode& node, YGConfigRef config); + [[deprecated("Will be removed imminently")]] YGNode( + const YGNode& node, + YGConfigRef config); // assignment means potential leaks of existing children, or alternatively // freeing unowned memory, double free, or freeing stack memory. @@ -300,7 +293,7 @@ struct YOGA_EXPORT YGNode { // TODO: rvalue override for setChildren - void setConfig(YGConfigRef config) { config_ = config; } + void setConfig(YGConfigRef config); void setDirty(bool isDirty); void setLayoutLastOwnerDirection(YGDirection direction);