Take margin into account on max dimension

Summary:
We need to take the margin into account if we clip on max dimension. Fixes #466.
Closes https://github.com/facebook/yoga/pull/467

Differential Revision: D4681342

Pulled By: emilsjolander

fbshipit-source-id: 56311df9864a284d553c31f1c6db382f337f1fad

csharp/tests/Facebook.Yoga/YGMarginTest.cs

@@ -1352,5 +1352,89 @@ namespace Facebook.Yoga
             Assert.AreEqual(50f, root_child1.LayoutHeight);
         }
 
+        [Test]
+        public void Test_margin_should_not_be_part_of_max_height()
+        {
+            YogaConfig config = new YogaConfig();
+
+            YogaNode root = new YogaNode(config);
+            root.Width = 250;
+            root.Height = 250;
+
+            YogaNode root_child0 = new YogaNode(config);
+            root_child0.MarginTop = 20;
+            root_child0.Width = 100;
+            root_child0.Height = 100;
+            root_child0.MaxHeight = 100;
+            root.Insert(0, root_child0);
+            root.StyleDirection = YogaDirection.LTR;
+            root.CalculateLayout();
+
+            Assert.AreEqual(0f, root.LayoutX);
+            Assert.AreEqual(0f, root.LayoutY);
+            Assert.AreEqual(250f, root.LayoutWidth);
+            Assert.AreEqual(250f, root.LayoutHeight);
+
+            Assert.AreEqual(0f, root_child0.LayoutX);
+            Assert.AreEqual(20f, root_child0.LayoutY);
+            Assert.AreEqual(100f, root_child0.LayoutWidth);
+            Assert.AreEqual(100f, root_child0.LayoutHeight);
+
+            root.StyleDirection = YogaDirection.RTL;
+            root.CalculateLayout();
+
+            Assert.AreEqual(0f, root.LayoutX);
+            Assert.AreEqual(0f, root.LayoutY);
+            Assert.AreEqual(250f, root.LayoutWidth);
+            Assert.AreEqual(250f, root.LayoutHeight);
+
+            Assert.AreEqual(150f, root_child0.LayoutX);
+            Assert.AreEqual(20f, root_child0.LayoutY);
+            Assert.AreEqual(100f, root_child0.LayoutWidth);
+            Assert.AreEqual(100f, root_child0.LayoutHeight);
+        }
+
+        [Test]
+        public void Test_margin_should_not_be_part_of_max_width()
+        {
+            YogaConfig config = new YogaConfig();
+
+            YogaNode root = new YogaNode(config);
+            root.Width = 250;
+            root.Height = 250;
+
+            YogaNode root_child0 = new YogaNode(config);
+            root_child0.MarginLeft = 20;
+            root_child0.Width = 100;
+            root_child0.MaxWidth = 100;
+            root_child0.Height = 100;
+            root.Insert(0, root_child0);
+            root.StyleDirection = YogaDirection.LTR;
+            root.CalculateLayout();
+
+            Assert.AreEqual(0f, root.LayoutX);
+            Assert.AreEqual(0f, root.LayoutY);
+            Assert.AreEqual(250f, root.LayoutWidth);
+            Assert.AreEqual(250f, root.LayoutHeight);
+
+            Assert.AreEqual(20f, root_child0.LayoutX);
+            Assert.AreEqual(0f, root_child0.LayoutY);
+            Assert.AreEqual(100f, root_child0.LayoutWidth);
+            Assert.AreEqual(100f, root_child0.LayoutHeight);
+
+            root.StyleDirection = YogaDirection.RTL;
+            root.CalculateLayout();
+
+            Assert.AreEqual(0f, root.LayoutX);
+            Assert.AreEqual(0f, root.LayoutY);
+            Assert.AreEqual(250f, root.LayoutWidth);
+            Assert.AreEqual(250f, root.LayoutHeight);
+
+            Assert.AreEqual(150f, root_child0.LayoutX);
+            Assert.AreEqual(0f, root_child0.LayoutY);
+            Assert.AreEqual(100f, root_child0.LayoutWidth);
+            Assert.AreEqual(100f, root_child0.LayoutHeight);
+        }
+
     }
 }

gentest/fixtures/YGMarginTest.html

@@ -116,3 +116,11 @@
   <div style="width: 50px; height: 50px; margin-top:auto; margin-bottom:auto;"></div>
   <div style="width: 50px; height: 50px;"></div>
 </div>
+
+<div id="margin_should_not_be_part_of_max_height" style="width: 250px; height: 250px;">
+  <div style="width: 100px; height: 100px; max-height: 100px; margin-top: 20px;"></div>
+</div>
+
+<div id="margin_should_not_be_part_of_max_width" style="width: 250px; height: 250px;">
+  <div style="width: 100px; height: 100px; max-width: 100px; margin-left: 20px;"></div>
+</div>

java/tests/com/facebook/yoga/YGMarginTest.java

@@ -1326,4 +1326,86 @@ public class YGMarginTest {
     assertEquals(50f, root_child1.getLayoutHeight(), 0.0f);
   }
 
+  @Test
+  public void test_margin_should_not_be_part_of_max_height() {
+    YogaConfig config = new YogaConfig();
+
+    final YogaNode root = new YogaNode(config);
+    root.setWidth(250f);
+    root.setHeight(250f);
+
+    final YogaNode root_child0 = new YogaNode(config);
+    root_child0.setMargin(YogaEdge.TOP, 20f);
+    root_child0.setWidth(100f);
+    root_child0.setHeight(100f);
+    root_child0.setMaxHeight(100f);
+    root.addChildAt(root_child0, 0);
+    root.setDirection(YogaDirection.LTR);
+    root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);
+
+    assertEquals(0f, root.getLayoutX(), 0.0f);
+    assertEquals(0f, root.getLayoutY(), 0.0f);
+    assertEquals(250f, root.getLayoutWidth(), 0.0f);
+    assertEquals(250f, root.getLayoutHeight(), 0.0f);
+
+    assertEquals(0f, root_child0.getLayoutX(), 0.0f);
+    assertEquals(20f, root_child0.getLayoutY(), 0.0f);
+    assertEquals(100f, root_child0.getLayoutWidth(), 0.0f);
+    assertEquals(100f, root_child0.getLayoutHeight(), 0.0f);
+
+    root.setDirection(YogaDirection.RTL);
+    root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);
+
+    assertEquals(0f, root.getLayoutX(), 0.0f);
+    assertEquals(0f, root.getLayoutY(), 0.0f);
+    assertEquals(250f, root.getLayoutWidth(), 0.0f);
+    assertEquals(250f, root.getLayoutHeight(), 0.0f);
+
+    assertEquals(150f, root_child0.getLayoutX(), 0.0f);
+    assertEquals(20f, root_child0.getLayoutY(), 0.0f);
+    assertEquals(100f, root_child0.getLayoutWidth(), 0.0f);
+    assertEquals(100f, root_child0.getLayoutHeight(), 0.0f);
+  }
+
+  @Test
+  public void test_margin_should_not_be_part_of_max_width() {
+    YogaConfig config = new YogaConfig();
+
+    final YogaNode root = new YogaNode(config);
+    root.setWidth(250f);
+    root.setHeight(250f);
+
+    final YogaNode root_child0 = new YogaNode(config);
+    root_child0.setMargin(YogaEdge.LEFT, 20f);
+    root_child0.setWidth(100f);
+    root_child0.setMaxWidth(100f);
+    root_child0.setHeight(100f);
+    root.addChildAt(root_child0, 0);
+    root.setDirection(YogaDirection.LTR);
+    root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);
+
+    assertEquals(0f, root.getLayoutX(), 0.0f);
+    assertEquals(0f, root.getLayoutY(), 0.0f);
+    assertEquals(250f, root.getLayoutWidth(), 0.0f);
+    assertEquals(250f, root.getLayoutHeight(), 0.0f);
+
+    assertEquals(20f, root_child0.getLayoutX(), 0.0f);
+    assertEquals(0f, root_child0.getLayoutY(), 0.0f);
+    assertEquals(100f, root_child0.getLayoutWidth(), 0.0f);
+    assertEquals(100f, root_child0.getLayoutHeight(), 0.0f);
+
+    root.setDirection(YogaDirection.RTL);
+    root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);
+
+    assertEquals(0f, root.getLayoutX(), 0.0f);
+    assertEquals(0f, root.getLayoutY(), 0.0f);
+    assertEquals(250f, root.getLayoutWidth(), 0.0f);
+    assertEquals(250f, root.getLayoutHeight(), 0.0f);
+
+    assertEquals(150f, root_child0.getLayoutX(), 0.0f);
+    assertEquals(0f, root_child0.getLayoutY(), 0.0f);
+    assertEquals(100f, root_child0.getLayoutWidth(), 0.0f);
+    assertEquals(100f, root_child0.getLayoutHeight(), 0.0f);
+  }
+
 }

javascript/tests/Facebook.Yoga/YGMarginTest.js

@@ -1321,3 +1321,85 @@ it("margin_auto_top_and_bottom_strech", function () {
     }
   }
 });
+it("margin_should_not_be_part_of_max_height", function () {
+  try {
+    var root = Yoga.Node.create(config);
+    root.setWidth(250);
+    root.setHeight(250);
+
+    var root_child0 = Yoga.Node.create(config);
+    root_child0.setMargin(Yoga.EDGE_TOP, 20);
+    root_child0.setWidth(100);
+    root_child0.setHeight(100);
+    root_child0.setMaxHeight(100);
+    root.insertChild(root_child0, 0);
+    root.calculateLayout(Yoga.UNDEFINED, Yoga.UNDEFINED, Yoga.DIRECTION_LTR);
+
+    console.assert(0 === root.getComputedLeft(), "0 === root.getComputedLeft() (" + root.getComputedLeft() + ")");
+    console.assert(0 === root.getComputedTop(), "0 === root.getComputedTop() (" + root.getComputedTop() + ")");
+    console.assert(250 === root.getComputedWidth(), "250 === root.getComputedWidth() (" + root.getComputedWidth() + ")");
+    console.assert(250 === root.getComputedHeight(), "250 === root.getComputedHeight() (" + root.getComputedHeight() + ")");
+
+    console.assert(0 === root_child0.getComputedLeft(), "0 === root_child0.getComputedLeft() (" + root_child0.getComputedLeft() + ")");
+    console.assert(20 === root_child0.getComputedTop(), "20 === root_child0.getComputedTop() (" + root_child0.getComputedTop() + ")");
+    console.assert(100 === root_child0.getComputedWidth(), "100 === root_child0.getComputedWidth() (" + root_child0.getComputedWidth() + ")");
+    console.assert(100 === root_child0.getComputedHeight(), "100 === root_child0.getComputedHeight() (" + root_child0.getComputedHeight() + ")");
+
+    root.calculateLayout(Yoga.UNDEFINED, Yoga.UNDEFINED, Yoga.DIRECTION_RTL);
+
+    console.assert(0 === root.getComputedLeft(), "0 === root.getComputedLeft() (" + root.getComputedLeft() + ")");
+    console.assert(0 === root.getComputedTop(), "0 === root.getComputedTop() (" + root.getComputedTop() + ")");
+    console.assert(250 === root.getComputedWidth(), "250 === root.getComputedWidth() (" + root.getComputedWidth() + ")");
+    console.assert(250 === root.getComputedHeight(), "250 === root.getComputedHeight() (" + root.getComputedHeight() + ")");
+
+    console.assert(150 === root_child0.getComputedLeft(), "150 === root_child0.getComputedLeft() (" + root_child0.getComputedLeft() + ")");
+    console.assert(20 === root_child0.getComputedTop(), "20 === root_child0.getComputedTop() (" + root_child0.getComputedTop() + ")");
+    console.assert(100 === root_child0.getComputedWidth(), "100 === root_child0.getComputedWidth() (" + root_child0.getComputedWidth() + ")");
+    console.assert(100 === root_child0.getComputedHeight(), "100 === root_child0.getComputedHeight() (" + root_child0.getComputedHeight() + ")");
+  } finally {
+    if (typeof root !== "undefined") {
+      root.freeRecursive();
+    }
+  }
+});
+it("margin_should_not_be_part_of_max_width", function () {
+  try {
+    var root = Yoga.Node.create(config);
+    root.setWidth(250);
+    root.setHeight(250);
+
+    var root_child0 = Yoga.Node.create(config);
+    root_child0.setMargin(Yoga.EDGE_LEFT, 20);
+    root_child0.setWidth(100);
+    root_child0.setMaxWidth(100);
+    root_child0.setHeight(100);
+    root.insertChild(root_child0, 0);
+    root.calculateLayout(Yoga.UNDEFINED, Yoga.UNDEFINED, Yoga.DIRECTION_LTR);
+
+    console.assert(0 === root.getComputedLeft(), "0 === root.getComputedLeft() (" + root.getComputedLeft() + ")");
+    console.assert(0 === root.getComputedTop(), "0 === root.getComputedTop() (" + root.getComputedTop() + ")");
+    console.assert(250 === root.getComputedWidth(), "250 === root.getComputedWidth() (" + root.getComputedWidth() + ")");
+    console.assert(250 === root.getComputedHeight(), "250 === root.getComputedHeight() (" + root.getComputedHeight() + ")");
+
+    console.assert(20 === root_child0.getComputedLeft(), "20 === root_child0.getComputedLeft() (" + root_child0.getComputedLeft() + ")");
+    console.assert(0 === root_child0.getComputedTop(), "0 === root_child0.getComputedTop() (" + root_child0.getComputedTop() + ")");
+    console.assert(100 === root_child0.getComputedWidth(), "100 === root_child0.getComputedWidth() (" + root_child0.getComputedWidth() + ")");
+    console.assert(100 === root_child0.getComputedHeight(), "100 === root_child0.getComputedHeight() (" + root_child0.getComputedHeight() + ")");
+
+    root.calculateLayout(Yoga.UNDEFINED, Yoga.UNDEFINED, Yoga.DIRECTION_RTL);
+
+    console.assert(0 === root.getComputedLeft(), "0 === root.getComputedLeft() (" + root.getComputedLeft() + ")");
+    console.assert(0 === root.getComputedTop(), "0 === root.getComputedTop() (" + root.getComputedTop() + ")");
+    console.assert(250 === root.getComputedWidth(), "250 === root.getComputedWidth() (" + root.getComputedWidth() + ")");
+    console.assert(250 === root.getComputedHeight(), "250 === root.getComputedHeight() (" + root.getComputedHeight() + ")");
+
+    console.assert(150 === root_child0.getComputedLeft(), "150 === root_child0.getComputedLeft() (" + root_child0.getComputedLeft() + ")");
+    console.assert(0 === root_child0.getComputedTop(), "0 === root_child0.getComputedTop() (" + root_child0.getComputedTop() + ")");
+    console.assert(100 === root_child0.getComputedWidth(), "100 === root_child0.getComputedWidth() (" + root_child0.getComputedWidth() + ")");
+    console.assert(100 === root_child0.getComputedHeight(), "100 === root_child0.getComputedHeight() (" + root_child0.getComputedHeight() + ")");
+  } finally {
+    if (typeof root !== "undefined") {
+      root.freeRecursive();
+    }
+  }
+});

tests/YGMarginTest.cpp

@@ -1346,3 +1346,87 @@ TEST(YogaTest, margin_auto_top_and_bottom_strech) {
 
   YGConfigFree(config);
 }
+
+TEST(YogaTest, margin_should_not_be_part_of_max_height) {
+  const YGConfigRef config = YGConfigNew();
+
+  const YGNodeRef root = YGNodeNewWithConfig(config);
+  YGNodeStyleSetWidth(root, 250);
+  YGNodeStyleSetHeight(root, 250);
+
+  const YGNodeRef root_child0 = YGNodeNewWithConfig(config);
+  YGNodeStyleSetMargin(root_child0, YGEdgeTop, 20);
+  YGNodeStyleSetWidth(root_child0, 100);
+  YGNodeStyleSetHeight(root_child0, 100);
+  YGNodeStyleSetMaxHeight(root_child0, 100);
+  YGNodeInsertChild(root, root_child0, 0);
+  YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
+
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
+  ASSERT_FLOAT_EQ(250, YGNodeLayoutGetWidth(root));
+  ASSERT_FLOAT_EQ(250, YGNodeLayoutGetHeight(root));
+
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetTop(root_child0));
+  ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root_child0));
+  ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root_child0));
+
+  YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionRTL);
+
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
+  ASSERT_FLOAT_EQ(250, YGNodeLayoutGetWidth(root));
+  ASSERT_FLOAT_EQ(250, YGNodeLayoutGetHeight(root));
+
+  ASSERT_FLOAT_EQ(150, YGNodeLayoutGetLeft(root_child0));
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetTop(root_child0));
+  ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root_child0));
+  ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root_child0));
+
+  YGNodeFreeRecursive(root);
+
+  YGConfigFree(config);
+}
+
+TEST(YogaTest, margin_should_not_be_part_of_max_width) {
+  const YGConfigRef config = YGConfigNew();
+
+  const YGNodeRef root = YGNodeNewWithConfig(config);
+  YGNodeStyleSetWidth(root, 250);
+  YGNodeStyleSetHeight(root, 250);
+
+  const YGNodeRef root_child0 = YGNodeNewWithConfig(config);
+  YGNodeStyleSetMargin(root_child0, YGEdgeLeft, 20);
+  YGNodeStyleSetWidth(root_child0, 100);
+  YGNodeStyleSetMaxWidth(root_child0, 100);
+  YGNodeStyleSetHeight(root_child0, 100);
+  YGNodeInsertChild(root, root_child0, 0);
+  YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
+
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
+  ASSERT_FLOAT_EQ(250, YGNodeLayoutGetWidth(root));
+  ASSERT_FLOAT_EQ(250, YGNodeLayoutGetHeight(root));
+
+  ASSERT_FLOAT_EQ(20, YGNodeLayoutGetLeft(root_child0));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
+  ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root_child0));
+  ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root_child0));
+
+  YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionRTL);
+
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
+  ASSERT_FLOAT_EQ(250, YGNodeLayoutGetWidth(root));
+  ASSERT_FLOAT_EQ(250, YGNodeLayoutGetHeight(root));
+
+  ASSERT_FLOAT_EQ(150, YGNodeLayoutGetLeft(root_child0));
+  ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
+  ASSERT_FLOAT_EQ(100, YGNodeLayoutGetWidth(root_child0));
+  ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root_child0));
+
+  YGNodeFreeRecursive(root);
+
+  YGConfigFree(config);
+}

yoga/Yoga.c

@@ -1334,7 +1334,14 @@ static float YGNodeRelativePosition(const YGNodeRef node,
                                                : -YGNodeTrailingPosition(node, axis, axisSize);
 }
 
-static void YGConstrainMaxSizeForMode(const float maxSize, YGMeasureMode *mode, float *size) {
+static void YGConstrainMaxSizeForMode(const YGNodeRef node,
+                                      const enum YGFlexDirection axis,
+                                      const float parentAxisSize,
+                                      const float parentWidth,
+                                      YGMeasureMode *mode,
+                                      float *size) {
+  const float maxSize = YGValueResolve(&node->style.maxDimensions[dim[axis]], parentAxisSize) +
+                        YGNodeMarginForAxis(node, axis, parentWidth);
   switch (*mode) {
     case YGMeasureModeExactly:
     case YGMeasureModeAtMost:
@@ -1480,14 +1487,10 @@ static void YGNodeComputeFlexBasisForChild(const YGNodeRef node,
       }
     }
 
-    YGConstrainMaxSizeForMode(YGValueResolve(&child->style.maxDimensions[YGDimensionWidth],
+    YGConstrainMaxSizeForMode(
-                                             parentWidth),
+        child, YGFlexDirectionRow, parentWidth, parentWidth, &childWidthMeasureMode, &childWidth);
-                              &childWidthMeasureMode,
+    YGConstrainMaxSizeForMode(
-                              &childWidth);
+        child, YGFlexDirectionColumn, parentHeight, parentWidth, &childHeightMeasureMode, &childHeight);
-    YGConstrainMaxSizeForMode(YGValueResolve(&child->style.maxDimensions[YGDimensionHeight],
-                                             parentHeight),
-                              &childHeightMeasureMode,
-                              &childHeight);
 
     // Measure the child
     YGLayoutNodeInternal(child,
@@ -2443,16 +2446,18 @@ static void YGNodelayoutImpl(const YGNodeRef node,
           childCrossSize += marginCross;
         }
 
-        YGConstrainMaxSizeForMode(
+        YGConstrainMaxSizeForMode(currentRelativeChild,
-            YGValueResolve(&currentRelativeChild->style.maxDimensions[dim[mainAxis]],
+                                  mainAxis,
-                           availableInnerWidth),
+                                  availableInnerMainDim,
-            &childMainMeasureMode,
+                                  availableInnerWidth,
-            &childMainSize);
+                                  &childMainMeasureMode,
-        YGConstrainMaxSizeForMode(
+                                  &childMainSize);
-            YGValueResolve(&currentRelativeChild->style.maxDimensions[dim[crossAxis]],
+        YGConstrainMaxSizeForMode(currentRelativeChild,
-                           availableInnerHeight),
+                                  crossAxis,
-            &childCrossMeasureMode,
+                                  availableInnerCrossDim,
-            &childCrossSize);
+                                  availableInnerWidth,
+                                  &childCrossMeasureMode,
+                                  &childCrossSize);
 
         const bool requiresStretchLayout =
             !YGNodeIsStyleDimDefined(currentRelativeChild, crossAxis, availableInnerCrossDim) &&
@@ -2690,12 +2695,16 @@ static void YGNodelayoutImpl(const YGNodeRef node,
 
               YGMeasureMode childMainMeasureMode = YGMeasureModeExactly;
               YGMeasureMode childCrossMeasureMode = YGMeasureModeExactly;
-              YGConstrainMaxSizeForMode(YGValueResolve(&child->style.maxDimensions[dim[mainAxis]],
+              YGConstrainMaxSizeForMode(child,
-                                                       availableInnerMainDim),
+                                        mainAxis,
+                                        availableInnerMainDim,
+                                        availableInnerWidth,
                                         &childMainMeasureMode,
                                         &childMainSize);
-              YGConstrainMaxSizeForMode(YGValueResolve(&child->style.maxDimensions[dim[crossAxis]],
+              YGConstrainMaxSizeForMode(child,
-                                                       availableInnerCrossDim),
+                                        crossAxis,
+                                        availableInnerCrossDim,
+                                        availableInnerWidth,
                                         &childCrossMeasureMode,
                                         &childCrossSize);