Extra spaces around `FlexWrap::Wrap` content if container width not specified with `Val::Px` #8082

jkb0o · 2023-03-14T09:27:41Z

Bevy version

bevy 0.11.0-dev, rev e77eb00
taffy v0.3.7

What you did

I put into flex-direction: column container (white) another flex-wrap: wrap container (grey) with 2-6 children (dark grey).

What went wrong

The wrap container contributes extra-space around content. It looks valid only if I specify the exact width in pixels of the wrap container. The amount of contributed space depends on the number of wrapped children.

Expected result:

Actual result:

Sample:

use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_startup_system(setup)
        .run();
}

fn setup(mut commands: Commands) {
    commands.spawn(Camera2dBundle::default());
    commands.spawn(NodeBundle {
        background_color: Color::WHITE.into(),
        style: Style { 
            margin: UiRect::all(Val::Px(20.)),
            size: Size::new(Val::Px(200.), Val::Px(400.)),
            align_items: AlignItems::Stretch,
            align_content: AlignContent::Center,
            justify_content: JustifyContent::FlexStart,
            flex_direction: FlexDirection::Column,
            ..default()
        },
        ..default()
    }).with_children(|parent| {
        parent.spawn(NodeBundle {
                background_color: Color::GRAY.into(),
                style: Style {
                    // Uncomment one or both of next two lines to get valid beahaviour
                    // size: Size::new(Val::Px(200.), Val::Auto),
                    // align_self: AlignSelf::Center,
                    align_items: AlignItems::FlexStart,
                    align_content: AlignContent::Center,
                    justify_content: JustifyContent::Center,
                    flex_wrap: FlexWrap::Wrap,
                    ..default()
                },
                ..default()
        }).with_children(|parent| {
            for _ in 0..4 {
                parent.spawn(NodeBundle {
                    background_color: Color::DARK_GRAY.into(),
                    style: Style {
                        size: Size::all(Val::Px(50.)),
                        margin: UiRect::all(Val::Px(10.)),
                        align_items: AlignItems::Center,
                        align_content: AlignContent::Center,
                        justify_content: JustifyContent::Center,
                        ..default()
                    },
                    ..default()
                });
            }
        });
    });
}

ui_wrap.zip

The text was updated successfully, but these errors were encountered:

doup · 2023-03-14T14:04:51Z

@nicoburns I've reduced the code a little bit… I'm pasting it here in case it's useful:

Reduced Rust code

commands.spawn(NodeBundle {
    background_color: Color::WHITE.into(),
    style: Style {
        size: Size::new(Val::Px(200.), Val::Px(400.)),
        flex_direction: FlexDirection::Column,
        ..default()
    },
    ..default()
}).with_children(|parent| {
    parent.spawn(NodeBundle {
            background_color: Color::GRAY.into(),
            style: Style {
                align_content: AlignContent::Center,
                justify_content: JustifyContent::Center,
                flex_wrap: FlexWrap::Wrap,
                ..default()
            },
            ..default()
    }).with_children(|parent| {
        for _ in 0..2 {
            parent.spawn(NodeBundle {
                background_color: Color::DARK_GRAY.into(),
                style: Style {
                    size: Size::all(Val::Px(50.)),
                    margin: UiRect::all(Val::Px(10.)),
                    ..default()
                },
                ..default()
            });
        }
    });
});

HTML version

<!DOCTYPE html>
<html lang="en">
<head>
    <script src="../scripts/gentest/test_helper.js"></script>
    <link rel="stylesheet" type="text/css" href="../scripts/gentest/test_base_style.css">
    <title>Bevy #8082</title>
</head>
<body>

<div id="test-root" style="width: 200px; height:400px; display: flex; flex-direction: column;">
    <div style="display: flex; align-content: center; justify-content: center; flex-wrap: wrap;">
        <div style="width: 50px; height: 50px; margin: 10px;"></div>
        <div style="width: 50px; height: 50px; margin: 10px;"></div>
        <div style="width: 50px; height: 50px; margin: 10px;"></div>
        <div style="width: 50px; height: 50px; margin: 10px;"></div>
    </div>
</div>

</body>
</html>

doup · 2023-03-14T14:09:44Z

I've updated the HTML above so you can directly use it as a Taffy test. Here is the test output:

---- generated::bevy_8082::bevy_8082 stdout ----

Computed tree:
TREE
└──  FLEX [x: 0    y: 0    width: 200  height: 400 ] (6v1)
    └──  FLEX [x: 0    y: 0    width: 200  height: 280 ] (5v1)
        ├──  LEAF [x: 40   y: 80   width: 50   height: 50  ] (1v1)
        ├──  LEAF [x: 110  y: 80   width: 50   height: 50  ] (2v1)
        ├──  LEAF [x: 40   y: 150  width: 50   height: 50  ] (3v1)
        └──  LEAF [x: 110  y: 150  width: 50   height: 50  ] (4v1)

thread 'generated::bevy_8082::bevy_8082' panicked at 'assertion failed: `(left == right)`
  left: `280.0`,
 right: `140.0`: height of node 5v1. Expected 140. Actual 280', tests/generated/bevy_8082.rs:104:5

nicoburns · 2023-03-14T15:20:30Z

I can see why this is happening. It's because when the flex column attempts to determine it's child's minimum size in the main (vertical) axis, it is passing "min-content" as the constraint in both axis. This is causing the flex-wrap node to wrap at every wrap point, which makes it twice as high. What should be happening is that "min-content" is passed as the constraint only in the main (vertical) axis, which will cause the child to wrap as expected.

I have a fix, but I want to do some additional testing to ensure that related cases are also covered.

* Add tests for bevyengine/bevy#8082 * Debug log parent size * Pass correct cross-axis parent_size/available_space when computing a flex item's min-content contribution

nicoburns · 2023-03-14T16:11:14Z

Taffy v0.3.8 has been released and fixes this issue.

jkb0o · 2023-03-14T17:24:25Z

Works like a charm! Thank you!

* Fix layout of direct flex children with `display:none` set (#380) * Add test for toggling display:none * Add test for toggling display:none on a flexbox child * Fix layout direct flex children with `display:none` set * Add test for toggling display:none on flexbox container * Fix setting display:none on a grid child (#382) * Prepare for 0.3.5 release (#381) * Prepare for 0.3.5 release * Add grid PR to release notes * Ignore align_content in non-wrapping flexbox containers (#383) * Add tests for bevyengine/bevy#7976 * Simplify code in compute_constants * Ignore align_contents if flex_wrap is set to no_wrap * Remove commented out code * Prepare for v0.3.6 release (#384) * Use mutable slice rather mutable vec parameter * Reduce style accesses (#386) * Use cached flex_grow and flex_shrink values when resolving flexible lengths * Cache auto margins on FlexItem struct * Use constants.is_wrap instead of re-resolving style * Use cached align_content value rather than re-resolving style * Cache justify_content style in AlgoConstants * Add failing tests for borders flooring node size * Add failing tests for padding flooring node size * Fix divide by zero in main size determination * Add seperate leaf and flexbox tests for padding/border flooring node size * Fix padding/border flooring leaf node size * Fix flexbox children being floored by padding/border * Combine padding and border tests + make edge dimensions uneven * Add padding/border floor absolute child size * Add padding/border floor node size tests for grid * Add grid container test for padding/border flooring node size * Fix padding/border flooring size of absolutely positioned children * Rename grid padding/border tests so that they are all located together * Make padding/border floor node size for grid children * Make padding/border floor node size for grid containers * Apply aspect ratio to leaf nodes whose size is determined by padding/border sum * Add test for padding/border not affecting flex basis * Convert flex basis determination to use break from block * Add test for flex-basis 0 with flex grow * Floor flex-basis limits by preferred size style value when determining flex container main size * Floor outer flex-basis by padding_border sum (floors inner flex-basis at 0) * Remove commented out code * Add missing 0 * Test cases for bevyengine/bevy#8017 and Taffy #387 * Prevent percentage sizes from contributing a flex item's min-content size * Prepare for 0.3.7 release (#389) * Prepare for 0.3.7 release * Fix duplicate content in the changelog * Fix markdown lint * Upgrade to better Github Actions for faster and better maintained CI (#390) * Use taiki-e/install-action for installing cargo-deny * Update CI to use dtolnay/rust-toolchain action * Enable CI for 0.3.x branch * Fix documentation CI * Dummy Cargo.toml change * Add fully reduced test case for #387 (#391) * Fix wrapping nodes generating an incorrect min-content size (#395) * Add tests for bevyengine/bevy#8082 * Debug log parent size * Pass correct cross-axis parent_size/available_space when computing a flex item's min-content contribution * Prepare for 0.3.8 release (#396) * Don't allow cached results to be used for sizings with greater available space (#397) * Don't allow cached results to be used for sizings with greater available space * Update caching tests to use tree 100 nodes deep (relax permitted measure count to 7) * Prepare for 0.3.9 release (#398) * Prepare for 0.3.9 release * Add bevyengine/bevy#8124 to release notes * Allow multiple syn versions * Tree creation benchmarks (#401) * Add tree creation benchmarks * Rename allocation benchmark to tree_creation * Update rstest requirement from 0.16.0 to 0.17.0 (#402) Updates the requirements on [rstest](https://github.com/la10736/rstest) to permit the latest version. - [Release notes](https://github.com/la10736/rstest/releases) - [Changelog](https://github.com/la10736/rstest/blob/master/CHANGELOG.md) - [Commits](la10736/rstest@0.16.0...0.17.0) --- updated-dependencies: - dependency-name: rstest dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Update syn requirement from 1.0.7 to 2.0.4 (#403) Updates the requirements on [syn](https://github.com/dtolnay/syn) to permit the latest version. - [Release notes](https://github.com/dtolnay/syn/releases) - [Commits](dtolnay/syn@1.0.7...2.0.4) --- updated-dependencies: - dependency-name: syn dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix: available space in the presence of a min width (#407) * Add `with_main` and `with_cross` methods to Size * Add test for min_width > available_space * Clamp available cross space by min/max height when computing flex basis * Clamp available cross space by min/max size when computing hypothetical cross size * Bump measure func call counts to 8 * Prepare for 0.3.10 release (#411) * Fix import lints (#416) (cherry picked from commit 5522573) * Add caching to CI (#418) * Optimise flexbox layouts with min/max sizes (#413) * Remove 2-pass min/max code path + implement main size min/max clamping within determine_container_main_size method * Fix typo in flex_grow_within_constrained_min_max_column test * Implement cross-size min/max clamping inline in algorithm * Remove debug log * Remove commented code * Prepare for 0.3.11 release (#419) --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Andreas Weibye <13300393+Weibye@users.noreply.github.com> Co-authored-by: TimJentzsch <commits@timjen.net>

jkb0o added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Mar 14, 2023

james7132 added A-UI Graphical user interfaces, styles, layouts, and widgets and removed S-Needs-Triage This issue needs to be labelled labels Mar 14, 2023

nicoburns added a commit to nicoburns/taffy that referenced this issue Mar 14, 2023

Add tests for bevyengine/bevy#8082

e670a06

nicoburns mentioned this issue Mar 14, 2023

Fix wrapping nodes generating an incorrect min-content size DioxusLabs/taffy#395

Merged

nicoburns closed this as completed in DioxusLabs/taffy#395 Mar 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Extra spaces around `FlexWrap::Wrap` content if container width not specified with `Val::Px` #8082

Extra spaces around `FlexWrap::Wrap` content if container width not specified with `Val::Px` #8082

jkb0o commented Mar 14, 2023 •

edited

Loading

doup commented Mar 14, 2023 •

edited

Loading

doup commented Mar 14, 2023

nicoburns commented Mar 14, 2023

nicoburns commented Mar 14, 2023

jkb0o commented Mar 14, 2023

Extra spaces around FlexWrap::Wrap content if container width not specified with Val::Px #8082

Extra spaces around FlexWrap::Wrap content if container width not specified with Val::Px #8082

Comments

jkb0o commented Mar 14, 2023 • edited Loading

Bevy version

What you did

What went wrong

Sample:

doup commented Mar 14, 2023 • edited Loading

doup commented Mar 14, 2023

nicoburns commented Mar 14, 2023

nicoburns commented Mar 14, 2023

jkb0o commented Mar 14, 2023

Extra spaces around `FlexWrap::Wrap` content if container width not specified with `Val::Px` #8082

Extra spaces around `FlexWrap::Wrap` content if container width not specified with `Val::Px` #8082

jkb0o commented Mar 14, 2023 •

edited

Loading

doup commented Mar 14, 2023 •

edited

Loading