Revert "Initial layout rounding implementation (#39712)"

John Tur created 3 weeks ago

This reverts commit 2fa54ddbbe408e31cac451c251e8241c1d083885.

This is causing some inscrutable issues with the title bar and window
borders on Linux with fractional DPI scaling. I can't debug this right
now, so let's revert it before the stable release.

Change summary

crates/gpui/src/geometry.rs  |   9 --
crates/gpui/src/taffy.rs     | 161 ++++++++++++-------------------------
crates/gpui/src/window.rs    |  22 +---
crates/workspace/src/pane.rs |   4 
4 files changed, 62 insertions(+), 134 deletions(-)

Detailed changes

crates/gpui/src/geometry.rs

@@ -2968,15 +2968,6 @@ impl ScaledPixels {
     /// # Returns
     ///
     /// Returns a new `ScaledPixels` instance with the rounded value.
-    pub fn round(&self) -> Self {
-        Self(self.0.round())
-    }
-
-    /// Ceils the `ScaledPixels` value to the nearest whole number.
-    ///
-    /// # Returns
-    ///
-    /// Returns a new `ScaledPixels` instance with the ceiled value.
     pub fn ceil(&self) -> Self {
         Self(self.0.ceil())
     }

crates/gpui/src/taffy.rs

@@ -1,6 +1,5 @@
 use crate::{
     AbsoluteLength, App, Bounds, DefiniteLength, Edges, Length, Pixels, Point, Size, Style, Window,
-    point, size,
 };
 use collections::{FxHashMap, FxHashSet};
 use smallvec::SmallVec;
@@ -38,7 +37,7 @@ const EXPECT_MESSAGE: &str = "we should avoid taffy layout errors by constructio
 impl TaffyLayoutEngine {
     pub fn new() -> Self {
         let mut taffy = TaffyTree::new();
-        taffy.enable_rounding();
+        taffy.disable_rounding();
         TaffyLayoutEngine {
             taffy,
             absolute_layout_bounds: FxHashMap::default(),
@@ -56,10 +55,9 @@ impl TaffyLayoutEngine {
         &mut self,
         style: Style,
         rem_size: Pixels,
-        scale_factor: f32,
         children: &[LayoutId],
     ) -> LayoutId {
-        let taffy_style = style.to_taffy(rem_size, scale_factor);
+        let taffy_style = style.to_taffy(rem_size);
 
         if children.is_empty() {
             self.taffy
@@ -81,7 +79,6 @@ impl TaffyLayoutEngine {
         &mut self,
         style: Style,
         rem_size: Pixels,
-        scale_factor: f32,
         measure: impl FnMut(
             Size<Option<Pixels>>,
             Size<AvailableSpace>,
@@ -90,7 +87,7 @@ impl TaffyLayoutEngine {
         ) -> Size<Pixels>
         + 'static,
     ) -> LayoutId {
-        let taffy_style = style.to_taffy(rem_size, scale_factor);
+        let taffy_style = style.to_taffy(rem_size);
 
         self.taffy
             .new_leaf_with_context(
@@ -184,20 +181,7 @@ impl TaffyLayoutEngine {
             }
         }
 
-        let scale_factor = window.scale_factor();
-
-        let transform = |v: AvailableSpace| match v {
-            AvailableSpace::Definite(pixels) => {
-                AvailableSpace::Definite(Pixels(pixels.0 * scale_factor))
-            }
-            AvailableSpace::MinContent => AvailableSpace::MinContent,
-            AvailableSpace::MaxContent => AvailableSpace::MaxContent,
-        };
-        let available_space = size(
-            transform(available_space.width),
-            transform(available_space.height),
-        );
-
+        // let started_at = std::time::Instant::now();
         self.taffy
             .compute_layout_with_measure(
                 id.into(),
@@ -208,50 +192,32 @@ impl TaffyLayoutEngine {
                     };
 
                     let known_dimensions = Size {
-                        width: known_dimensions.width.map(|e| Pixels(e / scale_factor)),
-                        height: known_dimensions.height.map(|e| Pixels(e / scale_factor)),
+                        width: known_dimensions.width.map(Pixels),
+                        height: known_dimensions.height.map(Pixels),
                     };
 
-                    let available_space: Size<AvailableSpace> = available_space.into();
-                    let untransform = |ev: AvailableSpace| match ev {
-                        AvailableSpace::Definite(pixels) => {
-                            AvailableSpace::Definite(Pixels(pixels.0 / scale_factor))
-                        }
-                        AvailableSpace::MinContent => AvailableSpace::MinContent,
-                        AvailableSpace::MaxContent => AvailableSpace::MaxContent,
-                    };
-                    let available_space = size(
-                        untransform(available_space.width),
-                        untransform(available_space.height),
-                    );
-
-                    let a: Size<Pixels> =
-                        (node_context.measure)(known_dimensions, available_space, window, cx);
-                    size(a.width.0 * scale_factor, a.height.0 * scale_factor).into()
+                    (node_context.measure)(known_dimensions, available_space.into(), window, cx)
+                        .into()
                 },
             )
             .expect(EXPECT_MESSAGE);
+
+        // println!("compute_layout took {:?}", started_at.elapsed());
     }
 
-    pub fn layout_bounds(&mut self, id: LayoutId, scale_factor: f32) -> Bounds<Pixels> {
+    pub fn layout_bounds(&mut self, id: LayoutId) -> Bounds<Pixels> {
         if let Some(layout) = self.absolute_layout_bounds.get(&id).cloned() {
             return layout;
         }
 
         let layout = self.taffy.layout(id.into()).expect(EXPECT_MESSAGE);
         let mut bounds = Bounds {
-            origin: point(
-                Pixels(layout.location.x / scale_factor),
-                Pixels(layout.location.y / scale_factor),
-            ),
-            size: size(
-                Pixels(layout.size.width / scale_factor),
-                Pixels(layout.size.height / scale_factor),
-            ),
+            origin: layout.location.into(),
+            size: layout.size.into(),
         };
 
         if let Some(parent_id) = self.taffy.parent(id.0) {
-            let parent_bounds = self.layout_bounds(parent_id.into(), scale_factor);
+            let parent_bounds = self.layout_bounds(parent_id.into());
             bounds.origin += parent_bounds.origin;
         }
         self.absolute_layout_bounds.insert(id, bounds);
@@ -284,11 +250,11 @@ impl From<LayoutId> for NodeId {
 }
 
 trait ToTaffy<Output> {
-    fn to_taffy(&self, rem_size: Pixels, scale_factor: f32) -> Output;
+    fn to_taffy(&self, rem_size: Pixels) -> Output;
 }
 
 impl ToTaffy<taffy::style::Style> for Style {
-    fn to_taffy(&self, rem_size: Pixels, scale_factor: f32) -> taffy::style::Style {
+    fn to_taffy(&self, rem_size: Pixels) -> taffy::style::Style {
         use taffy::style_helpers::{fr, length, minmax, repeat};
 
         fn to_grid_line(
@@ -311,24 +277,24 @@ impl ToTaffy<taffy::style::Style> for Style {
         taffy::style::Style {
             display: self.display.into(),
             overflow: self.overflow.into(),
-            scrollbar_width: self.scrollbar_width.to_taffy(rem_size, scale_factor),
+            scrollbar_width: self.scrollbar_width.to_taffy(rem_size),
             position: self.position.into(),
-            inset: self.inset.to_taffy(rem_size, scale_factor),
-            size: self.size.to_taffy(rem_size, scale_factor),
-            min_size: self.min_size.to_taffy(rem_size, scale_factor),
-            max_size: self.max_size.to_taffy(rem_size, scale_factor),
+            inset: self.inset.to_taffy(rem_size),
+            size: self.size.to_taffy(rem_size),
+            min_size: self.min_size.to_taffy(rem_size),
+            max_size: self.max_size.to_taffy(rem_size),
             aspect_ratio: self.aspect_ratio,
-            margin: self.margin.to_taffy(rem_size, scale_factor),
-            padding: self.padding.to_taffy(rem_size, scale_factor),
-            border: self.border_widths.to_taffy(rem_size, scale_factor),
+            margin: self.margin.to_taffy(rem_size),
+            padding: self.padding.to_taffy(rem_size),
+            border: self.border_widths.to_taffy(rem_size),
             align_items: self.align_items.map(|x| x.into()),
             align_self: self.align_self.map(|x| x.into()),
             align_content: self.align_content.map(|x| x.into()),
             justify_content: self.justify_content.map(|x| x.into()),
-            gap: self.gap.to_taffy(rem_size, scale_factor),
+            gap: self.gap.to_taffy(rem_size),
             flex_direction: self.flex_direction.into(),
             flex_wrap: self.flex_wrap.into(),
-            flex_basis: self.flex_basis.to_taffy(rem_size, scale_factor),
+            flex_basis: self.flex_basis.to_taffy(rem_size),
             flex_grow: self.flex_grow,
             flex_shrink: self.flex_shrink,
             grid_template_rows: to_grid_repeat(&self.grid_rows),
@@ -349,53 +315,41 @@ impl ToTaffy<taffy::style::Style> for Style {
 }
 
 impl ToTaffy<f32> for AbsoluteLength {
-    fn to_taffy(&self, rem_size: Pixels, scale_factor: f32) -> f32 {
+    fn to_taffy(&self, rem_size: Pixels) -> f32 {
         match self {
-            AbsoluteLength::Pixels(pixels) => {
-                let pixels: f32 = pixels.into();
-                pixels * scale_factor
-            }
-            AbsoluteLength::Rems(rems) => {
-                let pixels: f32 = (*rems * rem_size).into();
-                pixels * scale_factor
-            }
+            AbsoluteLength::Pixels(pixels) => pixels.into(),
+            AbsoluteLength::Rems(rems) => (*rems * rem_size).into(),
         }
     }
 }
 
 impl ToTaffy<taffy::style::LengthPercentageAuto> for Length {
-    fn to_taffy(
-        &self,
-        rem_size: Pixels,
-        scale_factor: f32,
-    ) -> taffy::prelude::LengthPercentageAuto {
+    fn to_taffy(&self, rem_size: Pixels) -> taffy::prelude::LengthPercentageAuto {
         match self {
-            Length::Definite(length) => length.to_taffy(rem_size, scale_factor),
+            Length::Definite(length) => length.to_taffy(rem_size),
             Length::Auto => taffy::prelude::LengthPercentageAuto::auto(),
         }
     }
 }
 
 impl ToTaffy<taffy::style::Dimension> for Length {
-    fn to_taffy(&self, rem_size: Pixels, scale_factor: f32) -> taffy::prelude::Dimension {
+    fn to_taffy(&self, rem_size: Pixels) -> taffy::prelude::Dimension {
         match self {
-            Length::Definite(length) => length.to_taffy(rem_size, scale_factor),
+            Length::Definite(length) => length.to_taffy(rem_size),
             Length::Auto => taffy::prelude::Dimension::auto(),
         }
     }
 }
 
 impl ToTaffy<taffy::style::LengthPercentage> for DefiniteLength {
-    fn to_taffy(&self, rem_size: Pixels, scale_factor: f32) -> taffy::style::LengthPercentage {
+    fn to_taffy(&self, rem_size: Pixels) -> taffy::style::LengthPercentage {
         match self {
             DefiniteLength::Absolute(length) => match length {
                 AbsoluteLength::Pixels(pixels) => {
-                    let pixels: f32 = pixels.into();
-                    taffy::style::LengthPercentage::length(pixels * scale_factor)
+                    taffy::style::LengthPercentage::length(pixels.into())
                 }
                 AbsoluteLength::Rems(rems) => {
-                    let pixels: f32 = (*rems * rem_size).into();
-                    taffy::style::LengthPercentage::length(pixels * scale_factor)
+                    taffy::style::LengthPercentage::length((*rems * rem_size).into())
                 }
             },
             DefiniteLength::Fraction(fraction) => {
@@ -406,16 +360,14 @@ impl ToTaffy<taffy::style::LengthPercentage> for DefiniteLength {
 }
 
 impl ToTaffy<taffy::style::LengthPercentageAuto> for DefiniteLength {
-    fn to_taffy(&self, rem_size: Pixels, scale_factor: f32) -> taffy::style::LengthPercentageAuto {
+    fn to_taffy(&self, rem_size: Pixels) -> taffy::style::LengthPercentageAuto {
         match self {
             DefiniteLength::Absolute(length) => match length {
                 AbsoluteLength::Pixels(pixels) => {
-                    let pixels: f32 = pixels.into();
-                    taffy::style::LengthPercentageAuto::length(pixels * scale_factor)
+                    taffy::style::LengthPercentageAuto::length(pixels.into())
                 }
                 AbsoluteLength::Rems(rems) => {
-                    let pixels: f32 = (*rems * rem_size).into();
-                    taffy::style::LengthPercentageAuto::length(pixels * scale_factor)
+                    taffy::style::LengthPercentageAuto::length((*rems * rem_size).into())
                 }
             },
             DefiniteLength::Fraction(fraction) => {
@@ -426,15 +378,12 @@ impl ToTaffy<taffy::style::LengthPercentageAuto> for DefiniteLength {
 }
 
 impl ToTaffy<taffy::style::Dimension> for DefiniteLength {
-    fn to_taffy(&self, rem_size: Pixels, scale_factor: f32) -> taffy::style::Dimension {
+    fn to_taffy(&self, rem_size: Pixels) -> taffy::style::Dimension {
         match self {
             DefiniteLength::Absolute(length) => match length {
-                AbsoluteLength::Pixels(pixels) => {
-                    let pixels: f32 = pixels.into();
-                    taffy::style::Dimension::length(pixels * scale_factor)
-                }
+                AbsoluteLength::Pixels(pixels) => taffy::style::Dimension::length(pixels.into()),
                 AbsoluteLength::Rems(rems) => {
-                    taffy::style::Dimension::length((*rems * rem_size * scale_factor).into())
+                    taffy::style::Dimension::length((*rems * rem_size).into())
                 }
             },
             DefiniteLength::Fraction(fraction) => taffy::style::Dimension::percent(*fraction),
@@ -443,15 +392,11 @@ impl ToTaffy<taffy::style::Dimension> for DefiniteLength {
 }
 
 impl ToTaffy<taffy::style::LengthPercentage> for AbsoluteLength {
-    fn to_taffy(&self, rem_size: Pixels, scale_factor: f32) -> taffy::style::LengthPercentage {
+    fn to_taffy(&self, rem_size: Pixels) -> taffy::style::LengthPercentage {
         match self {
-            AbsoluteLength::Pixels(pixels) => {
-                let pixels: f32 = pixels.into();
-                taffy::style::LengthPercentage::length(pixels * scale_factor)
-            }
+            AbsoluteLength::Pixels(pixels) => taffy::style::LengthPercentage::length(pixels.into()),
             AbsoluteLength::Rems(rems) => {
-                let pixels: f32 = (*rems * rem_size).into();
-                taffy::style::LengthPercentage::length(pixels * scale_factor)
+                taffy::style::LengthPercentage::length((*rems * rem_size).into())
             }
         }
     }
@@ -486,10 +431,10 @@ impl<T, U> ToTaffy<TaffySize<U>> for Size<T>
 where
     T: ToTaffy<U> + Clone + Debug + Default + PartialEq,
 {
-    fn to_taffy(&self, rem_size: Pixels, scale_factor: f32) -> TaffySize<U> {
+    fn to_taffy(&self, rem_size: Pixels) -> TaffySize<U> {
         TaffySize {
-            width: self.width.to_taffy(rem_size, scale_factor),
-            height: self.height.to_taffy(rem_size, scale_factor),
+            width: self.width.to_taffy(rem_size),
+            height: self.height.to_taffy(rem_size),
         }
     }
 }
@@ -498,12 +443,12 @@ impl<T, U> ToTaffy<TaffyRect<U>> for Edges<T>
 where
     T: ToTaffy<U> + Clone + Debug + Default + PartialEq,
 {
-    fn to_taffy(&self, rem_size: Pixels, scale_factor: f32) -> TaffyRect<U> {
+    fn to_taffy(&self, rem_size: Pixels) -> TaffyRect<U> {
         TaffyRect {
-            top: self.top.to_taffy(rem_size, scale_factor),
-            right: self.right.to_taffy(rem_size, scale_factor),
-            bottom: self.bottom.to_taffy(rem_size, scale_factor),
-            left: self.left.to_taffy(rem_size, scale_factor),
+            top: self.top.to_taffy(rem_size),
+            right: self.right.to_taffy(rem_size),
+            bottom: self.bottom.to_taffy(rem_size),
+            left: self.left.to_taffy(rem_size),
         }
     }
 }

crates/gpui/src/window.rs

@@ -3071,7 +3071,6 @@ impl Window {
 
         let element_opacity = self.element_opacity();
         let scale_factor = self.scale_factor();
-
         let bounds = bounds.scale(scale_factor);
         let params = RenderSvgParams {
             path,
@@ -3107,9 +3106,7 @@ impl Window {
         self.next_frame.scene.insert_primitive(MonochromeSprite {
             order: 0,
             pad: 0,
-            bounds: svg_bounds
-                .map_origin(|origin| origin.round())
-                .map_size(|size| size.ceil()),
+            bounds: svg_bounds,
             content_mask,
             color: color.opacity(element_opacity),
             tile,
@@ -3222,14 +3219,11 @@ impl Window {
         cx.layout_id_buffer.clear();
         cx.layout_id_buffer.extend(children);
         let rem_size = self.rem_size();
-        let scale_factor = self.scale_factor();
 
-        self.layout_engine.as_mut().unwrap().request_layout(
-            style,
-            rem_size,
-            scale_factor,
-            &cx.layout_id_buffer,
-        )
+        self.layout_engine
+            .as_mut()
+            .unwrap()
+            .request_layout(style, rem_size, &cx.layout_id_buffer)
     }
 
     /// Add a node to the layout tree for the current frame. Instead of taking a `Style` and children,
@@ -3251,11 +3245,10 @@ impl Window {
         self.invalidator.debug_assert_prepaint();
 
         let rem_size = self.rem_size();
-        let scale_factor = self.scale_factor();
         self.layout_engine
             .as_mut()
             .unwrap()
-            .request_measured_layout(style, rem_size, scale_factor, measure)
+            .request_measured_layout(style, rem_size, measure)
     }
 
     /// Compute the layout for the given id within the given available space.
@@ -3283,12 +3276,11 @@ impl Window {
     pub fn layout_bounds(&mut self, layout_id: LayoutId) -> Bounds<Pixels> {
         self.invalidator.debug_assert_prepaint();
 
-        let scale_factor = self.scale_factor();
         let mut bounds = self
             .layout_engine
             .as_mut()
             .unwrap()
-            .layout_bounds(layout_id, scale_factor)
+            .layout_bounds(layout_id)
             .map(Into::into);
         bounds.origin += self.element_offset();
         bounds

crates/workspace/src/pane.rs

@@ -6565,8 +6565,8 @@ mod tests {
         let scroll_bounds = tab_bar_scroll_handle.bounds();
         let scroll_offset = tab_bar_scroll_handle.offset();
         assert!(tab_bounds.right() <= scroll_bounds.right() + scroll_offset.x);
-        // -39.5 is the magic number for this setup
-        assert_eq!(scroll_offset.x, px(-39.5));
+        // -39.75 is the magic number for this setup
+        assert_eq!(scroll_offset.x, px(-39.75));
         assert!(
             !tab_bounds.intersects(&new_tab_button_bounds),
             "Tab should not overlap with the new tab button, if this is failing check if there's been a redesign!"