Fix resolving bug on changing layout and optimized memory usage

Summary:
Due to the changes in 46817a38. We have a bug if the layout changes.

This PR fixes this bug and adds a test for it.

Additionally it correctly marks negative percentage values as undefined dim.

It also changes the ```resolvedDimensions``` to use a reference instead of the full value in order to minimize the memory requirement of a ```YGNode``` and reduces the copying of the ```YGValue```.
Closes https://github.com/facebook/yoga/pull/379

Reviewed By: gkassabli

Differential Revision: D4528552

Pulled By: emilsjolander

fbshipit-source-id: c024fe3a009c3788af319b689858ea3374c46477
This commit is contained in:
Lukas Wöhrl
2017-02-11 08:32:50 -08:00
committed by Facebook Github Bot
parent 6a7ad2125d
commit adf8691093
2 changed files with 41 additions and 22 deletions

View File

@@ -29,3 +29,22 @@ TEST(YogaTest, dont_cache_computed_flex_basis_between_layouts) {
YGSetExperimentalFeatureEnabled(YGExperimentalFeatureWebFlexBasis, false); YGSetExperimentalFeatureEnabled(YGExperimentalFeatureWebFlexBasis, false);
} }
TEST(YogaTest, recalculate_resolvedDimonsion_onchange) {
const YGNodeRef root = YGNodeNew();
const YGNodeRef root_child0 = YGNodeNew();
YGNodeStyleSetMinHeight(root_child0, 10);
YGNodeStyleSetMaxHeight(root_child0, 10);
YGNodeInsertChild(root, root_child0, 0);
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
ASSERT_FLOAT_EQ(10, YGNodeLayoutGetHeight(root_child0));
YGNodeStyleSetMinHeight(root_child0, YGUndefined);
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetHeight(root_child0));
YGNodeFreeRecursive(root);
}

View File

@@ -112,7 +112,7 @@ typedef struct YGNode {
bool isDirty; bool isDirty;
bool hasNewLayout; bool hasNewLayout;
YGValue resolvedDimensions[2]; YGValue const *resolvedDimensions[2];
} YGNode; } YGNode;
#define YG_UNDEFINED_VALUES \ #define YG_UNDEFINED_VALUES \
@@ -138,7 +138,8 @@ static YGNode gYGNodeDefaults = {
.children = NULL, .children = NULL,
.hasNewLayout = true, .hasNewLayout = true,
.isDirty = false, .isDirty = false,
.resolvedDimensions = YG_DEFAULT_DIMENSION_VALUES_UNIT, .resolvedDimensions = {[YGDimensionWidth] = &YGValueUndefined,
[YGDimensionHeight] = &YGValueUndefined},
.style = .style =
{ {
@@ -643,9 +644,9 @@ static inline void YGResolveDimensions(YGNodeRef node) {
for (YGDimension dim = YGDimensionWidth; dim <= YGDimensionHeight; dim++) { for (YGDimension dim = YGDimensionWidth; dim <= YGDimensionHeight; dim++) {
if (node->style.maxDimensions[dim].unit != YGUnitUndefined && if (node->style.maxDimensions[dim].unit != YGUnitUndefined &&
YGValueEqual(node->style.maxDimensions[dim], node->style.minDimensions[dim])) { YGValueEqual(node->style.maxDimensions[dim], node->style.minDimensions[dim])) {
node->resolvedDimensions[dim] = node->style.maxDimensions[dim]; node->resolvedDimensions[dim] = &node->style.maxDimensions[dim];
} else { } else {
node->resolvedDimensions[dim] = node->style.dimensions[dim]; node->resolvedDimensions[dim] = &node->style.dimensions[dim];
} }
} }
} }
@@ -1095,11 +1096,11 @@ static inline float YGNodeDimWithMargin(const YGNodeRef node,
static inline bool YGNodeIsStyleDimDefined(const YGNodeRef node, static inline bool YGNodeIsStyleDimDefined(const YGNodeRef node,
const YGFlexDirection axis, const YGFlexDirection axis,
const float parentSize) { const float parentSize) {
return !(node->resolvedDimensions[dim[axis]].unit == YGUnitUndefined || return !(node->resolvedDimensions[dim[axis]]->unit == YGUnitUndefined ||
(node->resolvedDimensions[dim[axis]].unit == YGUnitPixel && (node->resolvedDimensions[dim[axis]]->unit == YGUnitPixel &&
node->resolvedDimensions[dim[axis]].value < 0.0f) || node->resolvedDimensions[dim[axis]]->value < 0.0f) ||
(node->resolvedDimensions[dim[axis]].unit == YGUnitPercent && (node->resolvedDimensions[dim[axis]]->unit == YGUnitPercent &&
YGFloatIsUndefined(parentSize))); (node->resolvedDimensions[dim[axis]]->value < 0.0f || YGFloatIsUndefined(parentSize))));
} }
static inline bool YGNodeIsLayoutDimDefined(const YGNodeRef node, const YGFlexDirection axis) { static inline bool YGNodeIsLayoutDimDefined(const YGNodeRef node, const YGFlexDirection axis) {
@@ -1286,12 +1287,12 @@ static void YGNodeComputeFlexBasisForChild(const YGNodeRef node,
} else if (isMainAxisRow && isRowStyleDimDefined) { } else if (isMainAxisRow && isRowStyleDimDefined) {
// The width is definite, so use that as the flex basis. // The width is definite, so use that as the flex basis.
child->layout.computedFlexBasis = child->layout.computedFlexBasis =
fmaxf(YGValueResolve(&child->resolvedDimensions[YGDimensionWidth], parentWidth), fmaxf(YGValueResolve(child->resolvedDimensions[YGDimensionWidth], parentWidth),
YGNodePaddingAndBorderForAxis(child, YGFlexDirectionRow, parentWidth)); YGNodePaddingAndBorderForAxis(child, YGFlexDirectionRow, parentWidth));
} else if (!isMainAxisRow && isColumnStyleDimDefined) { } else if (!isMainAxisRow && isColumnStyleDimDefined) {
// The height is definite, so use that as the flex basis. // The height is definite, so use that as the flex basis.
child->layout.computedFlexBasis = child->layout.computedFlexBasis =
fmaxf(YGValueResolve(&child->resolvedDimensions[YGDimensionHeight], parentHeight), fmaxf(YGValueResolve(child->resolvedDimensions[YGDimensionHeight], parentHeight),
YGNodePaddingAndBorderForAxis(child, YGFlexDirectionColumn, parentWidth)); YGNodePaddingAndBorderForAxis(child, YGFlexDirectionColumn, parentWidth));
} else { } else {
// Compute the flex basis and hypothetical main size (i.e. the clamped // Compute the flex basis and hypothetical main size (i.e. the clamped
@@ -1306,12 +1307,12 @@ static void YGNodeComputeFlexBasisForChild(const YGNodeRef node,
if (isRowStyleDimDefined) { if (isRowStyleDimDefined) {
childWidth = childWidth =
YGValueResolve(&child->resolvedDimensions[YGDimensionWidth], parentWidth) + marginRow; YGValueResolve(child->resolvedDimensions[YGDimensionWidth], parentWidth) + marginRow;
childWidthMeasureMode = YGMeasureModeExactly; childWidthMeasureMode = YGMeasureModeExactly;
} }
if (isColumnStyleDimDefined) { if (isColumnStyleDimDefined) {
childHeight = YGValueResolve(&child->resolvedDimensions[YGDimensionHeight], parentHeight) + childHeight =
marginColumn; YGValueResolve(child->resolvedDimensions[YGDimensionHeight], parentHeight) + marginColumn;
childHeightMeasureMode = YGMeasureModeExactly; childHeightMeasureMode = YGMeasureModeExactly;
} }
@@ -1410,7 +1411,7 @@ static void YGNodeAbsoluteLayoutChild(const YGNodeRef node,
const float marginColumn = YGNodeMarginForAxis(child, YGFlexDirectionColumn, width); const float marginColumn = YGNodeMarginForAxis(child, YGFlexDirectionColumn, width);
if (YGNodeIsStyleDimDefined(child, YGFlexDirectionRow, width)) { if (YGNodeIsStyleDimDefined(child, YGFlexDirectionRow, width)) {
childWidth = YGValueResolve(&child->resolvedDimensions[YGDimensionWidth], width) + marginRow; childWidth = YGValueResolve(child->resolvedDimensions[YGDimensionWidth], width) + marginRow;
} else { } else {
// If the child doesn't have a specified width, compute the width based // If the child doesn't have a specified width, compute the width based
// on the left/right // on the left/right
@@ -1428,7 +1429,7 @@ static void YGNodeAbsoluteLayoutChild(const YGNodeRef node,
if (YGNodeIsStyleDimDefined(child, YGFlexDirectionColumn, height)) { if (YGNodeIsStyleDimDefined(child, YGFlexDirectionColumn, height)) {
childHeight = childHeight =
YGValueResolve(&child->resolvedDimensions[YGDimensionHeight], height) + marginColumn; YGValueResolve(child->resolvedDimensions[YGDimensionHeight], height) + marginColumn;
} else { } else {
// If the child doesn't have a specified height, compute the height // If the child doesn't have a specified height, compute the height
// based on the top/bottom // based on the top/bottom
@@ -2297,7 +2298,7 @@ static void YGNodelayoutImpl(const YGNodeRef node,
YGFloatIsUndefined(childHeight) ? YGMeasureModeUndefined : YGMeasureModeAtMost; YGFloatIsUndefined(childHeight) ? YGMeasureModeUndefined : YGMeasureModeAtMost;
} else { } else {
childHeight = childHeight =
YGValueResolve(&currentRelativeChild->resolvedDimensions[YGDimensionHeight], YGValueResolve(currentRelativeChild->resolvedDimensions[YGDimensionHeight],
availableInnerHeight) + availableInnerHeight) +
marginColumn; marginColumn;
childHeightMeasureMode = childHeightMeasureMode =
@@ -2322,7 +2323,7 @@ static void YGNodelayoutImpl(const YGNodeRef node,
childWidthMeasureMode = childWidthMeasureMode =
YGFloatIsUndefined(childWidth) ? YGMeasureModeUndefined : YGMeasureModeAtMost; YGFloatIsUndefined(childWidth) ? YGMeasureModeUndefined : YGMeasureModeAtMost;
} else { } else {
childWidth = YGValueResolve(&currentRelativeChild->resolvedDimensions[YGDimensionWidth], childWidth = YGValueResolve(currentRelativeChild->resolvedDimensions[YGDimensionWidth],
availableInnerWidth) + availableInnerWidth) +
marginRow; marginRow;
childWidthMeasureMode = childWidthMeasureMode =
@@ -3201,7 +3202,7 @@ void YGNodeCalculateLayout(const YGNodeRef node,
if (!YGFloatIsUndefined(width)) { if (!YGFloatIsUndefined(width)) {
widthMeasureMode = YGMeasureModeExactly; widthMeasureMode = YGMeasureModeExactly;
} else if (YGNodeIsStyleDimDefined(node, YGFlexDirectionRow, availableWidth)) { } else if (YGNodeIsStyleDimDefined(node, YGFlexDirectionRow, availableWidth)) {
width = YGValueResolve(&node->resolvedDimensions[dim[YGFlexDirectionRow]], availableWidth) + width = YGValueResolve(node->resolvedDimensions[dim[YGFlexDirectionRow]], availableWidth) +
YGNodeMarginForAxis(node, YGFlexDirectionRow, availableWidth); YGNodeMarginForAxis(node, YGFlexDirectionRow, availableWidth);
widthMeasureMode = YGMeasureModeExactly; widthMeasureMode = YGMeasureModeExactly;
} else if (YGValueResolve(&node->style.maxDimensions[YGDimensionWidth], availableWidth) >= 0.0f) { } else if (YGValueResolve(&node->style.maxDimensions[YGDimensionWidth], availableWidth) >= 0.0f) {
@@ -3212,9 +3213,8 @@ void YGNodeCalculateLayout(const YGNodeRef node,
if (!YGFloatIsUndefined(height)) { if (!YGFloatIsUndefined(height)) {
heightMeasureMode = YGMeasureModeExactly; heightMeasureMode = YGMeasureModeExactly;
} else if (YGNodeIsStyleDimDefined(node, YGFlexDirectionColumn, availableHeight)) { } else if (YGNodeIsStyleDimDefined(node, YGFlexDirectionColumn, availableHeight)) {
height = height = YGValueResolve(node->resolvedDimensions[dim[YGFlexDirectionColumn]], availableHeight) +
YGValueResolve(&node->resolvedDimensions[dim[YGFlexDirectionColumn]], availableHeight) + YGNodeMarginForAxis(node, YGFlexDirectionColumn, availableWidth);
YGNodeMarginForAxis(node, YGFlexDirectionColumn, availableWidth);
heightMeasureMode = YGMeasureModeExactly; heightMeasureMode = YGMeasureModeExactly;
} else if (YGValueResolve(&node->style.maxDimensions[YGDimensionHeight], availableHeight) >= } else if (YGValueResolve(&node->style.maxDimensions[YGDimensionHeight], availableHeight) >=
0.0f) { 0.0f) {