Skip to content

Commit

Permalink
text aspect ratio bug fix (bevyengine#6825)
Browse files Browse the repository at this point in the history
## Objective 

Bevy UI uses a `MeasureFunc` that preserves the aspect ratio of text, not just images. This means that the extent of flex-items containing text may be calculated incorrectly depending on the ratio of the text size compared to the size of its containing node.

Fixes bevyengine#6748 
Related to bevyengine#6724

with Bevy 0.9:

![Capture_cols_0 9](https://user-images.githubusercontent.com/27962798/205435999-386d3400-fe9b-475a-aab1-18e61c4c074f.PNG)

with this PR (accurately matching the behavior of Flexbox):

![Capture_fixed](https://user-images.githubusercontent.com/27962798/205436005-6bafbcc2-cd87-4eb7-b5c6-9dbcb30fc795.PNG)

## Solution
Only perform the aspect ratio calculations if the uinode contains an image.

## Changelog
* Added a field `preserve_aspect_ratio` to `CalculatedSize`
* The `MeasureFunc` only preserves the aspect ratio when `preserve_aspect_ratio` is true.
* `update_image_calculated_size_system` sets `preserve_aspect_ratio` to true for nodes with images.
  • Loading branch information
ickshonpe authored and ItsDoot committed Feb 1, 2023
1 parent 5527f16 commit ff661ea
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 8 deletions.
20 changes: 12 additions & 8 deletions crates/bevy_ui/src/flex/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,15 @@ impl FlexSurface {
match (constraints.width, constraints.height) {
(Number::Undefined, Number::Undefined) => {}
(Number::Defined(width), Number::Undefined) => {
size.height = width * size.height / size.width;
if calculated_size.preserve_aspect_ratio {
size.height = width * size.height / size.width;
}
size.width = width;
}
(Number::Undefined, Number::Defined(height)) => {
size.width = height * size.width / size.height;
if calculated_size.preserve_aspect_ratio {
size.width = height * size.width / size.height;
}
size.height = height;
}
(Number::Defined(width), Number::Defined(height)) => {
Expand Down Expand Up @@ -236,12 +240,6 @@ pub fn flex_node_system(
let logical_to_physical_factor = windows.scale_factor(WindowId::primary());
let scale_factor = logical_to_physical_factor * ui_scale.scale;

if scale_factor_events.iter().next_back().is_some() || ui_scale.is_changed() {
update_changed(&mut flex_surface, scale_factor, full_node_query);
} else {
update_changed(&mut flex_surface, scale_factor, node_query);
}

fn update_changed<F: ReadOnlyWorldQuery>(
flex_surface: &mut FlexSurface,
scaling_factor: f64,
Expand All @@ -258,6 +256,12 @@ pub fn flex_node_system(
}
}

if scale_factor_events.iter().next_back().is_some() || ui_scale.is_changed() {
update_changed(&mut flex_surface, scale_factor, full_node_query);
} else {
update_changed(&mut flex_surface, scale_factor, node_query);
}

for (entity, style, calculated_size) in &changed_size_query {
flex_surface.upsert_leaf(entity, style, *calculated_size, scale_factor);
}
Expand Down
2 changes: 2 additions & 0 deletions crates/bevy_ui/src/ui_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,8 @@ pub enum FlexWrap {
pub struct CalculatedSize {
/// The size of the node
pub size: Size,
/// Whether to attempt to preserve the aspect ratio when determing the layout for this item
pub preserve_aspect_ratio: bool,
}

/// The background color of the node
Expand Down
1 change: 1 addition & 0 deletions crates/bevy_ui/src/widget/image.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub fn update_image_calculated_size_system(
// Update only if size has changed to avoid needless layout calculations
if size != calculated_size.size {
calculated_size.size = size;
calculated_size.preserve_aspect_ratio = true;
}
}
}
Expand Down

0 comments on commit ff661ea

Please sign in to comment.