Remove `Pixels: Mul<Pixels, Output = Pixels>` impl, add ScaledPixels ops (#27451)

Michael Sloan created

It doesn't make sense to have `Pixels: Mul<Pixels, Output = Pixels>` as
the output should be `Pixels^2` (area), so these impls are removed. All
code where these impls were used are improved by instead multiplying by
`f32` or `usize`.

Also adds math op impls that are present for `Pixels` but absent for
`ScaledPixels`. Adds missing `Mul<Pixels> for usize` to both.

Release Notes:

- N/A

Change summary

crates/editor/src/editor.rs                  |   2 
crates/editor/src/element.rs                 |  14 +-
crates/editor/src/hover_popover.rs           |   2 
crates/file_finder/src/file_finder.rs        |   2 
crates/gpui/src/geometry.rs                  | 108 +++++++++++++++++----
crates/outline_panel/src/outline_panel.rs    |   9 -
crates/project_panel/src/project_panel.rs    |  22 +--
crates/terminal_view/src/terminal_element.rs |   3 
crates/ui/src/components/indent_guides.rs    |   6 
9 files changed, 116 insertions(+), 52 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -6905,7 +6905,7 @@ impl Editor {
             )
             .child(
                 h_flex()
-                    .h(line_height + BORDER_WIDTH * px(2.))
+                    .h(line_height + BORDER_WIDTH * 2.)
                     .px_1p5()
                     .gap_1()
                     // Workaround: For some reason, there's a gap if we don't do this

crates/editor/src/element.rs 🔗

@@ -3303,7 +3303,7 @@ impl EditorElement {
             let offset = scroll_position - max_row as f32;
 
             if offset > 0.0 {
-                origin.y -= Pixels(offset) * line_height;
+                origin.y -= offset * line_height;
             }
             break;
         }
@@ -4524,7 +4524,7 @@ impl EditorElement {
                                     hunk_hitbox.origin.x - hunk_hitbox.size.width,
                                     hunk_hitbox.origin.y,
                                 ),
-                                size(hunk_hitbox.size.width * px(2.), hunk_hitbox.size.height),
+                                size(hunk_hitbox.size.width * 2., hunk_hitbox.size.height),
                             ),
                             cx.theme().colors().version_control_deleted,
                             Corners::all(1. * line_height),
@@ -7859,10 +7859,10 @@ impl ScrollbarRangeData {
         let text_bounds_size = size(longest_line_width, max_row.0 as f32 * letter_size.height);
 
         let settings = EditorSettings::get_global(cx);
-        let scroll_beyond_last_line: Pixels = match settings.scroll_beyond_last_line {
-            ScrollBeyondLastLine::OnePage => px(scrollbar_bounds.size.height / letter_size.height),
-            ScrollBeyondLastLine::Off => px(1.),
-            ScrollBeyondLastLine::VerticalScrollMargin => px(1.0 + settings.vertical_scroll_margin),
+        let scroll_beyond_last_line: f32 = match settings.scroll_beyond_last_line {
+            ScrollBeyondLastLine::OnePage => scrollbar_bounds.size.height / letter_size.height,
+            ScrollBeyondLastLine::Off => 1.0,
+            ScrollBeyondLastLine::VerticalScrollMargin => 1.0 + settings.vertical_scroll_margin,
         };
 
         let right_margin = if longest_line_width + longest_line_blame_width >= editor_width {
@@ -8534,7 +8534,7 @@ fn compute_auto_height_layout(
         snapshot = editor.snapshot(window, cx);
     }
 
-    let scroll_height = Pixels::from(snapshot.max_point().row().next_row().0) * line_height;
+    let scroll_height = (snapshot.max_point().row().next_row().0 as f32) * line_height;
     let height = scroll_height
         .max(line_height)
         .min(line_height * max_lines as f32);

crates/editor/src/hover_popover.rs 🔗

@@ -29,7 +29,7 @@ use workspace::{OpenOptions, OpenVisible, Workspace};
 pub const HOVER_REQUEST_DELAY_MILLIS: u64 = 200;
 
 pub const MIN_POPOVER_CHARACTER_WIDTH: f32 = 20.;
-pub const MIN_POPOVER_LINE_HEIGHT: Pixels = px(4.);
+pub const MIN_POPOVER_LINE_HEIGHT: f32 = 4.;
 pub const HOVER_POPOVER_GAP: Pixels = px(10.);
 
 /// Bindable action which uses the most recent selection head to trigger a hover

crates/file_finder/src/file_finder.rs 🔗

@@ -1067,7 +1067,7 @@ fn full_path_budget(
     small_em: Pixels,
     max_width: Pixels,
 ) -> usize {
-    ((px(max_width / px(0.8)) - px(file_name.len() as f32) * normal_em) / small_em) as usize
+    (((max_width / 0.8) - file_name.len() * normal_em) / small_em) as usize
 }
 
 impl PickerDelegate for FileFinderDelegate {

crates/gpui/src/geometry.rs 🔗

@@ -2468,7 +2468,7 @@ impl std::fmt::Display for Pixels {
     }
 }
 
-impl std::ops::Div for Pixels {
+impl Div for Pixels {
     type Output = f32;
 
     fn div(self, rhs: Self) -> Self::Output {
@@ -2497,32 +2497,40 @@ impl std::ops::Rem for Pixels {
 }
 
 impl Mul<f32> for Pixels {
+    type Output = Self;
+
+    fn mul(self, rhs: f32) -> Self {
+        Self(self.0 * rhs)
+    }
+}
+
+impl Mul<Pixels> for f32 {
     type Output = Pixels;
 
-    fn mul(self, other: f32) -> Pixels {
-        Pixels(self.0 * other)
+    fn mul(self, rhs: Pixels) -> Self::Output {
+        rhs * self
     }
 }
 
 impl Mul<usize> for Pixels {
-    type Output = Pixels;
+    type Output = Self;
 
-    fn mul(self, other: usize) -> Pixels {
-        Pixels(self.0 * other as f32)
+    fn mul(self, rhs: usize) -> Self {
+        self * (rhs as f32)
     }
 }
 
-impl Mul<Pixels> for f32 {
+impl Mul<Pixels> for usize {
     type Output = Pixels;
 
-    fn mul(self, rhs: Pixels) -> Self::Output {
-        Pixels(self * rhs.0)
+    fn mul(self, rhs: Pixels) -> Pixels {
+        rhs * self
     }
 }
 
 impl MulAssign<f32> for Pixels {
-    fn mul_assign(&mut self, other: f32) {
-        self.0 *= other;
+    fn mul_assign(&mut self, rhs: f32) {
+        self.0 *= rhs;
     }
 }
 
@@ -2617,14 +2625,6 @@ impl Pixels {
     }
 }
 
-impl Mul<Pixels> for Pixels {
-    type Output = Pixels;
-
-    fn mul(self, rhs: Pixels) -> Self::Output {
-        Pixels(self.0 * rhs.0)
-    }
-}
-
 impl Eq for Pixels {}
 
 impl PartialOrd for Pixels {
@@ -2821,7 +2821,9 @@ impl From<usize> for DevicePixels {
 /// a single logical pixel may correspond to multiple physical pixels. By using `ScaledPixels`,
 /// dimensions and positions can be specified in a way that scales appropriately across different
 /// display resolutions.
-#[derive(Clone, Copy, Default, Add, AddAssign, Sub, SubAssign, Div, PartialEq, PartialOrd)]
+#[derive(
+    Clone, Copy, Default, Add, AddAssign, Sub, SubAssign, Div, DivAssign, PartialEq, PartialOrd,
+)]
 #[repr(transparent)]
 pub struct ScaledPixels(pub(crate) f32);
 
@@ -2877,6 +2879,72 @@ impl From<ScaledPixels> for u32 {
     }
 }
 
+impl Div for ScaledPixels {
+    type Output = f32;
+
+    fn div(self, rhs: Self) -> Self::Output {
+        self.0 / rhs.0
+    }
+}
+
+impl std::ops::DivAssign for ScaledPixels {
+    fn div_assign(&mut self, rhs: Self) {
+        *self = Self(self.0 / rhs.0);
+    }
+}
+
+impl std::ops::RemAssign for ScaledPixels {
+    fn rem_assign(&mut self, rhs: Self) {
+        self.0 %= rhs.0;
+    }
+}
+
+impl std::ops::Rem for ScaledPixels {
+    type Output = Self;
+
+    fn rem(self, rhs: Self) -> Self {
+        Self(self.0 % rhs.0)
+    }
+}
+
+impl Mul<f32> for ScaledPixels {
+    type Output = Self;
+
+    fn mul(self, rhs: f32) -> Self {
+        Self(self.0 * rhs)
+    }
+}
+
+impl Mul<ScaledPixels> for f32 {
+    type Output = ScaledPixels;
+
+    fn mul(self, rhs: ScaledPixels) -> Self::Output {
+        rhs * self
+    }
+}
+
+impl Mul<usize> for ScaledPixels {
+    type Output = Self;
+
+    fn mul(self, rhs: usize) -> Self {
+        self * (rhs as f32)
+    }
+}
+
+impl Mul<ScaledPixels> for usize {
+    type Output = ScaledPixels;
+
+    fn mul(self, rhs: ScaledPixels) -> ScaledPixels {
+        rhs * self
+    }
+}
+
+impl MulAssign<f32> for ScaledPixels {
+    fn mul_assign(&mut self, rhs: f32) {
+        self.0 *= rhs;
+    }
+}
+
 /// Represents a length in rems, a unit based on the font-size of the window, which can be assigned with [`Window::set_rem_size`][set_rem_size].
 ///
 /// Rems are used for defining lengths that are scalable and consistent across different UI elements.

crates/outline_panel/src/outline_panel.rs 🔗

@@ -4586,7 +4586,7 @@ impl OutlinePanel {
                         .with_render_fn(
                             cx.entity().clone(),
                             move |outline_panel, params, _, _| {
-                                const LEFT_OFFSET: f32 = 14.;
+                                const LEFT_OFFSET: Pixels = px(14.);
 
                                 let indent_size = params.indent_size;
                                 let item_height = params.item_height;
@@ -4602,11 +4602,10 @@ impl OutlinePanel {
                                     .map(|(ix, layout)| {
                                         let bounds = Bounds::new(
                                             point(
-                                                px(layout.offset.x as f32) * indent_size
-                                                    + px(LEFT_OFFSET),
-                                                px(layout.offset.y as f32) * item_height,
+                                                layout.offset.x * indent_size + LEFT_OFFSET,
+                                                layout.offset.y * item_height,
                                             ),
-                                            size(px(1.), px(layout.length as f32) * item_height),
+                                            size(px(1.), layout.length * item_height),
                                         );
                                         ui::RenderedIndentGuide {
                                             bounds,

crates/project_panel/src/project_panel.rs 🔗

@@ -4571,9 +4571,9 @@ impl Render for ProjectPanel {
                             .with_render_fn(
                                 cx.entity().clone(),
                                 move |this, params, _, cx| {
-                                    const LEFT_OFFSET: f32 = 14.;
-                                    const PADDING_Y: f32 = 4.;
-                                    const HITBOX_OVERDRAW: f32 = 3.;
+                                    const LEFT_OFFSET: Pixels = px(14.);
+                                    const PADDING_Y: Pixels = px(4.);
+                                    const HITBOX_OVERDRAW: Pixels = px(3.);
 
                                     let active_indent_guide_index =
                                         this.find_active_indent_guide(&params.indent_guides, cx);
@@ -4589,19 +4589,16 @@ impl Render for ProjectPanel {
                                             let offset = if layout.continues_offscreen {
                                                 px(0.)
                                             } else {
-                                                px(PADDING_Y)
+                                                PADDING_Y
                                             };
                                             let bounds = Bounds::new(
                                                 point(
-                                                    px(layout.offset.x as f32) * indent_size
-                                                        + px(LEFT_OFFSET),
-                                                    px(layout.offset.y as f32) * item_height
-                                                        + offset,
+                                                    layout.offset.x * indent_size + LEFT_OFFSET,
+                                                    layout.offset.y * item_height + offset,
                                                 ),
                                                 size(
                                                     px(1.),
-                                                    px(layout.length as f32) * item_height
-                                                        - px(offset.0 * 2.),
+                                                    layout.length * item_height - offset * 2.,
                                                 ),
                                             );
                                             ui::RenderedIndentGuide {
@@ -4610,12 +4607,11 @@ impl Render for ProjectPanel {
                                                 is_active: Some(idx) == active_indent_guide_index,
                                                 hitbox: Some(Bounds::new(
                                                     point(
-                                                        bounds.origin.x - px(HITBOX_OVERDRAW),
+                                                        bounds.origin.x - HITBOX_OVERDRAW,
                                                         bounds.origin.y,
                                                     ),
                                                     size(
-                                                        bounds.size.width
-                                                            + px(2. * HITBOX_OVERDRAW),
+                                                        bounds.size.width + HITBOX_OVERDRAW * 2.,
                                                         bounds.size.height,
                                                     ),
                                                 )),

crates/terminal_view/src/terminal_element.rs 🔗

@@ -662,7 +662,8 @@ impl Element for TerminalElement {
                 let dimensions = {
                     let rem_size = window.rem_size();
                     let font_pixels = text_style.font_size.to_pixels(rem_size);
-                    let line_height = font_pixels * line_height.to_pixels(rem_size);
+                    // TODO: line_height should be an f32 not an AbsoluteLength.
+                    let line_height = font_pixels * line_height.to_pixels(rem_size).0;
                     let font_id = cx.text_system().resolve_font(&text_style.font());
 
                     let cell_width = text_system

crates/ui/src/components/indent_guides.rs 🔗

@@ -172,10 +172,10 @@ mod uniform_list {
                     .map(|layout| RenderedIndentGuide {
                         bounds: Bounds::new(
                             point(
-                                px(layout.offset.x as f32) * self.indent_size,
-                                px(layout.offset.y as f32) * item_height,
+                                layout.offset.x * self.indent_size,
+                                layout.offset.y * item_height,
                             ),
-                            size(px(1.), px(layout.length as f32) * item_height),
+                            size(px(1.), layout.length * item_height),
                         ),
                         layout,
                         is_active: false,