diff --git a/java/com/facebook/yoga/YogaConfig.java b/java/com/facebook/yoga/YogaConfig.java index 2d90eec9..ebb64aaf 100644 --- a/java/com/facebook/yoga/YogaConfig.java +++ b/java/com/facebook/yoga/YogaConfig.java @@ -25,7 +25,7 @@ public class YogaConfig { long mNativePointer; private YogaLogger mLogger; - private YogaNodeClonedFunction mNodeClonedFunction; + private YogaNodeCloneFunction mYogaNodeCloneFunction; private native long jni_YGConfigNew(); public YogaConfig() { @@ -97,16 +97,15 @@ public class YogaConfig { return mLogger; } - private native void jni_YGConfigSetHasNodeClonedFunc(long nativePointer, boolean hasClonedFunc); + private native void jni_YGConfigSetHasCloneNodeFunc(long nativePointer, boolean hasClonedFunc); - public void setOnNodeCloned(YogaNodeClonedFunction nodeClonedFunction) { - mNodeClonedFunction = nodeClonedFunction; - jni_YGConfigSetHasNodeClonedFunc(mNativePointer, nodeClonedFunction != null); + public void setOnCloneNode(YogaNodeCloneFunction cloneYogaNodeFunction) { + mYogaNodeCloneFunction = cloneYogaNodeFunction; + jni_YGConfigSetHasCloneNodeFunc(mNativePointer, cloneYogaNodeFunction != null); } @DoNotStrip - public final void onNodeCloned( - YogaNode oldNode, YogaNode newNode, YogaNode parent, int childIndex) { - mNodeClonedFunction.onNodeCloned(oldNode, newNode, parent, childIndex); + private final YogaNode cloneNode(YogaNode oldNode, YogaNode parent, int childIndex) { + return mYogaNodeCloneFunction.cloneNode(oldNode, parent, childIndex); } } diff --git a/java/com/facebook/yoga/YogaNode.java b/java/com/facebook/yoga/YogaNode.java index 1c6e67a5..577ee3fb 100644 --- a/java/com/facebook/yoga/YogaNode.java +++ b/java/com/facebook/yoga/YogaNode.java @@ -697,4 +697,18 @@ public class YogaNode implements Cloneable { public void print() { jni_YGNodePrint(mNativePointer); } + + /** + * This method replaces the child at childIndex position with the newNode received by parameter. + * This is different than calling removeChildAt and addChildAt because this method ONLY replaces + * the child in the mChildren datastructure. @DoNotStrip: called from JNI + * + * @return the nativePointer of the newNode {@linl YogaNode} + */ + @DoNotStrip + private final long replaceChild(YogaNode newNode, int childIndex) { + mChildren.remove(childIndex); + mChildren.add(childIndex, newNode); + return newNode.mNativePointer; + } } diff --git a/java/com/facebook/yoga/YogaNodeClonedFunction.java b/java/com/facebook/yoga/YogaNodeCloneFunction.java similarity index 69% rename from java/com/facebook/yoga/YogaNodeClonedFunction.java rename to java/com/facebook/yoga/YogaNodeCloneFunction.java index f4695464..0733776a 100644 --- a/java/com/facebook/yoga/YogaNodeClonedFunction.java +++ b/java/com/facebook/yoga/YogaNodeCloneFunction.java @@ -10,8 +10,8 @@ package com.facebook.yoga; import com.facebook.proguard.annotations.DoNotStrip; @DoNotStrip -public interface YogaNodeClonedFunction { +public interface YogaNodeCloneFunction { @DoNotStrip - void onNodeCloned(YogaNode oldNode, YogaNode newNode, YogaNode parent, int childIndex); + YogaNode cloneNode(YogaNode oldNode, YogaNode parent, int childIndex); } diff --git a/java/jni/YGJNI.cpp b/java/jni/YGJNI.cpp index cc75df96..7f92ba8e 100644 --- a/java/jni/YGJNI.cpp +++ b/java/jni/YGJNI.cpp @@ -142,31 +142,49 @@ static float YGJNIBaselineFunc(YGNodeRef node, float width, float height) { } } -static void YGJNIOnNodeClonedFunc( +static inline YGNodeRef _jlong2YGNodeRef(jlong addr) { + return reinterpret_cast(static_cast(addr)); +} + +static inline YGConfigRef _jlong2YGConfigRef(jlong addr) { + return reinterpret_cast(static_cast(addr)); +} + +static YGNodeRef YGJNIOnNodeClonedFunc( YGNodeRef oldNode, - YGNodeRef newNode, YGNodeRef parent, int childIndex) { auto config = oldNode->getConfig(); if (!config) { - return; + return nullptr; } + static auto onNodeClonedFunc = findClassStatic("com/facebook/yoga/YogaConfig") - ->getMethodgetMethod( local_ref, local_ref, - local_ref, - jint)>("onNodeCloned"); + jint)>("cloneNode"); auto context = reinterpret_cast(YGConfigGetContext(config)); auto javaConfig = context->config; - onNodeClonedFunc( + auto newNode = onNodeClonedFunc( javaConfig->get(), YGNodeJobject(oldNode)->lockLocal(), - YGNodeJobject(newNode)->lockLocal(), YGNodeJobject(parent)->lockLocal(), childIndex); + + static auto replaceChild = findClassStatic("com/facebook/yoga/YogaNode") + ->getMethod, + jint)>("replaceChild"); + + jlong newNodeNativePointer = replaceChild( + YGNodeJobject(parent)->lockLocal(), + newNode, + childIndex); + + return _jlong2YGNodeRef(newNodeNativePointer); } static YGSize YGJNIMeasureFunc( @@ -234,14 +252,6 @@ static int YGJNILogFunc(const YGConfigRef config, return result; } -static inline YGNodeRef _jlong2YGNodeRef(jlong addr) { - return reinterpret_cast(static_cast(addr)); -} - -static inline YGConfigRef _jlong2YGConfigRef(jlong addr) { - return reinterpret_cast(static_cast(addr)); -} - jlong jni_YGNodeNew(alias_ref thiz) { const YGNodeRef node = YGNodeNew(); node->setContext(new weak_ref(make_weak(thiz))); @@ -506,10 +516,10 @@ void jni_YGConfigSetUseLegacyStretchBehaviour(alias_ref, YGConfigSetUseLegacyStretchBehaviour(config, useLegacyStretchBehaviour); } -void jni_YGConfigSetHasNodeClonedFunc( +void jni_YGConfigSetHasCloneNodeFunc( alias_ref thiz, jlong nativePointer, - jboolean hasNodeClonedFunc) { + jboolean hasCloneNodeFunc) { const YGConfigRef config = _jlong2YGConfigRef(nativePointer); auto context = reinterpret_cast(YGConfigGetContext(config)); if (context && context->config) { @@ -517,15 +527,15 @@ void jni_YGConfigSetHasNodeClonedFunc( context->config = nullptr; } - if (hasNodeClonedFunc) { + if (hasCloneNodeFunc) { if (!context) { context = new YGConfigContext(); YGConfigSetContext(config, context); } context->config = new global_ref(make_global(thiz)); - YGConfigSetNodeClonedFunc(config, YGJNIOnNodeClonedFunc); + YGConfigSetCloneNodeFunc(config, YGJNIOnNodeClonedFunc); } else { - YGConfigSetNodeClonedFunc(config, nullptr); + YGConfigSetCloneNodeFunc(config, nullptr); } } @@ -652,7 +662,7 @@ jint JNI_OnLoad(JavaVM *vm, void *) { YGMakeNativeMethod(jni_YGConfigSetPointScaleFactor), YGMakeNativeMethod(jni_YGConfigSetUseLegacyStretchBehaviour), YGMakeNativeMethod(jni_YGConfigSetLogger), - YGMakeNativeMethod(jni_YGConfigSetHasNodeClonedFunc), + YGMakeNativeMethod(jni_YGConfigSetHasCloneNodeFunc), YGMakeNativeMethod( jni_YGConfigSetShouldDiffLayoutWithoutLegacyStretchBehaviour), }); diff --git a/java/tests/com/facebook/yoga/YogaNodeTest.java b/java/tests/com/facebook/yoga/YogaNodeTest.java index 1eae117f..49c26d46 100644 --- a/java/tests/com/facebook/yoga/YogaNodeTest.java +++ b/java/tests/com/facebook/yoga/YogaNodeTest.java @@ -255,12 +255,17 @@ public class YogaNodeTest { public void testCloneNodeListener() throws Exception { final AtomicBoolean onNodeClonedExecuted = new AtomicBoolean(false); YogaConfig config = new YogaConfig(); - config.setOnNodeCloned( - new YogaNodeClonedFunction() { + config.setOnCloneNode( + new YogaNodeCloneFunction() { @Override - public void onNodeCloned( - YogaNode oldNode, YogaNode newNode, YogaNode parent, int childIndex) { - onNodeClonedExecuted.set(true); + public YogaNode cloneNode(YogaNode oldNode, YogaNode parent, int childIndex) { + try { + onNodeClonedExecuted.set(true); + return oldNode.clone(); + } catch (CloneNotSupportedException ex) { + // DO nothing + return null; + } } }); YogaNode root = new YogaNode(config); @@ -268,6 +273,7 @@ public class YogaNodeTest { root.setHeight(100f); YogaNode child0 = new YogaNode(config); root.addChildAt(child0, 0); + child0.setWidth(50f); root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED); // Force a clone to happen. @@ -276,20 +282,29 @@ public class YogaNodeTest { root2.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED); assertTrue(onNodeClonedExecuted.get()); + assertEquals(1, root2.getChildCount()); + YogaNode clonedNode = root2.getChildAt(0); + assertNotSame(child0, clonedNode); + assertEquals(child0.getWidth(), clonedNode.getWidth()); + assertEquals(50f, clonedNode.getWidth().value, 0.01f); } @Test public void testOnNodeClonedLeak() throws Exception { YogaConfig config = new YogaConfig(); - config.setOnNodeCloned( - new YogaNodeClonedFunction() { + config.setOnCloneNode( + new YogaNodeCloneFunction() { @Override - public void onNodeCloned( - YogaNode oldNode, YogaNode newNode, YogaNode parent, int childIndex) { - // Do nothing + public YogaNode cloneNode(YogaNode oldNode, YogaNode parent, int childIndex) { + try { + return oldNode.clone(); + } catch (CloneNotSupportedException ex) { + // DO nothing + return null; + } } }); - config.setOnNodeCloned(null); + config.setOnCloneNode(null); WeakReference ref = new WeakReference(config); // noinspection UnusedAssignment config = null; diff --git a/yoga/YGNode.cpp b/yoga/YGNode.cpp index 8fcb44c3..f15105f2 100644 --- a/yoga/YGNode.cpp +++ b/yoga/YGNode.cpp @@ -558,15 +558,18 @@ void YGNode::cloneChildrenIfNeeded() { return; } - const YGNodeClonedFunc cloneNodeCallback = config_->cloneNodeCallback; + const YGCloneNodeFunc cloneNodeCallback = config_->cloneNodeCallback; for (uint32_t i = 0; i < childCount; ++i) { const YGNodeRef oldChild = children_[i]; - const YGNodeRef newChild = YGNodeClone(oldChild); + YGNodeRef newChild = nullptr; + if (cloneNodeCallback) { + newChild = cloneNodeCallback(oldChild, this, i); + } + if (newChild == nullptr) { + newChild = YGNodeClone(oldChild); + } replaceChild(newChild, i); newChild->setParent(this); - if (cloneNodeCallback) { - cloneNodeCallback(oldChild, newChild, this, i); - } } } diff --git a/yoga/Yoga-internal.h b/yoga/Yoga-internal.h index 56078d6c..d11f8c29 100644 --- a/yoga/Yoga-internal.h +++ b/yoga/Yoga-internal.h @@ -94,7 +94,7 @@ struct YGConfig { bool shouldDiffLayoutWithoutLegacyStretchBehaviour; float pointScaleFactor; YGLogger logger; - YGNodeClonedFunc cloneNodeCallback; + YGCloneNodeFunc cloneNodeCallback; void* context; }; diff --git a/yoga/Yoga.cpp b/yoga/Yoga.cpp index 1600b1b0..b597727b 100644 --- a/yoga/Yoga.cpp +++ b/yoga/Yoga.cpp @@ -428,7 +428,7 @@ void YGNodeRemoveChild(const YGNodeRef parent, const YGNodeRef excludedChild) { // 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. - const YGNodeClonedFunc cloneNodeCallback = + const YGCloneNodeFunc cloneNodeCallback = parent->getConfig()->cloneNodeCallback; uint32_t nextInsertIndex = 0; for (uint32_t i = 0; i < childCount; i++) { @@ -440,12 +440,16 @@ void YGNodeRemoveChild(const YGNodeRef parent, const YGNodeRef excludedChild) { parent->markDirtyAndPropogate(); continue; } - const YGNodeRef newChild = YGNodeClone(oldChild); + YGNodeRef newChild = nullptr; + if (cloneNodeCallback) { + newChild = cloneNodeCallback(oldChild, parent, nextInsertIndex); + } + if (newChild == nullptr) { + newChild = YGNodeClone(oldChild); + } parent->replaceChild(newChild, nextInsertIndex); newChild->setParent(parent); - if (cloneNodeCallback) { - cloneNodeCallback(oldChild, newChild, parent, nextInsertIndex); - } + nextInsertIndex++; } while (nextInsertIndex < childCount) { @@ -3964,7 +3968,7 @@ void *YGConfigGetContext(const YGConfigRef config) { return config->context; } -void YGConfigSetNodeClonedFunc(const YGConfigRef config, const YGNodeClonedFunc callback) { +void YGConfigSetCloneNodeFunc(const YGConfigRef config, const YGCloneNodeFunc callback) { config->cloneNodeCallback = callback; } diff --git a/yoga/Yoga.h b/yoga/Yoga.h index 1a985fa5..73e7c7fd 100644 --- a/yoga/Yoga.h +++ b/yoga/Yoga.h @@ -62,8 +62,7 @@ typedef int (*YGLogger)(const YGConfigRef config, YGLogLevel level, const char *format, va_list args); -typedef void (*YGNodeClonedFunc)(YGNodeRef oldNode, - YGNodeRef newNode, +typedef YGNodeRef (*YGCloneNodeFunc)(YGNodeRef oldNode, YGNodeRef parent, int childIndex); @@ -283,8 +282,8 @@ WIN_EXPORT bool YGConfigIsExperimentalFeatureEnabled(const YGConfigRef config, WIN_EXPORT void YGConfigSetUseWebDefaults(const YGConfigRef config, const bool enabled); WIN_EXPORT bool YGConfigGetUseWebDefaults(const YGConfigRef config); -WIN_EXPORT void YGConfigSetNodeClonedFunc(const YGConfigRef config, - const YGNodeClonedFunc callback); +WIN_EXPORT void YGConfigSetCloneNodeFunc(const YGConfigRef config, + const YGCloneNodeFunc callback); // Export only for C# WIN_EXPORT YGConfigRef YGConfigGetDefault(void);