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

JustifyText::Center doesn't place a text in the center of Text2dBounds #14266

Closed
m2ym opened this issue Jul 10, 2024 · 5 comments · Fixed by #17365
Closed

JustifyText::Center doesn't place a text in the center of Text2dBounds #14266

m2ym opened this issue Jul 10, 2024 · 5 comments · Fixed by #17365
Assignees
Labels
A-Text Rendering and layout for characters C-Bug An unexpected or incorrect behavior D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes P-Regression Functionality that used to work but no longer does. Add a test for this! S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Milestone

Comments

@m2ym
Copy link

m2ym commented Jul 10, 2024

Bevy version

v0.14.0

What you did

  • Tried to place a text in the center of some bounds (Text2dBounds)
use bevy::prelude::*;
use bevy::sprite::Anchor;
use bevy::text::Text2dBounds;

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

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

    commands.spawn(Text2dBundle {
        text: Text::from_section(
            "hello",
            TextStyle {
                font_size: 60.0,
                ..default()
            },
        )
        .with_justify(JustifyText::Center),
        text_2d_bounds: Text2dBounds {
            size: Vec2::new(600., 200.),
        },
        text_anchor: Anchor::Center,
        ..default()
    });
}

What went wrong

The text ("hello" in the code above) is not placed at the center, but at the right.

v0_14_0

In v0.13.2, the text is rendered as expected.

v0_13_2

@m2ym m2ym added C-Bug An unexpected or incorrect behavior S-Needs-Triage This issue needs to be labelled labels Jul 10, 2024
@mgi388
Copy link
Contributor

mgi388 commented Jul 10, 2024

@m2ym out of interest does the response #14251 (comment) help in your case?

@ickshonpe ickshonpe added A-Text Rendering and layout for characters and removed S-Needs-Triage This issue needs to be labelled labels Jul 10, 2024
@ickshonpe
Copy link
Contributor

ickshonpe commented Jul 10, 2024

This is a bug. JustifyText isn't meant to affect single lines of text. The Anchor component controls Text2d's alignment relative to the position of its transform.

The Text2dBounds constraint seems to be the problem. Without it the text is centered in Bevy 14 like in the second screenshot:

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

    commands.spawn(Text2dBundle {
        text: Text::from_section(
            "hello",
            TextStyle {
                font_size: 60.0,
                ..default()
            },
        ),
        text_anchor: Anchor::Center,
        ..default()
    });
}

@ickshonpe
Copy link
Contributor

ickshonpe commented Jul 10, 2024

This is really odd:

use bevy::color::palettes;
use bevy::math::vec2;
use bevy::prelude::*;
use bevy::sprite::Anchor;
use bevy::text::Text2dBounds;

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

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

    let size = vec2(600., 200.);
    commands.spawn(SpriteBundle {
        sprite: Sprite {
            color: palettes::css::NAVY.into(),
            custom_size: Some(size),
            anchor: Anchor::Center,
            ..Default::default()
        },
        transform: Transform::from_translation(300. * Vec3::Y),
        
        ..Default::default()
    });

    commands.spawn(SpriteBundle {
        sprite: Sprite {
            color: palettes::css::NAVY.into(),
            custom_size: Some(size),
            anchor: Anchor::Center,
            ..Default::default()
        },
        ..Default::default()
    });

    commands.spawn(Text2dBundle {
        text: Text::from_section(
            "hello",
            TextStyle {
                font_size: 60.0,
                ..default()
            },
        )
        .with_justify(JustifyText::Center),
        text_2d_bounds: Text2dBounds {
            size: Vec2::new(600., 200.),
        },
        text_anchor: Anchor::Center,
        ..default()
    });
}
hello

z-ordering

@ickshonpe
Copy link
Contributor

As a temporary fix you can add this system to PostUpdate after update_text2d_layout:

fn fix_text2d_layout(
    mut query: Query<&mut TextLayoutInfo, (Changed<TextLayoutInfo>, With<Anchor>)>,
) {
    for mut layout_info in query.iter_mut() {
        let mut min_x = f32::MAX;
        for glyph in layout_info.glyphs.iter() {
            min_x = (glyph.position.x - 0.5 * glyph.size.x).min(min_x);
        }

        for glyph in layout_info.glyphs.iter_mut() {
            glyph.position.x -= min_x;
        }
    }
}

Should work with both bevy 0.14 and main.

@m2ym
Copy link
Author

m2ym commented Jul 11, 2024

@ickshonpe The temporary fix works. Thanks for your work and a very quick response!

@rparrett rparrett added the P-Regression Functionality that used to work but no longer does. Add a test for this! label Jul 14, 2024
@alice-i-cecile alice-i-cecile added this to the 0.14.1 milestone Jul 14, 2024
@alice-i-cecile alice-i-cecile modified the milestones: 0.14.1, 0.14.2 Aug 2, 2024
@alice-i-cecile alice-i-cecile added S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it! D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes labels Sep 2, 2024
@mockersf mockersf modified the milestones: 0.14.2, 0.14.3 Sep 5, 2024
@JMS55 JMS55 modified the milestones: 0.14.3, 0.15 Sep 29, 2024
@alice-i-cecile alice-i-cecile modified the milestones: 0.15, 0.16 Oct 8, 2024
github-merge-queue bot pushed a commit that referenced this issue Jan 20, 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:
<img width="305" alt="hellox"
src="https://github.com/user-attachments/assets/8896c6f0-1b9f-4633-9c12-1de6eff5f3e1"
/>
(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()
    });
}
```

<img width="566" alt="cap"
src="https://github.com/user-attachments/assets/e6a98fa5-80b2-4380-a9b7-155bb49635b8"
/>

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,
    ));
}
```

<img width="338" alt="hello"
src="https://github.com/user-attachments/assets/e5e89364-afda-4baa-aca8-df4cdacbb4ed"
/>

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.
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-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes P-Regression Functionality that used to work but no longer does. Add a test for this! S-Ready-For-Implementation This issue is ready for an implementation PR. Go for it!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants