From 5e3ffb39a2acb05d4fe93d04f5ae4058c047f6b1 Mon Sep 17 00:00:00 2001 From: David Vacca Date: Sun, 1 Apr 2018 18:27:08 -0700 Subject: [PATCH] Extend Yoga to be able clone Yoga Node with new children Summary: This diff includes the following changes: 1 ) I extended the Java implementation of YogaNode to be able to get a full copy of a YogaNode object without copying the List of children of the original YogaNode. In other words, the new copy of the YogaNode will have the same data of the original YogaNode, but with an empty list of children. 2 ) We created a new method in Yoga.cpp called YGNodeInsertSharedChild. This new method is going to be used by Fabric in order to temporarily share a YogaNode between two "Yoga Trees" (the "current Yoga" tree and a partial "clone of the current Yoga tree"). We exposed this new functionality in the java implementation of Yoga (method addSharedChildAt) I'm including sebmarkbage for more context. Reviewed By: emilsjolander Differential Revision: D7245421 fbshipit-source-id: 72578c8261f29e4a12fc6c72a91f2f891cd58d48 --- java/com/facebook/yoga/YogaNode.java | 66 ++++++++++++++++--- java/jni/YGJNI.cpp | 16 +++++ .../tests/com/facebook/yoga/YogaNodeTest.java | 59 +++++++++++++---- yoga/Yoga.cpp | 21 ++++-- yoga/Yoga.h | 25 +++++-- 5 files changed, 155 insertions(+), 32 deletions(-) diff --git a/java/com/facebook/yoga/YogaNode.java b/java/com/facebook/yoga/YogaNode.java index ae5165e6..69bb3348 100644 --- a/java/com/facebook/yoga/YogaNode.java +++ b/java/com/facebook/yoga/YogaNode.java @@ -30,7 +30,7 @@ public class YogaNode implements Cloneable { static native int jni_YGNodeGetInstanceCount(); private YogaNode mOwner; - private List mChildren; + @Nullable private List mChildren; private YogaMeasureFunction mMeasureFunction; private YogaBaselineFunction mBaselineFunction; private long mNativePointer; @@ -147,6 +147,9 @@ public class YogaNode implements Cloneable { } public YogaNode getChildAt(int i) { + if (mChildren == null) { + throw new IllegalStateException("YogaNode does not have children"); + } return mChildren.get(i); } @@ -164,21 +167,62 @@ public class YogaNode implements Cloneable { jni_YGNodeInsertChild(mNativePointer, child.mNativePointer, i); } + private native void jni_YGNodeInsertSharedChild(long nativePointer, long childPointer, int index); + + public void addSharedChildAt(YogaNode child, int i) { + if (mChildren == null) { + mChildren = new ArrayList<>(4); + } + mChildren.add(i, child); + child.mOwner = null; + jni_YGNodeInsertSharedChild(mNativePointer, child.mNativePointer, i); + } + private native long jni_YGNodeClone(long nativePointer, Object newNode); @Override - public YogaNode clone() throws CloneNotSupportedException { - YogaNode clonedYogaNode = (YogaNode) super.clone(); - long clonedNativePointer = jni_YGNodeClone(mNativePointer, clonedYogaNode); - clonedYogaNode.mNativePointer = clonedNativePointer; - clonedYogaNode.mChildren = - mChildren != null ? (List) ((ArrayList) mChildren).clone() : null; - return clonedYogaNode; + public YogaNode clone() { + try { + YogaNode clonedYogaNode = (YogaNode) super.clone(); + long clonedNativePointer = jni_YGNodeClone(mNativePointer, clonedYogaNode); + clonedYogaNode.mNativePointer = clonedNativePointer; + clonedYogaNode.mOwner = null; + clonedYogaNode.mChildren = + mChildren != null ? (List) ((ArrayList) mChildren).clone() : null; + return clonedYogaNode; + } catch (CloneNotSupportedException ex) { + // This class implements Cloneable, this should not happen + throw new RuntimeException(ex); + } + } + + public YogaNode cloneWithNewChildren() { + try { + YogaNode clonedYogaNode = (YogaNode) super.clone(); + long clonedNativePointer = jni_YGNodeClone(mNativePointer, clonedYogaNode); + clonedYogaNode.mOwner = null; + clonedYogaNode.mNativePointer = clonedNativePointer; + clonedYogaNode.clearChildren(); + return clonedYogaNode; + } catch (CloneNotSupportedException ex) { + // This class implements Cloneable, this should not happen + throw new RuntimeException(ex); + } + } + + private native void jni_YGNodeClearChildren(long nativePointer); + + private void clearChildren() { + mChildren = null; + jni_YGNodeClearChildren(mNativePointer); } private native void jni_YGNodeRemoveChild(long nativePointer, long childPointer); public YogaNode removeChildAt(int i) { - + if (mChildren == null) { + throw new IllegalStateException( + "Trying to remove a child of a YogaNode that does not have children"); + } final YogaNode child = mChildren.remove(i); child.mOwner = null; jni_YGNodeRemoveChild(mNativePointer, child.mNativePointer); @@ -715,8 +759,12 @@ public class YogaNode implements Cloneable { */ @DoNotStrip private final long replaceChild(YogaNode newNode, int childIndex) { + if (mChildren == null) { + throw new IllegalStateException("Cannot replace child. YogaNode does not have children"); + } mChildren.remove(childIndex); mChildren.add(childIndex, newNode); + newNode.mOwner = this; return newNode.mNativePointer; } } diff --git a/java/jni/YGJNI.cpp b/java/jni/YGJNI.cpp index b303ce22..69cbb381 100644 --- a/java/jni/YGJNI.cpp +++ b/java/jni/YGJNI.cpp @@ -284,6 +284,11 @@ void jni_YGNodeFree(alias_ref thiz, jlong nativePointer) { YGNodeFree(node); } +void jni_YGNodeClearChildren(alias_ref thiz, jlong nativePointer) { + const YGNodeRef node = _jlong2YGNodeRef(nativePointer); + node->clearChildren(); +} + void jni_YGNodeReset(alias_ref thiz, jlong nativePointer) { const YGNodeRef node = _jlong2YGNodeRef(nativePointer); void* context = node->getContext(); @@ -303,6 +308,15 @@ void jni_YGNodeInsertChild(alias_ref, jlong nativePointer, jlong childP YGNodeInsertChild(_jlong2YGNodeRef(nativePointer), _jlong2YGNodeRef(childPointer), index); } +void jni_YGNodeInsertSharedChild( + alias_ref, + jlong nativePointer, + jlong childPointer, + jint index) { + YGNodeInsertSharedChild( + _jlong2YGNodeRef(nativePointer), _jlong2YGNodeRef(childPointer), index); +} + void jni_YGNodeRemoveChild(alias_ref, jlong nativePointer, jlong childPointer) { YGNodeRemoveChild(_jlong2YGNodeRef(nativePointer), _jlong2YGNodeRef(childPointer)); } @@ -577,7 +591,9 @@ jint JNI_OnLoad(JavaVM *vm, void *) { YGMakeNativeMethod(jni_YGNodeNewWithConfig), YGMakeNativeMethod(jni_YGNodeFree), YGMakeNativeMethod(jni_YGNodeReset), + YGMakeNativeMethod(jni_YGNodeClearChildren), YGMakeNativeMethod(jni_YGNodeInsertChild), + YGMakeNativeMethod(jni_YGNodeInsertSharedChild), YGMakeNativeMethod(jni_YGNodeRemoveChild), YGMakeNativeMethod(jni_YGNodeCalculateLayout), YGMakeNativeMethod(jni_YGNodeMarkDirty), diff --git a/java/tests/com/facebook/yoga/YogaNodeTest.java b/java/tests/com/facebook/yoga/YogaNodeTest.java index a502f43d..9cc26eb9 100644 --- a/java/tests/com/facebook/yoga/YogaNodeTest.java +++ b/java/tests/com/facebook/yoga/YogaNodeTest.java @@ -10,6 +10,7 @@ package com.facebook.yoga; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -251,6 +252,48 @@ public class YogaNodeTest { assertEquals(1, clonedChild.getChildCount()); } + @Test + public void testCloneWithNewChildren() throws Exception { + YogaConfig config = new YogaConfig(); + YogaNode root = new YogaNode(config); + YogaNode child = new YogaNode(config); + YogaNode grandChild = new YogaNode(config); + root.addChildAt(child, 0); + child.addChildAt(grandChild, 0); + child.setFlexDirection(YogaFlexDirection.ROW); + + YogaNode clonedChild = child.cloneWithNewChildren(); + + assertNotSame(clonedChild, child); + assertEquals(YogaFlexDirection.ROW, clonedChild.getFlexDirection()); + assertEquals(child.getFlexDirection(), clonedChild.getFlexDirection()); + assertEquals(0, clonedChild.getChildCount()); + assertEquals(1, child.getChildCount()); + } + + @Test + public void testAddSharedChildCloneWithNewChildren() throws Exception { + YogaConfig config = new YogaConfig(); + YogaNode root = new YogaNode(config); + YogaNode child = new YogaNode(config); + YogaNode grandChild = new YogaNode(config); + root.addChildAt(child, 0); + child.addChildAt(grandChild, 0); + child.setFlexDirection(YogaFlexDirection.ROW); + + YogaNode clonedChild = child.cloneWithNewChildren(); + + assertNotSame(clonedChild, child); + assertEquals(YogaFlexDirection.ROW, clonedChild.getFlexDirection()); + assertEquals(child.getFlexDirection(), clonedChild.getFlexDirection()); + assertEquals(0, clonedChild.getChildCount()); + assertEquals(1, child.getChildCount()); + + clonedChild.addSharedChildAt(grandChild, 0); + assertEquals(1, clonedChild.getChildCount()); + assertNull(grandChild.getOwner()); + } + @Test public void testCloneNodeListener() throws Exception { final AtomicBoolean onNodeClonedExecuted = new AtomicBoolean(false); @@ -259,13 +302,8 @@ public class YogaNodeTest { new YogaNodeCloneFunction() { @Override public YogaNode cloneNode(YogaNode oldNode, YogaNode owner, int childIndex) { - try { - onNodeClonedExecuted.set(true); - return oldNode.clone(); - } catch (CloneNotSupportedException ex) { - // DO nothing - return null; - } + onNodeClonedExecuted.set(true); + return oldNode.clone(); } }); YogaNode root = new YogaNode(config); @@ -296,12 +334,7 @@ public class YogaNodeTest { new YogaNodeCloneFunction() { @Override public YogaNode cloneNode(YogaNode oldNode, YogaNode owner, int childIndex) { - try { - return oldNode.clone(); - } catch (CloneNotSupportedException ex) { - // DO nothing - return null; - } + return oldNode.clone(); } }); config.setOnCloneNode(null); diff --git a/yoga/Yoga.cpp b/yoga/Yoga.cpp index 2e5979a2..7518e858 100644 --- a/yoga/Yoga.cpp +++ b/yoga/Yoga.cpp @@ -389,10 +389,6 @@ void YGConfigCopy(const YGConfigRef dest, const YGConfigRef src) { } void YGNodeInsertChild(const YGNodeRef node, const YGNodeRef child, const uint32_t index) { - YGAssertWithNode( - node, - child->getOwner() == nullptr, - "Child already has a owner, it must be removed first."); YGAssertWithNode( node, node->getMeasure() == nullptr, @@ -400,7 +396,22 @@ void YGNodeInsertChild(const YGNodeRef node, const YGNodeRef child, const uint32 node->cloneChildrenIfNeeded(); node->insertChild(child, index); - child->setOwner(node); + YGNodeRef owner = child->getOwner() ? nullptr : node; + child->setOwner(owner); + node->markDirtyAndPropogate(); +} + +void YGNodeInsertSharedChild( + const YGNodeRef node, + const YGNodeRef child, + const uint32_t index) { + YGAssertWithNode( + node, + node->getMeasure() == nullptr, + "Cannot add child: Nodes with measure functions cannot have children."); + + node->insertChild(child, index); + child->setOwner(nullptr); node->markDirtyAndPropogate(); } diff --git a/yoga/Yoga.h b/yoga/Yoga.h index 445c0b06..25eb0eb6 100644 --- a/yoga/Yoga.h +++ b/yoga/Yoga.h @@ -62,9 +62,8 @@ typedef int (*YGLogger)(const YGConfigRef config, YGLogLevel level, const char *format, va_list args); -typedef YGNodeRef (*YGCloneNodeFunc)(YGNodeRef oldNode, - YGNodeRef parent, - int childIndex); +typedef YGNodeRef ( + *YGCloneNodeFunc)(YGNodeRef oldNode, YGNodeRef owner, int childIndex); // YGNode WIN_EXPORT YGNodeRef YGNodeNew(void); @@ -78,12 +77,26 @@ WIN_EXPORT int32_t YGNodeGetInstanceCount(void); WIN_EXPORT void YGNodeInsertChild(const YGNodeRef node, const YGNodeRef child, const uint32_t index); + +// This function inserts the child YGNodeRef as a children of the node received +// by parameter and set the Owner of the child object to null. This function is +// expected to be called when using Yoga in persistent mode in order to share a +// YGNodeRef object as a child of two different Yoga trees. The child YGNodeRef +// is expected to be referenced from its original owner and from a clone of its +// original owner. +WIN_EXPORT void YGNodeInsertSharedChild( + const YGNodeRef node, + const YGNodeRef child, + const uint32_t index); WIN_EXPORT void YGNodeRemoveChild(const YGNodeRef node, const YGNodeRef child); WIN_EXPORT void YGNodeRemoveAllChildren(const YGNodeRef node); WIN_EXPORT YGNodeRef YGNodeGetChild(const YGNodeRef node, const uint32_t index); WIN_EXPORT YGNodeRef YGNodeGetOwner(const YGNodeRef node); WIN_EXPORT uint32_t YGNodeGetChildCount(const YGNodeRef node); -WIN_EXPORT void YGNodeSetChildren(YGNodeRef const parent, const YGNodeRef children[], const uint32_t count); +WIN_EXPORT void YGNodeSetChildren( + YGNodeRef const owner, + const YGNodeRef children[], + const uint32_t count); WIN_EXPORT void YGNodeCalculateLayout(const YGNodeRef node, const float availableWidth, @@ -307,6 +320,8 @@ YG_EXTERN_C_END // Calls f on each node in the tree including the given node argument. extern void YGTraversePreOrder(YGNodeRef const node, std::function&& f); -extern void YGNodeSetChildren(YGNodeRef const parent, const std::vector &children); +extern void YGNodeSetChildren( + YGNodeRef const owner, + const std::vector& children); #endif