From 10833bf9df35fff234b931a5354155add27fc891 Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Wed, 5 Mar 2025 21:23:40 +0000 Subject: [PATCH 1/9] Changed the `Val::resolve` method to always return always return a value in physical pixels. This required adding an extra parameter for the scale factor. --- crates/bevy_ui/src/geometry.rs | 58 ++++++++++++++++++++------------ crates/bevy_ui/src/layout/mod.rs | 34 ++++++++++--------- 2 files changed, 56 insertions(+), 36 deletions(-) diff --git a/crates/bevy_ui/src/geometry.rs b/crates/bevy_ui/src/geometry.rs index a5749e467a..635dc8ce54 100644 --- a/crates/bevy_ui/src/geometry.rs +++ b/crates/bevy_ui/src/geometry.rs @@ -255,15 +255,19 @@ pub enum ValArithmeticError { } impl Val { - /// Resolves a [`Val`] from the given context values and returns this as an [`f32`]. - /// The [`Val::Px`] value (if present), `parent_size` and `viewport_size` should all be in the same coordinate space. - /// Returns a [`ValArithmeticError::NonEvaluable`] if the [`Val`] is impossible to resolve into a concrete value. + /// Resolves this [`Val`] to a value in physical pixels from the given `scale_factor`, `parent_size`, + /// and `viewport_size`. /// - /// **Note:** If a [`Val::Px`] is resolved, its inner value is returned unchanged. - pub fn resolve(self, parent_size: f32, viewport_size: Vec2) -> Result { + /// Returns a [`ValArithmeticError::NonEvaluable`] if the [`Val`] is impossible to resolve into a concrete value. + pub fn resolve( + self, + scale_factor: f32, + parent_size: f32, + viewport_size: Vec2, + ) -> Result { match self { Val::Percent(value) => Ok(parent_size * value / 100.0), - Val::Px(value) => Ok(value), + Val::Px(value) => Ok(value * scale_factor), Val::Vw(value) => Ok(viewport_size.x * value / 100.0), Val::Vh(value) => Ok(viewport_size.y * value / 100.0), Val::VMin(value) => Ok(viewport_size.min_element() * value / 100.0), @@ -697,7 +701,7 @@ mod tests { fn val_evaluate() { let size = 250.; let viewport_size = vec2(1000., 500.); - let result = Val::Percent(80.).resolve(size, viewport_size).unwrap(); + let result = Val::Percent(80.).resolve(1., size, viewport_size).unwrap(); assert_eq!(result, size * 0.8); } @@ -706,7 +710,7 @@ mod tests { fn val_resolve_px() { let size = 250.; let viewport_size = vec2(1000., 500.); - let result = Val::Px(10.).resolve(size, viewport_size).unwrap(); + let result = Val::Px(10.).resolve(1., size, viewport_size).unwrap(); assert_eq!(result, 10.); } @@ -719,33 +723,45 @@ mod tests { for value in (-10..10).map(|value| value as f32) { // for a square viewport there should be no difference between `Vw` and `Vh` and between `Vmin` and `Vmax`. assert_eq!( - Val::Vw(value).resolve(size, viewport_size), - Val::Vh(value).resolve(size, viewport_size) + Val::Vw(value).resolve(1., size, viewport_size), + Val::Vh(value).resolve(1., size, viewport_size) ); assert_eq!( - Val::VMin(value).resolve(size, viewport_size), - Val::VMax(value).resolve(size, viewport_size) + Val::VMin(value).resolve(1., size, viewport_size), + Val::VMax(value).resolve(1., size, viewport_size) ); assert_eq!( - Val::VMin(value).resolve(size, viewport_size), - Val::Vw(value).resolve(size, viewport_size) + Val::VMin(value).resolve(1., size, viewport_size), + Val::Vw(value).resolve(1., size, viewport_size) ); } let viewport_size = vec2(1000., 500.); - assert_eq!(Val::Vw(100.).resolve(size, viewport_size).unwrap(), 1000.); - assert_eq!(Val::Vh(100.).resolve(size, viewport_size).unwrap(), 500.); - assert_eq!(Val::Vw(60.).resolve(size, viewport_size).unwrap(), 600.); - assert_eq!(Val::Vh(40.).resolve(size, viewport_size).unwrap(), 200.); - assert_eq!(Val::VMin(50.).resolve(size, viewport_size).unwrap(), 250.); - assert_eq!(Val::VMax(75.).resolve(size, viewport_size).unwrap(), 750.); + assert_eq!( + Val::Vw(100.).resolve(1., size, viewport_size).unwrap(), + 1000. + ); + assert_eq!( + Val::Vh(100.).resolve(1., size, viewport_size).unwrap(), + 500. + ); + assert_eq!(Val::Vw(60.).resolve(1., size, viewport_size).unwrap(), 600.); + assert_eq!(Val::Vh(40.).resolve(1., size, viewport_size).unwrap(), 200.); + assert_eq!( + Val::VMin(50.).resolve(1., size, viewport_size).unwrap(), + 250. + ); + assert_eq!( + Val::VMax(75.).resolve(1., size, viewport_size).unwrap(), + 750. + ); } #[test] fn val_auto_is_non_evaluable() { let size = 250.; let viewport_size = vec2(1000., 500.); - let resolve_auto = Val::Auto.resolve(size, viewport_size); + let resolve_auto = Val::Auto.resolve(1., size, viewport_size); assert_eq!(resolve_auto, Err(ValArithmeticError::NonEvaluable)); } diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index 1b74b90f33..6d6186bf4b 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -1,7 +1,7 @@ use crate::{ experimental::{UiChildren, UiRootNodes}, BorderRadius, ComputedNode, ComputedNodeTarget, ContentSize, Display, LayoutConfig, Node, - Outline, OverflowAxis, ScrollPosition, Val, + Outline, OverflowAxis, ScrollPosition, }; use bevy_ecs::{ change_detection::{DetectChanges, DetectChangesMut}, @@ -268,24 +268,28 @@ with UI components as a child of an entity without UI components, your UI layout // don't trigger change detection when only outlines are changed let node = node.bypass_change_detection(); node.outline_width = if style.display != Display::None { - match outline.width { - Val::Px(w) => Val::Px(w / inverse_target_scale_factor), - width => width, - } - .resolve(node.size().x, viewport_size) - .unwrap_or(0.) - .max(0.) + outline + .width + .resolve( + inverse_target_scale_factor.recip(), + node.size().x, + viewport_size, + ) + .unwrap_or(0.) + .max(0.) } else { 0. }; - node.outline_offset = match outline.offset { - Val::Px(offset) => Val::Px(offset / inverse_target_scale_factor), - offset => offset, - } - .resolve(node.size().x, viewport_size) - .unwrap_or(0.) - .max(0.); + node.outline_offset = outline + .offset + .resolve( + inverse_target_scale_factor.recip(), + node.size().x, + viewport_size, + ) + .unwrap_or(0.) + .max(0.); } if transform.translation.truncate() != node_center { From 236f5d0074ebfe719e32f13d5ff8a38c232974cc Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Fri, 2 May 2025 12:32:48 +0100 Subject: [PATCH 2/9] Update crates/bevy_ui/src/geometry.rs Co-authored-by: Greeble <166992735+greeble-dev@users.noreply.github.com> --- crates/bevy_ui/src/geometry.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/bevy_ui/src/geometry.rs b/crates/bevy_ui/src/geometry.rs index 9be9ceade6..d8dbed264d 100644 --- a/crates/bevy_ui/src/geometry.rs +++ b/crates/bevy_ui/src/geometry.rs @@ -703,6 +703,11 @@ mod tests { let result = Val::Px(10.).resolve(1., size, viewport_size).unwrap(); assert_eq!(result, 10.); + + let result = Val::Px(10.).resolve(3., size, viewport_size).unwrap(); + assert_eq!(result, 30.); + let result = Val::Px(10.).resolve(0.25, size, viewport_size).unwrap(); + assert_eq!(result, 2.5); } #[test] From bb3419075e24a5f0cb4e218d6449f4fb5147b551 Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Fri, 2 May 2025 12:42:15 +0100 Subject: [PATCH 3/9] Added working migration guide. --- working-migration-guides/add_scale_factor_to_Val-Resolve.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 working-migration-guides/add_scale_factor_to_Val-Resolve.md diff --git a/working-migration-guides/add_scale_factor_to_Val-Resolve.md b/working-migration-guides/add_scale_factor_to_Val-Resolve.md new file mode 100644 index 0000000000..a1448b20e1 --- /dev/null +++ b/working-migration-guides/add_scale_factor_to_Val-Resolve.md @@ -0,0 +1,5 @@ +# Add scale factor to `Val::resolve` + +prs = [18164] + +`Val::resolve` now has a scale factor parameter. To resolve a `Val` to a logical value pass in a `scale_factor` of `1.`. \ No newline at end of file From 25390486f134145d2f491988034d1ba23d4f94a6 Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Fri, 2 May 2025 12:56:51 +0100 Subject: [PATCH 4/9] edit migration guide --- working-migration-guides/add_scale_factor_to_Val-Resolve.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/working-migration-guides/add_scale_factor_to_Val-Resolve.md b/working-migration-guides/add_scale_factor_to_Val-Resolve.md index a1448b20e1..49ede34375 100644 --- a/working-migration-guides/add_scale_factor_to_Val-Resolve.md +++ b/working-migration-guides/add_scale_factor_to_Val-Resolve.md @@ -1,5 +1,5 @@ -# Add scale factor to `Val::resolve` +# `Val::resolve` scale factor support prs = [18164] -`Val::resolve` now has a scale factor parameter. To resolve a `Val` to a logical value pass in a `scale_factor` of `1.`. \ No newline at end of file +`Val::resolve` now has a `scale factor` parameter. To resolve a `Val` to a logical value use a `scale_factor` of `1.`. \ No newline at end of file From 590ac42948dbe4f89f66b812ec7cb803a1fce941 Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Fri, 2 May 2025 13:40:21 +0100 Subject: [PATCH 5/9] single-trailing-newline fix --- working-migration-guides/add_scale_factor_to_Val-Resolve.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/working-migration-guides/add_scale_factor_to_Val-Resolve.md b/working-migration-guides/add_scale_factor_to_Val-Resolve.md index 49ede34375..ce0d8e5f92 100644 --- a/working-migration-guides/add_scale_factor_to_Val-Resolve.md +++ b/working-migration-guides/add_scale_factor_to_Val-Resolve.md @@ -2,4 +2,4 @@ prs = [18164] -`Val::resolve` now has a `scale factor` parameter. To resolve a `Val` to a logical value use a `scale_factor` of `1.`. \ No newline at end of file +`Val::resolve` now has a `scale factor` parameter. To resolve a `Val` to a logical value use a `scale_factor` of `1.`. From 1a632343f7d85025b35bdc3e958d176a6e3f72b0 Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Mon, 12 May 2025 11:29:10 +0100 Subject: [PATCH 6/9] Added draft migration guide. --- .../migration-guides/val_resolve_scale_factor_support.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 release-content/migration-guides/val_resolve_scale_factor_support.md diff --git a/release-content/migration-guides/val_resolve_scale_factor_support.md b/release-content/migration-guides/val_resolve_scale_factor_support.md new file mode 100644 index 0000000000..e88e905f0d --- /dev/null +++ b/release-content/migration-guides/val_resolve_scale_factor_support.md @@ -0,0 +1,7 @@ +--- +title: `Val::resolve` scale factor support +pull_requests: [18164] +--- + +`Val::resolve` now has a scale factor parameter. To get the previous behaviour that would resolve a `Val` to a logical value use a scale factor of `1.`. + From 379f3ecef0d0b6462602e32e1dcae37900731133 Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Mon, 12 May 2025 12:36:40 +0100 Subject: [PATCH 7/9] Fix migration guide --- .../migration-guides/val_resolve_scale_factor_support.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/release-content/migration-guides/val_resolve_scale_factor_support.md b/release-content/migration-guides/val_resolve_scale_factor_support.md index e88e905f0d..ed586ba645 100644 --- a/release-content/migration-guides/val_resolve_scale_factor_support.md +++ b/release-content/migration-guides/val_resolve_scale_factor_support.md @@ -3,5 +3,4 @@ title: `Val::resolve` scale factor support pull_requests: [18164] --- -`Val::resolve` now has a scale factor parameter. To get the previous behaviour that would resolve a `Val` to a logical value use a scale factor of `1.`. - +`Val::resolve` now has a scale factor parameter. To resolve a `Val` to a logical value use a scale factor of `1.`. From f164e65994aa7a1a7b72f8720a1c61d1cdd26dfa Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Mon, 12 May 2025 13:42:52 +0100 Subject: [PATCH 8/9] try to fix migration guide --- .../migration-guides/val_resolve_scale_factor_support.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/release-content/migration-guides/val_resolve_scale_factor_support.md b/release-content/migration-guides/val_resolve_scale_factor_support.md index ed586ba645..efbb9c3319 100644 --- a/release-content/migration-guides/val_resolve_scale_factor_support.md +++ b/release-content/migration-guides/val_resolve_scale_factor_support.md @@ -1,6 +1,6 @@ --- -title: `Val::resolve` scale factor support +title: Val resolve scale factor support pull_requests: [18164] --- -`Val::resolve` now has a scale factor parameter. To resolve a `Val` to a logical value use a scale factor of `1.`. +`Val::resolve` now has a scale factor parameter. To resolve a `Val` to a logical value use a scale factor of `1.`. \ No newline at end of file From c7c84738c66b57d1074ea9e63ca6d40d840c9161 Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Mon, 12 May 2025 13:48:57 +0100 Subject: [PATCH 9/9] Fixed migration guide --- .../migration-guides/val_resolve_scale_factor_support.md | 4 ++-- working-migration-guides/add_scale_factor_to_Val-Resolve.md | 5 ----- 2 files changed, 2 insertions(+), 7 deletions(-) delete mode 100644 working-migration-guides/add_scale_factor_to_Val-Resolve.md diff --git a/release-content/migration-guides/val_resolve_scale_factor_support.md b/release-content/migration-guides/val_resolve_scale_factor_support.md index efbb9c3319..ed586ba645 100644 --- a/release-content/migration-guides/val_resolve_scale_factor_support.md +++ b/release-content/migration-guides/val_resolve_scale_factor_support.md @@ -1,6 +1,6 @@ --- -title: Val resolve scale factor support +title: `Val::resolve` scale factor support pull_requests: [18164] --- -`Val::resolve` now has a scale factor parameter. To resolve a `Val` to a logical value use a scale factor of `1.`. \ No newline at end of file +`Val::resolve` now has a scale factor parameter. To resolve a `Val` to a logical value use a scale factor of `1.`. diff --git a/working-migration-guides/add_scale_factor_to_Val-Resolve.md b/working-migration-guides/add_scale_factor_to_Val-Resolve.md deleted file mode 100644 index ce0d8e5f92..0000000000 --- a/working-migration-guides/add_scale_factor_to_Val-Resolve.md +++ /dev/null @@ -1,5 +0,0 @@ -# `Val::resolve` scale factor support - -prs = [18164] - -`Val::resolve` now has a `scale factor` parameter. To resolve a `Val` to a logical value use a `scale_factor` of `1.`.