Skip to content

Commit

Permalink
Fix SingleFlex Child condition
Browse files Browse the repository at this point in the history
Summary:
Fixes the improper `singleFlexChild` optimization. In the case when all the childs have `flex-grow:0 flex-grow:0` except one child with `flex-grow:1 flex-shrink:1`, then one can simply measure all the non-flexing children and then give the flexing child all the remaining space.

Also added a test case which reproduced the bug

Reviewed By: IanChilds

Differential Revision: D8782684

fbshipit-source-id: ffd4d35b6122f82111b987540efb23bd2a8da5a2
  • Loading branch information
priteshrnandgaonkar authored and facebook-github-bot committed Jul 11, 2018
1 parent 966f5ec commit 6548ddd
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 17 deletions.
94 changes: 89 additions & 5 deletions tests/YGFlexTest.cpp
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
/**
* Copyright (c) 2014-present, Facebook, Inc.
/*
* Copyright (c) 2014-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the LICENSE
* file in the root directory of this source tree.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

// @Generated by gentest/gentest.rb from gentest/fixtures/YGFlexTest.html

#include <gtest/gtest.h>
Expand Down Expand Up @@ -64,6 +64,90 @@ TEST(YogaTest, flex_basis_flex_grow_column) {
YGConfigFree(config);
}

TEST(YogaTest, flex_shrink_flex_grow_row) {
const YGConfigRef config = YGConfigNew();

const YGNodeRef root = YGNodeNewWithConfig(config);
YGNodeStyleSetFlexDirection(root, YGFlexDirectionRow);
YGNodeStyleSetWidth(root, 500);
YGNodeStyleSetHeight(root, 500);

const YGNodeRef root_child0 = YGNodeNewWithConfig(config);
YGNodeStyleSetFlexGrow(root_child0, 0);
YGNodeStyleSetFlexShrink(root_child0, 1);
YGNodeStyleSetWidth(root_child0, 500);
YGNodeStyleSetHeight(root_child0, 100);
YGNodeInsertChild(root, root_child0, 0);

const YGNodeRef root_child1 = YGNodeNewWithConfig(config);
YGNodeStyleSetFlexGrow(root_child1, 0);
YGNodeStyleSetFlexShrink(root_child1, 1);
YGNodeStyleSetWidth(root_child1, 500);
YGNodeStyleSetHeight(root_child1, 100);
YGNodeInsertChild(root, root_child1, 1);
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
ASSERT_FLOAT_EQ(500, YGNodeLayoutGetWidth(root));
ASSERT_FLOAT_EQ(500, YGNodeLayoutGetHeight(root));

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(250, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root_child0));

ASSERT_FLOAT_EQ(250, YGNodeLayoutGetLeft(root_child1));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child1));
ASSERT_FLOAT_EQ(250, YGNodeLayoutGetWidth(root_child1));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root_child1));
YGNodeFreeRecursive(root);

YGConfigFree(config);
}

TEST(YogaTest, flex_shrink_flex_grow_child_flex_shrink_other_child) {
const YGConfigRef config = YGConfigNew();

const YGNodeRef root = YGNodeNewWithConfig(config);
YGNodeStyleSetFlexDirection(root, YGFlexDirectionRow);
YGNodeStyleSetWidth(root, 500);
YGNodeStyleSetHeight(root, 500);

const YGNodeRef root_child0 = YGNodeNewWithConfig(config);
YGNodeStyleSetFlexGrow(root_child0, 0);
YGNodeStyleSetFlexShrink(root_child0, 1);
YGNodeStyleSetWidth(root_child0, 500);
YGNodeStyleSetHeight(root_child0, 100);
YGNodeInsertChild(root, root_child0, 0);

const YGNodeRef root_child1 = YGNodeNewWithConfig(config);
YGNodeStyleSetFlexGrow(root_child1, 1);
YGNodeStyleSetFlexShrink(root_child1, 1);
YGNodeStyleSetWidth(root_child1, 500);
YGNodeStyleSetHeight(root_child1, 100);
YGNodeInsertChild(root, root_child1, 1);
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
ASSERT_FLOAT_EQ(500, YGNodeLayoutGetWidth(root));
ASSERT_FLOAT_EQ(500, YGNodeLayoutGetHeight(root));

ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
ASSERT_FLOAT_EQ(250, YGNodeLayoutGetWidth(root_child0));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root_child0));

ASSERT_FLOAT_EQ(250, YGNodeLayoutGetLeft(root_child1));
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child1));
ASSERT_FLOAT_EQ(250, YGNodeLayoutGetWidth(root_child1));
ASSERT_FLOAT_EQ(100, YGNodeLayoutGetHeight(root_child1));
YGNodeFreeRecursive(root);

YGConfigFree(config);
}

TEST(YogaTest, flex_basis_flex_grow_row) {
const YGConfigRef config = YGConfigNew();

Expand Down
10 changes: 5 additions & 5 deletions tests/YGMeasureTest.cpp
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
/**
* Copyright (c) 2014-present, Facebook, Inc.
/*
* Copyright (c) 2014-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the LICENSE
* file in the root directory of this source tree.
*
* 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 <yoga/Yoga.h>
Expand Down
15 changes: 8 additions & 7 deletions yoga/Yoga.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1788,16 +1788,17 @@ static void YGNodeComputeFlexBasisForChildren(
// child to exactly match the remaining space
if (measureModeMainDim == YGMeasureModeExactly) {
for (auto child : children) {
if (singleFlexChild != nullptr) {
if (child->isNodeFlexible()) {
// There is already a flexible child, abort
if (child->isNodeFlexible()) {
if (singleFlexChild != nullptr ||
YGFloatsEqual(child->resolveFlexGrow(), 0.0f) ||
YGFloatsEqual(child->resolveFlexShrink(), 0.0f)) {
// There is already a flexible child, or this flexible child doesn't
// have flexGrow and flexShrink, abort
singleFlexChild = nullptr;
break;
} else {
singleFlexChild = child;
}
} else if (
child->resolveFlexGrow() > 0.0f &&
child->resolveFlexShrink() > 0.0f) {
singleFlexChild = child;
}
}
}
Expand Down

0 comments on commit 6548ddd

Please sign in to comment.