Skip to content

Commit cb91df8

Browse files
smitbarmasemikayla-maki
authored andcommitted
editor: Fix prepaint recursion when updating stale sizes (#42896)
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. <img width="500" alt="image" src="https://github.com/user-attachments/assets/da3d32ff-5972-46d9-8597-b438e162552b" /> Release Notes: - Fix issue where sometimes Zed used to experience freeze while working with inline diagnostics.
1 parent edfad43 commit cb91df8

File tree

1 file changed

+112
-23
lines changed

1 file changed

+112
-23
lines changed

crates/editor/src/element.rs

Lines changed: 112 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ use smallvec::{SmallVec, smallvec};
7474
use std::{
7575
any::TypeId,
7676
borrow::Cow,
77+
cell::Cell,
7778
cmp::{self, Ordering},
7879
fmt::{self, Write},
7980
iter, mem,
@@ -185,6 +186,13 @@ impl SelectionLayout {
185186
}
186187
}
187188

189+
#[derive(Default)]
190+
struct RenderBlocksOutput {
191+
blocks: Vec<BlockLayout>,
192+
row_block_types: HashMap<DisplayRow, bool>,
193+
resized_blocks: Option<HashMap<CustomBlockId, u32>>,
194+
}
195+
188196
pub struct EditorElement {
189197
editor: Entity<Editor>,
190198
style: EditorStyle,
@@ -3667,6 +3675,7 @@ impl EditorElement {
36673675
latest_selection_anchors: &HashMap<BufferId, Anchor>,
36683676
is_row_soft_wrapped: impl Copy + Fn(usize) -> bool,
36693677
sticky_header_excerpt_id: Option<ExcerptId>,
3678+
block_resize_offset: &mut i32,
36703679
window: &mut Window,
36713680
cx: &mut App,
36723681
) -> Option<(AnyElement, Size<Pixels>, DisplayRow, Pixels)> {
@@ -3820,7 +3829,10 @@ impl EditorElement {
38203829
};
38213830
let mut element_height_in_lines = ((final_size.height / line_height).ceil() as u32).max(1);
38223831

3823-
let mut row = block_row_start;
3832+
let effective_row_start = block_row_start.0 as i32 + *block_resize_offset;
3833+
debug_assert!(effective_row_start >= 0);
3834+
let mut row = DisplayRow(effective_row_start.max(0) as u32);
3835+
38243836
let mut x_offset = px(0.);
38253837
let mut is_block = true;
38263838

@@ -3850,6 +3862,7 @@ impl EditorElement {
38503862
}
38513863
};
38523864
if element_height_in_lines != block.height() {
3865+
*block_resize_offset += element_height_in_lines as i32 - block.height() as i32;
38533866
resized_blocks.insert(custom_block_id, element_height_in_lines);
38543867
}
38553868
}
@@ -4254,7 +4267,7 @@ impl EditorElement {
42544267
sticky_header_excerpt_id: Option<ExcerptId>,
42554268
window: &mut Window,
42564269
cx: &mut App,
4257-
) -> Result<(Vec<BlockLayout>, HashMap<DisplayRow, bool>), HashMap<CustomBlockId, u32>> {
4270+
) -> RenderBlocksOutput {
42584271
let (fixed_blocks, non_fixed_blocks) = snapshot
42594272
.blocks_in_range(rows.clone())
42604273
.partition::<Vec<_>, _>(|(_, block)| block.style() == BlockStyle::Fixed);
@@ -4266,6 +4279,7 @@ impl EditorElement {
42664279
let mut blocks = Vec::new();
42674280
let mut resized_blocks = HashMap::default();
42684281
let mut row_block_types = HashMap::default();
4282+
let mut block_resize_offset: i32 = 0;
42694283

42704284
for (row, block) in fixed_blocks {
42714285
let block_id = block.id();
@@ -4296,6 +4310,7 @@ impl EditorElement {
42964310
latest_selection_anchors,
42974311
is_row_soft_wrapped,
42984312
sticky_header_excerpt_id,
4313+
&mut block_resize_offset,
42994314
window,
43004315
cx,
43014316
) {
@@ -4354,6 +4369,7 @@ impl EditorElement {
43544369
latest_selection_anchors,
43554370
is_row_soft_wrapped,
43564371
sticky_header_excerpt_id,
4372+
&mut block_resize_offset,
43574373
window,
43584374
cx,
43594375
) {
@@ -4410,6 +4426,7 @@ impl EditorElement {
44104426
latest_selection_anchors,
44114427
is_row_soft_wrapped,
44124428
sticky_header_excerpt_id,
4429+
&mut block_resize_offset,
44134430
window,
44144431
cx,
44154432
) {
@@ -4429,9 +4446,12 @@ impl EditorElement {
44294446
if resized_blocks.is_empty() {
44304447
*scroll_width =
44314448
(*scroll_width).max(fixed_block_max_width - editor_margins.gutter.width);
4432-
Ok((blocks, row_block_types))
4433-
} else {
4434-
Err(resized_blocks)
4449+
}
4450+
4451+
RenderBlocksOutput {
4452+
blocks,
4453+
row_block_types,
4454+
resized_blocks: (!resized_blocks.is_empty()).then_some(resized_blocks),
44354455
}
44364456
}
44374457

@@ -8772,8 +8792,48 @@ impl EditorElement {
87728792
}
87738793
}
87748794

8795+
#[derive(Default)]
8796+
pub struct EditorRequestLayoutState {
8797+
// We use prepaint depth to limit the number of times prepaint is
8798+
// called recursively. We need this so that we can update stale
8799+
// data for e.g. block heights in block map.
8800+
prepaint_depth: Rc<Cell<usize>>,
8801+
}
8802+
8803+
impl EditorRequestLayoutState {
8804+
// In ideal conditions we only need one more subsequent prepaint call for resize to take effect.
8805+
// i.e. MAX_PREPAINT_DEPTH = 2, but since moving blocks inline (place_near), more lines from
8806+
// below get exposed, and we end up querying blocks for those lines too in subsequent renders.
8807+
// Setting MAX_PREPAINT_DEPTH = 3, passes all tests. Just to be on the safe side we set it to 5, so
8808+
// that subsequent shrinking does not lead to incorrect block placing.
8809+
const MAX_PREPAINT_DEPTH: usize = 5;
8810+
8811+
fn increment_prepaint_depth(&self) -> EditorPrepaintGuard {
8812+
let depth = self.prepaint_depth.get();
8813+
self.prepaint_depth.set(depth + 1);
8814+
EditorPrepaintGuard {
8815+
prepaint_depth: self.prepaint_depth.clone(),
8816+
}
8817+
}
8818+
8819+
fn can_prepaint(&self) -> bool {
8820+
self.prepaint_depth.get() < Self::MAX_PREPAINT_DEPTH
8821+
}
8822+
}
8823+
8824+
struct EditorPrepaintGuard {
8825+
prepaint_depth: Rc<Cell<usize>>,
8826+
}
8827+
8828+
impl Drop for EditorPrepaintGuard {
8829+
fn drop(&mut self) {
8830+
let depth = self.prepaint_depth.get();
8831+
self.prepaint_depth.set(depth.saturating_sub(1));
8832+
}
8833+
}
8834+
87758835
impl Element for EditorElement {
8776-
type RequestLayoutState = ();
8836+
type RequestLayoutState = EditorRequestLayoutState;
87778837
type PrepaintState = EditorLayout;
87788838

87798839
fn id(&self) -> Option<ElementId> {
@@ -8790,7 +8850,7 @@ impl Element for EditorElement {
87908850
_inspector_id: Option<&gpui::InspectorElementId>,
87918851
window: &mut Window,
87928852
cx: &mut App,
8793-
) -> (gpui::LayoutId, ()) {
8853+
) -> (gpui::LayoutId, Self::RequestLayoutState) {
87948854
let rem_size = self.rem_size(cx);
87958855
window.with_rem_size(rem_size, |window| {
87968856
self.editor.update(cx, |editor, cx| {
@@ -8857,7 +8917,7 @@ impl Element for EditorElement {
88578917
}
88588918
};
88598919

8860-
(layout_id, ())
8920+
(layout_id, EditorRequestLayoutState::default())
88618921
})
88628922
})
88638923
}
@@ -8867,10 +8927,11 @@ impl Element for EditorElement {
88678927
_: Option<&GlobalElementId>,
88688928
_inspector_id: Option<&gpui::InspectorElementId>,
88698929
bounds: Bounds<Pixels>,
8870-
_: &mut Self::RequestLayoutState,
8930+
request_layout: &mut Self::RequestLayoutState,
88718931
window: &mut Window,
88728932
cx: &mut App,
88738933
) -> Self::PrepaintState {
8934+
let _prepaint_depth_guard = request_layout.increment_prepaint_depth();
88748935
let text_style = TextStyleRefinement {
88758936
font_size: Some(self.style.text.font_size),
88768937
line_height: Some(self.style.text.line_height),
@@ -9394,7 +9455,20 @@ impl Element for EditorElement {
93949455
// If the fold widths have changed, we need to prepaint
93959456
// the element again to account for any changes in
93969457
// wrapping.
9397-
return self.prepaint(None, _inspector_id, bounds, &mut (), window, cx);
9458+
if request_layout.can_prepaint() {
9459+
return self.prepaint(
9460+
None,
9461+
_inspector_id,
9462+
bounds,
9463+
request_layout,
9464+
window,
9465+
cx,
9466+
);
9467+
} else {
9468+
debug_panic!(
9469+
"skipping recursive prepaint at max depth. renderer widths may be stale."
9470+
);
9471+
}
93989472
}
93999473

94009474
let longest_line_blame_width = self
@@ -9481,20 +9555,35 @@ impl Element for EditorElement {
94819555
)
94829556
})
94839557
})
9484-
.unwrap_or_else(|| Ok((Vec::default(), HashMap::default())));
9485-
let (mut blocks, row_block_types) = match blocks {
9486-
Ok(blocks) => blocks,
9487-
Err(resized_blocks) => {
9488-
self.editor.update(cx, |editor, cx| {
9489-
editor.resize_blocks(
9490-
resized_blocks,
9491-
autoscroll_request.map(|(autoscroll, _)| autoscroll),
9492-
cx,
9493-
)
9494-
});
9495-
return self.prepaint(None, _inspector_id, bounds, &mut (), window, cx);
9558+
.unwrap_or_default();
9559+
let RenderBlocksOutput {
9560+
mut blocks,
9561+
row_block_types,
9562+
resized_blocks,
9563+
} = blocks;
9564+
if let Some(resized_blocks) = resized_blocks {
9565+
self.editor.update(cx, |editor, cx| {
9566+
editor.resize_blocks(
9567+
resized_blocks,
9568+
autoscroll_request.map(|(autoscroll, _)| autoscroll),
9569+
cx,
9570+
)
9571+
});
9572+
if request_layout.can_prepaint() {
9573+
return self.prepaint(
9574+
None,
9575+
_inspector_id,
9576+
bounds,
9577+
request_layout,
9578+
window,
9579+
cx,
9580+
);
9581+
} else {
9582+
debug_panic!(
9583+
"skipping recursive prepaint at max depth. block layout may be stale."
9584+
);
94969585
}
9497-
};
9586+
}
94989587

94999588
let sticky_buffer_header = sticky_header_excerpt.map(|sticky_header_excerpt| {
95009589
window.with_element_namespace("blocks", |window| {

0 commit comments

Comments
 (0)