From 19a88d7f4addcd9f95fd4908d50db37b3604b5b1 Mon Sep 17 00:00:00 2001 From: David Aurelio Date: Mon, 20 May 2019 10:37:59 -0700 Subject: [PATCH] `YGNode`: Field for web defaults Summary: In order to remove the config pointer from nodes, we have to keep track of whether the node is using web defaults. This information fits into one bit that we can place in padding (i.e. no extra memory needed). This allows us to get rid of config usage withing `YGNode` with some exceptions: - `iterChildrenAfterCloningIfNeeded` -- this function will simply receive the configuration, or the cloning callback. - `setAndPropogateUseLegacyFlag` -- will be removed in D15316863 - in `YGNode::reset` -- will go away utomatically once we remove the config pointer Reviewed By: SidharthGuglani Differential Revision: D15391536 fbshipit-source-id: 0fa0d0805c6862bd741fe4a7d9b637ed534f56a4 --- .../view/yoga/YogaLayoutableShadowNode.cpp | 17 ++++++------- .../view/yoga/YogaLayoutableShadowNode.h | 12 +++++----- ReactCommon/yoga/yoga/YGNode.cpp | 24 ++++++++++++------- ReactCommon/yoga/yoga/YGNode.h | 23 +++++++++++++++--- ReactCommon/yoga/yoga/Yoga.cpp | 7 +----- 5 files changed, 49 insertions(+), 34 deletions(-) diff --git a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp index 89abffeb1bf..52c70153215 100644 --- a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp +++ b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.cpp @@ -22,21 +22,17 @@ namespace facebook { namespace react { YogaLayoutableShadowNode::YogaLayoutableShadowNode() - : yogaNode_({}), yogaConfig_(nullptr) { - initializeYogaConfig(yogaConfig_); - - yogaNode_.setConfig(&yogaConfig_); + : yogaConfig_(nullptr), yogaNode_(&initializeYogaConfig(yogaConfig_)) { yogaNode_.setContext(this); } YogaLayoutableShadowNode::YogaLayoutableShadowNode( const YogaLayoutableShadowNode &layoutableShadowNode) : LayoutableShadowNode(layoutableShadowNode), - yogaNode_(layoutableShadowNode.yogaNode_), - yogaConfig_(nullptr) { - initializeYogaConfig(yogaConfig_); - - yogaNode_.setConfig(&yogaConfig_); + yogaConfig_(nullptr), + yogaNode_( + layoutableShadowNode.yogaNode_, + &initializeYogaConfig(yogaConfig_)) { yogaNode_.setContext(this); yogaNode_.setOwner(nullptr); @@ -286,10 +282,11 @@ YGSize YogaLayoutableShadowNode::yogaNodeMeasureCallbackConnector( yogaFloatFromFloat(size.height)}; } -void YogaLayoutableShadowNode::initializeYogaConfig(YGConfig &config) { +YGConfig &YogaLayoutableShadowNode::initializeYogaConfig(YGConfig &config) { config.setCloneNodeCallback( YogaLayoutableShadowNode::yogaNodeCloneCallbackConnector); config.useLegacyStretchBehaviour = true; + return config; } } // namespace react diff --git a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h index 012f67091c3..afb369e60fe 100644 --- a/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h +++ b/ReactCommon/fabric/components/view/yoga/YogaLayoutableShadowNode.h @@ -94,6 +94,11 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode, LayoutableShadowNode::UnsharedList getLayoutableChildNodes() const override; protected: + /* + * Yoga config associated (only) with this particular node. + */ + YGConfig yogaConfig_; + /* * All Yoga functions only accept non-const arguments, so we have to mark * Yoga node as `mutable` here to avoid `static_cast`ing the pointer to this @@ -101,13 +106,8 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode, */ mutable YGNode yogaNode_; - /* - * Yoga config associated (only) with this particular node. - */ - YGConfig yogaConfig_; - private: - static void initializeYogaConfig(YGConfig &config); + static YGConfig &initializeYogaConfig(YGConfig &config); static YGNode *yogaNodeCloneCallbackConnector( YGNode *oldYogaNode, YGNode *parentYogaNode, diff --git a/ReactCommon/yoga/yoga/YGNode.cpp b/ReactCommon/yoga/yoga/YGNode.cpp index 2c68d6077bc..172da8713d5 100644 --- a/ReactCommon/yoga/yoga/YGNode.cpp +++ b/ReactCommon/yoga/yoga/YGNode.cpp @@ -22,6 +22,7 @@ YGNode::YGNode(YGNode&& node) { measureUsesContext_ = node.measureUsesContext_; baselineUsesContext_ = node.baselineUsesContext_; printUsesContext_ = node.printUsesContext_; + useWebDefaults_ = node.useWebDefaults_; measure_ = node.measure_; baseline_ = node.baseline_; print_ = node.print_; @@ -38,6 +39,13 @@ 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 (printUsesContext_) { @@ -349,7 +357,7 @@ YGValue YGNode::resolveFlexBasisPtr() const { return flexBasis; } if (!style_.flex().isUndefined() && style_.flex().unwrap() > 0.0f) { - return config_->useWebDefaults ? YGValueAuto : YGValueZero; + return useWebDefaults_ ? YGValueAuto : YGValueZero; } return YGValueAuto; } @@ -425,11 +433,11 @@ float YGNode::resolveFlexShrink() const { if (!style_.flexShrink().isUndefined()) { return style_.flexShrink().unwrap(); } - if (!config_->useWebDefaults && !style_.flex().isUndefined() && + if (!useWebDefaults_ && !style_.flex().isUndefined() && style_.flex().unwrap() < 0.0f) { return -style_.flex().unwrap(); } - return config_->useWebDefaults ? kWebDefaultFlexShrink : kDefaultFlexShrink; + return useWebDefaults_ ? kWebDefaultFlexShrink : kDefaultFlexShrink; } bool YGNode::isNodeFlexible() { @@ -581,11 +589,9 @@ void YGNode::reset() { clearChildren(); - auto config = getConfig(); - *this = YGNode{}; - if (config->useWebDefaults) { - style_.flexDirection() = YGFlexDirectionRow; - style_.alignContent() = YGAlignStretch; + auto webDefaults = useWebDefaults_; + *this = YGNode{getConfig()}; + if (webDefaults) { + useWebDefaults(); } - setConfig(config); } diff --git a/ReactCommon/yoga/yoga/YGNode.h b/ReactCommon/yoga/yoga/YGNode.h index bbd1b7031c9..d368bde0356 100644 --- a/ReactCommon/yoga/yoga/YGNode.h +++ b/ReactCommon/yoga/yoga/YGNode.h @@ -13,6 +13,8 @@ #include "YGStyle.h" #include "Yoga-internal.h" +YGConfigRef YGConfigGetDefault(); + struct YGNode { using MeasureWithContextFn = YGSize (*)(YGNode*, float, YGMeasureMode, float, YGMeasureMode, void*); @@ -28,6 +30,7 @@ private: bool measureUsesContext_ : 1; bool baselineUsesContext_ : 1; bool printUsesContext_ : 1; + bool useWebDefaults_ : 1; uint8_t reserved_ = 0; union { YGMeasureFunc noContext; @@ -58,6 +61,12 @@ private: void setMeasureFunc(decltype(measure_)); void setBaselineFunc(decltype(baseline_)); + void useWebDefaults() { + useWebDefaults_ = true; + style_.flexDirection() = YGFlexDirectionRow; + style_.alignContent() = YGAlignStretch; + } + // DANGER DANGER DANGER! // If the the node assigned to has children, we'd either have to deallocate // them (potentially incorrect) or ignore them (danger of leaks). Only ever @@ -68,8 +77,8 @@ private: using CompactValue = facebook::yoga::detail::CompactValue; public: - YGNode() : YGNode{nullptr} {} - explicit YGNode(const YGConfigRef newConfig) + YGNode() : YGNode{YGConfigGetDefault()} {} + explicit YGNode(const YGConfigRef config) : hasNewLayout_{true}, isReferenceBaseline_{false}, isDirty_{false}, @@ -77,7 +86,12 @@ public: measureUsesContext_{false}, baselineUsesContext_{false}, printUsesContext_{false}, - config_{newConfig} {}; + useWebDefaults_{config->useWebDefaults}, + config_{config} { + if (useWebDefaults_) { + useWebDefaults(); + } + }; ~YGNode() = default; // cleanup of owner/children relationships in YGNodeFree YGNode(YGNode&&); @@ -86,6 +100,9 @@ public: // Should we remove this? YGNode(const YGNode& node) = default; + // for RB fabric + YGNode(const YGNode& node, YGConfigRef config); + // assignment means potential leaks of existing children, or alternatively // freeing unowned memory, double free, or freeing stack memory. YGNode& operator=(const YGNode&) = delete; diff --git a/ReactCommon/yoga/yoga/Yoga.cpp b/ReactCommon/yoga/yoga/Yoga.cpp index f7322980fd3..bcca97951bb 100644 --- a/ReactCommon/yoga/yoga/Yoga.cpp +++ b/ReactCommon/yoga/yoga/Yoga.cpp @@ -210,18 +210,13 @@ void YGNodeMarkDirtyAndPropogateToDescendants(const YGNodeRef node) { int32_t gConfigInstanceCount = 0; WIN_EXPORT YGNodeRef YGNodeNewWithConfig(const YGConfigRef config) { - const YGNodeRef node = new YGNode(); + const YGNodeRef node = new YGNode{config}; YGAssertWithConfig( config, node != nullptr, "Could not allocate memory for node"); #ifdef YG_ENABLE_EVENTS Event::publish(node, {config}); #endif - if (config->useWebDefaults) { - node->getStyle().flexDirection() = YGFlexDirectionRow; - node->getStyle().alignContent() = YGAlignStretch; - } - node->setConfig(config); return node; }