Blockmap sync fix (#44743)

Release Notes:

- Improved display map rendering performance with many lines in the the multi-buffer.

---------

Co-authored-by: cameron <cameron.studdstreet@gmail.com>
Co-authored-by: Cole Miller <cole@zed.dev>
This commit is contained in:
Yara 🏳️‍⚧️
2025-12-17 17:14:57 +01:00
committed by GitHub
parent 80aefbe8e1
commit 1446d84941
7 changed files with 189 additions and 28 deletions

View File

@@ -141,6 +141,9 @@ Uladzislau Kaminski <i@uladkaminski.com>
Uladzislau Kaminski <i@uladkaminski.com> <uladzislau_kaminski@epam.com>
Vitaly Slobodin <vitaliy.slobodin@gmail.com>
Vitaly Slobodin <vitaliy.slobodin@gmail.com> <vitaly_slobodin@fastmail.com>
Yara <davidsk@zed.dev>
Yara <git@davidsk.dev>
Yara <git@yara.blue>
Will Bradley <williambbradley@gmail.com>
Will Bradley <williambbradley@gmail.com> <will@zed.dev>
WindSoilder <WindSoilder@outlook.com>

View File

@@ -14,8 +14,57 @@
//! - [`DisplayMap`] that adds background highlights to the regions of text.
//! Each one of those builds on top of preceding map.
//!
//! ## Structure of the display map layers
//!
//! Each layer in the map (and the multibuffer itself to some extent) has a few
//! structures that are used to implement the public API available to the layer
//! above:
//! - a `Transform` type - this represents a region of text that the layer in
//! question is "managing", that it transforms into a more "processed" text
//! for the layer above. For example, the inlay map has an `enum Transform`
//! that has two variants:
//! - `Isomorphic`, representing a region of text that has no inlay hints (i.e.
//! is passed through the map transparently)
//! - `Inlay`, representing a location where an inlay hint is to be inserted.
//! - a `TransformSummary` type, which is usually a struct with two fields:
//! [`input: TextSummary`][`TextSummary`] and [`output: TextSummary`][`TextSummary`]. Here,
//! `input` corresponds to "text in the layer below", and `output` corresponds to the text
//! exposed to the layer above. So in the inlay map case, a `Transform::Isomorphic`'s summary is
//! just `input = output = summary`, where `summary` is the [`TextSummary`] stored in that
//! variant. Conversely, a `Transform::Inlay` always has an empty `input` summary, because it's
//! not "replacing" any text that exists on disk. The `output` is the summary of the inlay text
//! to be injected. - Various newtype wrappers for co-ordinate spaces (e.g. [`WrapRow`]
//! represents a row index, after soft-wrapping (and all lower layers)).
//! - A `Snapshot` type (e.g. [`InlaySnapshot`]) that captures the state of a layer at a specific
//! point in time.
//! - various APIs which drill through the layers below to work with the underlying text. Notably:
//! - `fn text_summary_for_offset()` returns a [`TextSummary`] for the range in the co-ordinate
//! space that the map in question is responsible for.
//! - `fn <A>_point_to_<B>_point()` converts a point in co-ordinate space `A` into co-ordinate
//! space `B`.
//! - A [`RowInfo`] iterator (e.g. [`InlayBufferRows`]) and a [`Chunk`] iterator
//! (e.g. [`InlayChunks`])
//! - A `sync` function (e.g. [`InlayMap::sync`]) that takes a snapshot and list of [`Edit<T>`]s,
//! and returns a new snapshot and a list of transformed [`Edit<S>`]s. Note that the generic
//! parameter on `Edit` changes, since these methods take in edits in the co-ordinate space of
//! the lower layer, and return edits in their own co-ordinate space. The term "edit" is
//! slightly misleading, since an [`Edit<T>`] doesn't tell you what changed - rather it can be
//! thought of as a "region to invalidate". In theory, it would be correct to always use a
//! single edit that covers the entire range. However, this would lead to lots of unnecessary
//! recalculation.
//!
//! See the docs for the [`inlay_map`] module for a more in-depth explanation of how a single layer
//! works.
//!
//! [Editor]: crate::Editor
//! [EditorElement]: crate::element::EditorElement
//! [`TextSummary`]: multi_buffer::MBTextSummary
//! [`WrapRow`]: wrap_map::WrapRow
//! [`InlayBufferRows`]: inlay_map::InlayBufferRows
//! [`InlayChunks`]: inlay_map::InlayChunks
//! [`Edit<T>`]: text::Edit
//! [`Edit<S>`]: text::Edit
//! [`Chunk`]: language::Chunk
#[macro_use]
mod dimensions;

View File

@@ -545,7 +545,7 @@ impl BlockMap {
{
let max_point = wrap_snapshot.max_point();
let edit_start = wrap_snapshot.prev_row_boundary(max_point);
let edit_end = max_point.row() + WrapRow(1);
let edit_end = max_point.row() + WrapRow(1); // this is end of file
edits = edits.compose([WrapEdit {
old: edit_start..edit_end,
new: edit_start..edit_end,
@@ -715,6 +715,7 @@ impl BlockMap {
let placement = block.placement.to_wrap_row(wrap_snapshot)?;
if let BlockPlacement::Above(row) = placement
&& row < new_start
// this will be true more often now
{
return None;
}

View File

@@ -1,3 +1,10 @@
//! The inlay map. See the [`display_map`][super] docs for an overview of how the inlay map fits
//! into the rest of the [`DisplayMap`][super::DisplayMap]. Much of the documentation for this
//! module generalizes to other layers.
//!
//! The core of this module is the [`InlayMap`] struct, which maintains a vec of [`Inlay`]s, and
//! [`InlaySnapshot`], which holds a sum tree of [`Transform`]s.
use crate::{
ChunkRenderer, HighlightStyles,
inlays::{Inlay, InlayContent},
@@ -69,7 +76,9 @@ impl sum_tree::Item for Transform {
#[derive(Clone, Debug, Default)]
struct TransformSummary {
/// Summary of the text before inlays have been applied.
input: MBTextSummary,
/// Summary of the text after inlays have been applied.
output: MBTextSummary,
}

View File

@@ -840,35 +840,62 @@ impl WrapSnapshot {
self.tab_point_to_wrap_point(self.tab_snapshot.clip_point(self.to_tab_point(point), bias))
}
#[ztracing::instrument(skip_all, fields(point=?point, ret))]
pub fn prev_row_boundary(&self, mut point: WrapPoint) -> WrapRow {
/// Try to find a TabRow start that is also a WrapRow start
/// Every TabRow start is a WrapRow start
#[ztracing::instrument(skip_all, fields(point=?point))]
pub fn prev_row_boundary(&self, point: WrapPoint) -> WrapRow {
if self.transforms.is_empty() {
return WrapRow(0);
}
*point.column_mut() = 0;
let point = WrapPoint::new(point.row(), 0);
let mut cursor = self
.transforms
.cursor::<Dimensions<WrapPoint, TabPoint>>(());
// start
cursor.seek(&point, Bias::Right);
// end
if cursor.item().is_none() {
cursor.prev();
}
// start
// real newline fake fake
// text: helloworldasldlfjasd\njdlasfalsk\naskdjfasdkfj\n
// dimensions v v v v v
// transforms |-------|-----NW----|-----W------|-----W------|
// cursor ^ ^^^^^^^^^^^^^ ^
// (^) ^^^^^^^^^^^^^^
// point: ^
// point(col_zero): (^)
while let Some(transform) = cursor.item() {
if transform.is_isomorphic() && cursor.start().1.column() == 0 {
if transform.is_isomorphic() {
// this transform only has real linefeeds
let tab_summary = &transform.summary.input;
// is the wrap just before the end of the transform a tab row?
// thats only if this transform has at least one newline
//
// "this wrap row is a tab row" <=> self.to_tab_point(WrapPoint::new(wrap_row, 0)).column() == 0
// Note on comparison:
// We have code that relies on this to be row > 1
// It should work with row >= 1 but it does not :(
//
// That means that if every line is wrapped we walk back all the
// way to the start. Which invalidates the entire state triggering
// a full re-render.
if tab_summary.lines.row > 1 {
let wrap_point_at_end = cursor.end().0.row();
return cmp::min(wrap_point_at_end - RowDelta(1), point.row());
} else if cursor.start().1.column() == 0 {
return cmp::min(cursor.end().0.row(), point.row());
} else {
}
}
cursor.prev();
}
}
// end
unreachable!()
WrapRow(0)
}
#[ztracing::instrument(skip_all)]
@@ -891,13 +918,11 @@ impl WrapSnapshot {
}
#[cfg(test)]
#[ztracing::instrument(skip_all)]
pub fn text(&self) -> String {
self.text_chunks(WrapRow(0)).collect()
}
#[cfg(test)]
#[ztracing::instrument(skip_all)]
pub fn text_chunks(&self, wrap_row: WrapRow) -> impl Iterator<Item = &str> {
self.chunks(
wrap_row..self.max_point().row() + WrapRow(1),
@@ -1298,6 +1323,71 @@ mod tests {
use text::Rope;
use theme::LoadThemes;
#[gpui::test]
async fn test_prev_row_boundary(cx: &mut gpui::TestAppContext) {
init_test(cx);
fn test_wrap_snapshot(
text: &str,
soft_wrap_every: usize, // font size multiple
cx: &mut gpui::TestAppContext,
) -> WrapSnapshot {
let text_system = cx.read(|cx| cx.text_system().clone());
let tab_size = 4.try_into().unwrap();
let font = test_font();
let _font_id = text_system.resolve_font(&font);
let font_size = px(14.0);
// this is very much an estimate to try and get the wrapping to
// occur at `soft_wrap_every` we check that it pans out for every test case
let soft_wrapping = Some(font_size * soft_wrap_every * 0.6);
let buffer = cx.new(|cx| language::Buffer::local(text, cx));
let buffer = cx.new(|cx| MultiBuffer::singleton(buffer, cx));
let buffer_snapshot = buffer.read_with(cx, |buffer, cx| buffer.snapshot(cx));
let (_inlay_map, inlay_snapshot) = InlayMap::new(buffer_snapshot);
let (_fold_map, fold_snapshot) = FoldMap::new(inlay_snapshot);
let (mut tab_map, _) = TabMap::new(fold_snapshot, tab_size);
let tabs_snapshot = tab_map.set_max_expansion_column(32);
let (_wrap_map, wrap_snapshot) =
cx.update(|cx| WrapMap::new(tabs_snapshot, font, font_size, soft_wrapping, cx));
wrap_snapshot
}
// These two should pass but dont, see the comparison note in
// prev_row_boundary about why.
//
// // 0123 4567 wrap_rows
// let wrap_snapshot = test_wrap_snapshot("1234\n5678", 1, cx);
// assert_eq!(wrap_snapshot.text(), "1\n2\n3\n4\n5\n6\n7\n8");
// let row = wrap_snapshot.prev_row_boundary(wrap_snapshot.max_point());
// assert_eq!(row.0, 3);
// // 012 345 678 wrap_rows
// let wrap_snapshot = test_wrap_snapshot("123\n456\n789", 1, cx);
// assert_eq!(wrap_snapshot.text(), "1\n2\n3\n4\n5\n6\n7\n8\n9");
// let row = wrap_snapshot.prev_row_boundary(wrap_snapshot.max_point());
// assert_eq!(row.0, 5);
// 012345678 wrap_rows
let wrap_snapshot = test_wrap_snapshot("123456789", 1, cx);
assert_eq!(wrap_snapshot.text(), "1\n2\n3\n4\n5\n6\n7\n8\n9");
let row = wrap_snapshot.prev_row_boundary(wrap_snapshot.max_point());
assert_eq!(row.0, 0);
// 111 2222 44 wrap_rows
let wrap_snapshot = test_wrap_snapshot("123\n4567\n\n89", 4, cx);
assert_eq!(wrap_snapshot.text(), "123\n4567\n\n89");
let row = wrap_snapshot.prev_row_boundary(wrap_snapshot.max_point());
assert_eq!(row.0, 2);
// 11 2223 wrap_rows
let wrap_snapshot = test_wrap_snapshot("12\n3456\n\n", 3, cx);
assert_eq!(wrap_snapshot.text(), "12\n345\n6\n\n");
let row = wrap_snapshot.prev_row_boundary(wrap_snapshot.max_point());
assert_eq!(row.0, 3);
}
#[gpui::test(iterations = 100)]
async fn test_random_wraps(cx: &mut gpui::TestAppContext, mut rng: StdRng) {
// todo this test is flaky

View File

@@ -69,7 +69,10 @@ pub fn run_test(
std::mem::forget(error);
} else {
if is_multiple_runs {
eprintln!("failing seed: {}", seed);
eprintln!("failing seed: {seed}");
eprintln!(
"You can rerun from this seed by setting the environmental variable SEED to {seed}"
);
}
if let Some(on_fail_fn) = on_fail_fn {
on_fail_fn()

View File

@@ -1,8 +1,8 @@
pub use tracing::Level;
pub use tracing::{Level, field};
#[cfg(ztracing)]
pub use tracing::{
debug_span, error_span, event, info_span, instrument, span, trace_span, warn_span,
Span, debug_span, error_span, event, info_span, instrument, span, trace_span, warn_span,
};
#[cfg(not(ztracing))]
pub use ztracing_macro::instrument;
@@ -26,17 +26,23 @@ pub use __consume_all_tokens as span;
#[macro_export]
macro_rules! __consume_all_tokens {
($($t:tt)*) => {
$crate::FakeSpan
$crate::Span
};
}
pub struct FakeSpan;
impl FakeSpan {
pub fn enter(&self) {}
}
#[cfg(not(ztracing))]
pub struct Span;
// #[cfg(not(ztracing))]
// pub use span;
#[cfg(not(ztracing))]
impl Span {
pub fn current() -> Self {
Self
}
pub fn enter(&self) {}
pub fn record<T, S>(&self, _t: T, _s: S) {}
}
#[cfg(ztracing)]
pub fn init() {