Pull logic for clamping corner rounding radii out of `Corners::to_pixels` (#27460)

Michael Sloan created

This seems more correct as corners are not necessarily only for rounding
radii.

Also applies clamping after scaling in `paint_quad`, deduplicating that
logic. This also provides a more precise result by doing the clamping
after scaling, avoiding floating point rounding issues (probably a
non-issue).

Release Notes:

- N/A

Change summary

crates/gpui/src/elements/img.rs |  2 
crates/gpui/src/geometry.rs     | 64 +++++++++++++++++++++++-----------
crates/gpui/src/style.rs        |  6 +-
crates/gpui/src/window.rs       | 10 ++++-
4 files changed, 55 insertions(+), 27 deletions(-)

Detailed changes

crates/gpui/src/elements/img.rs 🔗

@@ -421,7 +421,7 @@ impl Element for Img {
             window,
             cx,
             |style, window, cx| {
-                let corner_radii = style.corner_radii.to_pixels(bounds.size, window.rem_size());
+                let corner_radii = style.corner_radii.to_pixels(window.rem_size());
 
                 if let Some(Ok(data)) = source.use_data(window, cx) {
                     let new_bounds = self

crates/gpui/src/geometry.rs 🔗

@@ -2165,21 +2165,15 @@ where
 }
 
 impl Corners<AbsoluteLength> {
-    /// Converts the `AbsoluteLength` to `Pixels` based on the provided size and rem size, ensuring the resulting
-    /// `Pixels` do not exceed half of the minimum of the provided size's width and height.
-    ///
-    /// This method is particularly useful when dealing with corner radii, where the radius in pixels should not
-    /// exceed half the size of the box it applies to, to avoid the corners overlapping.
+    /// Converts the `AbsoluteLength` to `Pixels` based on the provided rem size.
     ///
     /// # Arguments
     ///
-    /// * `size` - The `Size<Pixels>` against which the minimum allowable radius is determined.
     /// * `rem_size` - The size of one REM unit in pixels, used for conversion if the `AbsoluteLength` is in REMs.
     ///
     /// # Returns
     ///
-    /// Returns a `Corners<Pixels>` instance with each corner's length converted to pixels and clamped to the
-    /// minimum allowable radius based on the provided size.
+    /// Returns a `Corners<Pixels>` instance with each corner's length converted to pixels.
     ///
     /// # Examples
     ///
@@ -2191,23 +2185,20 @@ impl Corners<AbsoluteLength> {
     ///     bottom_right: AbsoluteLength::Pixels(Pixels(30.0)),
     ///     bottom_left: AbsoluteLength::Rems(Rems(2.0)),
     /// };
-    /// let size = Size { width: Pixels(100.0), height: Pixels(50.0) };
     /// let rem_size = Pixels(16.0);
     /// let corners_in_pixels = corners.to_pixels(size, rem_size);
     ///
-    /// // The resulting corners should not exceed half the size of the smallest dimension (50.0 / 2.0 = 25.0).
     /// assert_eq!(corners_in_pixels.top_left, Pixels(15.0));
     /// assert_eq!(corners_in_pixels.top_right, Pixels(16.0)); // 1 rem converted to pixels
-    /// assert_eq!(corners_in_pixels.bottom_right, Pixels(30.0).min(Pixels(25.0))); // Clamped to 25.0
-    /// assert_eq!(corners_in_pixels.bottom_left, Pixels(32.0).min(Pixels(25.0))); // 2 rems converted to pixels and clamped to 25.0
+    /// assert_eq!(corners_in_pixels.bottom_right, Pixels(30.0));
+    /// assert_eq!(corners_in_pixels.bottom_left, Pixels(32.0)); // 2 rems converted to pixels
     /// ```
-    pub fn to_pixels(&self, size: Size<Pixels>, rem_size: Pixels) -> Corners<Pixels> {
-        let max = size.width.min(size.height) / 2.;
+    pub fn to_pixels(&self, rem_size: Pixels) -> Corners<Pixels> {
         Corners {
-            top_left: self.top_left.to_pixels(rem_size).min(max),
-            top_right: self.top_right.to_pixels(rem_size).min(max),
-            bottom_right: self.bottom_right.to_pixels(rem_size).min(max),
-            bottom_left: self.bottom_left.to_pixels(rem_size).min(max),
+            top_left: self.top_left.to_pixels(rem_size),
+            top_right: self.top_right.to_pixels(rem_size),
+            bottom_right: self.bottom_right.to_pixels(rem_size),
+            bottom_left: self.bottom_left.to_pixels(rem_size),
         }
     }
 }
@@ -2263,6 +2254,27 @@ impl Corners<Pixels> {
     }
 }
 
+impl<T: Div<f32, Output = T> + Ord + Clone + Default + Debug> Corners<T> {
+    /// Clamps corner radii to be less than or equal to half the shortest side of a quad.
+    ///
+    /// # Arguments
+    ///
+    /// * `size` - The size of the quad which limits the size of the corner radii.
+    ///
+    /// # Returns
+    ///
+    /// Corner radii values clamped to fit.
+    pub fn clamp_radii_for_quad_size(self, size: Size<T>) -> Corners<T> {
+        let max = cmp::min(size.width, size.height) / 2.;
+        Corners {
+            top_left: cmp::min(self.top_left, max.clone()),
+            top_right: cmp::min(self.top_right, max.clone()),
+            bottom_right: cmp::min(self.bottom_right, max.clone()),
+            bottom_left: cmp::min(self.bottom_left, max),
+        }
+    }
+}
+
 impl<T: Clone + Default + Debug> Corners<T> {
     /// Applies a function to each field of the `Corners`, producing a new `Corners<U>`.
     ///
@@ -2821,9 +2833,7 @@ 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, DivAssign, PartialEq, PartialOrd,
-)]
+#[derive(Clone, Copy, Default, Add, AddAssign, Sub, SubAssign, Div, DivAssign, PartialEq)]
 #[repr(transparent)]
 pub struct ScaledPixels(pub(crate) f32);
 
@@ -2849,6 +2859,18 @@ impl ScaledPixels {
 
 impl Eq for ScaledPixels {}
 
+impl PartialOrd for ScaledPixels {
+    fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Ord for ScaledPixels {
+    fn cmp(&self, other: &Self) -> cmp::Ordering {
+        self.0.total_cmp(&other.0)
+    }
+}
+
 impl Debug for ScaledPixels {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "{} px (scaled)", self.0)

crates/gpui/src/style.rs 🔗

@@ -612,7 +612,7 @@ impl Style {
 
         window.paint_shadows(
             bounds,
-            self.corner_radii.to_pixels(bounds.size, rem_size),
+            self.corner_radii.to_pixels(rem_size),
             &self.box_shadow,
         );
 
@@ -633,7 +633,7 @@ impl Style {
             border_color.a = 0.;
             window.paint_quad(quad(
                 bounds,
-                self.corner_radii.to_pixels(bounds.size, rem_size),
+                self.corner_radii.to_pixels(rem_size),
                 background_color.unwrap_or_default(),
                 Edges::default(),
                 border_color,
@@ -644,7 +644,7 @@ impl Style {
         continuation(window, cx);
 
         if self.is_border_visible() {
-            let corner_radii = self.corner_radii.to_pixels(bounds.size, rem_size);
+            let corner_radii = self.corner_radii.to_pixels(rem_size);
             let border_widths = self.border_widths.to_pixels(rem_size);
             let max_border_width = border_widths.max();
             let max_corner_radius = corner_radii.max();

crates/gpui/src/window.rs 🔗

@@ -2333,13 +2333,19 @@ impl Window {
         let scale_factor = self.scale_factor();
         let content_mask = self.content_mask();
         let opacity = self.element_opacity();
+
+        let bounds = quad.bounds.scale(scale_factor);
+        let corner_radii = quad
+            .corner_radii
+            .scale(scale_factor)
+            .clamp_radii_for_quad_size(bounds.size);
         self.next_frame.scene.insert_primitive(Quad {
             order: 0,
-            bounds: quad.bounds.scale(scale_factor),
+            bounds,
             content_mask: content_mask.scale(scale_factor),
             background: quad.background.opacity(opacity),
             border_color: quad.border_color.opacity(opacity),
-            corner_radii: quad.corner_radii.scale(scale_factor),
+            corner_radii,
             border_widths: quad.border_widths.scale(scale_factor),
             border_style: quad.border_style,
         });