BREAKING - Change measure() api to remove need for MeasureOutput allocation

Summary: This is an API breaking change done to allow us to avoid an allocation during measurement. Instead we do the same trick as is done when passing measure results to C, we path them into a long.

Reviewed By: splhack

Differential Revision: D4081037

fbshipit-source-id: 28adbcdd160cbd3f59a0fdd4b9f1200ae18678f1

csharp/Facebook.CSSLayout/CSSNode.cs

@@ -543,9 +543,8 @@ namespace Facebook.CSSLayout
                 throw new InvalidOperationException("Measure function is not defined.");
             }
 
-            _measureFunction(this, width, widthMode, height, heightMode, _measureOutput);
-
-            return new CSSSize { width = _measureOutput.Width, height = _measureOutput.Height };
+            long output = _measureFunction(this, width, widthMode, height, heightMode);
+            return new CSSSize { width = MeasureOutput.GetWidth(output), height = MeasureOutput.GetHeight(output) };
         }
 
         public string Print(CSSPrintOptions options =

csharp/Facebook.CSSLayout/MeasureFunction.cs

@@ -9,5 +9,10 @@
 
 namespace Facebook.CSSLayout
 {
-    public delegate void MeasureFunction(CSSNode node, float width, CSSMeasureMode widthMode, float height, CSSMeasureMode heightMode, MeasureOutput measureOutput);
+  public delegate long MeasureFunction(
+      CSSNode node,
+      float width,
+      CSSMeasureMode widthMode,
+      float height,
+      CSSMeasureMode heightMode);
 }

csharp/Facebook.CSSLayout/MeasureOutput.cs

@@ -1,4 +1,4 @@
-/**
+/**
  * Copyright (c) 2014-present, Facebook, Inc.
  * All rights reserved.
  *
@@ -11,7 +11,19 @@ namespace Facebook.CSSLayout
 {
     public class MeasureOutput
     {
-        public float Width { get; set; }
-        public float Height { get; set; }
+        public static long Make(int width, int height)
+        {
+          return ((long) width) << 32 | ((long) height);
+        }
+
+        public static int GetWidth(long measureOutput)
+        {
+          return (int) (0xFFFFFFFF & (measureOutput >> 32));
+        }
+
+        public static int GetHeight(long measureOutput)
+        {
+          return (int) (0xFFFFFFFF & measureOutput);
+        }
     }
 }

csharp/tests/Facebook.CSSLayout/CSSNodeTest.cs

@@ -23,18 +23,18 @@ namespace Facebook.CSSLayout
         {
             CSSNode parent = new CSSNode();
             CSSNode child = new CSSNode();
-    
+
             Assert.IsNull(child.Parent);
             Assert.AreEqual(0, parent.Count);
-    
+
             parent.Insert(0, child);
-    
+
             Assert.AreEqual(1, parent.Count);
             Assert.AreEqual(child, parent[0]);
             Assert.AreEqual(parent, child.Parent);
-    
+
             parent.RemoveAt(0);
-    
+
             Assert.IsNull(child.Parent);
             Assert.AreEqual(0, parent.Count);
         }
@@ -152,9 +152,8 @@ namespace Facebook.CSSLayout
         public void TestMeasureFunc()
         {
             CSSNode node = new CSSNode();
-            node.SetMeasureFunction((_, width, widthMode, height, heightMode, measureResult) => {
-                measureResult.Width = 100;
-                measureResult.Height = 150;
+            node.SetMeasureFunction((_, width, widthMode, height, heightMode) => {
+                return MeasureOutput.Make(100, 150);
             });
             node.CalculateLayout();
             Assert.AreEqual(100, (int)node.LayoutWidth);

java/com/facebook/csslayout/CSSNode.java

@@ -487,15 +487,12 @@ public class CSSNode implements CSSNodeAPI<CSSNode> {
       throw new RuntimeException("Measure function isn't defined!");
     }
 
-    MeasureOutput output = new MeasureOutput();
-    mMeasureFunction.measure(
+    return mMeasureFunction.measure(
           this,
           width,
           CSSMeasureMode.values()[widthMode],
           height,
-          CSSMeasureMode.values()[heightMode],
-          output);
-    return ((long) output.width) << 32 | ((long) output.height);
+          CSSMeasureMode.values()[heightMode]);
   }
 
   @Override

java/com/facebook/csslayout/CSSNodeAPI.java

@@ -12,13 +12,15 @@ package com.facebook.csslayout;
 public interface CSSNodeAPI<CSSNodeType extends CSSNodeAPI> {
 
   interface MeasureFunction {
-    void measure(
+    /**
+     * Return a value created by MeasureOutput.make(width, height);
+     */
+    long measure(
         CSSNodeAPI node,
         float width,
         CSSMeasureMode widthMode,
         float height,
-        CSSMeasureMode heightMode,
-        MeasureOutput measureOutput);
+        CSSMeasureMode heightMode);
   }
 
   int getChildCount();

java/com/facebook/csslayout/CSSNodeDEPRECATED.java

@@ -134,14 +134,11 @@ public class CSSNodeDEPRECATED implements CSSNodeAPI<CSSNodeDEPRECATED> {
     return mIsTextNode;
   }
 
-  MeasureOutput measure(MeasureOutput measureOutput, float width, CSSMeasureMode widthMode, float height, CSSMeasureMode heightMode) {
+  long measure(float width, CSSMeasureMode widthMode, float height, CSSMeasureMode heightMode) {
     if (!isMeasureDefined()) {
       throw new RuntimeException("Measure function isn't defined!");
     }
-    measureOutput.height = CSSConstants.UNDEFINED;
-    measureOutput.width = CSSConstants.UNDEFINED;
-    Assertions.assertNotNull(mMeasureFunction).measure(this, width, widthMode, height, heightMode, measureOutput);
-    return measureOutput;
+    return Assertions.assertNotNull(mMeasureFunction).measure(this, width, widthMode, height, heightMode);
   }
 
   /**

java/com/facebook/csslayout/LayoutEngine.java

@@ -560,22 +560,23 @@ public class LayoutEngine {
       } else {
 
         // Measure the text under the current constraints.
-        MeasureOutput measureDim = node.measure(
-
-          layoutContext.measureOutput,
+        long measureOutput = node.measure(
           innerWidth,
           widthMeasureMode,
           innerHeight,
           heightMeasureMode
         );
 
+        int outputWidth = MeasureOutput.getWidth(measureOutput);
+        int outputHeight = MeasureOutput.getHeight(measureOutput);
+
         node.layout.measuredDimensions[DIMENSION_WIDTH] = boundAxis(node, CSS_FLEX_DIRECTION_ROW,
           (widthMeasureMode == CSSMeasureMode.UNDEFINED || widthMeasureMode == CSSMeasureMode.AT_MOST) ?
-            measureDim.width + paddingAndBorderAxisRow :
+            outputWidth + paddingAndBorderAxisRow :
             availableWidth - marginAxisRow);
         node.layout.measuredDimensions[DIMENSION_HEIGHT] = boundAxis(node, CSS_FLEX_DIRECTION_COLUMN,
           (heightMeasureMode == CSSMeasureMode.UNDEFINED || heightMeasureMode == CSSMeasureMode.AT_MOST) ?
-            measureDim.height + paddingAndBorderAxisColumn :
+            outputHeight + paddingAndBorderAxisColumn :
             availableHeight - marginAxisColumn);
       }
 

java/com/facebook/csslayout/MeasureOutput.java

@@ -10,10 +10,23 @@
 package com.facebook.csslayout;
 
 /**
- * POJO to hold the output of the measure function.
+ * Helpers for building measure output value.
  */
 public class MeasureOutput {
 
-  public float width;
-  public float height;
+  public static long make(float width, float height) {
+    return make((int) width, (int) height);
+  }
+
+  public static long make(int width, int height) {
+    return ((long) width) << 32 | ((long) height);
+  }
+
+  public static int getWidth(long measureOutput) {
+    return (int) (0xFFFFFFFF & (measureOutput >> 32));
+  }
+
+  public static int getHeight(long measureOutput) {
+    return (int) (0xFFFFFFFF & measureOutput);
+  }
 }

java/tests/com/facebook/csslayout/CSSNodeTest.java

@@ -26,15 +26,13 @@ public class CSSNodeTest {
   public void testMeasure() {
     final CSSNode node = new CSSNode();
     node.setMeasureFunction(new CSSNodeAPI.MeasureFunction() {
-        public void measure(
+        public long measure(
             CSSNodeAPI node,
             float width,
             CSSMeasureMode widthMode,
             float height,
-            CSSMeasureMode heightMode,
-            MeasureOutput measureOutput) {
-          measureOutput.width = 100;
-          measureOutput.height = 100;
+            CSSMeasureMode heightMode) {
+          return MeasureOutput.make(100, 100);
         }
     });
     node.calculateLayout(null);