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

Text 2d alignment fix #17365

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Jan 14, 2025

Objective

Text2d ignores TextBounds when calculating the offset for text aligment.
On main a text entity positioned in the center of the window with center justification and 600px horizontal text bounds isn't centered like it should be but shifted off to the right:
hellox
(second example in the testing section below)

Fixes #14266

I already had a PR in review for this (#14270) but it used post layout adjustment (which we want to avoid) and ignored TextBounds.

Solution

  • If TextBounds are present for an axis, use them instead of the size of the computed text layout size to calculate the offset.
  • Adjust the vertical offset of text so it's top is aligned with the top of the texts bounding rect (when present).

Testing

use bevy::prelude::*;
use bevy::color::palettes;
use bevy::sprite::Anchor;
use bevy::text::TextBounds;

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

fn example(commands: &mut Commands, dest: Vec3, justify: JustifyText) {
    commands.spawn((
        Sprite {
            color: palettes::css::YELLOW.into(),
            custom_size: Some(10. * Vec2::ONE),
            anchor: Anchor::Center,
            ..Default::default()
        },
        Transform::from_translation(dest),
    ));

    for a in [
        Anchor::TopLeft,
        Anchor::TopRight,
        Anchor::BottomRight,
        Anchor::BottomLeft,
    ] {
        commands.spawn((
            Text2d(format!("L R\n{:?}\n{:?}", a, justify)),
            TextFont {
                font_size: 14.0,
                ..default()
            },
            TextLayout {
                justify,
                ..Default::default()
            },
            TextBounds::new(300., 75.),
            Transform::from_translation(dest + Vec3::Z),
            a,
        ));
    }
}

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

    for (i, j) in [
        JustifyText::Left,
        JustifyText::Right,
        JustifyText::Center,
        JustifyText::Justified,
    ]
    .into_iter()
    .enumerate()
    {
        example(&mut commands, (300. - 150. * i as f32) * Vec3::Y, j);
    }

    commands.spawn(Sprite {
        color: palettes::css::YELLOW.into(),
        custom_size: Some(10. * Vec2::ONE),
        anchor: Anchor::Center,
        ..Default::default()
    });
}
cap

This probably looks really confusing but it should make sense if you imagine each block of text surrounded by a 300x75 rectangle that is anchored to the center of the yellow square.

use bevy::prelude::*;
use bevy::sprite::Anchor;
use bevy::text::TextBounds;

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

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

    commands.spawn((
        Text2d::new("hello"),
        TextFont {
            font_size: 60.0,
            ..default()
        },
        TextLayout::new_with_justify(JustifyText::Center),
        TextBounds::new(600., 200.),
        Anchor::Center,
    ));
}
hello

The text being above the center is intended. When TextBounds are present, the text block's offset is calculated using its TextBounds not the layout size returned by cosmic-text.

Probably we should add a vertical alignment setting for Text2d. Didn't do it here as this is intended for a 0.15.2 release.

@ickshonpe ickshonpe added C-Bug An unexpected or incorrect behavior A-Text Rendering and layout for characters S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 14, 2025
@rparrett rparrett self-requested a review January 14, 2025 16:42
Copy link
Contributor

@rparrett rparrett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@BenjaminBrienen BenjaminBrienen added D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes D-Straightforward Simple bug fixes and API improvements, docs, test and examples and removed D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes labels Jan 15, 2025
@rparrett rparrett added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Jan 17, 2025
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-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JustifyText::Center doesn't place a text in the center of Text2dBounds
4 participants