Skip to content

Conversation

@ickshonpe
Copy link
Contributor

@ickshonpe ickshonpe commented Nov 6, 2024

Objective

Text2d doesn't respond to changes to the window scalefactor.

Fixes #16223

Solution

In update_text2d_layout store the previous scale factor in a Local instead and check against the current scale factor to detect changes.

It seems like previously the text wasn't updated because of a bug with the WindowScaleFactorChanged event and it isn't emitted after changes to the scale factor. That needs to be looked into, but this will work for now.

Testing

Really simple app that draws a big message in the middle of the window:

use bevy::prelude::*;

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

fn setup(mut commands: Commands) {
    commands.spawn(Camera2d);
    commands.spawn((
        Text2d::new("Hello"),
        TextFont {
            font_size: 400.,
            ..Default::default()
        },
    ));
}

Looks fine:
hello1

On main, after changing the monitor's scale factor:
hello2

With this PR the text maintains the same size and position after the scale factor is changed.

In `update_text2d_layout` store the previous scale factor in a `Local` instead and check against the current scale factor to detect changes.
@alice-i-cecile alice-i-cecile added C-Bug An unexpected or incorrect behavior A-UI Graphical user interfaces, styles, layouts, and widgets A-Text Rendering and layout for characters and removed A-UI Graphical user interfaces, styles, layouts, and widgets labels Nov 6, 2024
@alice-i-cecile alice-i-cecile added the S-Needs-Review Needs reviewer attention (from anyone!) to move forward label Nov 6, 2024
@ickshonpe ickshonpe requested a review from NiseVoid November 6, 2024 21:21
@mockersf mockersf added this to the 0.15 milestone Nov 11, 2024
@mockersf
Copy link
Member

the WindowScaleFactorChanged event is also used to update ui layout, I would prefer a complete fix rather than "just" text.

The issue is probably about the first scale change on window creation. I can't reproduce on macOS, but it may be os dependent

@ickshonpe
Copy link
Contributor Author

ickshonpe commented Nov 12, 2024

the WindowScaleFactorChanged event is also used to update ui layout, I would prefer a complete fix rather than "just" text.

It's actually more complicated than just fixing the event, there are a couple of issues with update_text2d_layout:

  • It regenerates the text2d layout on recieving a WindowScaleFactorChanged event but the render target could be a different window or a texture.
  • It uses the scale factor from the primary window regardless of the render target.
  • It doesn't delete the old unused font atlases for the previous scale factors.

So storing the scale factor in a local variable is marginally preferable to watching the event. With the event (if it worked) it triggers updates on changes to the scale factor of any window, not just the primary window. But since text2d always uses the scale factor of the primary window, the text reupdate is needless on changes to non-primary window's scalefactors.

It's stupid how it works this way though and we need proper multi-window support.

The issue is probably about the first scale change on window creation. I can't reproduce on macOS, but it may be os dependent

It can be tricky sometimes to capture scale factor bugs in examples. Easiest way in this case is if you setup two monitors with different scale factors and drag the bevy app from one to the other.

@mockersf mockersf added this pull request to the merge queue Nov 13, 2024
Merged via the queue into bevyengine:main with commit c0fbadb Nov 13, 2024
29 checks passed
mockersf pushed a commit that referenced this pull request Nov 16, 2024
# Objective 

Text2d doesn't respond to changes to the window scalefactor.

Fixes #16223

## Solution 

In `update_text2d_layout` store the previous scale factor in a `Local`
instead and check against the current scale factor to detect changes.

It seems like previously the text wasn't updated because of a bug with
the `WindowScaleFactorChanged` event and it isn't emitted after changes
to the scale factor. That needs to be looked into, but this will work
for now.

## Testing

Really simple app that draws a big message in the middle of the window:

```
use bevy::prelude::*;

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

fn setup(mut commands: Commands) {
    commands.spawn(Camera2d);
    commands.spawn((
        Text2d::new("Hello"),
        TextFont {
            font_size: 400.,
            ..Default::default()
        },
    ));
}
```

Looks fine:
<img width="500" alt="hello1"
src="https://github.com/user-attachments/assets/5320746b-687e-4682-9e4c-bc43ab7ff9d3">

On main, after changing the monitor's scale factor:
<img width="500" alt="hello2"
src="https://github.com/user-attachments/assets/486cea16-fc44-4d66-9468-6f68905d4196">


With this PR the text maintains the same size and position after the
scale factor is changed.
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Dec 2, 2024
# Objective 

Text2d doesn't respond to changes to the window scalefactor.

Fixes bevyengine#16223

## Solution 

In `update_text2d_layout` store the previous scale factor in a `Local`
instead and check against the current scale factor to detect changes.

It seems like previously the text wasn't updated because of a bug with
the `WindowScaleFactorChanged` event and it isn't emitted after changes
to the scale factor. That needs to be looked into, but this will work
for now.

## Testing

Really simple app that draws a big message in the middle of the window:

```
use bevy::prelude::*;

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

fn setup(mut commands: Commands) {
    commands.spawn(Camera2d);
    commands.spawn((
        Text2d::new("Hello"),
        TextFont {
            font_size: 400.,
            ..Default::default()
        },
    ));
}
```

Looks fine:
<img width="500" alt="hello1"
src="https://github.com/user-attachments/assets/5320746b-687e-4682-9e4c-bc43ab7ff9d3">

On main, after changing the monitor's scale factor:
<img width="500" alt="hello2"
src="https://github.com/user-attachments/assets/486cea16-fc44-4d66-9468-6f68905d4196">


With this PR the text maintains the same size and position after the
scale factor is changed.
ecoskey pushed a commit to ecoskey/bevy that referenced this pull request Jan 6, 2025
# Objective 

Text2d doesn't respond to changes to the window scalefactor.

Fixes bevyengine#16223

## Solution 

In `update_text2d_layout` store the previous scale factor in a `Local`
instead and check against the current scale factor to detect changes.

It seems like previously the text wasn't updated because of a bug with
the `WindowScaleFactorChanged` event and it isn't emitted after changes
to the scale factor. That needs to be looked into, but this will work
for now.

## Testing

Really simple app that draws a big message in the middle of the window:

```
use bevy::prelude::*;

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

fn setup(mut commands: Commands) {
    commands.spawn(Camera2d);
    commands.spawn((
        Text2d::new("Hello"),
        TextFont {
            font_size: 400.,
            ..Default::default()
        },
    ));
}
```

Looks fine:
<img width="500" alt="hello1"
src="https://github.com/user-attachments/assets/5320746b-687e-4682-9e4c-bc43ab7ff9d3">

On main, after changing the monitor's scale factor:
<img width="500" alt="hello2"
src="https://github.com/user-attachments/assets/486cea16-fc44-4d66-9468-6f68905d4196">


With this PR the text maintains the same size and position after the
scale factor is changed.
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 S-Needs-Review Needs reviewer attention (from anyone!) to move forward

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Text doesn't respond correctly to scale changes

3 participants