Something?

John Tur created

Change summary

crates/editor/src/element.rs | 13 +++++--
crates/gpui/src/taffy.rs     | 15 +++++++-
crates/gpui/src/util.rs      | 60 ++++++++++++++++++++++++++++++-------
crates/gpui/src/window.rs    | 47 ++++++++++++++++++++---------
4 files changed, 101 insertions(+), 34 deletions(-)

Detailed changes

crates/editor/src/element.rs 🔗

@@ -10473,6 +10473,13 @@ impl Element for EditorElement {
                         }
                     });
 
+                    // Snap scroll pixel position to device-pixel boundaries so
+                    // that all paint coordinates derived from it land on clean
+                    // device pixels. The rounded value is converted back to
+                    // scroll-offset units for consistent use during this frame.
+                    // This rounded scroll_position must NOT be written back to
+                    // the editor's scroll state — doing so would accumulate
+                    // floating-point drift across frames.
                     let scroll_pixel_position = rounded_scroll_pixel_position(
                         window,
                         scroll_position,
@@ -12375,11 +12382,9 @@ fn rounded_scroll_pixel_position(
     em_layout_width: Pixels,
     line_height: Pixels,
 ) -> gpui::Point<ScrollPixelOffset> {
-    let dpi_scale = f64::from(window.scale_factor());
-    let snap_to_device_pixels = |value: f64| (value * dpi_scale).round() / dpi_scale;
     point(
-        snap_to_device_pixels(scroll_position.x * f64::from(em_layout_width)),
-        snap_to_device_pixels(scroll_position.y * f64::from(line_height)),
+        window.round_f64_to_nearest_device_pixel(scroll_position.x * f64::from(em_layout_width)),
+        window.round_f64_to_nearest_device_pixel(scroll_position.y * f64::from(line_height)),
     )
 }
 

crates/gpui/src/taffy.rs 🔗

@@ -1,6 +1,6 @@
 use crate::{
     AbsoluteLength, App, Bounds, DefiniteLength, Edges, Length, Pixels, Point, Size, Style, Window,
-    point, size, util::round_device_pixels_midpoint_down,
+    point, size, util::round_half_toward_zero,
 };
 use collections::{FxHashMap, FxHashSet};
 use stacksafe::{StackSafe, stacksafe};
@@ -239,6 +239,12 @@ impl TaffyLayoutEngine {
         }
 
         let layout = self.taffy.layout(id.into()).expect(EXPECT_MESSAGE);
+
+        // Taffy already rounds positions and sizes via a cumulative edge-based
+        // post-pass (see `round_layout` in taffy). That pass uses absolute
+        // positions to guarantee gap-free abutment between siblings. We must
+        // NOT re-round here — doing so would destroy the cumulative rounding
+        // invariant and could introduce 1-device-pixel gaps or size mismatches.
         let mut bounds = Bounds {
             origin: point(
                 Pixels(layout.location.x / scale_factor),
@@ -369,8 +375,11 @@ impl ToTaffy<taffy::style::Style> for Style {
 
 impl ToTaffy<f32> for AbsoluteLength {
     fn to_taffy(&self, rem_size: Pixels, scale_factor: f32) -> f32 {
-        let logical_pixels = self.to_pixels(rem_size).0;
-        round_device_pixels_midpoint_down((logical_pixels * scale_factor).max(0.0))
+        // Pre-round to integer device pixels so that Taffy's flex algorithm
+        // works with snapped values. Taffy's cumulative edge-based post-pass
+        // then ensures gap-free abutment between siblings.
+        // NOTE: no `.max(0.0)` here — negative values are valid (e.g. margins).
+        round_half_toward_zero(self.to_pixels(rem_size).0 * scale_factor)
     }
 }
 

crates/gpui/src/util.rs 🔗

@@ -125,14 +125,28 @@ pub(crate) fn atomic_incr_if_not_zero(counter: &AtomicUsize) -> usize {
     }
 }
 
-/// Rounds a device-pixel value to the nearest integer, with .5 ties rounded down.
+/// Rounds to the nearest integer with ±0.5 ties toward zero.
+///
+/// This is the single rounding policy for all device-pixel snapping in the
+/// rendering pipeline. A consistent midpoint rule prevents 1-device-pixel
+/// gaps or overlaps between adjacent elements.
 #[inline]
-pub(crate) fn round_device_pixels_midpoint_down(value: f32) -> f32 {
-    let floor = value.floor();
-    if value - floor > 0.5 {
-        floor + 1.0
+pub(crate) fn round_half_toward_zero(value: f32) -> f32 {
+    if value >= 0.0 {
+        (value - 0.5).ceil()
     } else {
-        floor
+        (value + 0.5).floor()
+    }
+}
+
+/// f64 variant of [`round_half_toward_zero`] for scroll-offset arithmetic
+/// that must preserve f64 precision.
+#[inline]
+pub(crate) fn round_half_toward_zero_f64(value: f64) -> f64 {
+    if value >= 0.0 {
+        (value - 0.5).ceil()
+    } else {
+        (value + 0.5).floor()
     }
 }
 
@@ -143,12 +157,34 @@ mod tests {
     use super::*;
 
     #[test]
-    fn test_round_device_pixels_midpoint_down() {
-        assert_eq!(round_device_pixels_midpoint_down(0.5), 0.0);
-        assert_eq!(round_device_pixels_midpoint_down(1.5), 1.0);
-        assert_eq!(round_device_pixels_midpoint_down(2.5), 2.0);
-        assert_eq!(round_device_pixels_midpoint_down(1.5001), 2.0);
-        assert_eq!(round_device_pixels_midpoint_down(-1.5), -2.0);
+    fn test_round_half_toward_zero() {
+        // Midpoint ties go toward zero
+        assert_eq!(round_half_toward_zero(0.5), 0.0);
+        assert_eq!(round_half_toward_zero(1.5), 1.0);
+        assert_eq!(round_half_toward_zero(2.5), 2.0);
+        assert_eq!(round_half_toward_zero(-0.5), 0.0);
+        assert_eq!(round_half_toward_zero(-1.5), -1.0);
+        assert_eq!(round_half_toward_zero(-2.5), -2.0);
+
+        // Non-midpoint values round to nearest
+        assert_eq!(round_half_toward_zero(1.5001), 2.0);
+        assert_eq!(round_half_toward_zero(1.4999), 1.0);
+        assert_eq!(round_half_toward_zero(-1.5001), -2.0);
+        assert_eq!(round_half_toward_zero(-1.4999), -1.0);
+
+        // Integers are unchanged
+        assert_eq!(round_half_toward_zero(0.0), 0.0);
+        assert_eq!(round_half_toward_zero(3.0), 3.0);
+        assert_eq!(round_half_toward_zero(-3.0), -3.0);
+    }
+
+    #[test]
+    fn test_round_half_toward_zero_f64() {
+        assert_eq!(round_half_toward_zero_f64(0.5), 0.0);
+        assert_eq!(round_half_toward_zero_f64(-0.5), 0.0);
+        assert_eq!(round_half_toward_zero_f64(1.5), 1.0);
+        assert_eq!(round_half_toward_zero_f64(-1.5), -1.0);
+        assert_eq!(round_half_toward_zero_f64(2.5001), 3.0);
     }
 
     #[gpui::test]

crates/gpui/src/window.rs 🔗

@@ -57,7 +57,7 @@ use uuid::Uuid;
 
 mod prompts;
 
-use crate::util::{atomic_incr_if_not_zero, round_device_pixels_midpoint_down};
+use crate::util::{atomic_incr_if_not_zero, round_half_toward_zero, round_half_toward_zero_f64};
 pub use prompts::*;
 
 /// Default window size used when no explicit size is provided.
@@ -2112,16 +2112,19 @@ impl Window {
     }
 
     /// Rounds a logical size or coordinate to the nearest device pixel for this window.
+    /// Uses round-half-toward-zero to match the layout engine's rounding policy.
     #[inline]
     pub fn round_to_nearest_device_pixel(&self, value: Pixels) -> Pixels {
         let scale_factor = self.scale_factor();
-        px((value.0 * scale_factor).round() / scale_factor)
+        px(round_half_toward_zero(value.0 * scale_factor) / scale_factor)
     }
 
-    /// Returns the logical length corresponding to one physical device pixel.
+    /// f64 variant of [`Self::round_to_nearest_device_pixel`] for scroll-offset
+    /// arithmetic that must preserve f64 precision.
     #[inline]
-    pub fn one_device_pixel(&self) -> Pixels {
-        px(1.0 / self.scale_factor())
+    pub fn round_f64_to_nearest_device_pixel(&self, value: f64) -> f64 {
+        let scale_factor = f64::from(self.scale_factor());
+        round_half_toward_zero_f64(value * scale_factor) / scale_factor
     }
 
     #[inline]
@@ -2148,13 +2151,21 @@ impl Window {
         )
     }
 
+    /// Converts logical-pixel bounds to device-pixel bounds by rounding origin
+    /// and size independently. This preserves consistent sizes: elements with
+    /// the same logical size always produce the same device-pixel size.
+    ///
+    /// Gap-free abutment between layout siblings is handled upstream by
+    /// Taffy's cumulative edge-based rounding post-pass. For elements
+    /// positioned by Taffy, the values arriving here are already on the
+    /// device-pixel grid and this rounding is effectively a no-op.
     #[inline]
     fn round_bounds_to_device_pixels(&self, bounds: Bounds<Pixels>) -> Bounds<ScaledPixels> {
         let scale_factor = self.scale_factor();
         Bounds {
             origin: point(
-                ScaledPixels((bounds.origin.x.0 * scale_factor).round()),
-                ScaledPixels((bounds.origin.y.0 * scale_factor).round()),
+                ScaledPixels(round_half_toward_zero(bounds.origin.x.0 * scale_factor)),
+                ScaledPixels(round_half_toward_zero(bounds.origin.y.0 * scale_factor)),
             ),
             size: size(
                 self.round_length_to_device_pixels(bounds.size.width),
@@ -2196,7 +2207,7 @@ impl Window {
     #[inline]
     fn round_length_to_device_pixels(&self, value: Pixels) -> ScaledPixels {
         let scaled = (value.0 * self.scale_factor()).max(0.0);
-        ScaledPixels(round_device_pixels_midpoint_down(scaled))
+        ScaledPixels(round_half_toward_zero(scaled))
     }
 
     #[inline]
@@ -3268,8 +3279,8 @@ impl Window {
         };
         let bounds = Bounds {
             origin: point(
-                ScaledPixels((origin.x.0 * scale_factor).round()),
-                ScaledPixels((origin.y.0 * scale_factor).round()),
+                ScaledPixels(round_half_toward_zero(origin.x.0 * scale_factor)),
+                ScaledPixels(round_half_toward_zero(origin.y.0 * scale_factor)),
             ),
             size: size(
                 self.round_nonzero_length_to_device_pixels(width),
@@ -3305,8 +3316,8 @@ impl Window {
         let height = style.thickness;
         let bounds = Bounds {
             origin: point(
-                ScaledPixels((origin.x.0 * scale_factor).round()),
-                ScaledPixels((origin.y.0 * scale_factor).round()),
+                ScaledPixels(round_half_toward_zero(origin.x.0 * scale_factor)),
+                ScaledPixels(round_half_toward_zero(origin.y.0 * scale_factor)),
             ),
             size: size(
                 self.round_nonzero_length_to_device_pixels(width),
@@ -3377,7 +3388,11 @@ impl Window {
                 })?
                 .expect("Callback above only errors or returns Some");
             let bounds = Bounds {
-                origin: point(glyph_origin.x.floor(), glyph_origin.y.round())
+                // X uses floor because subpixel_variant.x captures the fractional
+                // part — the glyph raster already contains the sub-pixel shift.
+                // Y uses round because there is no Y subpixel variant (y: 0),
+                // so rounding gives the most accurate integer placement.
+                origin: point(glyph_origin.x.floor(), ScaledPixels(round_half_toward_zero(glyph_origin.y.0)))
                     + raster_bounds.origin.map(Into::into),
                 size: tile.bounds.size.map(Into::into),
             };
@@ -3468,7 +3483,9 @@ impl Window {
                 .expect("Callback above only errors or returns Some");
 
             let bounds = Bounds {
-                origin: point(glyph_origin.x.floor(), glyph_origin.y.round())
+                // See comment in paint_glyph: floor for X (subpixel variant),
+                // round for Y (no subpixel variant).
+                origin: point(glyph_origin.x.floor(), ScaledPixels(round_half_toward_zero(glyph_origin.y.0)))
                     + raster_bounds.origin.map(Into::into),
                 size: tile.bounds.size.map(Into::into),
             };
@@ -3542,7 +3559,7 @@ impl Window {
             order: 0,
             pad: 0,
             bounds: svg_bounds
-                .map_origin(|origin| origin.round())
+                .map_origin(|v| ScaledPixels(round_half_toward_zero(v.0)))
                 .map_size(|size| size.ceil()),
             content_mask,
             color: color.opacity(element_opacity),