From 5e8aa7986bfd7adc77c7b824e7fddb02171ce855 Mon Sep 17 00:00:00 2001 From: ickshonpe Date: Tue, 1 Jul 2025 18:41:48 +0100 Subject: [PATCH] Newtyped `ScrollPosition` (#19881) # Objective Change `ScrollPosition` to newtype `Vec2`. It's easier to work with a `Vec2` wrapper than individual fields. I'm not sure why this wasn't newtyped to start with. Maybe the intent was to support responsive coordinates eventually but that probably isn't very useful or straightforward to implement. And even if we do want to support responsive coords in the future, it can newtype `Val2`. ## Solution Change `ScrollPosition` to newtype `Vec2`. Also added some extra details to the doc comments. ## Testing Try the `scroll` example. --------- Co-authored-by: Alice Cecile --- .../bevy_core_widgets/src/core_scrollbar.rs | 26 ++++-------- crates/bevy_ui/src/layout/mod.rs | 6 +-- crates/bevy_ui/src/ui_node.rs | 42 ++++++------------- examples/testbed/full_ui.rs | 4 +- examples/ui/scroll.rs | 4 +- examples/ui/scrollbars.rs | 5 +-- .../Newtype_ScrollPosition.md | 6 +++ 7 files changed, 34 insertions(+), 59 deletions(-) create mode 100644 release-content/migration-guides/Newtype_ScrollPosition.md diff --git a/crates/bevy_core_widgets/src/core_scrollbar.rs b/crates/bevy_core_widgets/src/core_scrollbar.rs index 2d0fd49fb6..d997f565ce 100644 --- a/crates/bevy_core_widgets/src/core_scrollbar.rs +++ b/crates/bevy_core_widgets/src/core_scrollbar.rs @@ -136,23 +136,13 @@ fn scrollbar_on_pointer_down( ControlOrientation::Horizontal => { if node.size().x > 0. { let click_pos = local_pos.x * content_size.x / node.size().x; - adjust_scroll_pos( - &mut scroll_pos.offset_x, - click_pos, - visible_size.x, - max_range.x, - ); + adjust_scroll_pos(&mut scroll_pos.x, click_pos, visible_size.x, max_range.x); } } ControlOrientation::Vertical => { if node.size().y > 0. { let click_pos = local_pos.y * content_size.y / node.size().y; - adjust_scroll_pos( - &mut scroll_pos.offset_y, - click_pos, - visible_size.y, - max_range.y, - ); + adjust_scroll_pos(&mut scroll_pos.y, click_pos, visible_size.y, max_range.y); } } } @@ -171,8 +161,8 @@ fn scrollbar_on_drag_start( if let Ok(scroll_area) = q_scroll_area.get(scrollbar.target) { drag.dragging = true; drag.drag_origin = match scrollbar.orientation { - ControlOrientation::Horizontal => scroll_area.offset_x, - ControlOrientation::Vertical => scroll_area.offset_y, + ControlOrientation::Horizontal => scroll_area.x, + ControlOrientation::Vertical => scroll_area.y, }; } } @@ -204,13 +194,13 @@ fn scrollbar_on_drag( match scrollbar.orientation { ControlOrientation::Horizontal => { let range = (content_size.x - visible_size.x).max(0.); - scroll_pos.offset_x = (drag.drag_origin + scroll_pos.x = (drag.drag_origin + (distance.x * content_size.x) / scrollbar_size.x) .clamp(0., range); } ControlOrientation::Vertical => { let range = (content_size.y - visible_size.y).max(0.); - scroll_pos.offset_y = (drag.drag_origin + scroll_pos.y = (drag.drag_origin + (distance.y * content_size.y) / scrollbar_size.y) .clamp(0., range); } @@ -296,7 +286,7 @@ fn update_scrollbar_thumb( visible_size.x, track_length.x, scrollbar.min_thumb_length, - scroll_area.0.offset_x, + scroll_area.0.x, ); thumb.top = Val::Px(0.); @@ -310,7 +300,7 @@ fn update_scrollbar_thumb( visible_size.y, track_length.y, scrollbar.min_thumb_length, - scroll_area.0.offset_y, + scroll_area.0.y, ); thumb.left = Val::Px(0.); diff --git a/crates/bevy_ui/src/layout/mod.rs b/crates/bevy_ui/src/layout/mod.rs index 484acbd4af..4d5bec8f07 100644 --- a/crates/bevy_ui/src/layout/mod.rs +++ b/crates/bevy_ui/src/layout/mod.rs @@ -311,12 +311,12 @@ with UI components as a child of an entity without UI components, your UI layout .map(|scroll_pos| { Vec2::new( if style.overflow.x == OverflowAxis::Scroll { - scroll_pos.offset_x + scroll_pos.x } else { 0.0 }, if style.overflow.y == OverflowAxis::Scroll { - scroll_pos.offset_y + scroll_pos.y } else { 0.0 }, @@ -333,7 +333,7 @@ with UI components as a child of an entity without UI components, your UI layout if clamped_scroll_position != scroll_position { commands .entity(entity) - .insert(ScrollPosition::from(clamped_scroll_position)); + .insert(ScrollPosition(clamped_scroll_position)); } let physical_scroll_position = diff --git a/crates/bevy_ui/src/ui_node.rs b/crates/bevy_ui/src/ui_node.rs index 09ae4bf56e..f6b8ee27b3 100644 --- a/crates/bevy_ui/src/ui_node.rs +++ b/crates/bevy_ui/src/ui_node.rs @@ -321,45 +321,27 @@ impl Default for ComputedNode { } } -/// The scroll position of the node. +/// The scroll position of the node. Values are in logical pixels, increasing from top-left to bottom-right. /// -/// Updating the values of `ScrollPosition` will reposition the children of the node by the offset amount. +/// Increasing the x-coordinate causes the scrolled content to visibly move left on the screen, while increasing the y-coordinate causes the scrolled content to move up. +/// This might seem backwards, however what's really happening is that +/// the scroll position is moving the visible "window" in the local coordinate system of the scrolled content - +/// moving the window down causes the content to move up. +/// +/// Updating the values of `ScrollPosition` will reposition the children of the node by the offset amount in logical pixels. /// `ScrollPosition` may be updated by the layout system when a layout change makes a previously valid `ScrollPosition` invalid. /// Changing this does nothing on a `Node` without setting at least one `OverflowAxis` to `OverflowAxis::Scroll`. -#[derive(Component, Debug, Clone, Reflect)] +#[derive(Component, Debug, Clone, Default, Deref, DerefMut, Reflect)] #[reflect(Component, Default, Clone)] -pub struct ScrollPosition { - /// How far across the node is scrolled, in logical pixels. (0 = not scrolled / scrolled to right) - pub offset_x: f32, - /// How far down the node is scrolled, in logical pixels. (0 = not scrolled / scrolled to top) - pub offset_y: f32, -} +pub struct ScrollPosition(pub Vec2); impl ScrollPosition { - pub const DEFAULT: Self = Self { - offset_x: 0.0, - offset_y: 0.0, - }; -} - -impl Default for ScrollPosition { - fn default() -> Self { - Self::DEFAULT - } -} - -impl From<&ScrollPosition> for Vec2 { - fn from(scroll_pos: &ScrollPosition) -> Self { - Vec2::new(scroll_pos.offset_x, scroll_pos.offset_y) - } + pub const DEFAULT: Self = Self(Vec2::ZERO); } impl From for ScrollPosition { - fn from(vec: Vec2) -> Self { - ScrollPosition { - offset_x: vec.x, - offset_y: vec.y, - } + fn from(value: Vec2) -> Self { + Self(value) } } diff --git a/examples/testbed/full_ui.rs b/examples/testbed/full_ui.rs index 634945f057..da42dd9732 100644 --- a/examples/testbed/full_ui.rs +++ b/examples/testbed/full_ui.rs @@ -454,8 +454,8 @@ pub fn update_scroll_position( for (_pointer, pointer_map) in hover_map.iter() { for (entity, _hit) in pointer_map.iter() { if let Ok(mut scroll_position) = scrolled_node_query.get_mut(*entity) { - scroll_position.offset_x -= dx; - scroll_position.offset_y -= dy; + scroll_position.x -= dx; + scroll_position.y -= dy; } } } diff --git a/examples/ui/scroll.rs b/examples/ui/scroll.rs index 594c89a3a5..9bedb719d2 100644 --- a/examples/ui/scroll.rs +++ b/examples/ui/scroll.rs @@ -338,8 +338,8 @@ pub fn update_scroll_position( for (_pointer, pointer_map) in hover_map.iter() { for (entity, _hit) in pointer_map.iter() { if let Ok(mut scroll_position) = scrolled_node_query.get_mut(*entity) { - scroll_position.offset_x -= dx; - scroll_position.offset_y -= dy; + scroll_position.x -= dx; + scroll_position.y -= dy; } } } diff --git a/examples/ui/scrollbars.rs b/examples/ui/scrollbars.rs index 2726df12fb..72cdfd8229 100644 --- a/examples/ui/scrollbars.rs +++ b/examples/ui/scrollbars.rs @@ -86,10 +86,7 @@ fn scroll_area_demo() -> impl Bundle { ..default() }, BackgroundColor(colors::GRAY1.into()), - ScrollPosition { - offset_x: 0.0, - offset_y: 10.0, - }, + ScrollPosition(Vec2::new(0.0, 10.0)), Children::spawn(( // The actual content of the scrolling area Spawn(text_row("Alpha Wolf")), diff --git a/release-content/migration-guides/Newtype_ScrollPosition.md b/release-content/migration-guides/Newtype_ScrollPosition.md new file mode 100644 index 0000000000..5aa3196646 --- /dev/null +++ b/release-content/migration-guides/Newtype_ScrollPosition.md @@ -0,0 +1,6 @@ +--- +title: Make `ScrollPosition` newtype `Vec2` +pull_requests: [19881] +--- + +`ScrollPosition` now newtypes `Vec2`, its `offset_x` and `offset_y` fields have been removed.