Call measure and baseline fns within YGNode

Summary:
@public

Stricter encapsulation of baseline and measure callbacks withing `YGNode`.

Instead of invoking these callbacks directly (`node->getBaseline()(...)`), they are invoked via methods on `YGNode` (`node->baseline(...)`).

This change will allow us to add the concept of a *Layout Context,* where measure and baseline functions will be able to receive an additional `void *` argument if configured accordingly. This API will be used internally for Yoga’s JNI bindings, to avoid storing a weak JNI reference for each node, and avoid reference table overflows.

Changed API:

- `YGNodeGetMeasureFunc()` -> `YGNodeHasMeasureFunc()`
- `YGNodeGetBaselineFunc()` -> `YGNodeHasBaselineFunc()`
- `YGNode::getMeasure()` -> `YGNode::hasMeasureFunc()` + `YGNode::measure()`
- `YGNpde::getBaseline()` -> `YGNode::hasBaselineFunc()` + `YGNode::baseline()`

Reviewed By: SidharthGuglani

Differential Revision: D14099550

fbshipit-source-id: 2653ab36acc252a9747986bc88d21dac22d8c91b

YogaKit/Source/YGLayout.m

@@ -180,7 +180,7 @@ static YGConfigRef globalConfig;
   // the measure function. Since we already know that this is a leaf,
   // this *should* be fine. Forgive me Hack Gods.
   const YGNodeRef node = self.node;
-  if (YGNodeGetMeasureFunc(node) == NULL) {
+  if (YGNodeHasMeasureFunc(node)) {
     YGNodeSetMeasureFunc(node, YGMeasureView);
   }
 

tests/YGMeasureTest.cpp

@@ -8,22 +8,25 @@
 #include <yoga/YGNode.h>
 #include <yoga/Yoga.h>
 
-static YGSize _measure(YGNodeRef node,
+static YGSize _measure(
+    YGNodeRef node,
     float width,
     YGMeasureMode widthMode,
     float height,
     YGMeasureMode heightMode) {
-  int* measureCount = (int*)node->getContext();
+  int* measureCount = (int*) node->getContext();
   if (measureCount) {
     (*measureCount)++;
   }
 
   return YGSize{
-      .width = 10, .height = 10,
+      .width = 10,
+      .height = 10,
   };
 }
 
-static YGSize _simulate_wrapping_text(YGNodeRef node,
+static YGSize _simulate_wrapping_text(
+    YGNodeRef node,
     float width,
     YGMeasureMode widthMode,
     float height,
@@ -33,11 +36,13 @@ static YGSize _simulate_wrapping_text(YGNodeRef node,
   }
 
   return YGSize{
-      .width = 50, .height = 32,
+      .width = 50,
+      .height = 32,
   };
 }
 
-static YGSize _measure_assert_negative(YGNodeRef node,
+static YGSize _measure_assert_negative(
+    YGNodeRef node,
     float width,
     YGMeasureMode widthMode,
     float height,
@@ -46,7 +51,8 @@ static YGSize _measure_assert_negative(YGNodeRef node,
   EXPECT_GE(height, 0);
 
   return YGSize{
-    .width = 0, .height = 0,
+      .width = 0,
+      .height = 0,
   };
 }
 
@@ -148,7 +154,6 @@ TEST(YogaTest, dont_measure_when_min_equals_max_percentages) {
   YGNodeFreeRecursive(root);
 }
 
-
 TEST(YogaTest, measure_nodes_with_margin_auto_and_stretch) {
   const YGNodeRef root = YGNodeNew();
   YGNodeStyleSetWidth(root, 500);
@@ -593,7 +598,7 @@ TEST(YogaTest, can_nullify_measure_func_on_any_node) {
   const YGNodeRef root = YGNodeNew();
   YGNodeInsertChild(root, YGNodeNew(), 0);
   root->setMeasureFunc(nullptr);
-  ASSERT_TRUE(root->getMeasure() == NULL);
+  ASSERT_TRUE(!root->hasMeasureFunc());
   YGNodeFreeRecursive(root);
 }
 
@@ -635,14 +640,16 @@ TEST(YogaTest, cant_call_negative_measure_horizontal) {
   YGConfigFree(config);
 }
 
-static YGSize _measure_90_10(YGNodeRef node,
+static YGSize _measure_90_10(
+    YGNodeRef node,
     float width,
     YGMeasureMode widthMode,
     float height,
     YGMeasureMode heightMode) {
 
   return YGSize{
-    .width = 90, .height = 10,
+      .width = 90,
+      .height = 10,
   };
 }
 

tests/YGNodeCallbackTest.cpp

@@ -0,0 +1,81 @@
+/**
+ * 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.
+ */
+#include <gtest/gtest.h>
+#include <yoga/YGNode.h>
+#include <ostream>
+
+inline bool operator==(const YGSize& lhs, const YGSize& rhs) {
+  return lhs.width == rhs.width && lhs.height == rhs.height;
+}
+
+void PrintTo(const YGSize&, std::ostream*);
+
+TEST(YGNode, hasMeasureFunc_initial) {
+  auto n = YGNode{};
+  ASSERT_FALSE(n.hasMeasureFunc());
+}
+
+TEST(YGNode, hasMeasureFunc_with_measure_fn) {
+  auto n = YGNode{};
+  n.setMeasureFunc([](YGNode*, float, YGMeasureMode, float, YGMeasureMode) {
+    return YGSize{};
+  });
+  ASSERT_TRUE(n.hasMeasureFunc());
+}
+
+TEST(YGNode, measure_with_measure_fn) {
+  auto n = YGNode{};
+
+  n.setMeasureFunc(
+      [](YGNode*, float w, YGMeasureMode wm, float h, YGMeasureMode hm) {
+        return YGSize{w * wm, h / hm};
+      });
+
+  ASSERT_EQ(
+      n.measure(23, YGMeasureModeExactly, 24, YGMeasureModeAtMost),
+      (YGSize{23, 12}));
+}
+
+TEST(YGNode, hasMeasureFunc_after_unset) {
+  auto n = YGNode{};
+  n.setMeasureFunc([](YGNode*, float, YGMeasureMode, float, YGMeasureMode) {
+    return YGSize{};
+  });
+
+  n.setMeasureFunc(nullptr);
+  ASSERT_FALSE(n.hasMeasureFunc());
+}
+
+TEST(YGNode, hasBaselineFunc_initial) {
+  auto n = YGNode{};
+  ASSERT_FALSE(n.hasBaselineFunc());
+}
+
+TEST(YGNode, hasBaselineFunc_with_baseline_fn) {
+  auto n = YGNode{};
+  n.setBaseLineFunc([](YGNode*, float, float) { return 0.0f; });
+  ASSERT_TRUE(n.hasBaselineFunc());
+}
+
+TEST(YGNode, baseline_with_baseline_fn) {
+  auto n = YGNode{};
+  n.setBaseLineFunc([](YGNode*, float w, float h) { return w + h; });
+
+  ASSERT_EQ(n.baseline(1.25f, 2.5f), 3.75f);
+}
+
+TEST(YGNode, hasBaselineFunc_after_unset) {
+  auto n = YGNode{};
+  n.setBaseLineFunc([](YGNode*, float, float) { return 0.0f; });
+
+  n.setBaseLineFunc(nullptr);
+  ASSERT_FALSE(n.hasBaselineFunc());
+}
+
+void PrintTo(const YGSize& size, std::ostream* os) {
+  *os << "YGSize{" << size.width << ", " << size.height << "}";
+}

yoga/YGNode.cpp

@@ -270,8 +270,8 @@ YGNode& YGNode::operator=(const YGNode& node) {
   print_ = node.getPrintFunc();
   hasNewLayout_ = node.getHasNewLayout();
   nodeType_ = node.getNodeType();
-  measure_ = node.getMeasure();
-  baseline_ = node.getBaseline();
+  measure_ = node.measure_;
+  baseline_ = node.baseline_;
   dirtied_ = node.getDirtied();
   style_ = node.style_;
   layout_ = node.layout_;

yoga/YGNode.h

@@ -63,12 +63,24 @@ public:
     return nodeType_;
   }
 
-  YGMeasureFunc getMeasure() const {
-    return measure_;
+  bool hasMeasureFunc() const noexcept {
+    return measure_ != nullptr;
   }
 
-  YGBaselineFunc getBaseline() const {
-    return baseline_;
+  YGSize measure(
+      float width,
+      YGMeasureMode widthMode,
+      float height,
+      YGMeasureMode heightMode) {
+    return measure_(this, width, widthMode, height, heightMode);
+  }
+
+  bool hasBaselineFunc() const noexcept {
+    return baseline_ != nullptr;
+  }
+
+  float baseline(float width, float height) {
+    return baseline_(this, width, height);
   }
 
   YGDirtiedFunc getDirtied() const {

yoga/YGNodePrint.cpp

@@ -214,7 +214,7 @@ void YGNodeToString(
         str, "bottom", node->getStyle().position, YGEdgeBottom);
     appendFormatedString(str, "\" ");
 
-    if (node->getMeasure() != nullptr) {
+    if (node->hasMeasureFunc()) {
       appendFormatedString(str, "has-custom-measure=\"true\"");
     }
   }

yoga/Yoga.cpp

@@ -145,16 +145,16 @@ void YGNodeSetContext(YGNodeRef node, void* context) {
   return node->setContext(context);
 }
 
-YGMeasureFunc YGNodeGetMeasureFunc(YGNodeRef node) {
-  return node->getMeasure();
+bool YGNodeHasMeasureFunc(YGNodeRef node) {
+  return node->hasMeasureFunc();
 }
 
 void YGNodeSetMeasureFunc(YGNodeRef node, YGMeasureFunc measureFunc) {
   node->setMeasureFunc(measureFunc);
 }
 
-YGBaselineFunc YGNodeGetBaselineFunc(YGNodeRef node) {
-  return node->getBaseline();
+bool YGNodeHasBaselineFunc(YGNodeRef node) {
+  return node->hasBaselineFunc();
 }
 
 void YGNodeSetBaselineFunc(YGNodeRef node, YGBaselineFunc baselineFunc) {
@@ -395,7 +395,7 @@ void YGNodeInsertChild(
 
   YGAssertWithNode(
       node,
-      node->getMeasure() == nullptr,
+      !node->hasMeasureFunc(),
       "Cannot add child: Nodes with measure functions cannot have children.");
 
   node->cloneChildrenIfNeeded();
@@ -555,7 +555,7 @@ YGNodeRef YGNodeGetParent(const YGNodeRef node) {
 void YGNodeMarkDirty(const YGNodeRef node) {
   YGAssertWithNode(
       node,
-      node->getMeasure() != nullptr,
+      node->hasMeasureFunc(),
       "Only leaf nodes with custom measure functions"
       "should manually mark themselves as dirty");
 
@@ -1075,11 +1075,10 @@ static inline YGAlign YGNodeAlignItem(
 }
 
 static float YGBaseline(const YGNodeRef node) {
-  if (node->getBaseline() != nullptr) {
+  if (node->hasBaselineFunc()) {
     const float baseline = marker::MarkerSection<YGMarkerBaselineFn>::wrap(
         node,
-        node->getBaseline(),
-        node,
+        &YGNode::baseline,
         node->getLayout().measuredDimensions[YGDimensionWidth],
         node->getLayout().measuredDimensions[YGDimensionHeight]);
     YGAssertWithNode(
@@ -1650,7 +1649,7 @@ static void YGNodeWithMeasureFuncSetMeasuredDimensions(
     const float ownerHeight) {
   YGAssertWithNode(
       node,
-      node->getMeasure() != nullptr,
+      node->hasMeasureFunc(),
       "Expected node to have custom measure function");
 
   const float paddingAndBorderAxisRow =
@@ -1694,8 +1693,7 @@ static void YGNodeWithMeasureFuncSetMeasuredDimensions(
     // Measure the text under the current constraints.
     const YGSize measuredSize = marker::MarkerSection<YGMarkerMeasure>::wrap(
         node,
-        node->getMeasure(),
-        node,
+        &YGNode::measure,
         innerWidth,
         widthMeasureMode,
         innerHeight,
@@ -2737,7 +2735,7 @@ static void YGNodelayoutImpl(
       node->getTrailingPadding(flexColumnDirection, ownerWidth).unwrap(),
       YGEdgeBottom);
 
-  if (node->getMeasure() != nullptr) {
+  if (node->hasMeasureFunc()) {
     YGNodeWithMeasureFuncSetMeasuredDimensions(
         node,
         availableWidth,
@@ -3741,7 +3739,7 @@ bool YGLayoutNodeInternal(
   // dimensions. We handle nodes with measure functions specially here because
   // they are the most expensive to measure, so it's worth avoiding redundant
   // measurements if at all possible.
-  if (node->getMeasure() != nullptr) {
+  if (node->hasMeasureFunc()) {
     const float marginAxisRow =
         node->getMarginForAxis(YGFlexDirectionRow, ownerWidth).unwrap();
     const float marginAxisColumn =

yoga/Yoga.h

@@ -132,9 +132,9 @@ WIN_EXPORT void YGNodeCopyStyle(
 WIN_EXPORT void* YGNodeGetContext(YGNodeRef node);
 WIN_EXPORT void YGNodeSetContext(YGNodeRef node, void* context);
 void YGConfigSetPrintTreeFlag(YGConfigRef config, bool enabled);
-YGMeasureFunc YGNodeGetMeasureFunc(YGNodeRef node);
+bool YGNodeHasMeasureFunc(YGNodeRef node);
 WIN_EXPORT void YGNodeSetMeasureFunc(YGNodeRef node, YGMeasureFunc measureFunc);
-YGBaselineFunc YGNodeGetBaselineFunc(YGNodeRef node);
+bool YGNodeHasBaselineFunc(YGNodeRef node);
 void YGNodeSetBaselineFunc(YGNodeRef node, YGBaselineFunc baselineFunc);
 YGDirtiedFunc YGNodeGetDirtiedFunc(YGNodeRef node);
 void YGNodeSetDirtiedFunc(YGNodeRef node, YGDirtiedFunc dirtiedFunc);

yoga/instrumentation.h

@@ -28,9 +28,12 @@ public:
   typename Data::type data = {};
 
   template <typename Ret, typename... Args>
-  static Ret wrap(YGNodeRef node, Ret (*fn)(Args...), Args... args) {
+  static Ret wrap(
+      YGNodeRef node,
+      Ret (YGNode::*method)(Args...),
+      Args... args) {
     MarkerSection<MarkerType> section{node};
-    return fn(std::forward<Args>(args)...);
+    return (node->*method)(std::forward<Args>(args)...);
   }
 
 private: