Skip to content

Commit 4b5ca50

Browse files
NickGerlemanfacebook-github-bot
NickGerleman
authored and
facebook-github-bot
committed
Reland Fix possible invalid measurements when width or height is zero pixels (#1823)
Summary: Pull Request resolved: #1823 X-link: facebook/react-native#52348 Fixes #1819 Yoga has a fast path when measuring a node, if it thinks it knows its dimensions ahead of time. This path has some eroneous logic, to set both axis to owner size, if *either* will evaluate to zero, while having an `YGMeasureModeAtMost`/`FitContent` constraint. This means that if a node is given a zero width, and Yoga later measures with with `FitContent`, its height will become the maximum allowable height, even if it shouldn't be that large. We can fix this, by only allowing if both axis are this fixed case, instead of just one. This bug has existed for about a decade (going back to at least D3312496). Changelog: [General][Fixed] - Fix possible invalid measurements with width or height is zero pixels Reviewed By: yungsters Differential Revision: D76851589 fbshipit-source-id: 6f5a0e6beccc51f591726c9e83e9b90f3350ed0f
1 parent 3029139 commit 4b5ca50

File tree

5 files changed

+268
-9
lines changed

5 files changed

+268
-9
lines changed

‎gentest/fixtures/YGAlignItemsTest.html‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,3 +240,14 @@
240240
<div style="position:relative; width:50px; height:50px;">
241241
</div>
242242
</div>
243+
244+
<div id="align_items_non_stretch_s526008"
245+
style="flex-direction: column; width: 400px; height: 400px;">
246+
<div style="flex-direction: row;">
247+
<div style="flex-direction: column; align-items: flex-start;">
248+
<div>
249+
<div style="width: 0; height: 10px;"></div>
250+
</div>
251+
</div>
252+
</div>
253+
</div>

‎java/tests/generated/com/facebook/yoga/YGAlignItemsTest.java‎

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<e8d11c0e97041bb2f1487f0d87363fdc>>
7+
* @generated SignedSource<<84597dd8b2c4f3aac1f21abe68f25308>>
88
* generated by gentest/gentest-driver.ts from gentest/fixtures/YGAlignItemsTest.html
99
*/
1010

@@ -2293,6 +2293,86 @@ public void test_align_stretch_with_row_reverse() {
22932293
assertEquals(50f, root_child1.getLayoutHeight(), 0.0f);
22942294
}
22952295

2296+
@Test
2297+
public void test_align_items_non_stretch_s526008() {
2298+
YogaConfig config = YogaConfigFactory.create();
2299+
2300+
final YogaNode root = createNode(config);
2301+
root.setPositionType(YogaPositionType.ABSOLUTE);
2302+
root.setWidth(400f);
2303+
root.setHeight(400f);
2304+
2305+
final YogaNode root_child0 = createNode(config);
2306+
root_child0.setFlexDirection(YogaFlexDirection.ROW);
2307+
root.addChildAt(root_child0, 0);
2308+
2309+
final YogaNode root_child0_child0 = createNode(config);
2310+
root_child0_child0.setAlignItems(YogaAlign.FLEX_START);
2311+
root_child0.addChildAt(root_child0_child0, 0);
2312+
2313+
final YogaNode root_child0_child0_child0 = createNode(config);
2314+
root_child0_child0.addChildAt(root_child0_child0_child0, 0);
2315+
2316+
final YogaNode root_child0_child0_child0_child0 = createNode(config);
2317+
root_child0_child0_child0_child0.setHeight(10f);
2318+
root_child0_child0_child0.addChildAt(root_child0_child0_child0_child0, 0);
2319+
root.setDirection(YogaDirection.LTR);
2320+
root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);
2321+
2322+
assertEquals(0f, root.getLayoutX(), 0.0f);
2323+
assertEquals(0f, root.getLayoutY(), 0.0f);
2324+
assertEquals(400f, root.getLayoutWidth(), 0.0f);
2325+
assertEquals(400f, root.getLayoutHeight(), 0.0f);
2326+
2327+
assertEquals(0f, root_child0.getLayoutX(), 0.0f);
2328+
assertEquals(0f, root_child0.getLayoutY(), 0.0f);
2329+
assertEquals(400f, root_child0.getLayoutWidth(), 0.0f);
2330+
assertEquals(10f, root_child0.getLayoutHeight(), 0.0f);
2331+
2332+
assertEquals(0f, root_child0_child0.getLayoutX(), 0.0f);
2333+
assertEquals(0f, root_child0_child0.getLayoutY(), 0.0f);
2334+
assertEquals(0f, root_child0_child0.getLayoutWidth(), 0.0f);
2335+
assertEquals(10f, root_child0_child0.getLayoutHeight(), 0.0f);
2336+
2337+
assertEquals(0f, root_child0_child0_child0.getLayoutX(), 0.0f);
2338+
assertEquals(0f, root_child0_child0_child0.getLayoutY(), 0.0f);
2339+
assertEquals(0f, root_child0_child0_child0.getLayoutWidth(), 0.0f);
2340+
assertEquals(10f, root_child0_child0_child0.getLayoutHeight(), 0.0f);
2341+
2342+
assertEquals(0f, root_child0_child0_child0_child0.getLayoutX(), 0.0f);
2343+
assertEquals(0f, root_child0_child0_child0_child0.getLayoutY(), 0.0f);
2344+
assertEquals(0f, root_child0_child0_child0_child0.getLayoutWidth(), 0.0f);
2345+
assertEquals(10f, root_child0_child0_child0_child0.getLayoutHeight(), 0.0f);
2346+
2347+
root.setDirection(YogaDirection.RTL);
2348+
root.calculateLayout(YogaConstants.UNDEFINED, YogaConstants.UNDEFINED);
2349+
2350+
assertEquals(0f, root.getLayoutX(), 0.0f);
2351+
assertEquals(0f, root.getLayoutY(), 0.0f);
2352+
assertEquals(400f, root.getLayoutWidth(), 0.0f);
2353+
assertEquals(400f, root.getLayoutHeight(), 0.0f);
2354+
2355+
assertEquals(0f, root_child0.getLayoutX(), 0.0f);
2356+
assertEquals(0f, root_child0.getLayoutY(), 0.0f);
2357+
assertEquals(400f, root_child0.getLayoutWidth(), 0.0f);
2358+
assertEquals(10f, root_child0.getLayoutHeight(), 0.0f);
2359+
2360+
assertEquals(400f, root_child0_child0.getLayoutX(), 0.0f);
2361+
assertEquals(0f, root_child0_child0.getLayoutY(), 0.0f);
2362+
assertEquals(0f, root_child0_child0.getLayoutWidth(), 0.0f);
2363+
assertEquals(10f, root_child0_child0.getLayoutHeight(), 0.0f);
2364+
2365+
assertEquals(0f, root_child0_child0_child0.getLayoutX(), 0.0f);
2366+
assertEquals(0f, root_child0_child0_child0.getLayoutY(), 0.0f);
2367+
assertEquals(0f, root_child0_child0_child0.getLayoutWidth(), 0.0f);
2368+
assertEquals(10f, root_child0_child0_child0.getLayoutHeight(), 0.0f);
2369+
2370+
assertEquals(0f, root_child0_child0_child0_child0.getLayoutX(), 0.0f);
2371+
assertEquals(0f, root_child0_child0_child0_child0.getLayoutY(), 0.0f);
2372+
assertEquals(0f, root_child0_child0_child0_child0.getLayoutWidth(), 0.0f);
2373+
assertEquals(10f, root_child0_child0_child0_child0.getLayoutHeight(), 0.0f);
2374+
}
2375+
22962376
private YogaNode createNode(YogaConfig config) {
22972377
return mNodeFactory.create(config);
22982378
}

‎javascript/tests/generated/YGAlignItemsTest.test.ts‎

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* This source code is licensed under the MIT license found in the
55
* LICENSE file in the root directory of this source tree.
66
*
7-
* @generated SignedSource<<075a0b67003e5c4a1f51bd99da71c700>>
7+
* @generated SignedSource<<fd80af208d8635435f86ffa8f3810b39>>
88
* generated by gentest/gentest-driver.ts from gentest/fixtures/YGAlignItemsTest.html
99
*/
1010

@@ -2442,3 +2442,88 @@ test('align_stretch_with_row_reverse', () => {
24422442
config.free();
24432443
}
24442444
});
2445+
test('align_items_non_stretch_s526008', () => {
2446+
const config = Yoga.Config.create();
2447+
let root;
2448+
2449+
try {
2450+
root = Yoga.Node.create(config);
2451+
root.setPositionType(PositionType.Absolute);
2452+
root.setWidth(400);
2453+
root.setHeight(400);
2454+
2455+
const root_child0 = Yoga.Node.create(config);
2456+
root_child0.setFlexDirection(FlexDirection.Row);
2457+
root.insertChild(root_child0, 0);
2458+
2459+
const root_child0_child0 = Yoga.Node.create(config);
2460+
root_child0_child0.setAlignItems(Align.FlexStart);
2461+
root_child0.insertChild(root_child0_child0, 0);
2462+
2463+
const root_child0_child0_child0 = Yoga.Node.create(config);
2464+
root_child0_child0.insertChild(root_child0_child0_child0, 0);
2465+
2466+
const root_child0_child0_child0_child0 = Yoga.Node.create(config);
2467+
root_child0_child0_child0_child0.setHeight(10);
2468+
root_child0_child0_child0.insertChild(root_child0_child0_child0_child0, 0);
2469+
root.calculateLayout(undefined, undefined, Direction.LTR);
2470+
2471+
expect(root.getComputedLeft()).toBe(0);
2472+
expect(root.getComputedTop()).toBe(0);
2473+
expect(root.getComputedWidth()).toBe(400);
2474+
expect(root.getComputedHeight()).toBe(400);
2475+
2476+
expect(root_child0.getComputedLeft()).toBe(0);
2477+
expect(root_child0.getComputedTop()).toBe(0);
2478+
expect(root_child0.getComputedWidth()).toBe(400);
2479+
expect(root_child0.getComputedHeight()).toBe(10);
2480+
2481+
expect(root_child0_child0.getComputedLeft()).toBe(0);
2482+
expect(root_child0_child0.getComputedTop()).toBe(0);
2483+
expect(root_child0_child0.getComputedWidth()).toBe(0);
2484+
expect(root_child0_child0.getComputedHeight()).toBe(10);
2485+
2486+
expect(root_child0_child0_child0.getComputedLeft()).toBe(0);
2487+
expect(root_child0_child0_child0.getComputedTop()).toBe(0);
2488+
expect(root_child0_child0_child0.getComputedWidth()).toBe(0);
2489+
expect(root_child0_child0_child0.getComputedHeight()).toBe(10);
2490+
2491+
expect(root_child0_child0_child0_child0.getComputedLeft()).toBe(0);
2492+
expect(root_child0_child0_child0_child0.getComputedTop()).toBe(0);
2493+
expect(root_child0_child0_child0_child0.getComputedWidth()).toBe(0);
2494+
expect(root_child0_child0_child0_child0.getComputedHeight()).toBe(10);
2495+
2496+
root.calculateLayout(undefined, undefined, Direction.RTL);
2497+
2498+
expect(root.getComputedLeft()).toBe(0);
2499+
expect(root.getComputedTop()).toBe(0);
2500+
expect(root.getComputedWidth()).toBe(400);
2501+
expect(root.getComputedHeight()).toBe(400);
2502+
2503+
expect(root_child0.getComputedLeft()).toBe(0);
2504+
expect(root_child0.getComputedTop()).toBe(0);
2505+
expect(root_child0.getComputedWidth()).toBe(400);
2506+
expect(root_child0.getComputedHeight()).toBe(10);
2507+
2508+
expect(root_child0_child0.getComputedLeft()).toBe(400);
2509+
expect(root_child0_child0.getComputedTop()).toBe(0);
2510+
expect(root_child0_child0.getComputedWidth()).toBe(0);
2511+
expect(root_child0_child0.getComputedHeight()).toBe(10);
2512+
2513+
expect(root_child0_child0_child0.getComputedLeft()).toBe(0);
2514+
expect(root_child0_child0_child0.getComputedTop()).toBe(0);
2515+
expect(root_child0_child0_child0.getComputedWidth()).toBe(0);
2516+
expect(root_child0_child0_child0.getComputedHeight()).toBe(10);
2517+
2518+
expect(root_child0_child0_child0_child0.getComputedLeft()).toBe(0);
2519+
expect(root_child0_child0_child0_child0.getComputedTop()).toBe(0);
2520+
expect(root_child0_child0_child0_child0.getComputedWidth()).toBe(0);
2521+
expect(root_child0_child0_child0_child0.getComputedHeight()).toBe(10);
2522+
} finally {
2523+
if (typeof root !== 'undefined') {
2524+
root.freeRecursive();
2525+
}
2526+
2527+
config.free();
2528+
}
2529+
});

‎tests/generated/YGAlignItemsTest.cpp‎

Lines changed: 82 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
* LICENSE file in the root directory of this source tree.
66
*
77
* clang-format off
8-
* @generated SignedSource<<71295a398c89ea424490884a05e2c77c>>
8+
* @generated SignedSource<<1db57b05babb408c08efcec7dbdf16fb>>
99
* generated by gentest/gentest-driver.ts from gentest/fixtures/YGAlignItemsTest.html
1010
*/
1111

@@ -2310,3 +2310,84 @@ TEST(YogaTest, align_stretch_with_row_reverse) {
23102310

23112311
YGConfigFree(config);
23122312
}
2313+
2314+
TEST(YogaTest, align_items_non_stretch_s526008) {
2315+
YGConfigRef config = YGConfigNew();
2316+
2317+
YGNodeRef root = YGNodeNewWithConfig(config);
2318+
YGNodeStyleSetPositionType(root, YGPositionTypeAbsolute);
2319+
YGNodeStyleSetWidth(root, 400);
2320+
YGNodeStyleSetHeight(root, 400);
2321+
2322+
YGNodeRef root_child0 = YGNodeNewWithConfig(config);
2323+
YGNodeStyleSetFlexDirection(root_child0, YGFlexDirectionRow);
2324+
YGNodeInsertChild(root, root_child0, 0);
2325+
2326+
YGNodeRef root_child0_child0 = YGNodeNewWithConfig(config);
2327+
YGNodeStyleSetAlignItems(root_child0_child0, YGAlignFlexStart);
2328+
YGNodeInsertChild(root_child0, root_child0_child0, 0);
2329+
2330+
YGNodeRef root_child0_child0_child0 = YGNodeNewWithConfig(config);
2331+
YGNodeInsertChild(root_child0_child0, root_child0_child0_child0, 0);
2332+
2333+
YGNodeRef root_child0_child0_child0_child0 = YGNodeNewWithConfig(config);
2334+
YGNodeStyleSetHeight(root_child0_child0_child0_child0, 10);
2335+
YGNodeInsertChild(root_child0_child0_child0, root_child0_child0_child0_child0, 0);
2336+
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionLTR);
2337+
2338+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
2339+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
2340+
ASSERT_FLOAT_EQ(400, YGNodeLayoutGetWidth(root));
2341+
ASSERT_FLOAT_EQ(400, YGNodeLayoutGetHeight(root));
2342+
2343+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0));
2344+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
2345+
ASSERT_FLOAT_EQ(400, YGNodeLayoutGetWidth(root_child0));
2346+
ASSERT_FLOAT_EQ(10, YGNodeLayoutGetHeight(root_child0));
2347+
2348+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0_child0));
2349+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0_child0));
2350+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetWidth(root_child0_child0));
2351+
ASSERT_FLOAT_EQ(10, YGNodeLayoutGetHeight(root_child0_child0));
2352+
2353+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0_child0_child0));
2354+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0_child0_child0));
2355+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetWidth(root_child0_child0_child0));
2356+
ASSERT_FLOAT_EQ(10, YGNodeLayoutGetHeight(root_child0_child0_child0));
2357+
2358+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0_child0_child0_child0));
2359+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0_child0_child0_child0));
2360+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetWidth(root_child0_child0_child0_child0));
2361+
ASSERT_FLOAT_EQ(10, YGNodeLayoutGetHeight(root_child0_child0_child0_child0));
2362+
2363+
YGNodeCalculateLayout(root, YGUndefined, YGUndefined, YGDirectionRTL);
2364+
2365+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root));
2366+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root));
2367+
ASSERT_FLOAT_EQ(400, YGNodeLayoutGetWidth(root));
2368+
ASSERT_FLOAT_EQ(400, YGNodeLayoutGetHeight(root));
2369+
2370+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0));
2371+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0));
2372+
ASSERT_FLOAT_EQ(400, YGNodeLayoutGetWidth(root_child0));
2373+
ASSERT_FLOAT_EQ(10, YGNodeLayoutGetHeight(root_child0));
2374+
2375+
ASSERT_FLOAT_EQ(400, YGNodeLayoutGetLeft(root_child0_child0));
2376+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0_child0));
2377+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetWidth(root_child0_child0));
2378+
ASSERT_FLOAT_EQ(10, YGNodeLayoutGetHeight(root_child0_child0));
2379+
2380+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0_child0_child0));
2381+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0_child0_child0));
2382+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetWidth(root_child0_child0_child0));
2383+
ASSERT_FLOAT_EQ(10, YGNodeLayoutGetHeight(root_child0_child0_child0));
2384+
2385+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetLeft(root_child0_child0_child0_child0));
2386+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetTop(root_child0_child0_child0_child0));
2387+
ASSERT_FLOAT_EQ(0, YGNodeLayoutGetWidth(root_child0_child0_child0_child0));
2388+
ASSERT_FLOAT_EQ(10, YGNodeLayoutGetHeight(root_child0_child0_child0_child0));
2389+
2390+
YGNodeFreeRecursive(root);
2391+
2392+
YGConfigFree(config);
2393+
}

‎yoga/algorithm/CalculateLayout.cpp‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,12 @@ static void measureNodeWithoutChildren(
415415
Dimension::Height);
416416
}
417417

418+
inline bool isFixedSize(float dim, SizingMode sizingMode) {
419+
return sizingMode == SizingMode::StretchFit ||
420+
(yoga::isDefined(dim) && sizingMode == SizingMode::FitContent &&
421+
dim <= 0.0);
422+
}
423+
418424
static bool measureNodeWithFixedSize(
419425
yoga::Node* const node,
420426
const Direction direction,
@@ -424,12 +430,8 @@ static bool measureNodeWithFixedSize(
424430
const SizingMode heightSizingMode,
425431
const float ownerWidth,
426432
const float ownerHeight) {
427-
if ((yoga::isDefined(availableWidth) &&
428-
widthSizingMode == SizingMode::FitContent && availableWidth <= 0.0f) ||
429-
(yoga::isDefined(availableHeight) &&
430-
heightSizingMode == SizingMode::FitContent && availableHeight <= 0.0f) ||
431-
(widthSizingMode == SizingMode::StretchFit &&
432-
heightSizingMode == SizingMode::StretchFit)) {
433+
if (isFixedSize(availableWidth, widthSizingMode) &&
434+
isFixedSize(availableHeight, heightSizingMode)) {
433435
node->setLayoutMeasuredDimension(
434436
boundAxis(
435437
node,

0 commit comments

Comments
 (0)