Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix non-exact text h-alignment #13846

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

robtfm
Copy link
Contributor

@robtfm robtfm commented Jun 14, 2024

Objective

when a parent container is auto-sized, text alignments Center and Right don't align to the center and right properly. fix it

Solution

ab_glyph positions return +/- values from an anchor point. we currently transform them to positive values from the min-x of the glyphs, and then offset from the left of the bounds. instead, we can keep the negative values as ab_glyph intended and offset from the left/middle/right of the bounds as appropriate.

Testing

texts with align left, center, right, all contained in the purple boxes:
before (0.14.0-rc.2):
Screenshot 2024-06-14 165456

after:

Screenshot 2024-06-14 164449

code:

use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .run();
}

fn setup(mut commands: Commands) {
    commands.spawn(Camera2dBundle::default());

    for (left, justify) in [
        (100.0, JustifyText::Left),
        (500.0, JustifyText::Center),
        (900.0, JustifyText::Right),
    ] {
        commands
        // container
        .spawn(NodeBundle {
            style: Style {
                flex_direction: FlexDirection::Column,
                position_type: PositionType::Absolute,
                left: Val::Px(left),
                top: Val::Px(100.0),
                width: Val::Px(300.0),
                ..Default::default()
            },
            ..Default::default()
        })
        .with_children(|commands| {
            commands.spawn(NodeBundle{
                style: Style {
                    flex_direction: FlexDirection::Row,
                    height: Val::Px(75.0),
                    ..Default::default()
                },
                background_color: Color::srgb(1.0, 0.0, 1.0).into(),
                ..Default::default()
            }).with_children(|commands| {
                // a div that reduces the available size
                commands.spawn(NodeBundle {
                    style: Style {
                        width: Val::Px(75.0),
                        ..Default::default()
                    },
                    background_color: Color::srgb(0.0, 1.0, 0.0).into(),
                    ..Default::default()
                });

                // text with width=auto, but actual size will not be what it expcets due to the sibling div above
                commands.spawn(TextBundle {
                    text: Text::from_section("Some text that wraps onto a second line", Default::default()).with_justify(justify),
                    style: Style {
                        align_self: AlignSelf::Center,
                        ..Default::default()
                    },
                    ..Default::default()
                });
            });
        });
    }
}

@robtfm robtfm added C-Bug An unexpected or incorrect behavior A-Text Rendering and layout for characters labels Jun 14, 2024
@robtfm
Copy link
Contributor Author

robtfm commented Jun 14, 2024

i haven't tested subpixel offsets explicitly (not really sure what i'm looking for), but i did floor the anchor so that the new position will be a whole number of pixels from where it was originally. hopefully that works.

@alice-i-cecile alice-i-cecile added X-Uncontroversial This work is generally agreed upon D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jun 14, 2024
@alice-i-cecile alice-i-cecile added this to the 0.14 milestone Jun 14, 2024
@mockersf mockersf added this pull request to the merge queue Jun 14, 2024
Merged via the queue into bevyengine:main with commit 01971f2 Jun 14, 2024
33 checks passed
mockersf pushed a commit that referenced this pull request Jun 14, 2024
# Objective

when a parent container is auto-sized, text alignments `Center` and
`Right` don't align to the center and right properly. fix it

## Solution

ab_glyph positions return +/- values from an anchor point. we currently
transform them to positive values from the min-x of the glyphs, and then
offset from the left of the bounds. instead, we can keep the negative
values as ab_glyph intended and offset from the left/middle/right of the
bounds as appropriate.

## Testing

texts with align left, center, right, all contained in the purple boxes:
before (0.14.0-rc.2):
![Screenshot 2024-06-14
165456](https://github.com/bevyengine/bevy/assets/50659922/90fb73b0-d8bd-4ae8-abf3-7106eafc93ba)

after:

![Screenshot 2024-06-14
164449](https://github.com/bevyengine/bevy/assets/50659922/0a75ff09-b51d-4fbe-a491-b655a145c08b)

code:
```rs
use bevy::prelude::*;

fn main() {
    App::new()
        .add_plugins(DefaultPlugins)
        .add_systems(Startup, setup)
        .run();
}

fn setup(mut commands: Commands) {
    commands.spawn(Camera2dBundle::default());

    for (left, justify) in [
        (100.0, JustifyText::Left),
        (500.0, JustifyText::Center),
        (900.0, JustifyText::Right),
    ] {
        commands
        // container
        .spawn(NodeBundle {
            style: Style {
                flex_direction: FlexDirection::Column,
                position_type: PositionType::Absolute,
                left: Val::Px(left),
                top: Val::Px(100.0),
                width: Val::Px(300.0),
                ..Default::default()
            },
            ..Default::default()
        })
        .with_children(|commands| {
            commands.spawn(NodeBundle{
                style: Style {
                    flex_direction: FlexDirection::Row,
                    height: Val::Px(75.0),
                    ..Default::default()
                },
                background_color: Color::srgb(1.0, 0.0, 1.0).into(),
                ..Default::default()
            }).with_children(|commands| {
                // a div that reduces the available size
                commands.spawn(NodeBundle {
                    style: Style {
                        width: Val::Px(75.0),
                        ..Default::default()
                    },
                    background_color: Color::srgb(0.0, 1.0, 0.0).into(),
                    ..Default::default()
                });

                // text with width=auto, but actual size will not be what it expcets due to the sibling div above
                commands.spawn(TextBundle {
                    text: Text::from_section("Some text that wraps onto a second line", Default::default()).with_justify(justify),
                    style: Style {
                        align_self: AlignSelf::Center,
                        ..Default::default()
                    },
                    ..Default::default()
                });
            });
        });
    }
}
```
github-merge-queue bot pushed a commit that referenced this pull request Jun 16, 2024
# Objective

- #13846 introduced a bug where text not bound was not displayed

## Solution

- bounds are infinite
- use computed size instead, that already should be using the available
bounds
mockersf added a commit that referenced this pull request Jun 16, 2024
# Objective

- #13846 introduced a bug where text not bound was not displayed

## Solution

- bounds are infinite
- use computed size instead, that already should be using the available
bounds
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Text Rendering and layout for characters C-Bug An unexpected or incorrect behavior D-Straightforward Simple bug fixes and API improvements, docs, test and examples S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Uncontroversial This work is generally agreed upon
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants