From 52f494d33070e08765b465d43db7556d5d0f13b1 Mon Sep 17 00:00:00 2001 From: "Joseph T. Lyons" Date: Wed, 19 Nov 2025 10:51:37 -0500 Subject: [PATCH] Revert "editor: Fix prepaint recursion when updating stale sizes (#42896)" This reverts commit 79be5cbfe21eabeadb2d910d4ba7c355256bd0e3. --- crates/editor/src/element.rs | 135 ++++++----------------------------- 1 file changed, 23 insertions(+), 112 deletions(-) diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 8801f20323338e28bb7ed62923be65db785af312..785fd9de00888a7f658785e689df34bd2cffdf8d 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -74,7 +74,6 @@ use smallvec::{SmallVec, smallvec}; use std::{ any::TypeId, borrow::Cow, - cell::Cell, cmp::{self, Ordering}, fmt::{self, Write}, iter, mem, @@ -186,13 +185,6 @@ impl SelectionLayout { } } -#[derive(Default)] -struct RenderBlocksOutput { - blocks: Vec, - row_block_types: HashMap, - resized_blocks: Option>, -} - pub struct EditorElement { editor: Entity, style: EditorStyle, @@ -3675,7 +3667,6 @@ 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)> { @@ -3829,10 +3820,7 @@ impl EditorElement { }; let mut element_height_in_lines = ((final_size.height / line_height).ceil() as u32).max(1); - 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 row = block_row_start; let mut x_offset = px(0.); let mut is_block = true; @@ -3862,7 +3850,6 @@ 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); } } @@ -4267,7 +4254,7 @@ impl EditorElement { sticky_header_excerpt_id: Option, window: &mut Window, cx: &mut App, - ) -> RenderBlocksOutput { + ) -> Result<(Vec, HashMap), HashMap> { let (fixed_blocks, non_fixed_blocks) = snapshot .blocks_in_range(rows.clone()) .partition::, _>(|(_, block)| block.style() == BlockStyle::Fixed); @@ -4279,7 +4266,6 @@ 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(); @@ -4310,7 +4296,6 @@ impl EditorElement { latest_selection_anchors, is_row_soft_wrapped, sticky_header_excerpt_id, - &mut block_resize_offset, window, cx, ) { @@ -4369,7 +4354,6 @@ impl EditorElement { latest_selection_anchors, is_row_soft_wrapped, sticky_header_excerpt_id, - &mut block_resize_offset, window, cx, ) { @@ -4426,7 +4410,6 @@ impl EditorElement { latest_selection_anchors, is_row_soft_wrapped, sticky_header_excerpt_id, - &mut block_resize_offset, window, cx, ) { @@ -4446,12 +4429,9 @@ impl EditorElement { if resized_blocks.is_empty() { *scroll_width = (*scroll_width).max(fixed_block_max_width - editor_margins.gutter.width); - } - - RenderBlocksOutput { - blocks, - row_block_types, - resized_blocks: (!resized_blocks.is_empty()).then_some(resized_blocks), + Ok((blocks, row_block_types)) + } else { + Err(resized_blocks) } } @@ -8792,48 +8772,8 @@ 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 = EditorRequestLayoutState; + type RequestLayoutState = (); type PrepaintState = EditorLayout; fn id(&self) -> Option { @@ -8850,7 +8790,7 @@ impl Element for EditorElement { _inspector_id: Option<&gpui::InspectorElementId>, window: &mut Window, cx: &mut App, - ) -> (gpui::LayoutId, Self::RequestLayoutState) { + ) -> (gpui::LayoutId, ()) { let rem_size = self.rem_size(cx); window.with_rem_size(rem_size, |window| { self.editor.update(cx, |editor, cx| { @@ -8917,7 +8857,7 @@ impl Element for EditorElement { } }; - (layout_id, EditorRequestLayoutState::default()) + (layout_id, ()) }) }) } @@ -8927,11 +8867,10 @@ impl Element for EditorElement { _: Option<&GlobalElementId>, _inspector_id: Option<&gpui::InspectorElementId>, bounds: Bounds, - request_layout: &mut Self::RequestLayoutState, + _: &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), @@ -9455,20 +9394,7 @@ impl Element for EditorElement { // If the fold widths have changed, we need to prepaint // the element again to account for any changes in // wrapping. - 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." - ); - } + return self.prepaint(None, _inspector_id, bounds, &mut (), window, cx); } let longest_line_blame_width = self @@ -9555,35 +9481,20 @@ impl Element for EditorElement { ) }) }) - .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." - ); + .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); } - } + }; let sticky_buffer_header = sticky_header_excerpt.map(|sticky_header_excerpt| { window.with_element_namespace("blocks", |window| {