From 79be5cbfe21eabeadb2d910d4ba7c355256bd0e3 Mon Sep 17 00:00:00 2001 From: Smit Barmase Date: Wed, 19 Nov 2025 21:02:31 +0530 Subject: [PATCH] editor: Fix prepaint recursion when updating stale sizes (#42896) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bug is in the `place_near` logic, specifically the `!row_block_types.contains_key(&(row - 1))` check. The problem isn’t really that condition itself, but it’s that it relies on `row_block_types`, which does not take into account that upon block resizes, subsequent block start row moves up/down. Since `place_near` depends on this incorrect map, it ends up causing incorrect resize syncs to the block map, which then triggers more bad recursive calls. The reason it worked till now in most of the cases is that recursive resizes eventually lead to stabilizing it. Before `place_near`, we never touched `row_block_types` during the first prepaint pass because we knew it was based on outdated heights. Once all heights are finalized, using it is fine. The fix is to make sure `row_block_types` is accurate from the very first prepaint pass by keeping an offset whenever a block shrinks or expands. Now ideally it should take only one subsequent prepaint. But due to shrinking, new custom/diagnostics blocks might come into the view from below, which needs further prepaint calls for resolving. Right now, tests pass after 2 subsequent prepaint calls. Just to be safe, we have set it to 5. image Release Notes: - Fix issue where sometimes Zed used to experience freeze while working with inline diagnostics. --- crates/editor/src/element.rs | 135 +++++++++++++++++++++++++++++------ 1 file changed, 112 insertions(+), 23 deletions(-) diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 785fd9de00888a7f658785e689df34bd2cffdf8d..8801f20323338e28bb7ed62923be65db785af312 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -74,6 +74,7 @@ use smallvec::{SmallVec, smallvec}; use std::{ any::TypeId, borrow::Cow, + cell::Cell, cmp::{self, Ordering}, fmt::{self, Write}, iter, mem, @@ -185,6 +186,13 @@ impl SelectionLayout { } } +#[derive(Default)] +struct RenderBlocksOutput { + blocks: Vec, + row_block_types: HashMap, + resized_blocks: Option>, +} + pub struct EditorElement { editor: Entity, style: EditorStyle, @@ -3667,6 +3675,7 @@ impl EditorElement { latest_selection_anchors: &HashMap, is_row_soft_wrapped: impl Copy + Fn(usize) -> bool, sticky_header_excerpt_id: Option, + block_resize_offset: &mut i32, window: &mut Window, cx: &mut App, ) -> Option<(AnyElement, Size, DisplayRow, Pixels)> { @@ -3820,7 +3829,10 @@ impl EditorElement { }; let mut element_height_in_lines = ((final_size.height / line_height).ceil() as u32).max(1); - let mut row = block_row_start; + let effective_row_start = block_row_start.0 as i32 + *block_resize_offset; + debug_assert!(effective_row_start >= 0); + let mut row = DisplayRow(effective_row_start.max(0) as u32); + let mut x_offset = px(0.); let mut is_block = true; @@ -3850,6 +3862,7 @@ impl EditorElement { } }; if element_height_in_lines != block.height() { + *block_resize_offset += element_height_in_lines as i32 - block.height() as i32; resized_blocks.insert(custom_block_id, element_height_in_lines); } } @@ -4254,7 +4267,7 @@ impl EditorElement { sticky_header_excerpt_id: Option, window: &mut Window, cx: &mut App, - ) -> Result<(Vec, HashMap), HashMap> { + ) -> RenderBlocksOutput { let (fixed_blocks, non_fixed_blocks) = snapshot .blocks_in_range(rows.clone()) .partition::, _>(|(_, block)| block.style() == BlockStyle::Fixed); @@ -4266,6 +4279,7 @@ impl EditorElement { let mut blocks = Vec::new(); let mut resized_blocks = HashMap::default(); let mut row_block_types = HashMap::default(); + let mut block_resize_offset: i32 = 0; for (row, block) in fixed_blocks { let block_id = block.id(); @@ -4296,6 +4310,7 @@ impl EditorElement { latest_selection_anchors, is_row_soft_wrapped, sticky_header_excerpt_id, + &mut block_resize_offset, window, cx, ) { @@ -4354,6 +4369,7 @@ impl EditorElement { latest_selection_anchors, is_row_soft_wrapped, sticky_header_excerpt_id, + &mut block_resize_offset, window, cx, ) { @@ -4410,6 +4426,7 @@ impl EditorElement { latest_selection_anchors, is_row_soft_wrapped, sticky_header_excerpt_id, + &mut block_resize_offset, window, cx, ) { @@ -4429,9 +4446,12 @@ impl EditorElement { if resized_blocks.is_empty() { *scroll_width = (*scroll_width).max(fixed_block_max_width - editor_margins.gutter.width); - Ok((blocks, row_block_types)) - } else { - Err(resized_blocks) + } + + RenderBlocksOutput { + blocks, + row_block_types, + resized_blocks: (!resized_blocks.is_empty()).then_some(resized_blocks), } } @@ -8772,8 +8792,48 @@ impl EditorElement { } } +#[derive(Default)] +pub struct EditorRequestLayoutState { + // We use prepaint depth to limit the number of times prepaint is + // called recursively. We need this so that we can update stale + // data for e.g. block heights in block map. + prepaint_depth: Rc>, +} + +impl EditorRequestLayoutState { + // In ideal conditions we only need one more subsequent prepaint call for resize to take effect. + // i.e. MAX_PREPAINT_DEPTH = 2, but since moving blocks inline (place_near), more lines from + // below get exposed, and we end up querying blocks for those lines too in subsequent renders. + // Setting MAX_PREPAINT_DEPTH = 3, passes all tests. Just to be on the safe side we set it to 5, so + // that subsequent shrinking does not lead to incorrect block placing. + const MAX_PREPAINT_DEPTH: usize = 5; + + fn increment_prepaint_depth(&self) -> EditorPrepaintGuard { + let depth = self.prepaint_depth.get(); + self.prepaint_depth.set(depth + 1); + EditorPrepaintGuard { + prepaint_depth: self.prepaint_depth.clone(), + } + } + + fn can_prepaint(&self) -> bool { + self.prepaint_depth.get() < Self::MAX_PREPAINT_DEPTH + } +} + +struct EditorPrepaintGuard { + prepaint_depth: Rc>, +} + +impl Drop for EditorPrepaintGuard { + fn drop(&mut self) { + let depth = self.prepaint_depth.get(); + self.prepaint_depth.set(depth.saturating_sub(1)); + } +} + impl Element for EditorElement { - type RequestLayoutState = (); + type RequestLayoutState = EditorRequestLayoutState; type PrepaintState = EditorLayout; fn id(&self) -> Option { @@ -8790,7 +8850,7 @@ impl Element for EditorElement { _inspector_id: Option<&gpui::InspectorElementId>, window: &mut Window, cx: &mut App, - ) -> (gpui::LayoutId, ()) { + ) -> (gpui::LayoutId, Self::RequestLayoutState) { let rem_size = self.rem_size(cx); window.with_rem_size(rem_size, |window| { self.editor.update(cx, |editor, cx| { @@ -8857,7 +8917,7 @@ impl Element for EditorElement { } }; - (layout_id, ()) + (layout_id, EditorRequestLayoutState::default()) }) }) } @@ -8867,10 +8927,11 @@ impl Element for EditorElement { _: Option<&GlobalElementId>, _inspector_id: Option<&gpui::InspectorElementId>, bounds: Bounds, - _: &mut Self::RequestLayoutState, + request_layout: &mut Self::RequestLayoutState, window: &mut Window, cx: &mut App, ) -> Self::PrepaintState { + let _prepaint_depth_guard = request_layout.increment_prepaint_depth(); let text_style = TextStyleRefinement { font_size: Some(self.style.text.font_size), line_height: Some(self.style.text.line_height), @@ -9394,7 +9455,20 @@ impl Element for EditorElement { // If the fold widths have changed, we need to prepaint // the element again to account for any changes in // wrapping. - return self.prepaint(None, _inspector_id, bounds, &mut (), window, cx); + if request_layout.can_prepaint() { + return self.prepaint( + None, + _inspector_id, + bounds, + request_layout, + window, + cx, + ); + } else { + debug_panic!( + "skipping recursive prepaint at max depth. renderer widths may be stale." + ); + } } let longest_line_blame_width = self @@ -9481,20 +9555,35 @@ impl Element for EditorElement { ) }) }) - .unwrap_or_else(|| Ok((Vec::default(), HashMap::default()))); - let (mut blocks, row_block_types) = match blocks { - Ok(blocks) => blocks, - Err(resized_blocks) => { - self.editor.update(cx, |editor, cx| { - editor.resize_blocks( - resized_blocks, - autoscroll_request.map(|(autoscroll, _)| autoscroll), - cx, - ) - }); - return self.prepaint(None, _inspector_id, bounds, &mut (), window, cx); + .unwrap_or_default(); + let RenderBlocksOutput { + mut blocks, + row_block_types, + resized_blocks, + } = blocks; + if let Some(resized_blocks) = resized_blocks { + self.editor.update(cx, |editor, cx| { + editor.resize_blocks( + resized_blocks, + autoscroll_request.map(|(autoscroll, _)| autoscroll), + cx, + ) + }); + if request_layout.can_prepaint() { + return self.prepaint( + None, + _inspector_id, + bounds, + request_layout, + window, + cx, + ); + } else { + debug_panic!( + "skipping recursive prepaint at max depth. block layout may be stale." + ); } - }; + } let sticky_buffer_header = sticky_header_excerpt.map(|sticky_header_excerpt| { window.with_element_namespace("blocks", |window| {