From 251de4fa0c0da470c0298fd3a6ed5bfd14bfb6ee Mon Sep 17 00:00:00 2001 From: John Tur Date: Sat, 14 Mar 2026 15:23:58 -0400 Subject: [PATCH] Something? --- 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(-) diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 66281fc80c8374a241481a95b4cfd86e8ccbf4c9..686cbb3c454b81fa51eced842898cf14a978377f 100644 --- a/crates/editor/src/element.rs +++ b/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 { - 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)), ) } diff --git a/crates/gpui/src/taffy.rs b/crates/gpui/src/taffy.rs index 88019746566a51e4c85227c9bc0b5b631a7f3a26..10f97ac30c0fcfa78210c3687aad3ebeba8c6f80 100644 --- a/crates/gpui/src/taffy.rs +++ b/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 for Style { impl ToTaffy 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) } } diff --git a/crates/gpui/src/util.rs b/crates/gpui/src/util.rs index bd8c03167e80ef98fceaa35d06d2e2f202424d34..5119683d3cb1b84af5407aa0afe787394cad91b8 100644 --- a/crates/gpui/src/util.rs +++ b/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] diff --git a/crates/gpui/src/window.rs b/crates/gpui/src/window.rs index 2a36da065c19862a5cbfc82e33ff7c40a49644ea..e69696cc9d795f767c8132c2a212c24c49d8a09e 100644 --- a/crates/gpui/src/window.rs +++ b/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) -> Bounds { 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),