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 blurry text #12429

Merged
merged 3 commits into from
Mar 13, 2024
Merged

Fix blurry text #12429

merged 3 commits into from
Mar 13, 2024

Conversation

rparrett
Copy link
Contributor

@rparrett rparrett commented Mar 11, 2024

Objective

Fixes #12064

Solution

Prior to #11326, the "global physical" translation of text was rounded.

After #11326, only the "offset" is being rounded.

This moves things around so that the "global translation" is converted to physical pixels, rounded, and then converted back to logical pixels, which is what I believe was happening before / what the comments above describe.

Discussion

This seems to work and fix an obvious mistake in some code, but I don't fully grok the ui / text pipelines / math here.

Before / After and test example

Expand Code
use std::f32::consts::FRAC_PI_2;

use bevy::prelude::*;
use bevy_internal::window::WindowResolution;

const FONT_SIZE: f32 = 25.0;
const PADDING: f32 = 5.0;

fn main() {
    App::new()
        .add_plugins(
            DefaultPlugins.set(WindowPlugin {
                primary_window: Some(Window {
                    resolution: WindowResolution::default().with_scale_factor_override(1.0),
                    ..default()
                }),
                ..default()
            }),
            //.set(ImagePlugin::default_nearest()),
        )
        .add_systems(Startup, setup)
        .run();
}

fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
    commands.spawn(Camera2dBundle::default());

    let font = asset_server.load("fonts/FiraSans-Bold.ttf");

    for x in [20.5, 140.0] {
        for i in 1..10 {
            text(
                &mut commands,
                font.clone(),
                x,
                (FONT_SIZE + PADDING) * i as f32,
                i,
                Quat::default(),
                1.0,
            );
        }
    }

    for x in [450.5, 700.0] {
        for i in 1..10 {
            text(
                &mut commands,
                font.clone(),
                x,
                ((FONT_SIZE * 2.0) + PADDING) * i as f32,
                i,
                Quat::default(),
                2.0,
            );
        }
    }

    for y in [400.0, 600.0] {
        for i in 1..10 {
            text(
                &mut commands,
                font.clone(),
                (FONT_SIZE + PADDING) * i as f32,
                y,
                i,
                Quat::from_rotation_z(FRAC_PI_2),
                1.0,
            );
        }
    }
}

fn text(
    commands: &mut Commands,
    font: Handle<Font>,
    x: f32,
    y: f32,
    i: usize,
    rot: Quat,
    scale: f32,
) {
    let text = (65..(65 + i)).map(|a| a as u8 as char).collect::<String>();

    commands.spawn(TextBundle {
        style: Style {
            position_type: PositionType::Absolute,
            left: Val::Px(x),
            top: Val::Px(y),
            ..default()
        },
        text: Text::from_section(
            text,
            TextStyle {
                font,
                font_size: FONT_SIZE,
                ..default()
            },
        ),
        transform: Transform::from_rotation(rot).with_scale(Vec2::splat(scale).extend(1.)),
        ..default()
    });
}

Open both images in new tabs and swap back and forth. Pay attention to the "A" and "ABCD" lines.

Before main3
After pr3

@alice-i-cecile alice-i-cecile added this to the 0.13.1 milestone Mar 11, 2024
Copy link
Contributor

@Shatur Shatur left a comment

Choose a reason for hiding this comment

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

Applied this patch for my game and looks like it fixes the problem.

@rparrett
Copy link
Contributor Author

rparrett commented Mar 12, 2024

I am not totally sure this should be merged as-is.

The new rounding behavior seems to be causing animated scale / rotation on text to be jerky.

See the overflow_debug example (testing with scale factor 2.0, press space to turn on animations).

edit: Pretty sure I'm doing something silly and coincidentally fixing this by effectively rounding in logical pixels or something.

@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets labels Mar 12, 2024
@rparrett
Copy link
Contributor Author

rparrett commented Mar 12, 2024

Taking this out of draft mode. The fix is still basically the same as described in the description, just executed better, I hope.

I got my local screenshot diffing infrastructure back up and running and checked examples in the ui folder and found slight differences in

  • overflow_debug (bottom right square)
  • render_ui_to_texture

I believe that both of these (definitely render_ui_to_texture) are just "the bug being reproduced / fixed."

But I only did that particular testing at scale factor 2.0 and it might be wise for another set of eyes to take a look and make sure this isn't introducing another bug.

CI failure seems unrelated and pretty strange.

@rparrett rparrett marked this pull request as ready for review March 12, 2024 05:11
@alice-i-cecile alice-i-cecile added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Mar 12, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Mar 13, 2024
Merged via the queue into bevyengine:main with commit 55b786c Mar 13, 2024
25 checks passed
mockersf added a commit that referenced this pull request Mar 13, 2024
# Objective

Fixes #12064

## Solution

Prior to #11326, the "global physical" translation of text was rounded.

After #11326, only the "offset" is being rounded.

This moves things around so that the "global translation" is converted
to physical pixels, rounded, and then converted back to logical pixels,
which is what I believe was happening before / what the comments above
describe.

## Discussion

This seems to work and fix an obvious mistake in some code, but I don't
fully grok the ui / text pipelines / math here.

## Before / After and test example

<details>
<summary>Expand Code</summary>

```rust
use std::f32::consts::FRAC_PI_2;

use bevy::prelude::*;
use bevy_internal::window::WindowResolution;

const FONT_SIZE: f32 = 25.0;
const PADDING: f32 = 5.0;

fn main() {
    App::new()
        .add_plugins(
            DefaultPlugins.set(WindowPlugin {
                primary_window: Some(Window {
                    resolution: WindowResolution::default().with_scale_factor_override(1.0),
                    ..default()
                }),
                ..default()
            }),
            //.set(ImagePlugin::default_nearest()),
        )
        .add_systems(Startup, setup)
        .run();
}

fn setup(mut commands: Commands, asset_server: Res<AssetServer>) {
    commands.spawn(Camera2dBundle::default());

    let font = asset_server.load("fonts/FiraSans-Bold.ttf");

    for x in [20.5, 140.0] {
        for i in 1..10 {
            text(
                &mut commands,
                font.clone(),
                x,
                (FONT_SIZE + PADDING) * i as f32,
                i,
                Quat::default(),
                1.0,
            );
        }
    }

    for x in [450.5, 700.0] {
        for i in 1..10 {
            text(
                &mut commands,
                font.clone(),
                x,
                ((FONT_SIZE * 2.0) + PADDING) * i as f32,
                i,
                Quat::default(),
                2.0,
            );
        }
    }

    for y in [400.0, 600.0] {
        for i in 1..10 {
            text(
                &mut commands,
                font.clone(),
                (FONT_SIZE + PADDING) * i as f32,
                y,
                i,
                Quat::from_rotation_z(FRAC_PI_2),
                1.0,
            );
        }
    }
}

fn text(
    commands: &mut Commands,
    font: Handle<Font>,
    x: f32,
    y: f32,
    i: usize,
    rot: Quat,
    scale: f32,
) {
    let text = (65..(65 + i)).map(|a| a as u8 as char).collect::<String>();

    commands.spawn(TextBundle {
        style: Style {
            position_type: PositionType::Absolute,
            left: Val::Px(x),
            top: Val::Px(y),
            ..default()
        },
        text: Text::from_section(
            text,
            TextStyle {
                font,
                font_size: FONT_SIZE,
                ..default()
            },
        ),
        transform: Transform::from_rotation(rot).with_scale(Vec2::splat(scale).extend(1.)),
        ..default()
    });
}
```

</details>

Open both images in new tabs and swap back and forth. Pay attention to
the "A" and "ABCD" lines.

<details>
<summary>Before</summary>

<img width="640" alt="main3"
src="https:/bevyengine/bevy/assets/200550/248d7a55-d06d-433f-80da-1914803c3551">

</details>

<details>
<summary>After</summary>

<img width="640" alt="pr3"
src="https:/bevyengine/bevy/assets/200550/26a9d292-07ae-4af3-b035-e187b2529ace">

</details>

---------

Co-authored-by: François Mockers <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen A-UI Graphical user interfaces, styles, layouts, and widgets C-Bug An unexpected or incorrect behavior 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.

Font rendering is blurry
4 participants