From ca46c67e9efa5343bf524096d332be170fe8991a Mon Sep 17 00:00:00 2001 From: David Aurelio Date: Fri, 22 Mar 2019 10:31:18 -0700 Subject: [PATCH] Create `YogaValue` instances in Java, not C++ Summary: @public Passing primitive data via JNI is more efficient than passing objects. Here, we avoid creating `YogaValue` (Java) instances via JNI, and rather pass a `long` back to Java. The instance is then created by extracting the necessary bytes on the Java side. Reviewed By: foghina Differential Revision: D14576755 fbshipit-source-id: 22d09ad50c3ac6c49b0a797a0dad639ea4829df9 --- java/com/facebook/yoga/YogaNative.java | 20 +++++------ java/com/facebook/yoga/YogaNodeJNIBase.java | 24 +++++++------ java/com/facebook/yoga/YogaValue.java | 4 --- java/jni/YGJNI.cpp | 35 +++++++++++++------ java/jni/YGJTypes.h | 8 ----- .../yoga/YogaNodeStylePropertiesTest.java | 24 ++++++++++--- 6 files changed, 67 insertions(+), 48 deletions(-) diff --git a/java/com/facebook/yoga/YogaNative.java b/java/com/facebook/yoga/YogaNative.java index 65ded834..ef0b44ee 100644 --- a/java/com/facebook/yoga/YogaNative.java +++ b/java/com/facebook/yoga/YogaNative.java @@ -69,40 +69,40 @@ public class YogaNative { static native void jni_YGNodeStyleSetFlexGrow(long nativePointer, float flexGrow); static native float jni_YGNodeStyleGetFlexShrink(long nativePointer); static native void jni_YGNodeStyleSetFlexShrink(long nativePointer, float flexShrink); - static native Object jni_YGNodeStyleGetFlexBasis(long nativePointer); + static native long jni_YGNodeStyleGetFlexBasis(long nativePointer); static native void jni_YGNodeStyleSetFlexBasis(long nativePointer, float flexBasis); static native void jni_YGNodeStyleSetFlexBasisPercent(long nativePointer, float percent); static native void jni_YGNodeStyleSetFlexBasisAuto(long nativePointer); - static native Object jni_YGNodeStyleGetMargin(long nativePointer, int edge); + static native long jni_YGNodeStyleGetMargin(long nativePointer, int edge); static native void jni_YGNodeStyleSetMargin(long nativePointer, int edge, float margin); static native void jni_YGNodeStyleSetMarginPercent(long nativePointer, int edge, float percent); static native void jni_YGNodeStyleSetMarginAuto(long nativePointer, int edge); - static native Object jni_YGNodeStyleGetPadding(long nativePointer, int edge); + static native long jni_YGNodeStyleGetPadding(long nativePointer, int edge); static native void jni_YGNodeStyleSetPadding(long nativePointer, int edge, float padding); static native void jni_YGNodeStyleSetPaddingPercent(long nativePointer, int edge, float percent); static native float jni_YGNodeStyleGetBorder(long nativePointer, int edge); static native void jni_YGNodeStyleSetBorder(long nativePointer, int edge, float border); - static native Object jni_YGNodeStyleGetPosition(long nativePointer, int edge); + static native long jni_YGNodeStyleGetPosition(long nativePointer, int edge); static native void jni_YGNodeStyleSetPosition(long nativePointer, int edge, float position); static native void jni_YGNodeStyleSetPositionPercent(long nativePointer, int edge, float percent); - static native Object jni_YGNodeStyleGetWidth(long nativePointer); + static native long jni_YGNodeStyleGetWidth(long nativePointer); static native void jni_YGNodeStyleSetWidth(long nativePointer, float width); static native void jni_YGNodeStyleSetWidthPercent(long nativePointer, float percent); static native void jni_YGNodeStyleSetWidthAuto(long nativePointer); - static native Object jni_YGNodeStyleGetHeight(long nativePointer); + static native long jni_YGNodeStyleGetHeight(long nativePointer); static native void jni_YGNodeStyleSetHeight(long nativePointer, float height); static native void jni_YGNodeStyleSetHeightPercent(long nativePointer, float percent); static native void jni_YGNodeStyleSetHeightAuto(long nativePointer); - static native Object jni_YGNodeStyleGetMinWidth(long nativePointer); + static native long jni_YGNodeStyleGetMinWidth(long nativePointer); static native void jni_YGNodeStyleSetMinWidth(long nativePointer, float minWidth); static native void jni_YGNodeStyleSetMinWidthPercent(long nativePointer, float percent); - static native Object jni_YGNodeStyleGetMinHeight(long nativePointer); + static native long jni_YGNodeStyleGetMinHeight(long nativePointer); static native void jni_YGNodeStyleSetMinHeight(long nativePointer, float minHeight); static native void jni_YGNodeStyleSetMinHeightPercent(long nativePointer, float percent); - static native Object jni_YGNodeStyleGetMaxWidth(long nativePointer); + static native long jni_YGNodeStyleGetMaxWidth(long nativePointer); static native void jni_YGNodeStyleSetMaxWidth(long nativePointer, float maxWidth); static native void jni_YGNodeStyleSetMaxWidthPercent(long nativePointer, float percent); - static native Object jni_YGNodeStyleGetMaxHeight(long nativePointer); + static native long jni_YGNodeStyleGetMaxHeight(long nativePointer); static native void jni_YGNodeStyleSetMaxHeight(long nativePointer, float maxheight); static native void jni_YGNodeStyleSetMaxHeightPercent(long nativePointer, float percent); static native float jni_YGNodeStyleGetAspectRatio(long nativePointer); diff --git a/java/com/facebook/yoga/YogaNodeJNIBase.java b/java/com/facebook/yoga/YogaNodeJNIBase.java index 4ec53d80..4981ebd1 100644 --- a/java/com/facebook/yoga/YogaNodeJNIBase.java +++ b/java/com/facebook/yoga/YogaNodeJNIBase.java @@ -349,7 +349,7 @@ public abstract class YogaNodeJNIBase extends YogaNode { } public YogaValue getFlexBasis() { - return (YogaValue) YogaNative.jni_YGNodeStyleGetFlexBasis(mNativePointer); + return valueFromLong(YogaNative.jni_YGNodeStyleGetFlexBasis(mNativePointer)); } public void setFlexBasis(float flexBasis) { @@ -365,7 +365,7 @@ public abstract class YogaNodeJNIBase extends YogaNode { } public YogaValue getMargin(YogaEdge edge) { - return (YogaValue) YogaNative.jni_YGNodeStyleGetMargin(mNativePointer, edge.intValue()); + return valueFromLong(YogaNative.jni_YGNodeStyleGetMargin(mNativePointer, edge.intValue())); } public void setMargin(YogaEdge edge, float margin) { @@ -381,7 +381,7 @@ public abstract class YogaNodeJNIBase extends YogaNode { } public YogaValue getPadding(YogaEdge edge) { - return (YogaValue) YogaNative.jni_YGNodeStyleGetPadding(mNativePointer, edge.intValue()); + return valueFromLong(YogaNative.jni_YGNodeStyleGetPadding(mNativePointer, edge.intValue())); } public void setPadding(YogaEdge edge, float padding) { @@ -401,7 +401,7 @@ public abstract class YogaNodeJNIBase extends YogaNode { } public YogaValue getPosition(YogaEdge edge) { - return (YogaValue) YogaNative.jni_YGNodeStyleGetPosition(mNativePointer, edge.intValue()); + return valueFromLong(YogaNative.jni_YGNodeStyleGetPosition(mNativePointer, edge.intValue())); } public void setPosition(YogaEdge edge, float position) { @@ -413,7 +413,7 @@ public abstract class YogaNodeJNIBase extends YogaNode { } public YogaValue getWidth() { - return (YogaValue) YogaNative.jni_YGNodeStyleGetWidth(mNativePointer); + return valueFromLong(YogaNative.jni_YGNodeStyleGetWidth(mNativePointer)); } public void setWidth(float width) { @@ -429,7 +429,7 @@ public abstract class YogaNodeJNIBase extends YogaNode { } public YogaValue getHeight() { - return (YogaValue) YogaNative.jni_YGNodeStyleGetHeight(mNativePointer); + return valueFromLong(YogaNative.jni_YGNodeStyleGetHeight(mNativePointer)); } public void setHeight(float height) { @@ -445,7 +445,7 @@ public abstract class YogaNodeJNIBase extends YogaNode { } public YogaValue getMinWidth() { - return (YogaValue) YogaNative.jni_YGNodeStyleGetMinWidth(mNativePointer); + return valueFromLong(YogaNative.jni_YGNodeStyleGetMinWidth(mNativePointer)); } public void setMinWidth(float minWidth) { @@ -457,7 +457,7 @@ public abstract class YogaNodeJNIBase extends YogaNode { } public YogaValue getMinHeight() { - return (YogaValue) YogaNative.jni_YGNodeStyleGetMinHeight(mNativePointer); + return valueFromLong(YogaNative.jni_YGNodeStyleGetMinHeight(mNativePointer)); } public void setMinHeight(float minHeight) { @@ -469,7 +469,7 @@ public abstract class YogaNodeJNIBase extends YogaNode { } public YogaValue getMaxWidth() { - return (YogaValue) YogaNative.jni_YGNodeStyleGetMaxWidth(mNativePointer); + return valueFromLong(YogaNative.jni_YGNodeStyleGetMaxWidth(mNativePointer)); } public void setMaxWidth(float maxWidth) { @@ -481,7 +481,7 @@ public abstract class YogaNodeJNIBase extends YogaNode { } public YogaValue getMaxHeight() { - return (YogaValue) YogaNative.jni_YGNodeStyleGetMaxHeight(mNativePointer); + return valueFromLong(YogaNative.jni_YGNodeStyleGetMaxHeight(mNativePointer)); } public void setMaxHeight(float maxheight) { @@ -656,4 +656,8 @@ public abstract class YogaNodeJNIBase extends YogaNode { newNode.mOwner = this; return newNode.mNativePointer; } + + private static YogaValue valueFromLong(long raw) { + return new YogaValue(Float.intBitsToFloat((int) raw), (int) (raw >> 32)); + } } diff --git a/java/com/facebook/yoga/YogaValue.java b/java/com/facebook/yoga/YogaValue.java index 6f234609..9b012bfd 100644 --- a/java/com/facebook/yoga/YogaValue.java +++ b/java/com/facebook/yoga/YogaValue.java @@ -6,9 +6,6 @@ */ package com.facebook.yoga; -import com.facebook.proguard.annotations.DoNotStrip; - -@DoNotStrip public class YogaValue { static final YogaValue UNDEFINED = new YogaValue(YogaConstants.UNDEFINED, YogaUnit.UNDEFINED); static final YogaValue ZERO = new YogaValue(0, YogaUnit.POINT); @@ -22,7 +19,6 @@ public class YogaValue { this.unit = unit; } - @DoNotStrip YogaValue(float value, int unit) { this(value, YogaUnit.fromInt(unit)); } diff --git a/java/jni/YGJNI.cpp b/java/jni/YGJNI.cpp index c001ce28..15e5635c 100644 --- a/java/jni/YGJNI.cpp +++ b/java/jni/YGJNI.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -128,6 +129,19 @@ public: } }; +struct YogaValue { + static constexpr jint NAN_BYTES = 0x7fc00000; + + static jlong asJavaLong(const YGValue& value) { + uint32_t valueBytes = 0; + memcpy(&valueBytes, &value.value, sizeof valueBytes); + return ((jlong) value.unit) << 32 | valueBytes; + } + constexpr static jlong undefinedAsJavaLong() { + return ((jlong) YGUnitUndefined) << 32 | NAN_BYTES; + } +}; + } // namespace static inline local_ref YGNodeJobject( @@ -472,9 +486,8 @@ void jni_YGNodeCopyStyle(jlong dstNativePointer, jlong srcNativePointer) { } #define YG_NODE_JNI_STYLE_UNIT_PROP(name) \ - local_ref jni_YGNodeStyleGet##name( \ - alias_ref, jlong nativePointer) { \ - return JYogaValue::create( \ + jlong jni_YGNodeStyleGet##name(alias_ref, jlong nativePointer) { \ + return YogaValue::asJavaLong( \ YGNodeStyleGet##name(_jlong2YGNodeRef(nativePointer))); \ } \ \ @@ -509,9 +522,9 @@ void jni_YGNodeCopyStyle(jlong dstNativePointer, jlong srcNativePointer) { } #define YG_NODE_JNI_STYLE_EDGE_UNIT_PROP(name) \ - local_ref jni_YGNodeStyleGet##name( \ + jlong jni_YGNodeStyleGet##name( \ alias_ref, jlong nativePointer, jint edge) { \ - return JYogaValue::create(YGNodeStyleGet##name( \ + return YogaValue::asJavaLong(YGNodeStyleGet##name( \ _jlong2YGNodeRef(nativePointer), static_cast(edge))); \ } \ \ @@ -841,15 +854,15 @@ jint jni_YGNodeGetInstanceCount() { return YGNodeGetInstanceCount(); } -local_ref jni_YGNodeStyleGetMargin( +jlong jni_YGNodeStyleGetMargin( alias_ref, jlong nativePointer, jint edge) { YGNodeRef yogaNodeRef = _jlong2YGNodeRef(nativePointer); if (!YGNodeEdges{yogaNodeRef}.has(YGNodeEdges::MARGIN)) { - return JYogaValue::create(YGValueUndefined); + return YogaValue::undefinedAsJavaLong(); } - return JYogaValue::create( + return YogaValue::asJavaLong( YGNodeStyleGetMargin(yogaNodeRef, static_cast(edge))); } @@ -876,15 +889,15 @@ void jni_YGNodeStyleSetMarginAuto(jlong nativePointer, jint edge) { YGNodeStyleSetMarginAuto(yogaNodeRef, static_cast(edge)); } -local_ref jni_YGNodeStyleGetPadding( +jlong jni_YGNodeStyleGetPadding( alias_ref, jlong nativePointer, jint edge) { YGNodeRef yogaNodeRef = _jlong2YGNodeRef(nativePointer); if (!YGNodeEdges{yogaNodeRef}.has(YGNodeEdges::PADDING)) { - return JYogaValue::create(YGValueUndefined); + return YogaValue::undefinedAsJavaLong(); } - return JYogaValue::create( + return YogaValue::asJavaLong( YGNodeStyleGetPadding(yogaNodeRef, static_cast(edge))); } diff --git a/java/jni/YGJTypes.h b/java/jni/YGJTypes.h index 53f606f7..c2c5fc51 100644 --- a/java/jni/YGJTypes.h +++ b/java/jni/YGJTypes.h @@ -28,11 +28,3 @@ struct JYogaLogger : public facebook::jni::JavaClass { facebook::jni::alias_ref, jstring); }; - -struct JYogaValue : public facebook::jni::JavaClass { - constexpr static auto kJavaDescriptor = "Lcom/facebook/yoga/YogaValue;"; - - static facebook::jni::local_ref create(YGValue value) { - return newInstance(value.value, static_cast(value.unit)); - } -}; diff --git a/java/tests/com/facebook/yoga/YogaNodeStylePropertiesTest.java b/java/tests/com/facebook/yoga/YogaNodeStylePropertiesTest.java index 75ad6730..7cfb82a2 100644 --- a/java/tests/com/facebook/yoga/YogaNodeStylePropertiesTest.java +++ b/java/tests/com/facebook/yoga/YogaNodeStylePropertiesTest.java @@ -1,9 +1,8 @@ -/* - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the LICENSE - * file in the root directory of this source tree. +/** + * Copyright (c) Facebook, Inc. and its affiliates. * + * This source code is licensed under the MIT license found in the LICENSE + * file in the root directory of this source tree. */ package com.facebook.yoga; @@ -401,6 +400,21 @@ public class YogaNodeStylePropertiesTest { } } + @Test + public void testNegativeMarginAssignment() { + final YogaNode node = createNode(); + for (YogaEdge edge : YogaEdge.values()) { + node.setMargin(edge, -25); + assertEquals(new YogaValue(-25, YogaUnit.POINT), node.getMargin(edge)); + + node.setMarginPercent(edge, -5); + assertEquals(new YogaValue(-5, YogaUnit.PERCENT), node.getMargin(edge)); + + node.setMarginAuto(edge); + assertEquals(YogaValue.AUTO, node.getMargin(edge)); + } + } + @Test public void testMarginPointAffectsLayout() { final YogaNode node = style().margin(YogaEdge.TOP, 42).node();