Remove custom rounding

crates/bevy_ui/src/layout/mod.rs

@@ -296,14 +296,13 @@ with UI components as a child of an entity without UI components, your UI layout
             update_uinode_geometry_recursive(
                 &mut commands,
                 *root,
-                &ui_surface,
+                &mut ui_surface,
                 None,
                 &mut node_transform_query,
                 &ui_children,
                 inverse_target_scale_factor,
                 Vec2::ZERO,
                 Vec2::ZERO,
-                Vec2::ZERO,
             );
         }
 
@@ -315,7 +314,7 @@ with UI components as a child of an entity without UI components, your UI layout
     fn update_uinode_geometry_recursive(
         commands: &mut Commands,
         entity: Entity,
-        ui_surface: &UiSurface,
+        ui_surface: &mut UiSurface,
         root_size: Option<Vec2>,
         node_transform_query: &mut Query<(
             &mut ComputedNode,
@@ -329,7 +328,6 @@ with UI components as a child of an entity without UI components, your UI layout
         inverse_target_scale_factor: f32,
         parent_size: Vec2,
         parent_scroll_position: Vec2,
-        mut absolute_location: Vec2,
     ) {
         if let Ok((
             mut node,
@@ -340,28 +338,23 @@ with UI components as a child of an entity without UI components, your UI layout
             maybe_scroll_position,
         )) = node_transform_query.get_mut(entity)
         {
-            let Ok(layout) = ui_surface.get_layout(entity) else {
+            let Ok((layout, unrounded_unscaled_size)) = ui_surface.get_layout(entity) else {
                 return;
             };
 
             let layout_size =
                 inverse_target_scale_factor * Vec2::new(layout.size.width, layout.size.height);
+            let unrounded_size = inverse_target_scale_factor * unrounded_unscaled_size;
             let layout_location =
                 inverse_target_scale_factor * Vec2::new(layout.location.x, layout.location.y);
 
-            absolute_location += layout_location;
-
-            let rounded_size = approx_round_layout_coords(absolute_location + layout_size)
-                - approx_round_layout_coords(absolute_location);
-
             let rounded_location =
-                approx_round_layout_coords(layout_location - parent_scroll_position)
-                    + 0.5 * (rounded_size - parent_size);
+                layout_location - parent_scroll_position + 0.5 * (layout_size - parent_size);
 
             // only trigger change detection when the new values are different
-            if node.calculated_size != rounded_size || node.unrounded_size != layout_size {
-                node.calculated_size = rounded_size;
-                node.unrounded_size = layout_size;
+            if node.calculated_size != layout_size || node.unrounded_size != unrounded_size {
+                node.calculated_size = layout_size;
+                node.unrounded_size = unrounded_size;
             }
 
             let taffy_rect_to_border_rect = |rect: taffy::Rect<f32>| BorderRect {
@@ -423,11 +416,10 @@ with UI components as a child of an entity without UI components, your UI layout
                 })
                 .unwrap_or_default();
 
-            let round_content_size = approx_round_layout_coords(
+            let round_content_size =
                 Vec2::new(layout.content_size.width, layout.content_size.height)
-                    * inverse_target_scale_factor,
-            );
-            let max_possible_offset = (round_content_size - rounded_size).max(Vec2::ZERO);
+                    * inverse_target_scale_factor;
+            let max_possible_offset = (round_content_size - layout_size).max(Vec2::ZERO);
             let clamped_scroll_position = scroll_position.clamp(Vec2::ZERO, max_possible_offset);
 
             if clamped_scroll_position != scroll_position {
@@ -445,36 +437,14 @@ with UI components as a child of an entity without UI components, your UI layout
                     node_transform_query,
                     ui_children,
                     inverse_target_scale_factor,
-                    rounded_size,
+                    layout_size,
                     clamped_scroll_position,
-                    absolute_location,
                 );
             }
         }
     }
 }
 
-#[inline]
-/// Round `value` to the nearest whole integer, with ties (values with a fractional part equal to 0.5) rounded towards positive infinity.
-fn approx_round_ties_up(value: f32) -> f32 {
-    (value + 0.5).floor()
-}
-
-#[inline]
-/// Rounds layout coordinates by rounding ties upwards.
-///
-/// Rounding ties up avoids gaining a pixel when rounding bounds that span from negative to positive.
-///
-/// Example: The width between bounds of -50.5 and 49.5 before rounding is 100, using:
-/// - `f32::round`: width becomes 101 (rounds to -51 and 50).
-/// - `round_ties_up`: width is 100 (rounds to -50 and 50).
-fn approx_round_layout_coords(value: Vec2) -> Vec2 {
-    Vec2 {
-        x: approx_round_ties_up(value.x),
-        y: approx_round_ties_up(value.y),
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use taffy::TraversePartialTree;
@@ -493,7 +463,7 @@ mod tests {
     use bevy_hierarchy::{
         despawn_with_children_recursive, BuildChildren, ChildBuild, Children, Parent,
     };
-    use bevy_math::{vec2, Rect, UVec2, Vec2};
+    use bevy_math::{Rect, UVec2, Vec2};
     use bevy_render::{
         camera::{ManualTextureViews, OrthographicProjection},
         prelude::Camera,
@@ -510,21 +480,10 @@ mod tests {
     };
 
     use crate::{
-        layout::{approx_round_layout_coords, ui_surface::UiSurface},
-        prelude::*,
-        ui_layout_system,
-        update::update_target_camera_system,
-        ContentSize, LayoutContext,
+        layout::ui_surface::UiSurface, prelude::*, ui_layout_system,
+        update::update_target_camera_system, ContentSize, LayoutContext,
     };
 
-    #[test]
-    fn round_layout_coords_must_round_ties_up() {
-        assert_eq!(
-            approx_round_layout_coords(vec2(-50.5, 49.5)),
-            vec2(-50., 50.)
-        );
-    }
-
     // these window dimensions are easy to convert to and from percentage values
     const WINDOW_WIDTH: f32 = 1000.;
     const WINDOW_HEIGHT: f32 = 100.;
@@ -598,10 +557,10 @@ mod tests {
         world.entity_mut(ui_root).add_child(ui_child);
 
         ui_schedule.run(&mut world);
-        let ui_surface = world.resource::<UiSurface>();
+        let mut ui_surface = world.resource_mut::<UiSurface>();
 
         for ui_entity in [ui_root, ui_child] {
-            let layout = ui_surface.get_layout(ui_entity).unwrap();
+            let layout = ui_surface.get_layout(ui_entity).unwrap().0;
             assert_eq!(layout.size.width, WINDOW_WIDTH);
             assert_eq!(layout.size.height, WINDOW_HEIGHT);
         }
@@ -955,11 +914,12 @@ mod tests {
                 .get_single(world)
                 .expect("missing MovingUiNode");
             assert_eq!(expected_camera_entity, target_camera_entity);
-            let ui_surface = world.resource::<UiSurface>();
+            let mut ui_surface = world.resource_mut::<UiSurface>();
 
             let layout = ui_surface
                 .get_layout(ui_node_entity)
-                .expect("failed to get layout");
+                .expect("failed to get layout")
+                .0;
 
             // negative test for #12255
             assert_eq!(Vec2::new(layout.location.x, layout.location.y), new_pos);
@@ -1043,8 +1003,8 @@ mod tests {
 
         ui_schedule.run(&mut world);
 
-        let ui_surface = world.resource::<UiSurface>();
-        let layout = ui_surface.get_layout(ui_entity).unwrap();
+        let mut ui_surface = world.resource_mut::<UiSurface>();
+        let layout = ui_surface.get_layout(ui_entity).unwrap().0;
 
         // the node should takes its size from the fixed size measure func
         assert_eq!(layout.size.width, content_size.x);
@@ -1068,25 +1028,25 @@ mod tests {
 
         ui_schedule.run(&mut world);
 
-        let ui_surface = world.resource::<UiSurface>();
+        let mut ui_surface = world.resource_mut::<UiSurface>();
         let ui_node = ui_surface.entity_to_taffy[&ui_entity];
 
         // a node with a content size should have taffy context
         assert!(ui_surface.taffy.get_node_context(ui_node).is_some());
-        let layout = ui_surface.get_layout(ui_entity).unwrap();
+        let layout = ui_surface.get_layout(ui_entity).unwrap().0;
         assert_eq!(layout.size.width, content_size.x);
         assert_eq!(layout.size.height, content_size.y);
 
         world.entity_mut(ui_entity).remove::<ContentSize>();
 
         ui_schedule.run(&mut world);
 
-        let ui_surface = world.resource::<UiSurface>();
+        let mut ui_surface = world.resource_mut::<UiSurface>();
         // a node without a content size should not have taffy context
         assert!(ui_surface.taffy.get_node_context(ui_node).is_none());
 
         // Without a content size, the node has no width or height constraints so the length of both dimensions is 0.
-        let layout = ui_surface.get_layout(ui_entity).unwrap();
+        let layout = ui_surface.get_layout(ui_entity).unwrap().0;
         assert_eq!(layout.size.width, 0.);
         assert_eq!(layout.size.height, 0.);
     }
@@ -1123,7 +1083,8 @@ mod tests {
             .collect::<Vec<Entity>>();
 
         for r in [2, 3, 5, 7, 11, 13, 17, 19, 21, 23, 29, 31].map(|n| (n as f32).recip()) {
-            let mut s = r;
+            // This fails with very small / unrealistic scale values
+            let mut s = 1. - r;
             while s <= 5. {
                 world.resource_mut::<UiScale>().0 = s;
                 ui_schedule.run(&mut world);

crates/bevy_ui/src/layout/ui_surface.rs

@@ -6,7 +6,7 @@ use bevy_ecs::{
     entity::{Entity, EntityHashMap},
     prelude::Resource,
 };
-use bevy_math::UVec2;
+use bevy_math::{UVec2, Vec2};
 use bevy_utils::default;
 
 use crate::{layout::convert, LayoutContext, LayoutError, Measure, MeasureArgs, Node, NodeMeasure};
@@ -51,8 +51,7 @@ impl fmt::Debug for UiSurface {
 
 impl Default for UiSurface {
     fn default() -> Self {
-        let mut taffy: TaffyTree<NodeMeasure> = TaffyTree::new();
-        taffy.disable_rounding();
+        let taffy: TaffyTree<NodeMeasure> = TaffyTree::new();
         Self {
             entity_to_taffy: Default::default(),
             camera_entity_to_taffy: Default::default(),
@@ -276,14 +275,23 @@ impl UiSurface {
 
     /// Get the layout geometry for the taffy node corresponding to the ui node [`Entity`].
     /// Does not compute the layout geometry, `compute_window_layouts` should be run before using this function.
-    pub fn get_layout(&self, entity: Entity) -> Result<&taffy::Layout, LayoutError> {
-        if let Some(taffy_node) = self.entity_to_taffy.get(&entity) {
-            self.taffy
-                .layout(*taffy_node)
-                .map_err(LayoutError::TaffyError)
-        } else {
-            Err(LayoutError::InvalidHierarchy)
-        }
+    pub fn get_layout(&mut self, entity: Entity) -> Result<(taffy::Layout, Vec2), LayoutError> {
+        let Some(taffy_node) = self.entity_to_taffy.get(&entity) else {
+            return Err(LayoutError::InvalidHierarchy);
+        };
+
+        let layout = self
+            .taffy
+            .layout(*taffy_node)
+            .cloned()
+            .map_err(LayoutError::TaffyError)?;
+
+        self.taffy.disable_rounding();
+        let taffy_size = self.taffy.layout(*taffy_node).unwrap().size;
+        let size = Vec2::new(taffy_size.width, taffy_size.height);
+        self.taffy.enable_rounding();
+
+        Ok((layout, size))
     }
 }