Create YogaValue instances in Java, not C++

davidaurelio · facebook-github-bot · commit ca46c67e9efa · 2019-03-22T10:36:55.000-07:00
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
diff --git 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
@@ -349,7 +349,7 @@ public void setFlexShrink(float flexShrink) {
   }
 
   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 void setFlexBasisAuto() {
   }
 
   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 void setMarginAuto(YogaEdge edge) {
   }
 
   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 void setBorder(YogaEdge edge, float border) {
   }
 
   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 void setPositionPercent(YogaEdge edge, float percent) {
   }
 
   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 void setWidthAuto() {
   }
 
   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 void setHeightAuto() {
   }
 
   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 void setMinWidthPercent(float percent) {
   }
 
   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 void setMinHeightPercent(float percent) {
   }
 
   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 void setMaxWidthPercent(float percent) {
   }
 
   public YogaValue getMaxHeight() {
-    return (YogaValue) YogaNative.jni_YGNodeStyleGetMaxHeight(mNativePointer);
+    return valueFromLong(YogaNative.jni_YGNodeStyleGetMaxHeight(mNativePointer));
   }
 
   public void setMaxHeight(float maxheight) {
@@ -656,4 +656,8 @@ private final long replaceChild(YogaNodeJNIBase newNode, int childIndex) {
     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
@@ -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 YogaValue(float value, YogaUnit unit) {
     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
@@ -9,6 +9,7 @@
 #include <yoga/Yoga.h>
 #include <yoga/log.h>
 #include <cstdint>
+#include <cstring>
 #include <iostream>
 #include <map>
 
@@ -128,6 +129,19 @@ class YGNodeEdges {
   }
 };
 
+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<JYogaNode> YGNodeJobject(
@@ -472,9 +486,8 @@ void jni_YGNodeCopyStyle(jlong dstNativePointer, jlong srcNativePointer) {
   }
 
 #define YG_NODE_JNI_STYLE_UNIT_PROP(name)                                     \
-  local_ref<jobject> jni_YGNodeStyleGet##name(                                \
-      alias_ref<jclass>, jlong nativePointer) {                               \
-    return JYogaValue::create(                                                \
+  jlong jni_YGNodeStyleGet##name(alias_ref<jclass>, 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<jobject> jni_YGNodeStyleGet##name(                        \
+  jlong jni_YGNodeStyleGet##name(                                     \
       alias_ref<jclass>, jlong nativePointer, jint edge) {            \
-    return JYogaValue::create(YGNodeStyleGet##name(                   \
+    return YogaValue::asJavaLong(YGNodeStyleGet##name(                \
         _jlong2YGNodeRef(nativePointer), static_cast<YGEdge>(edge))); \
   }                                                                   \
                                                                       \
@@ -841,15 +854,15 @@ jint jni_YGNodeGetInstanceCount() {
   return YGNodeGetInstanceCount();
 }
 
-local_ref<jobject> jni_YGNodeStyleGetMargin(
+jlong jni_YGNodeStyleGetMargin(
     alias_ref<jclass>,
     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<YGEdge>(edge)));
 }
 
@@ -876,15 +889,15 @@ void jni_YGNodeStyleSetMarginAuto(jlong nativePointer, jint edge) {
   YGNodeStyleSetMarginAuto(yogaNodeRef, static_cast<YGEdge>(edge));
 }
 
-local_ref<jobject> jni_YGNodeStyleGetPadding(
+jlong jni_YGNodeStyleGetPadding(
     alias_ref<jclass>,
     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<YGEdge>(edge)));
 }
 
diff --git a/java/jni/YGJTypes.h b/java/jni/YGJTypes.h
@@ -28,11 +28,3 @@ struct JYogaLogger : public facebook::jni::JavaClass<JYogaLogger> {
       facebook::jni::alias_ref<JYogaLogLevel>,
       jstring);
 };
-
-struct JYogaValue : public facebook::jni::JavaClass<JYogaValue> {
-  constexpr static auto kJavaDescriptor = "Lcom/facebook/yoga/YogaValue;";
-
-  static facebook::jni::local_ref<javaobject> create(YGValue value) {
-    return newInstance(value.value, static_cast<int>(value.unit));
-  }
-};
diff --git 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 void testMarginAssignment() {
     }
   }
 
+  @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();

Original file line number	Diff line number	Diff line change
`@@ -349,7 +349,7 @@ public void setFlexShrink(float flexShrink) {`
`349`	`349`	`}`
`350`	`350`
`351`	`351`	`public YogaValue getFlexBasis() {`
`352`		`- return (YogaValue) YogaNative.jni_YGNodeStyleGetFlexBasis(mNativePointer);`
	`352`	`+ return valueFromLong(YogaNative.jni_YGNodeStyleGetFlexBasis(mNativePointer));`
`353`	`353`	`}`
`354`	`354`
`355`	`355`	`public void setFlexBasis(float flexBasis) {`
`@@ -365,7 +365,7 @@ public void setFlexBasisAuto() {`
`365`	`365`	`}`
`366`	`366`
`367`	`367`	`public YogaValue getMargin(YogaEdge edge) {`
`368`		`- return (YogaValue) YogaNative.jni_YGNodeStyleGetMargin(mNativePointer, edge.intValue());`
	`368`	`+ return valueFromLong(YogaNative.jni_YGNodeStyleGetMargin(mNativePointer, edge.intValue()));`
`369`	`369`	`}`
`370`	`370`
`371`	`371`	`public void setMargin(YogaEdge edge, float margin) {`
`@@ -381,7 +381,7 @@ public void setMarginAuto(YogaEdge edge) {`
`381`	`381`	`}`
`382`	`382`
`383`	`383`	`public YogaValue getPadding(YogaEdge edge) {`
`384`		`- return (YogaValue) YogaNative.jni_YGNodeStyleGetPadding(mNativePointer, edge.intValue());`
	`384`	`+ return valueFromLong(YogaNative.jni_YGNodeStyleGetPadding(mNativePointer, edge.intValue()));`
`385`	`385`	`}`
`386`	`386`
`387`	`387`	`public void setPadding(YogaEdge edge, float padding) {`
`@@ -401,7 +401,7 @@ public void setBorder(YogaEdge edge, float border) {`
`401`	`401`	`}`
`402`	`402`
`403`	`403`	`public YogaValue getPosition(YogaEdge edge) {`
`404`		`- return (YogaValue) YogaNative.jni_YGNodeStyleGetPosition(mNativePointer, edge.intValue());`
	`404`	`+ return valueFromLong(YogaNative.jni_YGNodeStyleGetPosition(mNativePointer, edge.intValue()));`
`405`	`405`	`}`
`406`	`406`
`407`	`407`	`public void setPosition(YogaEdge edge, float position) {`
`@@ -413,7 +413,7 @@ public void setPositionPercent(YogaEdge edge, float percent) {`
`413`	`413`	`}`
`414`	`414`
`415`	`415`	`public YogaValue getWidth() {`
`416`		`- return (YogaValue) YogaNative.jni_YGNodeStyleGetWidth(mNativePointer);`
	`416`	`+ return valueFromLong(YogaNative.jni_YGNodeStyleGetWidth(mNativePointer));`
`417`	`417`	`}`
`418`	`418`
`419`	`419`	`public void setWidth(float width) {`
`@@ -429,7 +429,7 @@ public void setWidthAuto() {`
`429`	`429`	`}`
`430`	`430`
`431`	`431`	`public YogaValue getHeight() {`
`432`		`- return (YogaValue) YogaNative.jni_YGNodeStyleGetHeight(mNativePointer);`
	`432`	`+ return valueFromLong(YogaNative.jni_YGNodeStyleGetHeight(mNativePointer));`
`433`	`433`	`}`
`434`	`434`
`435`	`435`	`public void setHeight(float height) {`
`@@ -445,7 +445,7 @@ public void setHeightAuto() {`
`445`	`445`	`}`
`446`	`446`
`447`	`447`	`public YogaValue getMinWidth() {`
`448`		`- return (YogaValue) YogaNative.jni_YGNodeStyleGetMinWidth(mNativePointer);`
	`448`	`+ return valueFromLong(YogaNative.jni_YGNodeStyleGetMinWidth(mNativePointer));`
`449`	`449`	`}`
`450`	`450`
`451`	`451`	`public void setMinWidth(float minWidth) {`
`@@ -457,7 +457,7 @@ public void setMinWidthPercent(float percent) {`
`457`	`457`	`}`
`458`	`458`
`459`	`459`	`public YogaValue getMinHeight() {`
`460`		`- return (YogaValue) YogaNative.jni_YGNodeStyleGetMinHeight(mNativePointer);`
	`460`	`+ return valueFromLong(YogaNative.jni_YGNodeStyleGetMinHeight(mNativePointer));`
`461`	`461`	`}`
`462`	`462`
`463`	`463`	`public void setMinHeight(float minHeight) {`
`@@ -469,7 +469,7 @@ public void setMinHeightPercent(float percent) {`
`469`	`469`	`}`
`470`	`470`
`471`	`471`	`public YogaValue getMaxWidth() {`
`472`		`- return (YogaValue) YogaNative.jni_YGNodeStyleGetMaxWidth(mNativePointer);`
	`472`	`+ return valueFromLong(YogaNative.jni_YGNodeStyleGetMaxWidth(mNativePointer));`
`473`	`473`	`}`
`474`	`474`
`475`	`475`	`public void setMaxWidth(float maxWidth) {`
`@@ -481,7 +481,7 @@ public void setMaxWidthPercent(float percent) {`
`481`	`481`	`}`
`482`	`482`
`483`	`483`	`public YogaValue getMaxHeight() {`
`484`		`- return (YogaValue) YogaNative.jni_YGNodeStyleGetMaxHeight(mNativePointer);`
	`484`	`+ return valueFromLong(YogaNative.jni_YGNodeStyleGetMaxHeight(mNativePointer));`
`485`	`485`	`}`
`486`	`486`
`487`	`487`	`public void setMaxHeight(float maxheight) {`
`@@ -656,4 +656,8 @@ private final long replaceChild(YogaNodeJNIBase newNode, int childIndex) {`
`656`	`656`	`newNode.mOwner = this;`
`657`	`657`	`return newNode.mNativePointer;`
`658`	`658`	`}`
	`659`	`+`
	`660`	`+ private static YogaValue valueFromLong(long raw) {`
	`661`	`+ return new YogaValue(Float.intBitsToFloat((int) raw), (int) (raw >> 32));`
	`662`	`+ }`
`659`	`663`	`}`