Skip to content

Commit

Permalink
Cleanup YGNode for explicit per-node config
Browse files Browse the repository at this point in the history
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
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Apr 25, 2023
1 parent ba92700 commit 0e5d54a
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,8 @@ YogaLayoutableShadowNode::YogaLayoutableShadowNode(
ShadowNodeFragment const &fragment)
: LayoutableShadowNode(sourceShadowNode, fragment),
yogaConfig_(FabricDefaultYogaLog),
yogaNode_(
static_cast<YogaLayoutableShadowNode const &>(sourceShadowNode)
.yogaNode_,
&initializeYogaConfig(yogaConfig_)) {
yogaNode_(static_cast<YogaLayoutableShadowNode const &>(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.
Expand All @@ -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.
Expand Down
49 changes: 33 additions & 16 deletions packages/react-native/ReactCommon/yoga/yoga/YGNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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();
}
}
19 changes: 6 additions & 13 deletions packages/react-native/ReactCommon/yoga/yoga/YGNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ struct YGNodeFlags {
bool measureUsesContext : 1;
bool baselineUsesContext : 1;
bool printUsesContext : 1;
bool useWebDefaults : 1;
};
#pragma pack(pop)

Expand Down Expand Up @@ -72,7 +71,6 @@ struct YOGA_EXPORT YGNode {
void setBaselineFunc(decltype(baseline_));

void useWebDefaults() {
flags_.useWebDefaults = true;
style_.flexDirection() = YGFlexDirectionRow;
style_.alignContent() = YGAlignStretch;
}
Expand All @@ -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&&);
Expand All @@ -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.
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 0e5d54a

Please sign in to comment.