From ca663839415f4e28c2ff067d22799bd6066d9148 Mon Sep 17 00:00:00 2001 From: Emil Sjolander Date: Fri, 8 Jul 2016 09:22:45 -0700 Subject: [PATCH] Correctly size cross axis when measuring flex basis Summary: https://github.com/facebook/css-layout/pull/199 - Nodes were measured with the assumption of being text nodes (height depends on width) when determining flex basis. This is not always true. Even when we are just interested in the main axis (flex basis) we need to correctly constrain the cross axis. - Some tests were wrong. Measuring texts.big and expecting it to have textSizes.smallHeight which doesn't make a lot of sense. Reviewed By: vjeux Differential Revision: D3510163 fbshipit-source-id: ee53b548dd078005fdd153d279e4c7fef3dd02d0 --- React/Layout/Layout.c | 89 ++++++++++--------- React/Layout/README | 2 +- .../com/facebook/csslayout/LayoutEngine.java | 89 ++++++++++--------- .../main/java/com/facebook/csslayout/README | 2 +- 4 files changed, 96 insertions(+), 86 deletions(-) diff --git a/React/Layout/Layout.c b/React/Layout/Layout.c index 22412a8d5..d9bc697fc 100644 --- a/React/Layout/Layout.c +++ b/React/Layout/Layout.c @@ -7,7 +7,7 @@ */ // NOTE: this file is auto-copied from https://github.com/facebook/css-layout -// @generated SignedSource<<484d0d17453521463896ce24d946412b>> +// @generated SignedSource<<8af322a110307b3277db5b00573da822>> #include @@ -829,56 +829,61 @@ static void layoutNodeImpl(css_node_t* node, float availableWidth, float availab child->layout.flex_basis = fmaxf(0, getPaddingAndBorderAxis(child, mainAxis)); } else { - // Compute the flex basis and hypothetical main size (i.e. the clamped flex basis). childWidth = CSS_UNDEFINED; childHeight = CSS_UNDEFINED; childWidthMeasureMode = CSS_MEASURE_MODE_UNDEFINED; childHeightMeasureMode = CSS_MEASURE_MODE_UNDEFINED; - if (isStyleDimDefined(child, CSS_FLEX_DIRECTION_ROW)) { - childWidth = child->style.dimensions[CSS_WIDTH] + getMarginAxis(child, CSS_FLEX_DIRECTION_ROW); - childWidthMeasureMode = CSS_MEASURE_MODE_EXACTLY; - } - if (isStyleDimDefined(child, CSS_FLEX_DIRECTION_COLUMN)) { - childHeight = child->style.dimensions[CSS_HEIGHT] + getMarginAxis(child, CSS_FLEX_DIRECTION_COLUMN); - childHeightMeasureMode = CSS_MEASURE_MODE_EXACTLY; - } - - // According to the spec, if the main size is not definite and the - // child's inline axis is parallel to the main axis (i.e. it's - // horizontal), the child should be sized using "UNDEFINED" in - // the main size. Otherwise use "AT_MOST" in the cross axis. - if (!isMainAxisRow && isUndefined(childWidth) && !isUndefined(availableInnerWidth)) { - childWidth = availableInnerWidth; - childWidthMeasureMode = CSS_MEASURE_MODE_AT_MOST; - } - - // The W3C spec doesn't say anything about the 'overflow' property, - // but all major browsers appear to implement the following logic. - if (node->style.overflow == CSS_OVERFLOW_HIDDEN) { - if (isMainAxisRow && isUndefined(childHeight) && !isUndefined(availableInnerHeight)) { - childHeight = availableInnerHeight; + // Main axis + if (isMainAxisRow) { + if (widthMeasureMode == CSS_MEASURE_MODE_UNDEFINED || isUndefined(availableInnerMainDim)) { + childWidth = CSS_UNDEFINED; + childWidthMeasureMode = CSS_MEASURE_MODE_UNDEFINED; + } else { + childWidth = availableInnerMainDim; + childWidthMeasureMode = CSS_MEASURE_MODE_AT_MOST; + } + } else if (node->style.overflow == CSS_OVERFLOW_HIDDEN) { + if (heightMeasureMode == CSS_MEASURE_MODE_UNDEFINED || isUndefined(availableInnerMainDim)) { + childHeight = CSS_UNDEFINED; + childHeightMeasureMode = CSS_MEASURE_MODE_UNDEFINED; + } else { + childHeight = availableInnerMainDim; childHeightMeasureMode = CSS_MEASURE_MODE_AT_MOST; } } - // If child has no defined size in the cross axis and is set to stretch, set the cross - // axis to be measured exactly with the available inner width - if (!isMainAxisRow && - !isUndefined(availableInnerWidth) && - !isStyleDimDefined(child, CSS_FLEX_DIRECTION_ROW) && - widthMeasureMode == CSS_MEASURE_MODE_EXACTLY && - getAlignItem(node, child) == CSS_ALIGN_STRETCH) { - childWidth = availableInnerWidth; - childWidthMeasureMode = CSS_MEASURE_MODE_EXACTLY; - } - if (isMainAxisRow && - !isUndefined(availableInnerHeight) && - !isStyleDimDefined(child, CSS_FLEX_DIRECTION_COLUMN) && - heightMeasureMode == CSS_MEASURE_MODE_EXACTLY && - getAlignItem(node, child) == CSS_ALIGN_STRETCH) { - childHeight = availableInnerHeight; - childHeightMeasureMode = CSS_MEASURE_MODE_EXACTLY; + // Cross axis + if (isMainAxisRow) { + if (node->style.overflow == CSS_OVERFLOW_HIDDEN) { + if (!isUndefined(availableInnerCrossDim) && + !isStyleDimDefined(child, CSS_FLEX_DIRECTION_COLUMN) && + heightMeasureMode == CSS_MEASURE_MODE_EXACTLY && + getAlignItem(node, child) == CSS_ALIGN_STRETCH) { + childHeight = availableInnerCrossDim; + childHeightMeasureMode = CSS_MEASURE_MODE_EXACTLY; + } else if (!isStyleDimDefined(child, CSS_FLEX_DIRECTION_COLUMN)) { + childHeight = availableInnerCrossDim; + childHeightMeasureMode = isUndefined(childHeight) ? CSS_MEASURE_MODE_UNDEFINED : CSS_MEASURE_MODE_AT_MOST; + } else { + childHeight = child->style.dimensions[CSS_HEIGHT] + getMarginAxis(child, CSS_FLEX_DIRECTION_COLUMN); + childHeightMeasureMode = CSS_MEASURE_MODE_EXACTLY; + } + } + } else { + if (!isUndefined(availableInnerCrossDim) && + !isStyleDimDefined(child, CSS_FLEX_DIRECTION_ROW) && + widthMeasureMode == CSS_MEASURE_MODE_EXACTLY && + getAlignItem(node, child) == CSS_ALIGN_STRETCH) { + childWidth = availableInnerCrossDim; + childWidthMeasureMode = CSS_MEASURE_MODE_EXACTLY; + } else if (!isStyleDimDefined(child, CSS_FLEX_DIRECTION_ROW)) { + childWidth = availableInnerCrossDim; + childWidthMeasureMode = isUndefined(childWidth) ? CSS_MEASURE_MODE_UNDEFINED : CSS_MEASURE_MODE_AT_MOST; + } else { + childWidth = child->style.dimensions[CSS_WIDTH] + getMarginAxis(child, CSS_FLEX_DIRECTION_ROW); + childWidthMeasureMode = CSS_MEASURE_MODE_EXACTLY; + } } // Measure the child diff --git a/React/Layout/README b/React/Layout/README index 9d85b26ff..5a2ca3b1e 100644 --- a/React/Layout/README +++ b/React/Layout/README @@ -1,7 +1,7 @@ The source of truth for css-layout is: https://github.com/facebook/css-layout The code here should be kept in sync with GitHub. -HEAD at the time this code was synced: https://github.com/facebook/css-layout/commit/a1f36b53f5464c8ee7abc311765dc3ecb1b879c6 +HEAD at the time this code was synced: https://github.com/facebook/css-layout/commit/ca34ff44460bce122d02b27c0409a825f8de90b1 There is generated code in: - README (this file) diff --git a/ReactAndroid/src/main/java/com/facebook/csslayout/LayoutEngine.java b/ReactAndroid/src/main/java/com/facebook/csslayout/LayoutEngine.java index 582fc44aa..735eec687 100644 --- a/ReactAndroid/src/main/java/com/facebook/csslayout/LayoutEngine.java +++ b/ReactAndroid/src/main/java/com/facebook/csslayout/LayoutEngine.java @@ -7,7 +7,7 @@ */ // NOTE: this file is auto-copied from https://github.com/facebook/css-layout -// @generated SignedSource<> +// @generated SignedSource<<9805b148b88d298edb7fcd7c13738ede>> package com.facebook.csslayout; @@ -726,56 +726,61 @@ public class LayoutEngine { child.layout.flexBasis = Math.max(0, ((child.style.padding.getWithFallback(leadingSpacing[mainAxis], leading[mainAxis]) + child.style.border.getWithFallback(leadingSpacing[mainAxis], leading[mainAxis])) + (child.style.padding.getWithFallback(trailingSpacing[mainAxis], trailing[mainAxis]) + child.style.border.getWithFallback(trailingSpacing[mainAxis], trailing[mainAxis])))); } else { - // Compute the flex basis and hypothetical main size (i.e. the clamped flex basis). childWidth = CSSConstants.UNDEFINED; childHeight = CSSConstants.UNDEFINED; childWidthMeasureMode = CSSMeasureMode.UNDEFINED; childHeightMeasureMode = CSSMeasureMode.UNDEFINED; - if ((child.style.dimensions[dim[CSS_FLEX_DIRECTION_ROW]] >= 0.0)) { - childWidth = child.style.dimensions[DIMENSION_WIDTH] + (child.style.margin.getWithFallback(leadingSpacing[CSS_FLEX_DIRECTION_ROW], leading[CSS_FLEX_DIRECTION_ROW]) + child.style.margin.getWithFallback(trailingSpacing[CSS_FLEX_DIRECTION_ROW], trailing[CSS_FLEX_DIRECTION_ROW])); - childWidthMeasureMode = CSSMeasureMode.EXACTLY; - } - if ((child.style.dimensions[dim[CSS_FLEX_DIRECTION_COLUMN]] >= 0.0)) { - childHeight = child.style.dimensions[DIMENSION_HEIGHT] + (child.style.margin.getWithFallback(leadingSpacing[CSS_FLEX_DIRECTION_COLUMN], leading[CSS_FLEX_DIRECTION_COLUMN]) + child.style.margin.getWithFallback(trailingSpacing[CSS_FLEX_DIRECTION_COLUMN], trailing[CSS_FLEX_DIRECTION_COLUMN])); - childHeightMeasureMode = CSSMeasureMode.EXACTLY; - } - - // According to the spec, if the main size is not definite and the - // child's inline axis is parallel to the main axis (i.e. it's - // horizontal), the child should be sized using "UNDEFINED" in - // the main size. Otherwise use "AT_MOST" in the cross axis. - if (!isMainAxisRow && Float.isNaN(childWidth) && !Float.isNaN(availableInnerWidth)) { - childWidth = availableInnerWidth; - childWidthMeasureMode = CSSMeasureMode.AT_MOST; - } - - // The W3C spec doesn't say anything about the 'overflow' property, - // but all major browsers appear to implement the following logic. - if (node.style.overflow == CSSOverflow.HIDDEN) { - if (isMainAxisRow && Float.isNaN(childHeight) && !Float.isNaN(availableInnerHeight)) { - childHeight = availableInnerHeight; + // Main axis + if (isMainAxisRow) { + if (widthMeasureMode == CSSMeasureMode.UNDEFINED || Float.isNaN(availableInnerMainDim)) { + childWidth = CSSConstants.UNDEFINED; + childWidthMeasureMode = CSSMeasureMode.UNDEFINED; + } else { + childWidth = availableInnerMainDim; + childWidthMeasureMode = CSSMeasureMode.AT_MOST; + } + } else if (node.style.overflow == CSSOverflow.HIDDEN) { + if (heightMeasureMode == CSSMeasureMode.UNDEFINED || Float.isNaN(availableInnerMainDim)) { + childHeight = CSSConstants.UNDEFINED; + childHeightMeasureMode = CSSMeasureMode.UNDEFINED; + } else { + childHeight = availableInnerMainDim; childHeightMeasureMode = CSSMeasureMode.AT_MOST; } } - // If child has no defined size in the cross axis and is set to stretch, set the cross - // axis to be measured exactly with the available inner width - if (!isMainAxisRow && - !Float.isNaN(availableInnerWidth) && - !(child.style.dimensions[dim[CSS_FLEX_DIRECTION_ROW]] >= 0.0) && - widthMeasureMode == CSSMeasureMode.EXACTLY && - getAlignItem(node, child) == CSSAlign.STRETCH) { - childWidth = availableInnerWidth; - childWidthMeasureMode = CSSMeasureMode.EXACTLY; - } - if (isMainAxisRow && - !Float.isNaN(availableInnerHeight) && - !(child.style.dimensions[dim[CSS_FLEX_DIRECTION_COLUMN]] >= 0.0) && - heightMeasureMode == CSSMeasureMode.EXACTLY && - getAlignItem(node, child) == CSSAlign.STRETCH) { - childHeight = availableInnerHeight; - childHeightMeasureMode = CSSMeasureMode.EXACTLY; + // Cross axis + if (isMainAxisRow) { + if (node.style.overflow == CSSOverflow.HIDDEN) { + if (!Float.isNaN(availableInnerCrossDim) && + !(child.style.dimensions[dim[CSS_FLEX_DIRECTION_COLUMN]] >= 0.0) && + heightMeasureMode == CSSMeasureMode.EXACTLY && + getAlignItem(node, child) == CSSAlign.STRETCH) { + childHeight = availableInnerCrossDim; + childHeightMeasureMode = CSSMeasureMode.EXACTLY; + } else if (!(child.style.dimensions[dim[CSS_FLEX_DIRECTION_COLUMN]] >= 0.0)) { + childHeight = availableInnerCrossDim; + childHeightMeasureMode = Float.isNaN(childHeight) ? CSSMeasureMode.UNDEFINED : CSSMeasureMode.AT_MOST; + } else { + childHeight = child.style.dimensions[DIMENSION_HEIGHT] + (child.style.margin.getWithFallback(leadingSpacing[CSS_FLEX_DIRECTION_COLUMN], leading[CSS_FLEX_DIRECTION_COLUMN]) + child.style.margin.getWithFallback(trailingSpacing[CSS_FLEX_DIRECTION_COLUMN], trailing[CSS_FLEX_DIRECTION_COLUMN])); + childHeightMeasureMode = CSSMeasureMode.EXACTLY; + } + } + } else { + if (!Float.isNaN(availableInnerCrossDim) && + !(child.style.dimensions[dim[CSS_FLEX_DIRECTION_ROW]] >= 0.0) && + widthMeasureMode == CSSMeasureMode.EXACTLY && + getAlignItem(node, child) == CSSAlign.STRETCH) { + childWidth = availableInnerCrossDim; + childWidthMeasureMode = CSSMeasureMode.EXACTLY; + } else if (!(child.style.dimensions[dim[CSS_FLEX_DIRECTION_ROW]] >= 0.0)) { + childWidth = availableInnerCrossDim; + childWidthMeasureMode = Float.isNaN(childWidth) ? CSSMeasureMode.UNDEFINED : CSSMeasureMode.AT_MOST; + } else { + childWidth = child.style.dimensions[DIMENSION_WIDTH] + (child.style.margin.getWithFallback(leadingSpacing[CSS_FLEX_DIRECTION_ROW], leading[CSS_FLEX_DIRECTION_ROW]) + child.style.margin.getWithFallback(trailingSpacing[CSS_FLEX_DIRECTION_ROW], trailing[CSS_FLEX_DIRECTION_ROW])); + childWidthMeasureMode = CSSMeasureMode.EXACTLY; + } } // Measure the child diff --git a/ReactAndroid/src/main/java/com/facebook/csslayout/README b/ReactAndroid/src/main/java/com/facebook/csslayout/README index 9d85b26ff..5a2ca3b1e 100644 --- a/ReactAndroid/src/main/java/com/facebook/csslayout/README +++ b/ReactAndroid/src/main/java/com/facebook/csslayout/README @@ -1,7 +1,7 @@ The source of truth for css-layout is: https://github.com/facebook/css-layout The code here should be kept in sync with GitHub. -HEAD at the time this code was synced: https://github.com/facebook/css-layout/commit/a1f36b53f5464c8ee7abc311765dc3ecb1b879c6 +HEAD at the time this code was synced: https://github.com/facebook/css-layout/commit/ca34ff44460bce122d02b27c0409a825f8de90b1 There is generated code in: - README (this file)