From e37c78059b62d0ecbc31a486294718d5a16178e4 Mon Sep 17 00:00:00 2001 From: Ickshonpe Date: Fri, 2 Dec 2022 18:31:59 +0000 Subject: [PATCH 1/5] Text Aspect Ratio Bug Fix Bevy UI was using the MeasureFunc that preserves aspect ratio for both images and text. This meant that the cross-axis of flex items containing text would be calculated incorrectly sometimes depending on how the aspect ratio of the text compared to the size of the containing node. changes: * Added parameter preserve_aspect_ratio to the upsert_leaf function. * The MeasureFunc only preserves the aspect ratio when preserve_aspect_ratio is true. * flex_node_system queries for UiImage before it calls upsert_leaf and sets preserve_aspect_ratio appropriately. --- crates/bevy_ui/src/flex/mod.rs | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/crates/bevy_ui/src/flex/mod.rs b/crates/bevy_ui/src/flex/mod.rs index ebe6e1ad4d394..3e3f1ef5fd7dc 100644 --- a/crates/bevy_ui/src/flex/mod.rs +++ b/crates/bevy_ui/src/flex/mod.rs @@ -1,6 +1,6 @@ mod convert; -use crate::{CalculatedSize, Node, Style, UiScale}; +use crate::{CalculatedSize, Node, Style, UiScale, UiImage}; use bevy_ecs::{ entity::Entity, event::EventReader, @@ -77,6 +77,7 @@ impl FlexSurface { style: &Style, calculated_size: CalculatedSize, scale_factor: f64, + preserve_aspect_ratio: bool, ) { let taffy = &mut self.taffy; let taffy_style = convert::from_style(scale_factor, style); @@ -86,11 +87,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 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 preserve_aspect_ratio { + size.width = height * size.width / size.height; + } size.height = height; } (Number::Defined(width), Number::Defined(height)) => { @@ -225,6 +230,7 @@ pub fn flex_node_system( children_query: Query<(Entity, &Children), (With, Changed)>, removed_children: RemovedComponents, mut node_transform_query: Query<(Entity, &mut Node, &mut Transform, Option<&Parent>)>, + image_query: Query>, removed_nodes: RemovedComponents, ) { // update window root nodes @@ -236,30 +242,31 @@ 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( flex_surface: &mut FlexSurface, scaling_factor: f64, query: Query<(Entity, &Style, Option<&CalculatedSize>), F>, + image_query: &Query>, ) { // update changed nodes for (entity, style, calculated_size) in &query { // TODO: remove node from old hierarchy if its root has changed if let Some(calculated_size) = calculated_size { - flex_surface.upsert_leaf(entity, style, *calculated_size, scaling_factor); + flex_surface.upsert_leaf(entity, style, *calculated_size, scaling_factor, image_query.contains(entity)); } else { flex_surface.upsert_node(entity, style, scaling_factor); } } } + if scale_factor_events.iter().next_back().is_some() || ui_scale.is_changed() { + update_changed(&mut flex_surface, scale_factor, full_node_query, &image_query); + } else { + update_changed(&mut flex_surface, scale_factor, node_query, &image_query); + } + for (entity, style, calculated_size) in &changed_size_query { - flex_surface.upsert_leaf(entity, style, *calculated_size, scale_factor); + flex_surface.upsert_leaf(entity, style, *calculated_size, scale_factor, image_query.contains(entity)); } // clean up removed nodes From 89e647ce9d2f043125e248002e38e83026781ed8 Mon Sep 17 00:00:00 2001 From: Ickshonpe Date: Fri, 2 Dec 2022 19:08:27 +0000 Subject: [PATCH 2/5] cargo fmt --all --- crates/bevy_ui/src/flex/mod.rs | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/crates/bevy_ui/src/flex/mod.rs b/crates/bevy_ui/src/flex/mod.rs index 3e3f1ef5fd7dc..8459b9f3ddea3 100644 --- a/crates/bevy_ui/src/flex/mod.rs +++ b/crates/bevy_ui/src/flex/mod.rs @@ -1,6 +1,6 @@ mod convert; -use crate::{CalculatedSize, Node, Style, UiScale, UiImage}; +use crate::{CalculatedSize, Node, Style, UiImage, UiScale}; use bevy_ecs::{ entity::Entity, event::EventReader, @@ -252,7 +252,13 @@ pub fn flex_node_system( for (entity, style, calculated_size) in &query { // TODO: remove node from old hierarchy if its root has changed if let Some(calculated_size) = calculated_size { - flex_surface.upsert_leaf(entity, style, *calculated_size, scaling_factor, image_query.contains(entity)); + flex_surface.upsert_leaf( + entity, + style, + *calculated_size, + scaling_factor, + image_query.contains(entity), + ); } else { flex_surface.upsert_node(entity, style, scaling_factor); } @@ -260,13 +266,24 @@ 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, &image_query); + update_changed( + &mut flex_surface, + scale_factor, + full_node_query, + &image_query, + ); } else { update_changed(&mut flex_surface, scale_factor, node_query, &image_query); } for (entity, style, calculated_size) in &changed_size_query { - flex_surface.upsert_leaf(entity, style, *calculated_size, scale_factor, image_query.contains(entity)); + flex_surface.upsert_leaf( + entity, + style, + *calculated_size, + scale_factor, + image_query.contains(entity), + ); } // clean up removed nodes From 21e7531a7660f6a30890f0732793cd2f52a626cf Mon Sep 17 00:00:00 2001 From: Ickshonpe Date: Sat, 3 Dec 2022 10:43:34 +0000 Subject: [PATCH 3/5] Instead of querying for images in flex_node_system, added a preserve_aspect_ratio field to CalculatedSize which is set to true for image nodes by update_image_calculated_size_system. --- crates/bevy_ui/src/flex/mod.rs | 14 ++++---------- crates/bevy_ui/src/ui_node.rs | 2 ++ crates/bevy_ui/src/widget/image.rs | 1 + 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/crates/bevy_ui/src/flex/mod.rs b/crates/bevy_ui/src/flex/mod.rs index 8459b9f3ddea3..7280b69be094c 100644 --- a/crates/bevy_ui/src/flex/mod.rs +++ b/crates/bevy_ui/src/flex/mod.rs @@ -1,6 +1,6 @@ mod convert; -use crate::{CalculatedSize, Node, Style, UiImage, UiScale}; +use crate::{CalculatedSize, Node, Style, UiScale}; use bevy_ecs::{ entity::Entity, event::EventReader, @@ -77,7 +77,6 @@ impl FlexSurface { style: &Style, calculated_size: CalculatedSize, scale_factor: f64, - preserve_aspect_ratio: bool, ) { let taffy = &mut self.taffy; let taffy_style = convert::from_style(scale_factor, style); @@ -87,13 +86,13 @@ impl FlexSurface { match (constraints.width, constraints.height) { (Number::Undefined, Number::Undefined) => {} (Number::Defined(width), Number::Undefined) => { - if preserve_aspect_ratio { + if calculated_size.preserve_aspect_ratio { size.height = width * size.height / size.width; } size.width = width; } (Number::Undefined, Number::Defined(height)) => { - if preserve_aspect_ratio { + if calculated_size.preserve_aspect_ratio { size.width = height * size.width / size.height; } size.height = height; @@ -230,7 +229,6 @@ pub fn flex_node_system( children_query: Query<(Entity, &Children), (With, Changed)>, removed_children: RemovedComponents, mut node_transform_query: Query<(Entity, &mut Node, &mut Transform, Option<&Parent>)>, - image_query: Query>, removed_nodes: RemovedComponents, ) { // update window root nodes @@ -246,7 +244,6 @@ pub fn flex_node_system( flex_surface: &mut FlexSurface, scaling_factor: f64, query: Query<(Entity, &Style, Option<&CalculatedSize>), F>, - image_query: &Query>, ) { // update changed nodes for (entity, style, calculated_size) in &query { @@ -257,7 +254,6 @@ pub fn flex_node_system( style, *calculated_size, scaling_factor, - image_query.contains(entity), ); } else { flex_surface.upsert_node(entity, style, scaling_factor); @@ -270,10 +266,9 @@ pub fn flex_node_system( &mut flex_surface, scale_factor, full_node_query, - &image_query, ); } else { - update_changed(&mut flex_surface, scale_factor, node_query, &image_query); + update_changed(&mut flex_surface, scale_factor, node_query); } for (entity, style, calculated_size) in &changed_size_query { @@ -282,7 +277,6 @@ pub fn flex_node_system( style, *calculated_size, scale_factor, - image_query.contains(entity), ); } diff --git a/crates/bevy_ui/src/ui_node.rs b/crates/bevy_ui/src/ui_node.rs index 738d578d6e5a3..12cfbf62671ac 100644 --- a/crates/bevy_ui/src/ui_node.rs +++ b/crates/bevy_ui/src/ui_node.rs @@ -434,6 +434,8 @@ pub enum FlexWrap { pub struct CalculatedSize { /// The size of the node pub size: Size, + /// Whether to preserve the aspect ratio of this item + pub preserve_aspect_ratio: bool } /// The background color of the node diff --git a/crates/bevy_ui/src/widget/image.rs b/crates/bevy_ui/src/widget/image.rs index 2916a6295223a..fa2e011232cec 100644 --- a/crates/bevy_ui/src/widget/image.rs +++ b/crates/bevy_ui/src/widget/image.rs @@ -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; } } } From de4d940a607b9357b4aaa2ea0e1d44b713db753f Mon Sep 17 00:00:00 2001 From: Ickshonpe Date: Sat, 3 Dec 2022 10:53:34 +0000 Subject: [PATCH 4/5] edit comment --- crates/bevy_ui/src/ui_node.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/bevy_ui/src/ui_node.rs b/crates/bevy_ui/src/ui_node.rs index 12cfbf62671ac..904a51c0259a3 100644 --- a/crates/bevy_ui/src/ui_node.rs +++ b/crates/bevy_ui/src/ui_node.rs @@ -434,7 +434,7 @@ pub enum FlexWrap { pub struct CalculatedSize { /// The size of the node pub size: Size, - /// Whether to preserve the aspect ratio of this item + /// Whether to attempt to preserve the aspect ratio when determing the layout for this item pub preserve_aspect_ratio: bool } From 8cf96a9f5e42aa3c1a43b7778ad36b55f38c1bdc Mon Sep 17 00:00:00 2001 From: Ickshonpe Date: Sat, 3 Dec 2022 11:18:27 +0000 Subject: [PATCH 5/5] cargo fmt --- crates/bevy_ui/src/flex/mod.rs | 20 +++----------------- crates/bevy_ui/src/ui_node.rs | 2 +- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/crates/bevy_ui/src/flex/mod.rs b/crates/bevy_ui/src/flex/mod.rs index 7280b69be094c..da31a692d5630 100644 --- a/crates/bevy_ui/src/flex/mod.rs +++ b/crates/bevy_ui/src/flex/mod.rs @@ -249,12 +249,7 @@ pub fn flex_node_system( for (entity, style, calculated_size) in &query { // TODO: remove node from old hierarchy if its root has changed if let Some(calculated_size) = calculated_size { - flex_surface.upsert_leaf( - entity, - style, - *calculated_size, - scaling_factor, - ); + flex_surface.upsert_leaf(entity, style, *calculated_size, scaling_factor); } else { flex_surface.upsert_node(entity, style, scaling_factor); } @@ -262,22 +257,13 @@ 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, - ); + 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, - ); + flex_surface.upsert_leaf(entity, style, *calculated_size, scale_factor); } // clean up removed nodes diff --git a/crates/bevy_ui/src/ui_node.rs b/crates/bevy_ui/src/ui_node.rs index 904a51c0259a3..bc6661b2ed81c 100644 --- a/crates/bevy_ui/src/ui_node.rs +++ b/crates/bevy_ui/src/ui_node.rs @@ -435,7 +435,7 @@ 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 + pub preserve_aspect_ratio: bool, } /// The background color of the node