Skip to content

Commit

Permalink
Fabric: Using unique_ptr for storing YGNode inside YogaLayoutableShad…
Browse files Browse the repository at this point in the history
…owNode

Summary:
I recently realized (Thanks David!) that we should not use `shared_ptr` for storing YGNode*
because ShadowNode does not share ownership of the Yoga node with anybody.
So the lifecycle of shadow node and yoga node must be synchronized (this is already the case but changing to unique_ptr makes this explicit and a bit more performant).

Reviewed By: fkgozali

Differential Revision: D8030417

fbshipit-source-id: c7f85ea309598d2a5ebfed55b1d182d3fe1336ae
  • Loading branch information
shergin authored and facebook-github-bot committed May 19, 2018
1 parent 6cc597e commit caaea38
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 34 deletions.
54 changes: 25 additions & 29 deletions ReactCommon/fabric/view/yoga/YogaLayoutableShadowNode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,35 +37,32 @@ YogaLayoutableShadowNode::YogaLayoutableShadowNode(
assert(props);
assert(children);

auto yogaNode = std::make_shared<YGNode>();
yogaNode->setConfig(suitableYogaConfig().get());
yogaNode->setStyle(props->yogaStyle);
yogaNode->setContext(this);
yogaNode->setDirty(true);
YogaLayoutableShadowNode::setYogaNodeChildrenBasedOnShadowNodeChildren(*yogaNode, children);
yogaNode_ = yogaNode;
yogaNode_ = std::make_unique<YGNode>();
yogaNode_->setConfig(suitableYogaConfig().get());
yogaNode_->setStyle(props->yogaStyle);
yogaNode_->setContext(this);
yogaNode_->setDirty(true);
YogaLayoutableShadowNode::setYogaNodeChildrenBasedOnShadowNodeChildren(yogaNode_.get(), children);
}

YogaLayoutableShadowNode::YogaLayoutableShadowNode(
const SharedYogaLayoutableShadowNode &shadowNode,
const SharedYogaStylableProps &props,
const SharedShadowNodeSharedList &children
) {
auto yogaNode = std::make_shared<YGNode>(*shadowNode->yogaNode_);
yogaNode->setConfig(suitableYogaConfig().get());
yogaNode->setContext(this);
yogaNode->setOwner(nullptr);
yogaNode->setDirty(true);
yogaNode_ = std::make_unique<YGNode>(*shadowNode->yogaNode_);
yogaNode_->setConfig(suitableYogaConfig().get());
yogaNode_->setContext(this);
yogaNode_->setOwner(nullptr);
yogaNode_->setDirty(true);

if (props) {
yogaNode->setStyle(props->yogaStyle);
yogaNode_->setStyle(props->yogaStyle);
}

if (children) {
YogaLayoutableShadowNode::setYogaNodeChildrenBasedOnShadowNodeChildren(*yogaNode, children);
YogaLayoutableShadowNode::setYogaNodeChildrenBasedOnShadowNodeChildren(yogaNode_.get(), children);
}

yogaNode_ = yogaNode;
}

void YogaLayoutableShadowNode::cleanLayout() {
Expand Down Expand Up @@ -99,21 +96,20 @@ void YogaLayoutableShadowNode::enableMeasurement() {
void YogaLayoutableShadowNode::appendChild(SharedYogaLayoutableShadowNode child) {
ensureUnsealed();

auto nonConstYogaNode = std::const_pointer_cast<YGNode>(yogaNode_);
auto nonConstChildYogaNode = std::const_pointer_cast<YGNode>(child->yogaNode_);
nonConstYogaNode->insertChild(nonConstChildYogaNode.get(), nonConstYogaNode->getChildrenCount());
auto yogaNodeRawPtr = yogaNode_.get();
auto childYogaNodeRawPtr = child->yogaNode_.get();
yogaNodeRawPtr->insertChild(childYogaNodeRawPtr, yogaNodeRawPtr->getChildrenCount());

if (nonConstChildYogaNode->getOwner() == nullptr) {
if (childYogaNodeRawPtr->getOwner() == nullptr) {
child->ensureUnsealed();
nonConstChildYogaNode->setOwner(nonConstYogaNode.get());
childYogaNodeRawPtr->setOwner(yogaNodeRawPtr);
}
}

void YogaLayoutableShadowNode::layout(LayoutContext layoutContext) {
if (!getIsLayoutClean()) {
ensureUnsealed();
YGNode *yogaNode = const_cast<YGNode *>(yogaNode_.get());
YGNodeCalculateLayout(yogaNode, YGUndefined, YGUndefined, YGDirectionInherit);
YGNodeCalculateLayout(yogaNode_.get(), YGUndefined, YGUndefined, YGDirectionInherit);
}

LayoutableShadowNode::layout(layoutContext);
Expand Down Expand Up @@ -206,7 +202,7 @@ YGSize YogaLayoutableShadowNode::yogaNodeMeasureCallbackConnector(YGNode *yogaNo
};
}

void YogaLayoutableShadowNode::setYogaNodeChildrenBasedOnShadowNodeChildren(YGNode &yogaNode, const SharedShadowNodeSharedList &children) {
void YogaLayoutableShadowNode::setYogaNodeChildrenBasedOnShadowNodeChildren(YGNode *yogaNodeRawPtr, const SharedShadowNodeSharedList &children) {
auto yogaNodeChildren = YGVector();

for (const SharedShadowNode &shadowNode : *children) {
Expand All @@ -216,17 +212,17 @@ void YogaLayoutableShadowNode::setYogaNodeChildrenBasedOnShadowNodeChildren(YGNo
continue;
}

YGNode *yogaNodeChild = (YGNode *)yogaLayoutableShadowNode->yogaNode_.get();
auto &&childYogaNodeRawPtr = (YGNode *)yogaLayoutableShadowNode->yogaNode_.get();

yogaNodeChildren.push_back(yogaNodeChild);
yogaNodeChildren.push_back(childYogaNodeRawPtr);

if (yogaNodeChild->getOwner() == nullptr) {
if (childYogaNodeRawPtr->getOwner() == nullptr) {
yogaLayoutableShadowNode->ensureUnsealed();
yogaNodeChild->setOwner(&yogaNode);
childYogaNodeRawPtr->setOwner(yogaNodeRawPtr);
}
}

yogaNode.setChildren(yogaNodeChildren);
yogaNodeRawPtr->setChildren(yogaNodeChildren);
}

} // namespace react
Expand Down
7 changes: 2 additions & 5 deletions ReactCommon/fabric/view/yoga/YogaLayoutableShadowNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@ namespace react {

class YogaLayoutableShadowNode;

// We accept that Yoga node is highly mutable thing and we don't try to enforce immutability,
// so it does not have `const` qualifier (and we mark it as `mutable` in the class).
using SharedYogaNode = std::shared_ptr<YGNode>;
using SharedYogaConfig = std::shared_ptr<YGConfig>;

using SharedYogaLayoutableShadowNode = std::shared_ptr<const YogaLayoutableShadowNode>;
Expand Down Expand Up @@ -83,11 +80,11 @@ class YogaLayoutableShadowNode:
void layoutChildren(LayoutContext layoutContext) override;

protected:
mutable SharedYogaNode yogaNode_;
std::unique_ptr<YGNode> yogaNode_;

private:
static SharedYogaConfig suitableYogaConfig();
static void setYogaNodeChildrenBasedOnShadowNodeChildren(YGNode &yogaNode, const SharedShadowNodeSharedList &children);
static void setYogaNodeChildrenBasedOnShadowNodeChildren(YGNode *yogaNodeRawPtr, const SharedShadowNodeSharedList &children);
static YGNode *yogaNodeCloneCallbackConnector(YGNode *oldYogaNode, YGNode *parentYogaNode, int childIndex);
static YGSize yogaNodeMeasureCallbackConnector(YGNode *yogaNode, float width, YGMeasureMode widthMode, float height, YGMeasureMode heightMode);
};
Expand Down

0 comments on commit caaea38

Please sign in to comment.