DaddyFrosty/yoga
1
0
Fork 0
Code Issues 96 Pull Requests 31 Actions Packages Projects Releases 33 Wiki Activity

Clone children only during layout, allow mixing shared + owned children

Browse Source 
Summary:
@public

Limit child cloning to layout calculation. This also allows for mixing shared and owned children.

Rationale:
We do allow for shared children if the caller manages themselves. The single known use case is React Native.

So far, we have cloned children eagerly whenever child lists are mutated, or layout is run. This was to allow for a quick check of the owner of any first child, assuming that either *all* or *no* child of a node are shared.

For Yoga/Java, we want to get rid of global weak JNI refs, and these are also used to invoke clone callbacks. We can achieve that goal by switching to an alternative approach, passing additional data to the layout pass. This additional data has to be passed to any configured cloning callback. Therefore, it is desirable to **only call cloning functions during the layout pass.**

The obvious solution seems to be to not uphold the invariant of the first child determining shared/owned state of all siblings, and allow for a mix of shared and own children.

Reviewed By: shergin

Differential Revision: D14136223

fbshipit-source-id: 34490cfeeb2170c99d6ed1b9bdcbcedb316813af
This commit is contained in:
David Aurelio
2019-02-20 11:52:38 -08:00
committed by Facebook Github Bot
parent 367a93de88
commit b1c749075d
4 changed files with 82 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files

41
tests/YGPersistenceTest.cpp
View File

@@ -6,6 +6,7 @@
*/
#include <gtest/gtest.h>
#include <yoga/Yoga.h>
#include <yoga/YGNode.h>
TEST(YogaTest, cloning_shared_root) {
const YGConfigRef config = YGConfigNew();
@@ -104,7 +105,7 @@ TEST(YogaTest, cloning_shared_root) {
YGConfigFree(config);
}
TEST(YogaTest, mutating_children_of_a_clone_clones) {
TEST(YogaTest, mutating_children_of_a_clone_clones_only_after_layout) {
const YGConfigRef config = YGConfigNew();
const YGNodeRef root = YGNodeNewWithConfig(config);
@@ -129,7 +130,7 @@ TEST(YogaTest, mutating_children_of_a_clone_clones) {
ASSERT_EQ(1, YGNodeGetChildCount(root2));
ASSERT_EQ(2, YGNodeGetChildCount(root3));
ASSERT_EQ(root3_child1, YGNodeGetChild(root3, 1));
ASSERT_NE(YGNodeGetChild(root2, 0), YGNodeGetChild(root3, 0));
ASSERT_EQ(YGNodeGetChild(root2, 0), YGNodeGetChild(root3, 0));
const YGNodeRef root4 = YGNodeClone(root3);
ASSERT_EQ(root3_child1, YGNodeGetChild(root4, 1));
@@ -137,7 +138,12 @@ TEST(YogaTest, mutating_children_of_a_clone_clones) {
YGNodeRemoveChild(root4, root3_child1);
ASSERT_EQ(2, YGNodeGetChildCount(root3));
ASSERT_EQ(1, YGNodeGetChildCount(root4));
ASSERT_EQ(YGNodeGetChild(root3, 0), YGNodeGetChild(root4, 0));
YGNodeCalculateLayout(root4, YGUndefined, YGUndefined, YGDirectionLTR);
ASSERT_NE(YGNodeGetChild(root3, 0), YGNodeGetChild(root4, 0));
YGNodeCalculateLayout(root3, YGUndefined, YGUndefined, YGDirectionLTR);
ASSERT_NE(YGNodeGetChild(root2, 0), YGNodeGetChild(root3, 0));
YGNodeFreeRecursive(root4);
YGNodeFreeRecursive(root3);
@@ -245,3 +251,34 @@ TEST(YogaTest, cloning_and_freeing) {
ASSERT_EQ(initialInstanceCount, YGNodeGetInstanceCount());
}
TEST(YogaTest, mixed_shared_and_owned_children) {
// Don't try this at home!
YGNodeRef root0 = YGNodeNew();
YGNodeRef root1 = YGNodeNew();
YGNodeRef root0_child0 = YGNodeNew();
YGNodeRef root0_child0_0 = YGNodeNew();
YGNodeInsertChild(root0, root0_child0, 0);
YGNodeInsertChild(root0_child0, root0_child0_0, 0);
YGNodeRef root1_child0 = YGNodeNew();
YGNodeRef root1_child2 = YGNodeNew();
YGNodeInsertChild(root1, root1_child0, 0);
YGNodeInsertChild(root1, root1_child2, 1);
auto children = root1->getChildren();
children.insert(children.begin() + 1, root0_child0);
root1->setChildren(children);
auto secondChild = YGNodeGetChild(root1, 1);
ASSERT_EQ(secondChild, YGNodeGetChild(root0, 0));
ASSERT_EQ(YGNodeGetChild(secondChild, 0), YGNodeGetChild(root0_child0, 0));
YGNodeCalculateLayout(root1, YGUndefined, YGUndefined, YGDirectionLTR);
secondChild = YGNodeGetChild(root1, 1);
ASSERT_NE(secondChild, YGNodeGetChild(root0, 0));
ASSERT_EQ(YGNodeGetOwner(secondChild), root1);
ASSERT_NE(YGNodeGetChild(secondChild, 0), YGNodeGetChild(root0_child0, 0));
ASSERT_EQ(YGNodeGetOwner(YGNodeGetChild(secondChild, 0)), secondChild);
}

25
yoga/YGNode.cpp
View File

@@ -386,30 +386,7 @@ void YGNode::clearChildren() {
// Other Methods
void YGNode::cloneChildrenIfNeeded() {
// YGNodeRemoveChild in yoga.cpp has a forked variant of this algorithm
// optimized for deletions.
const uint32_t childCount = static_cast<uint32_t>(children_.size());
if (childCount == 0) {
// This is an empty set. Nothing to clone.
return;
}
const YGNodeRef firstChild = children_.front();
if (firstChild->getOwner() == this) {
// If the first child has this node as its owner, we assume that it is
// already unique. We can do this because if we have it has a child, that
// means that its owner was at some point cloned which made that subtree
// immutable. We also assume that all its sibling are cloned as well.
return;
}
for (uint32_t i = 0; i < childCount; ++i) {
const YGNodeRef oldChild = children_[i];
YGNodeRef newChild = config_->cloneNode(oldChild, this, i);
replaceChild(newChild, i);
newChild->setOwner(this);
}
iterChildrenAfterCloningIfNeeded([](YGNodeRef) {});
}
void YGNode::markDirtyAndPropogate() {

16
yoga/YGNode.h
View File

@@ -143,6 +143,22 @@ public:
return children_;
}
// Applies a callback to all children, after cloning them if they are not
// owned.
template <typename T>
void iterChildrenAfterCloningIfNeeded(T callback) {
int i = 0;
for (YGNodeRef& child : children_) {
if (child->getOwner() != this) {
child = config_->cloneNode(child, this, i);
child->setOwner(this);
}
i += 1;
callback(child);
}
}
YGNodeRef getChild(uint32_t index) const {
return children_.at(index);
}

84
yoga/Yoga.cpp
View File

@@ -302,14 +302,16 @@ static void YGConfigFreeRecursive(const YGNodeRef root) {
void YGNodeFreeRecursiveWithCleanupFunc(
const YGNodeRef root,
YGNodeCleanupFunc cleanup) {
while (YGNodeGetChildCount(root) > 0) {
const YGNodeRef child = YGNodeGetChild(root, 0);
uint32_t skipped = 0;
while (YGNodeGetChildCount(root) > skipped) {
const YGNodeRef child = YGNodeGetChild(root, skipped);
if (child->getOwner() != root) {
// Don't free shared nodes that we don't own.
break;
skipped += 1;
} else {
YGNodeRemoveChild(root, child);
YGNodeFreeRecursive(child);
}
YGNodeRemoveChild(root, child);
YGNodeFreeRecursive(child);
}
if (cleanup != nullptr) {
cleanup(root);
@@ -381,70 +383,40 @@ bool YGNodeIsReferenceBaseline(YGNodeRef node) {
}
void YGNodeInsertChild(
const YGNodeRef node,
const YGNodeRef owner,
const YGNodeRef child,
const uint32_t index) {
YGAssertWithNode(
node,
owner,
child->getOwner() == nullptr,
"Child already has a owner, it must be removed first.");
YGAssertWithNode(
node,
!node->hasMeasureFunc(),
owner,
!owner->hasMeasureFunc(),
"Cannot add child: Nodes with measure functions cannot have children.");
node->cloneChildrenIfNeeded();
node->insertChild(child, index);
YGNodeRef owner = child->getOwner() ? nullptr : node;
owner->insertChild(child, index);
child->setOwner(owner);
node->markDirtyAndPropogate();
owner->markDirtyAndPropogate();
}
void YGNodeRemoveChild(const YGNodeRef owner, const YGNodeRef excludedChild) {
// This algorithm is a forked variant from cloneChildrenIfNeeded in YGNode
// that excludes a child.
const uint32_t childCount = YGNodeGetChildCount(owner);
if (childCount == 0) {
if (YGNodeGetChildCount(owner) == 0) {
// This is an empty set. Nothing to remove.
return;
}
const YGNodeRef firstChild = YGNodeGetChild(owner, 0);
if (firstChild->getOwner() == owner) {
// If the first child has this node as its owner, we assume that it is
// already unique. We can now try to delete a child in this list.
if (owner->removeChild(excludedChild)) {
excludedChild->setLayout(
YGNode().getLayout()); // layout is no longer valid
excludedChild->setOwner(nullptr);
owner->markDirtyAndPropogate();
}
return;
}
// Otherwise we have to clone the node list except for the child we're trying
// to delete. We don't want to simply clone all children, because then the
// host will need to free the clone of the child that was just deleted.
uint32_t nextInsertIndex = 0;
for (uint32_t i = 0; i < childCount; i++) {
const YGNodeRef oldChild = owner->getChild(i);
if (excludedChild == oldChild) {
// Ignore the deleted child. Don't reset its layout or owner since it is
// still valid in the other owner. However, since this owner has now
// changed, we need to mark it as dirty.
owner->markDirtyAndPropogate();
continue;
}
YGNodeRef newChild =
owner->getConfig()->cloneNode(oldChild, owner, nextInsertIndex);
owner->replaceChild(newChild, nextInsertIndex);
newChild->setOwner(owner);
nextInsertIndex++;
}
while (nextInsertIndex < childCount) {
owner->removeChild(nextInsertIndex);
nextInsertIndex++;
// Children may be shared between parents, which is indicated by not having an
// owner. We only want to reset the child completely if it is owned
// exclusively by one node.
auto childOwner = excludedChild->getOwner();
if (owner->removeChild(excludedChild)) {
if (owner == childOwner) {
excludedChild->setLayout({}); // layout is no longer valid
excludedChild->setOwner(nullptr);
}
owner->markDirtyAndPropogate();
}
}
@@ -1822,12 +1794,8 @@ static void YGZeroOutLayoutRecursivly(const YGNodeRef node) {
node->setLayoutDimension(0, 0);
node->setLayoutDimension(0, 1);
node->setHasNewLayout(true);
node->cloneChildrenIfNeeded();
const uint32_t childCount = YGNodeGetChildCount(node);
for (uint32_t i = 0; i < childCount; i++) {
const YGNodeRef child = node->getChild(i);
YGZeroOutLayoutRecursivly(child);
}
node->iterChildrenAfterCloningIfNeeded(YGZeroOutLayoutRecursivly);
}
static float YGNodeCalculateAvailableInnerDim(