From 10bbcdfec5b33168c013e3b8298d8801f2239ea5 Mon Sep 17 00:00:00 2001 From: Cameron Mcloughlin Date: Thu, 26 Feb 2026 10:47:10 +0000 Subject: [PATCH] Side by side diff spacer polish (#50182) Co-authored-by: Cole Miller --- crates/editor/src/display_map/block_map.rs | 13 +- crates/editor/src/element.rs | 338 +++++++++++++++------ crates/git_ui/src/conflict_view.rs | 2 +- crates/language/src/language_settings.rs | 16 + 4 files changed, 277 insertions(+), 92 deletions(-) diff --git a/crates/editor/src/display_map/block_map.rs b/crates/editor/src/display_map/block_map.rs index 0073166e3d5ee8989d7c5d16112b86395fc7cebf..8f30264a7f8ef6af86eedfb772235bfbe2d17465 100644 --- a/crates/editor/src/display_map/block_map.rs +++ b/crates/editor/src/display_map/block_map.rs @@ -265,6 +265,10 @@ impl Debug for BlockProperties

{ pub enum BlockStyle { Fixed, Flex, + /// Like `Flex` but doesn't use the gutter: + /// - block content scrolls with buffer content + /// - doesn't paint in gutter + Spacer, Sticky, } @@ -272,6 +276,7 @@ pub enum BlockStyle { pub struct EditorMargins { pub gutter: GutterDimensions, pub right: Pixels, + pub extended_right: Pixels, } #[derive(gpui::AppContext, gpui::VisualContext)] @@ -289,6 +294,7 @@ pub struct BlockContext<'a, 'b> { pub height: u32, pub selected: bool, pub editor_style: &'b EditorStyle, + pub indent_guide_padding: Pixels, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Ord, PartialOrd, Hash)] @@ -393,8 +399,8 @@ impl Block { Block::Custom(block) => block.style, Block::ExcerptBoundary { .. } | Block::FoldedBuffer { .. } - | Block::BufferHeader { .. } - | Block::Spacer { .. } => BlockStyle::Sticky, + | Block::BufferHeader { .. } => BlockStyle::Sticky, + Block::Spacer { .. } => BlockStyle::Spacer, } } @@ -1702,12 +1708,13 @@ pub(crate) fn balancing_block( Some(BlockProperties { placement: their_placement, height: my_block.height, - style: BlockStyle::Sticky, + style: BlockStyle::Spacer, render: Arc::new(move |cx| { crate::EditorElement::render_spacer_block( cx.block_id, cx.height, cx.line_height, + cx.indent_guide_padding, cx.window, cx.app, ) diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 8131cdcb9e94754b03b6fc672d05652107a7e6b9..59086fbedf397e05fbec50481d04771f878eff7c 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -47,8 +47,8 @@ use gpui::{ MouseDownEvent, MouseMoveEvent, MousePressureEvent, MouseUpEvent, PaintQuad, ParentElement, Pixels, PressureStage, ScrollDelta, ScrollHandle, ScrollWheelEvent, ShapedLine, SharedString, Size, StatefulInteractiveElement, Style, Styled, StyledText, TextAlign, TextRun, - TextStyleRefinement, WeakEntity, Window, anchored, checkerboard, deferred, div, fill, - linear_color_stop, linear_gradient, outline, point, px, quad, relative, size, solid_background, + TextStyleRefinement, WeakEntity, Window, anchored, deferred, div, fill, linear_color_stop, + linear_gradient, outline, pattern_slash, point, px, quad, relative, size, solid_background, transparent_black, }; use itertools::Itertools; @@ -186,7 +186,10 @@ impl SelectionLayout { #[derive(Default)] struct RenderBlocksOutput { - blocks: Vec, + // We store spacer blocks separately because they paint in a different order + // (spacers -> indent guides -> non-spacers) + non_spacer_blocks: Vec, + spacer_blocks: Vec, row_block_types: HashMap, resized_blocks: Option>, } @@ -2970,11 +2973,12 @@ impl EditorElement { - scroll_pixel_position.x, ); if start_x >= text_origin.x { - let (offset_y, length) = Self::calculate_indent_guide_bounds( - indent_guide.start_row..indent_guide.end_row, - line_height, - snapshot, - ); + let (offset_y, length, display_row_range) = + Self::calculate_indent_guide_bounds( + indent_guide.start_row..indent_guide.end_row, + line_height, + snapshot, + ); let start_y = Pixels::from( ScrollOffset::from(content_origin.y) + offset_y @@ -2985,6 +2989,7 @@ impl EditorElement { origin: point(start_x, start_y), length, single_indent_width, + display_row_range, depth: indent_guide.depth, active: active_indent_guide_indices.contains(&i), settings: indent_guide.settings, @@ -2997,6 +3002,22 @@ impl EditorElement { ) } + fn depth_zero_indent_guide_padding_for_row( + indent_guides: &[IndentGuideLayout], + row: DisplayRow, + ) -> Pixels { + indent_guides + .iter() + .find(|guide| guide.depth == 0 && guide.display_row_range.contains(&row)) + .and_then(|guide| { + guide + .settings + .visible_line_width(guide.active) + .map(|width| px(width as f32 * 2.0)) + }) + .unwrap_or(px(0.0)) + } + fn layout_wrap_guides( &self, em_advance: Pixels, @@ -3034,11 +3055,11 @@ impl EditorElement { row_range: Range, line_height: Pixels, snapshot: &DisplaySnapshot, - ) -> (f64, gpui::Pixels) { + ) -> (f64, gpui::Pixels, Range) { let start_point = Point::new(row_range.start.0, 0); let end_point = Point::new(row_range.end.0, 0); - let row_range = start_point.to_display_point(snapshot).row() + let mut row_range = start_point.to_display_point(snapshot).row() ..end_point.to_display_point(snapshot).row(); let mut prev_line = start_point; @@ -3076,6 +3097,7 @@ impl EditorElement { if !found_excerpt_header { offset_y -= block_offset as f64 * f64::from(line_height); length += block_height as f32 * line_height; + row_range = DisplayRow(row_range.start.0.saturating_sub(block_offset))..row_range.end; } // If there is a block (e.g. diagnostic) at the end of an multibuffer excerpt, @@ -3093,9 +3115,11 @@ impl EditorElement { } if found_excerpt_header { length -= block_height as f32 * line_height; + } else { + row_range = row_range.start..cons_line; } - (offset_y, length) + (offset_y, length, row_range) } fn layout_breakpoints( @@ -3857,6 +3881,7 @@ impl EditorElement { latest_selection_anchors: &HashMap, is_row_soft_wrapped: impl Copy + Fn(usize) -> bool, sticky_header_excerpt_id: Option, + indent_guides: &Option>, block_resize_offset: &mut i32, window: &mut Window, cx: &mut App, @@ -3908,19 +3933,30 @@ impl EditorElement { div() .size_full() - .child(custom.render(&mut BlockContext { - window, - app: cx, - anchor_x, - margins: editor_margins, - line_height, - em_width, - block_id, - height: custom.height.unwrap_or(1), - selected, - max_width: text_hitbox.size.width.max(*scroll_width), - editor_style: &self.style, - })) + .child( + custom.render(&mut BlockContext { + window, + app: cx, + anchor_x, + margins: editor_margins, + line_height, + em_width, + block_id, + height: custom.height.unwrap_or(1), + selected, + max_width: text_hitbox.size.width.max(*scroll_width), + editor_style: &self.style, + indent_guide_padding: indent_guides + .as_ref() + .map(|guides| { + Self::depth_zero_indent_guide_padding_for_row( + guides, + block_row_start, + ) + }) + .unwrap_or(px(0.0)), + }), + ) .into_any() } @@ -4008,7 +4044,20 @@ impl EditorElement { } Block::Spacer { height, .. } => { - Self::render_spacer_block(block_id, *height, line_height, window, cx) + let indent_guide_padding = indent_guides + .as_ref() + .map(|guides| { + Self::depth_zero_indent_guide_padding_for_row(guides, block_row_start) + }) + .unwrap_or(px(0.0)); + Self::render_spacer_block( + block_id, + *height, + line_height, + indent_guide_padding, + window, + cx, + ) } }; @@ -4070,10 +4119,13 @@ impl EditorElement { Some((element, final_size, row, x_offset)) } - /// The checkerboard pattern height must be an even factor of the line - /// height, so that two consecutive spacer blocks can render contiguously - /// without an obvious break in the pattern. - fn checkerboard_size(line_height: f32, target_height: f32) -> f32 { + /// The spacer pattern period must be an even factor of the line height, so + /// that two consecutive spacer blocks can render contiguously without an + /// obvious break in the pattern. + /// + /// Two consecutive spacers can appear when the other side has a diff hunk + /// and a custom block next to each other (e.g. merge conflict buttons). + fn spacer_pattern_period(line_height: f32, target_height: f32) -> f32 { let k_approx = line_height / (2.0 * target_height); let k_floor = (k_approx.floor() as u32).max(1); let k_ceil = (k_approx.ceil() as u32).max(1); @@ -4092,24 +4144,39 @@ impl EditorElement { block_id: BlockId, block_height: u32, line_height: Pixels, + indent_guide_padding: Pixels, window: &mut Window, cx: &App, ) -> AnyElement { + let target_size = 16.0; + let scale = window.scale_factor(); + let pattern_size = + Self::spacer_pattern_period(f32::from(line_height) * scale, target_size * scale); + let color = cx.theme().colors().panel_background; + let background = pattern_slash(color, 2.0, pattern_size - 2.0); + div() .id(block_id) .w_full() .h((block_height as f32) * line_height) - // the checkerboard pattern is semi-transparent, so we render a - // solid background to prevent indent guides peeking through - .bg(cx.theme().colors().editor_background) + .flex() + .flex_row() + .child(div().flex_shrink_0().w(indent_guide_padding).h_full()) .child( div() - .size_full() - .bg(checkerboard(cx.theme().colors().panel_background, { - let target_size = 16.0; - let scale = window.scale_factor(); - Self::checkerboard_size(f32::from(line_height) * scale, target_size * scale) - })), + .flex_1() + .h_full() + .relative() + .overflow_x_hidden() + .child( + div() + .absolute() + .top_0() + .bottom_0() + .right_0() + .left(-indent_guide_padding) + .bg(background), + ), ) .into_any() } @@ -4154,6 +4221,7 @@ impl EditorElement { latest_selection_anchors: &HashMap, is_row_soft_wrapped: impl Copy + Fn(usize) -> bool, sticky_header_excerpt_id: Option, + indent_guides: &Option>, window: &mut Window, cx: &mut App, ) -> RenderBlocksOutput { @@ -4166,6 +4234,7 @@ impl EditorElement { .update(cx, |editor, _| editor.take_focused_block()); let mut fixed_block_max_width = Pixels::ZERO; let mut blocks = Vec::new(); + let mut spacer_blocks = Vec::new(); let mut resized_blocks = HashMap::default(); let mut row_block_types = HashMap::default(); let mut block_resize_offset: i32 = 0; @@ -4199,6 +4268,7 @@ impl EditorElement { latest_selection_anchors, is_row_soft_wrapped, sticky_header_excerpt_id, + indent_guides, &mut block_resize_offset, window, cx, @@ -4226,7 +4296,15 @@ impl EditorElement { .size .width .max(fixed_block_max_width) - .max(editor_margins.gutter.width + *scroll_width) + .max( + editor_margins.gutter.width + *scroll_width + editor_margins.extended_right, + ) + .into(), + (BlockStyle::Spacer, _) => hitbox + .size + .width + .max(fixed_block_max_width) + .max(*scroll_width + editor_margins.extended_right) .into(), (BlockStyle::Fixed, _) => unreachable!(), }; @@ -4258,20 +4336,26 @@ impl EditorElement { latest_selection_anchors, is_row_soft_wrapped, sticky_header_excerpt_id, + indent_guides, &mut block_resize_offset, window, cx, ) { - blocks.push(BlockLayout { + let layout = BlockLayout { id: block_id, x_offset, row: Some(row), element, available_space: size(width, element_size.height.into()), style, - overlaps_gutter: !block.place_near(), + overlaps_gutter: !block.place_near() && style != BlockStyle::Spacer, is_buffer_header: block.is_buffer_header(), - }); + }; + if style == BlockStyle::Spacer { + spacer_blocks.push(layout); + } else { + blocks.push(layout); + } } } @@ -4283,12 +4367,17 @@ impl EditorElement { let style = block.style(); let width = match style { BlockStyle::Fixed => AvailableSpace::MinContent, - BlockStyle::Flex => AvailableSpace::Definite( + BlockStyle::Flex => { + AvailableSpace::Definite(hitbox.size.width.max(fixed_block_max_width).max( + editor_margins.gutter.width + *scroll_width + editor_margins.extended_right, + )) + } + BlockStyle::Spacer => AvailableSpace::Definite( hitbox .size .width .max(fixed_block_max_width) - .max(editor_margins.gutter.width + *scroll_width), + .max(*scroll_width + editor_margins.extended_right), ), BlockStyle::Sticky => AvailableSpace::Definite(hitbox.size.width), }; @@ -4315,6 +4404,7 @@ impl EditorElement { latest_selection_anchors, is_row_soft_wrapped, sticky_header_excerpt_id, + indent_guides, &mut block_resize_offset, window, cx, @@ -4338,7 +4428,8 @@ impl EditorElement { } RenderBlocksOutput { - blocks, + non_spacer_blocks: blocks, + spacer_blocks, row_block_types, resized_blocks: (!resized_blocks.is_empty()).then_some(resized_blocks), } @@ -4348,9 +4439,11 @@ impl EditorElement { &self, blocks: &mut Vec, hitbox: &Hitbox, + gutter_hitbox: &Hitbox, line_height: Pixels, scroll_position: gpui::Point, scroll_pixel_position: gpui::Point, + editor_margins: &EditorMargins, window: &mut Window, cx: &mut App, ) { @@ -4369,6 +4462,13 @@ impl EditorElement { hitbox.origin + point(Pixels::ZERO, hitbox.size.height) }; + if block.style == BlockStyle::Spacer { + origin += point( + gutter_hitbox.size.width + editor_margins.gutter.margin, + Pixels::ZERO, + ); + } + if !matches!(block.style, BlockStyle::Sticky) { origin += point(Pixels::from(-scroll_pixel_position.x), Pixels::ZERO); } @@ -6023,22 +6123,18 @@ impl EditorElement { )), }; - let requested_line_width = if indent_guide.active { - settings.active_line_width - } else { - settings.line_width - } - .clamp(1, 10); let mut line_indicator_width = 0.; - if let Some(color) = line_color { - window.paint_quad(fill( - Bounds { - origin: indent_guide.origin, - size: size(px(requested_line_width as f32), indent_guide.length), - }, - color, - )); - line_indicator_width = requested_line_width as f32; + if let Some(requested_line_width) = settings.visible_line_width(indent_guide.active) { + if let Some(color) = line_color { + window.paint_quad(fill( + Bounds { + origin: indent_guide.origin, + size: size(px(requested_line_width as f32), indent_guide.length), + }, + color, + )); + line_indicator_width = requested_line_width as f32; + } } if let Some(color) = background_color { @@ -7468,7 +7564,27 @@ impl EditorElement { } } - fn paint_blocks(&mut self, layout: &mut EditorLayout, window: &mut Window, cx: &mut App) { + fn paint_spacer_blocks( + &mut self, + layout: &mut EditorLayout, + window: &mut Window, + cx: &mut App, + ) { + for mut block in layout.spacer_blocks.drain(..) { + let mut bounds = layout.hitbox.bounds; + bounds.origin.x += layout.gutter_hitbox.bounds.size.width; + window.with_content_mask(Some(ContentMask { bounds }), |window| { + block.element.paint(window, cx); + }) + } + } + + fn paint_non_spacer_blocks( + &mut self, + layout: &mut EditorLayout, + window: &mut Window, + cx: &mut App, + ) { for mut block in layout.blocks.drain(..) { if block.overlaps_gutter { block.element.paint(window, cx); @@ -9591,11 +9707,12 @@ impl Element for EditorElement { let right_margin = minimap_width + vertical_scrollbar_width; - let editor_width = - text_width - gutter_dimensions.margin - 2 * em_width - right_margin; + let extended_right = 2 * em_width + right_margin; + let editor_width = text_width - gutter_dimensions.margin - extended_right; let editor_margins = EditorMargins { gutter: gutter_dimensions, right: right_margin, + extended_right, }; snapshot = self.editor.update(cx, |editor, cx| { @@ -10216,6 +10333,26 @@ impl Element for EditorElement { let sticky_header_excerpt_id = sticky_header_excerpt.as_ref().map(|top| top.excerpt.id); + let buffer = snapshot.buffer_snapshot(); + let start_buffer_row = MultiBufferRow(start_anchor.to_point(&buffer).row); + let end_buffer_row = MultiBufferRow(end_anchor.to_point(&buffer).row); + + let preliminary_scroll_pixel_position = point( + scroll_position.x * f64::from(em_layout_width), + scroll_position.y * f64::from(line_height), + ); + let indent_guides = self.layout_indent_guides( + content_origin, + text_hitbox.origin, + start_buffer_row..end_buffer_row, + preliminary_scroll_pixel_position, + line_height, + &snapshot, + window, + cx, + ); + let indent_guides_for_spacers = indent_guides.clone(); + let blocks = (!is_minimap) .then(|| { window.with_element_namespace("blocks", |window| { @@ -10236,6 +10373,7 @@ impl Element for EditorElement { &latest_selection_anchors, is_row_soft_wrapped, sticky_header_excerpt_id, + &indent_guides_for_spacers, window, cx, ) @@ -10243,7 +10381,8 @@ impl Element for EditorElement { }) .unwrap_or_default(); let RenderBlocksOutput { - mut blocks, + non_spacer_blocks: mut blocks, + mut spacer_blocks, row_block_types, resized_blocks, } = blocks; @@ -10294,11 +10433,6 @@ impl Element for EditorElement { None }; - let start_buffer_row = - MultiBufferRow(start_anchor.to_point(&snapshot.buffer_snapshot()).row); - let end_buffer_row = - MultiBufferRow(end_anchor.to_point(&snapshot.buffer_snapshot()).row); - let scroll_max: gpui::Point = point( ScrollPixelOffset::from( ((scroll_width - editor_width) / em_layout_width).max(0.0), @@ -10359,16 +10493,21 @@ impl Element for EditorElement { sticky_headers.as_ref().map_or(0, |h| h.lines.len()), ); }); - let indent_guides = self.layout_indent_guides( - content_origin, - text_hitbox.origin, - start_buffer_row..end_buffer_row, - scroll_pixel_position, - line_height, - &snapshot, - window, - cx, - ); + let indent_guides = + if scroll_pixel_position != preliminary_scroll_pixel_position { + self.layout_indent_guides( + content_origin, + text_hitbox.origin, + start_buffer_row..end_buffer_row, + scroll_pixel_position, + line_height, + &snapshot, + window, + cx, + ) + } else { + indent_guides + }; let crease_trailers = window.with_element_namespace("crease_trailers", |window| { @@ -10507,9 +10646,22 @@ impl Element for EditorElement { self.layout_blocks( &mut blocks, &hitbox, + &gutter_hitbox, + line_height, + scroll_position, + scroll_pixel_position, + &editor_margins, + window, + cx, + ); + self.layout_blocks( + &mut spacer_blocks, + &hitbox, + &gutter_hitbox, line_height, scroll_position, scroll_pixel_position, + &editor_margins, window, cx, ); @@ -10904,6 +11056,7 @@ impl Element for EditorElement { inline_blame_layout, inline_code_actions, blocks, + spacer_blocks, cursors, visible_cursors, selections, @@ -10965,6 +11118,13 @@ impl Element for EditorElement { window.with_content_mask(Some(ContentMask { bounds }), |window| { self.paint_mouse_listeners(layout, window, cx); self.paint_background(layout, window, cx); + + if !layout.spacer_blocks.is_empty() { + window.with_element_namespace("blocks", |window| { + self.paint_spacer_blocks(layout, window, cx); + }); + } + self.paint_indent_guides(layout, window, cx); if layout.gutter_hitbox.size.width > Pixels::ZERO { @@ -10981,7 +11141,7 @@ impl Element for EditorElement { if !layout.blocks.is_empty() { window.with_element_namespace("blocks", |window| { - self.paint_blocks(layout, window, cx); + self.paint_non_spacer_blocks(layout, window, cx); }); } @@ -11083,6 +11243,7 @@ pub struct EditorLayout { inline_blame_layout: Option, inline_code_actions: Option, blocks: Vec, + spacer_blocks: Vec, highlighted_ranges: Vec<(Range, Hsla)>, highlighted_gutter_ranges: Vec<(Range, Hsla)>, redacted_ranges: Vec>, @@ -11847,11 +12008,12 @@ pub fn layout_line( .unwrap() } -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct IndentGuideLayout { origin: gpui::Point, length: Pixels, single_indent_width: Pixels, + display_row_range: Range, depth: u32, active: bool, settings: IndentGuideSettings, @@ -13357,26 +13519,26 @@ mod tests { } #[test] - fn test_checkerboard_size() { + fn test_spacer_pattern_period() { // line height is smaller than target height, so we just return half the line height - assert_eq!(EditorElement::checkerboard_size(10.0, 20.0), 5.0); + assert_eq!(EditorElement::spacer_pattern_period(10.0, 20.0), 5.0); // line height is exactly half the target height, perfect match - assert_eq!(EditorElement::checkerboard_size(20.0, 10.0), 10.0); + assert_eq!(EditorElement::spacer_pattern_period(20.0, 10.0), 10.0); // line height is close to half the target height - assert_eq!(EditorElement::checkerboard_size(20.0, 9.0), 10.0); + assert_eq!(EditorElement::spacer_pattern_period(20.0, 9.0), 10.0); // line height is close to 1/4 the target height - assert_eq!(EditorElement::checkerboard_size(20.0, 4.8), 5.0); + assert_eq!(EditorElement::spacer_pattern_period(20.0, 4.8), 5.0); } #[gpui::test(iterations = 100)] - fn test_random_checkerboard_size(mut rng: StdRng) { + fn test_random_spacer_pattern_period(mut rng: StdRng) { let line_height = rng.next_u32() as f32; let target_height = rng.next_u32() as f32; - let result = EditorElement::checkerboard_size(line_height, target_height); + let result = EditorElement::spacer_pattern_period(line_height, target_height); let k = line_height / result; assert!(k - k.round() < 0.0000001); // approximately integer diff --git a/crates/git_ui/src/conflict_view.rs b/crates/git_ui/src/conflict_view.rs index 838ec886fdb400b67fa284df9182bab9766548bd..82571b541e692141f843a4c3ef6e082c72e55e48 100644 --- a/crates/git_ui/src/conflict_view.rs +++ b/crates/git_ui/src/conflict_view.rs @@ -290,7 +290,7 @@ fn conflicts_updated( blocks.push(BlockProperties { placement: BlockPlacement::Above(anchor), height: Some(1), - style: BlockStyle::Fixed, + style: BlockStyle::Sticky, render: Arc::new({ let conflict = conflict.clone(); move |cx| render_conflict_buttons(&conflict, excerpt_id, editor_handle.clone(), cx) diff --git a/crates/language/src/language_settings.rs b/crates/language/src/language_settings.rs index 1741d6163c13b5622192fca43b3204911486f25a..40e3da789d4785cc5fd56589b09735ba8592ebc7 100644 --- a/crates/language/src/language_settings.rs +++ b/crates/language/src/language_settings.rs @@ -229,6 +229,22 @@ pub struct IndentGuideSettings { pub background_coloring: settings::IndentGuideBackgroundColoring, } +impl IndentGuideSettings { + /// Returns the clamped line width in pixels for an indent guide based on + /// whether it is active, or `None` when line coloring is disabled. + pub fn visible_line_width(&self, active: bool) -> Option { + if self.coloring == settings::IndentGuideColoring::Disabled { + return None; + } + let width = if active { + self.active_line_width + } else { + self.line_width + }; + Some(width.clamp(1, 10)) + } +} + #[derive(Debug, Clone, PartialEq)] pub struct LanguageTaskSettings { /// Extra task variables to set for a particular language.