Fix handling of negative flex gap (#1405)

Summary:
X-link: https://github.com/facebook/react-native/pull/39596

Pull Request resolved: https://github.com/facebook/yoga/pull/1405

I noticed that we weren't clamping negative flex gap values to zero. This fixes that bug.

Reviewed By: rshest

Differential Revision: D49530494

fbshipit-source-id: 069db7312f72a085c5c4b01ead7bc66a353a07e5

gentest/gentest.js

@@ -701,7 +701,7 @@ function calculateTree(root, roundToPixelGrid) {
 
 function getYogaStyle(node) {
   // TODO: Relying on computed style means we cannot test shorthand props like
-  // "padding", "margin", "gap".
+  // "padding", "margin", "gap", or negative values.
   return [
     'direction',
     'flex-direction',

tests/FlexGapTest.cpp

@@ -0,0 +1,93 @@
+/*
+ * Copyright (c) Meta Platforms, Inc. and 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/Yoga.h>
+
+// TODO: move this to a fixture based test once it supports parsing negative
+// values
+TEST(FlexGap, gap_negative_value) {
+  const YGConfigRef config = YGConfigNew();
+
+  const YGNodeRef root = YGNodeNewWithConfig(config);
+  YGNodeStyleSetFlexDirection(root, YGFlexDirectionRow);
+  YGNodeStyleSetGap(root, YGGutterAll, -20);
+  YGNodeStyleSetHeight(root, 200);
+
+  const YGNodeRef root_child0 = YGNodeNewWithConfig(config);
+  YGNodeStyleSetWidth(root_child0, 20);
+  YGNodeInsertChild(root, root_child0, 0);
+
+  const YGNodeRef root_child1 = YGNodeNewWithConfig(config);
+  YGNodeStyleSetWidth(root_child1, 20);
+  YGNodeInsertChild(root, root_child1, 1);
+
+  const YGNodeRef root_child2 = YGNodeNewWithConfig(config);
+  YGNodeStyleSetWidth(root_child2, 20);
+  YGNodeInsertChild(root, root_child2, 2);
+
+  const YGNodeRef root_child3 = YGNodeNewWithConfig(config);
+  YGNodeStyleSetWidth(root_child3, 20);
+  YGNodeInsertChild(root, root_child3, 3);
+  YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
+
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
+  ASSERT_FLOAT_EQ(80, YGNodeLayoutGetWidth(root));
+  ASSERT_FLOAT_EQ(200, YGNodeLayoutGetHeight(root));
+
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetWidth(root_child0));
+  ASSERT_FLOAT_EQ(200, YGNodeLayoutGetHeight(root_child0));
+
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetLeft(root_child1));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child1));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetWidth(root_child1));
+  ASSERT_FLOAT_EQ(200, YGNodeLayoutGetHeight(root_child1));
+
+  ASSERT_FLOAT_EQ(40, YGNodeLayoutGetLeft(root_child2));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child2));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetWidth(root_child2));
+  ASSERT_FLOAT_EQ(200, YGNodeLayoutGetHeight(root_child2));
+
+  ASSERT_FLOAT_EQ(60, YGNodeLayoutGetLeft(root_child3));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child3));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetWidth(root_child3));
+  ASSERT_FLOAT_EQ(200, YGNodeLayoutGetHeight(root_child3));
+
+  YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionRTL);
+
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
+  ASSERT_FLOAT_EQ(80, YGNodeLayoutGetWidth(root));
+  ASSERT_FLOAT_EQ(200, YGNodeLayoutGetHeight(root));
+
+  ASSERT_FLOAT_EQ(60, YGNodeLayoutGetLeft(root_child0));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetWidth(root_child0));
+  ASSERT_FLOAT_EQ(200, YGNodeLayoutGetHeight(root_child0));
+
+  ASSERT_FLOAT_EQ(40, YGNodeLayoutGetLeft(root_child1));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child1));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetWidth(root_child1));
+  ASSERT_FLOAT_EQ(200, YGNodeLayoutGetHeight(root_child1));
+
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetLeft(root_child2));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child2));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetWidth(root_child2));
+  ASSERT_FLOAT_EQ(200, YGNodeLayoutGetHeight(root_child2));
+
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child3));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child3));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetWidth(root_child3));
+  ASSERT_FLOAT_EQ(200, YGNodeLayoutGetHeight(root_child3));
+
+  YGNodeFreeRecursive(root);
+
+  YGConfigFree(config);
+}

yoga/algorithm/CalculateLayout.cpp

@@ -1215,7 +1215,7 @@ static void justifyMainAxis(
       node->getLeadingPaddingAndBorder(mainAxis, ownerWidth).unwrap();
   const float trailingPaddingAndBorderMain =
       node->getTrailingPaddingAndBorder(mainAxis, ownerWidth).unwrap();
-  const float gap = node->getGapForAxis(mainAxis, ownerWidth).unwrap();
+  const float gap = node->getGapForAxis(mainAxis, ownerWidth);
   // If we are using "at most" rules in the main axis, make sure that
   // remainingFreeSpace is 0 when min main dimension is not given
   if (measureModeMainDim == MeasureMode::AtMost &&
@@ -1666,8 +1666,7 @@ static void calculateLayoutImpl(
       generationCount);
 
   if (childCount > 1) {
-    totalMainDim +=
+    totalMainDim += node->getGapForAxis(mainAxis, availableInnerCrossDim) *
-        node->getGapForAxis(mainAxis, availableInnerCrossDim).unwrap() *
         static_cast<float>(childCount - 1);
   }
 
@@ -1692,7 +1691,7 @@ static void calculateLayoutImpl(
   float totalLineCrossDim = 0;
 
   const float crossAxisGap =
-      node->getGapForAxis(crossAxis, availableInnerCrossDim).unwrap();
+      node->getGapForAxis(crossAxis, availableInnerCrossDim);
 
   // Max main dimension of all the lines.
   float maxLineMainDim = 0;

yoga/algorithm/FlexLine.cpp

@@ -33,7 +33,7 @@ FlexLine calculateFlexLine(
   const FlexDirection mainAxis = resolveDirection(
       node->getStyle().flexDirection(), node->resolveDirection(ownerDirection));
   const bool isNodeFlexWrap = node->getStyle().flexWrap() != Wrap::NoWrap;
-  const float gap = node->getGapForAxis(mainAxis, availableInnerWidth).unwrap();
+  const float gap = node->getGapForAxis(mainAxis, availableInnerWidth);
 
   // Add items to the current line until it's full or we run out of items.
   for (; endOfLineIndex < node->getChildren().size(); endOfLineIndex++) {

yoga/node/Node.cpp

@@ -202,13 +202,13 @@ FloatOptional Node::getMarginForAxis(
   return getLeadingMargin(axis, widthSize) + getTrailingMargin(axis, widthSize);
 }
 
-FloatOptional Node::getGapForAxis(
+float Node::getGapForAxis(const FlexDirection axis, const float widthSize)
-    const FlexDirection axis,
+    const {
-    const float widthSize) const {
   auto gap = isRow(axis)
-      ? computeColumnGap(style_.gap(), CompactValue::ofZero())
+      ? computeColumnGap(style_.gap(), CompactValue::ofUndefined())
-      : computeRowGap(style_.gap(), CompactValue::ofZero());
+      : computeRowGap(style_.gap(), CompactValue::ofUndefined());
-  return yoga::resolveValue(gap, widthSize);
+  auto resolvedGap = yoga::resolveValue(gap, widthSize);
+  return maxOrDefined(resolvedGap.unwrap(), 0);
 }
 
 YGSize Node::measure(

yoga/node/Node.h

@@ -231,8 +231,7 @@ class YG_EXPORT Node : public ::YGNode {
   FloatOptional getMarginForAxis(
       const FlexDirection axis,
       const float widthSize) const;
-  FloatOptional getGapForAxis(const FlexDirection axis, const float widthSize)
+  float getGapForAxis(const FlexDirection axis, const float widthSize) const;
-      const;
   // Setters
 
   void setContext(void* context) {