Skip to content

Commit

Permalink
YGNode: Field for web defaults
Browse files Browse the repository at this point in the history
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
  • Loading branch information
davidaurelio authored and facebook-github-bot committed May 20, 2019
1 parent 9785a57 commit 19a88d7
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,20 +94,20 @@ 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
* all the time.
*/
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,
Expand Down
24 changes: 15 additions & 9 deletions ReactCommon/yoga/yoga/YGNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand All @@ -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_) {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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);
}
23 changes: 20 additions & 3 deletions ReactCommon/yoga/yoga/YGNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
#include "YGStyle.h"
#include "Yoga-internal.h"

YGConfigRef YGConfigGetDefault();

struct YGNode {
using MeasureWithContextFn =
YGSize (*)(YGNode*, float, YGMeasureMode, float, YGMeasureMode, void*);
Expand All @@ -28,6 +30,7 @@ struct YGNode {
bool measureUsesContext_ : 1;
bool baselineUsesContext_ : 1;
bool printUsesContext_ : 1;
bool useWebDefaults_ : 1;
uint8_t reserved_ = 0;
union {
YGMeasureFunc noContext;
Expand Down Expand Up @@ -58,6 +61,12 @@ struct YGNode {
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
Expand All @@ -68,16 +77,21 @@ struct YGNode {
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},
nodeType_{YGNodeTypeDefault},
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&&);
Expand All @@ -86,6 +100,9 @@ struct YGNode {
// 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;
Expand Down
7 changes: 1 addition & 6 deletions ReactCommon/yoga/yoga/Yoga.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<Event::NodeAllocation>(node, {config});
#endif

if (config->useWebDefaults) {
node->getStyle().flexDirection() = YGFlexDirectionRow;
node->getStyle().alignContent() = YGAlignStretch;
}
node->setConfig(config);
return node;
}

Expand Down

0 comments on commit 19a88d7

Please sign in to comment.