From 9665c678c6b907b138389cdf2426794a65d30764 Mon Sep 17 00:00:00 2001 From: "warp-agent-staging[bot]" <240773466+warp-agent-staging[bot]@users.noreply.github.com> Date: Sat, 5 Sep 2026 01:19:12 +0000 Subject: [PATCH 1/2] Avoid parallel layout when opening lazy Mermaid plans --- app/src/notebooks/editor/model.rs | 15 +++- app/src/notebooks/editor/model_tests.rs | 44 +++++++++ crates/editor/src/content/edit.rs | 114 ++++++++++++++++++------ crates/editor/src/content/edit_tests.rs | 56 ++++++++++++ crates/editor/src/render/model/mod.rs | 31 ++++++- 5 files changed, 233 insertions(+), 27 deletions(-) diff --git a/app/src/notebooks/editor/model.rs b/app/src/notebooks/editor/model.rs index 0b6d22dd3d3..74a0bf704af 100644 --- a/app/src/notebooks/editor/model.rs +++ b/app/src/notebooks/editor/model.rs @@ -108,6 +108,7 @@ pub struct NotebooksEditorModel { /// Context used to generate clickable file path links for notebooks. file_link_resolution_context: Option, default_mermaid_display_mode: MarkdownDisplayMode, + lazy_layout: bool, } #[derive(Clone)] @@ -239,6 +240,7 @@ impl NotebooksEditorModel { resize_tx, file_link_resolution_context: None, default_mermaid_display_mode: MarkdownDisplayMode::Raw, + lazy_layout, } } @@ -282,8 +284,19 @@ impl NotebooksEditorModel { } /// Set the window this editor model is associated with. Should be called when the pane attaches. - pub fn set_window_id(&mut self, window_id: WindowId, _ctx: &mut ModelContext) { + pub fn set_window_id(&mut self, window_id: WindowId, ctx: &mut ModelContext) { self.rte_window_id = Some(window_id); + self.child_models.update( + self.interaction_state.clone(), + self.content.clone(), + self.selection_model.clone(), + window_id, + self.default_mermaid_display_mode, + ctx, + ); + if self.sync_mermaid_render_offsets(ctx) && !self.lazy_layout { + self.rebuild_layout(ctx); + } } /// Get the context for the session and working directory associated with this editor, if any. diff --git a/app/src/notebooks/editor/model_tests.rs b/app/src/notebooks/editor/model_tests.rs index f1c40335134..98f785b5093 100644 --- a/app/src/notebooks/editor/model_tests.rs +++ b/app/src/notebooks/editor/model_tests.rs @@ -176,6 +176,50 @@ fn command_range( }) } +#[test] +fn test_lazy_model_initializes_rendered_mermaid_before_first_layout() { + App::test((), |mut app| async move { + initialize_deps(&mut app); + let _flag = FeatureFlag::MarkdownMermaid.override_enabled(true); + let _editable_flag = FeatureFlag::EditableMarkdownMermaid.override_enabled(true); + let window = setup_editor_window(&mut app, true); + let model = app.add_model(|ctx| { + let styles = rich_text_styles(Appearance::as_ref(ctx), FontSettings::as_ref(ctx)); + let mut model = NotebooksEditorModel::new_unbound_lazy(styles, ctx); + model.set_default_mermaid_display_mode(MarkdownDisplayMode::Rendered, ctx); + model.reset_with_markdown("```mermaid\ngraph TD\nA --> B\n```", ctx); + model + }); + layout_model(&mut app, &model).await; + + model.update(&mut app, |model, ctx| model.set_window_id(window, ctx)); + + let commands = command_models(&model, &mut app); + let mermaid = commands + .into_iter() + .exactly_one() + .expect("expected one Mermaid command model"); + let (render_offset, options) = app.read(|ctx| { + let render_offset = mermaid + .as_ref(ctx) + .start_offset(ctx) + .expect("Mermaid start offset should resolve") + + CharOffset::from(1); + let options = model + .as_ref(ctx) + .render_state() + .as_ref(ctx) + .layout_options(); + (render_offset, options) + }); + + assert_eq!( + options.mermaid_render_offsets, + HashSet::from([render_offset]) + ); + }); +} + /// Wait for text layout to finish. async fn layout_model(app: &mut App, model: &ModelHandle) { app.read(|ctx| model.as_ref(ctx).render_state.as_ref(ctx).layout_complete()) diff --git a/crates/editor/src/content/edit.rs b/crates/editor/src/content/edit.rs index a23ec064041..6e9f635187f 100644 --- a/crates/editor/src/content/edit.rs +++ b/crates/editor/src/content/edit.rs @@ -106,6 +106,13 @@ pub fn resolve_asset_source_relative_to_directory( } } +#[derive(Clone, Copy)] +enum LayoutConcurrency { + Parallel, + #[cfg(any(target_os = "macos", test))] + Sequential, +} + /// Resolve an image source when its Markdown block is laid out. /// /// Local-file metadata is read here so refreshes get a new cache key, while @@ -578,6 +585,44 @@ impl EditDelta { layout_options: &RenderLayoutOptions, hidden_ranges: Option>, app: &AppContext, + ) -> LaidOutRenderDelta { + self.layout_delta_with_concurrency( + layout, + document_path, + layout_options, + hidden_ranges, + app, + LayoutConcurrency::Parallel, + ) + } + + #[cfg(any(target_os = "macos", test))] + pub(crate) fn layout_delta_sequential( + &self, + layout: &TextLayout, + document_path: Option<&Path>, + layout_options: &RenderLayoutOptions, + hidden_ranges: Option>, + app: &AppContext, + ) -> LaidOutRenderDelta { + self.layout_delta_with_concurrency( + layout, + document_path, + layout_options, + hidden_ranges, + app, + LayoutConcurrency::Sequential, + ) + } + + fn layout_delta_with_concurrency( + &self, + layout: &TextLayout, + document_path: Option<&Path>, + layout_options: &RenderLayoutOptions, + hidden_ranges: Option>, + app: &AppContext, + concurrency: LayoutConcurrency, ) -> LaidOutRenderDelta { let hidden_ranges = hidden_ranges.unwrap_or_default(); @@ -623,34 +668,53 @@ impl EditDelta { for chunk in chunk_layout_tasks(layout_tasks) { let chunk_len = chunk.len(); - let (chunk_items, chunk_trailing_newline): (Vec<_>, Last<_>) = chunk - .into_par_iter() - .enumerate() - .filter_map(|(local_idx, (task, is_hidden))| { - let idx = chunk_start + local_idx; - let location = if idx == 0 { - BlockLocation::Start - } else if idx >= last_task { - BlockLocation::End - } else { - BlockLocation::Middle - }; - - match task.run(layout, location, is_hidden) { - Ok(result) => Some(result), - Err(e) => { - report_error!( - e.context("Failed to lay out BlockItem"), - extra: { "offset" => ?self.old_offset } - ); - None - } + let layout_task = |(local_idx, (task, is_hidden)): (usize, (LayoutTask<'_>, bool))| { + let idx = chunk_start + local_idx; + let location = if idx == 0 { + BlockLocation::Start + } else if idx >= last_task { + BlockLocation::End + } else { + BlockLocation::Middle + }; + + match task.run(layout, location, is_hidden) { + Ok(result) => Some(result), + Err(e) => { + report_error!( + e.context("Failed to lay out BlockItem"), + extra: { "offset" => ?self.old_offset } + ); + None } - }) - .unzip(); + } + }; + let (chunk_items, chunk_trailing_newline) = match concurrency { + LayoutConcurrency::Parallel => { + let (items, trailing_newline): (Vec<_>, Last<_>) = chunk + .into_par_iter() + .enumerate() + .filter_map(layout_task) + .unzip(); + (items, trailing_newline.into_inner()) + } + #[cfg(any(target_os = "macos", test))] + LayoutConcurrency::Sequential => { + let results: Vec<_> = chunk + .into_iter() + .enumerate() + .filter_map(layout_task) + .collect(); + let trailing_newline = results + .last() + .map(|(_, trailing_newline)| *trailing_newline); + let items = results.into_iter().map(|(item, _)| item).collect(); + (items, trailing_newline) + } + }; block_items.extend(chunk_items); - if let Some(trailing_newline) = chunk_trailing_newline.into_inner() { + if let Some(trailing_newline) = chunk_trailing_newline { has_trailing_newline = Some(trailing_newline); } chunk_start += chunk_len; diff --git a/crates/editor/src/content/edit_tests.rs b/crates/editor/src/content/edit_tests.rs index 552d4791bcd..b5e62fd8a5d 100644 --- a/crates/editor/src/content/edit_tests.rs +++ b/crates/editor/src/content/edit_tests.rs @@ -1404,6 +1404,62 @@ fn test_layout_delta_single_chunk_matches_direct_layout() { }) } +#[test] +fn test_sequential_layout_preserves_parallel_layout_results() { + App::test((), |app| async move { + app.read(|ctx| { + let layout_cache = LayoutCache::new(); + let text_layout = TextLayout::new( + &layout_cache, + ctx.font_cache().text_layout_system(), + &TEST_STYLES, + f32::MAX, + ); + let delta = EditDelta { + old_offset: CharOffset::from(1)..CharOffset::from(13), + new_lines: Arc::new(vec![ + identifiable_text_block(3), + identifiable_text_block(4), + identifiable_text_block(5), + ]), + ..Default::default() + }; + + let parallel = delta.layout_delta( + &text_layout, + None, + &RenderLayoutOptions::default(), + None, + ctx, + ); + let sequential = delta.layout_delta_sequential( + &text_layout, + None, + &RenderLayoutOptions::default(), + None, + ctx, + ); + + assert_eq!(sequential.old_offset, parallel.old_offset); + assert_eq!( + sequential + .laid_out_line + .iter() + .map(BlockItem::content_length) + .collect::>(), + parallel + .laid_out_line + .iter() + .map(BlockItem::content_length) + .collect::>() + ); + assert_eq!( + sequential.trailing_newline.is_some(), + parallel.trailing_newline.is_some() + ); + }); + }) +} /// Builds a hidden, isolated `CodeBlock`-styled block whose gutter-button count (and thus its /// laid-out `line_count`) directly observes the `BlockLocation` it was laid out with: Start/End /// always get one button, but a genuine Middle location with `run_count >= diff --git a/crates/editor/src/render/model/mod.rs b/crates/editor/src/render/model/mod.rs index 2bc7c3d0ab3..1e158a5e7a7 100644 --- a/crates/editor/src/render/model/mod.rs +++ b/crates/editor/src/render/model/mod.rs @@ -3398,6 +3398,35 @@ impl RenderState { self.layout_pending_edit(laid_out_edit, hidden_ranges); } + #[cfg(target_os = "macos")] + fn layout_lazy_edit_delta( + &self, + delta: EditDelta, + hidden_ranges: Option>, + app: &AppContext, + ) { + let layout_cache = LayoutCache::new(); + let layout_context = self.layout_context(&layout_cache, app); + let laid_out_edit = delta.layout_delta_sequential( + &layout_context, + self.document_path.as_deref(), + &self.layout_options, + hidden_ranges.clone(), + app, + ); + self.layout_pending_edit(laid_out_edit, hidden_ranges); + } + + #[cfg(not(target_os = "macos"))] + fn layout_lazy_edit_delta( + &self, + delta: EditDelta, + hidden_ranges: Option>, + app: &AppContext, + ) { + self.layout_edit_delta(delta, hidden_ranges, app); + } + /// Construct a throwaway layout cache. We only lay out modified text, so in effect, /// the entire RenderState is a cache. fn layout_context<'a>( @@ -3431,7 +3460,7 @@ impl RenderState { delta, hidden_ranges, } => { - self.layout_edit_delta(delta, hidden_ranges, app); + self.layout_lazy_edit_delta(delta, hidden_ranges, app); } PendingLayout::TemporaryBlocks(blocks) => { self.layout_temporary_blocks(blocks, app); From 0005ccf7243e736d2b628fc780d269ed88b96043 Mon Sep 17 00:00:00 2001 From: "warp-agent-staging[bot]" <240773466+warp-agent-staging[bot]@users.noreply.github.com> Date: Sat, 5 Sep 2026 02:22:05 +0000 Subject: [PATCH 2/2] Strengthen lazy Mermaid layout coverage --- app/src/notebooks/editor/model.rs | 10 +- app/src/notebooks/editor/model_tests.rs | 52 +++++--- crates/editor/src/content/edit_tests.rs | 155 ++++++++++++++++++++---- 3 files changed, 172 insertions(+), 45 deletions(-) diff --git a/app/src/notebooks/editor/model.rs b/app/src/notebooks/editor/model.rs index 74a0bf704af..9c2a7681144 100644 --- a/app/src/notebooks/editor/model.rs +++ b/app/src/notebooks/editor/model.rs @@ -2194,9 +2194,13 @@ impl ChildModels { .cloned() } - /// Update the sub-model state with [`NotebookCommand`] models for every runnable command - /// in the buffer. This should be called after text layout completes, so that the offsets of - /// each block line up between the render and content models. + /// Update the sub-model state with [`NotebookCommand`] models for every runnable command in the + /// buffer. + /// + /// Synchronization after text layout keeps edited render and content offsets aligned. A lazy + /// model may also update before its first layout once it is bound to a window: buffer outlines + /// already have stable content offsets, and the child models provide the rendered Mermaid + /// offsets that first layout consumes. pub fn update( &mut self, interaction_state: ModelHandle, diff --git a/app/src/notebooks/editor/model_tests.rs b/app/src/notebooks/editor/model_tests.rs index 98f785b5093..992849a7149 100644 --- a/app/src/notebooks/editor/model_tests.rs +++ b/app/src/notebooks/editor/model_tests.rs @@ -199,24 +199,44 @@ fn test_lazy_model_initializes_rendered_mermaid_before_first_layout() { .into_iter() .exactly_one() .expect("expected one Mermaid command model"); - let (render_offset, options) = app.read(|ctx| { - let render_offset = mermaid - .as_ref(ctx) - .start_offset(ctx) - .expect("Mermaid start offset should resolve") - + CharOffset::from(1); - let options = model - .as_ref(ctx) - .render_state() - .as_ref(ctx) - .layout_options(); - (render_offset, options) + let expected_range = command_range(&mermaid, &mut app); + let render_state = app.read(|ctx| model.as_ref(ctx).render_state().clone()); + render_state.update(&mut app, |render_state, ctx| { + render_state.set_viewport_size( + SizeInfo { + viewport_size: Vector2F::new(800., 600.), + needs_layout: false, + }, + ctx, + ); }); + let pending_edits_flushed = + app.read(|ctx| render_state.as_ref(ctx).try_layout_pending_edits(ctx)); + assert!(pending_edits_flushed); - assert_eq!( - options.mermaid_render_offsets, - HashSet::from([render_offset]) - ); + app.read(|ctx| { + let model = model.as_ref(ctx); + let render_state = model.render_state().as_ref(ctx); + let content = render_state.content(); + let mut offset = CharOffset::zero(); + let (mermaid_offset, mermaid_item) = content + .block_items() + .find_map(|item| { + let item_offset = offset; + offset += item.content_length(); + matches!(item, BlockItem::MermaidDiagram { .. }).then_some((item_offset, item)) + }) + .expect("first lazy layout should produce a Mermaid diagram"); + + assert_eq!(mermaid_offset, expected_range.start); + assert_eq!( + mermaid_item.content_length(), + expected_range.end - expected_range.start + ); + assert_eq!(mermaid_item.lines(), 1.into()); + assert!(mermaid_item.content_width().as_f32() > 0.); + assert!(mermaid_item.content_height().as_f32() > 0.); + }); }); } diff --git a/crates/editor/src/content/edit_tests.rs b/crates/editor/src/content/edit_tests.rs index b5e62fd8a5d..2c69729e1ea 100644 --- a/crates/editor/src/content/edit_tests.rs +++ b/crates/editor/src/content/edit_tests.rs @@ -1251,6 +1251,66 @@ fn identifiable_text_block(content_len: usize) -> StyledBufferBlock { }) } +fn text_block(content: &str, style: BufferBlockStyle) -> StyledBufferBlock { + StyledBufferBlock::Text(StyledTextBlock { + block: vec![StyledBufferRun { + run: content.to_string(), + text_styles: TextStylesWithMetadata::default(), + block_style: style.clone(), + }], + style, + content_length: CharOffset::from(content.chars().count()), + }) +} + +fn assert_same_rendered_block(parallel: &BlockItem, sequential: &BlockItem) { + assert_eq!(sequential.content_length(), parallel.content_length()); + assert_eq!(sequential.lines(), parallel.lines()); + assert_eq!(sequential.content_width(), parallel.content_width()); + assert_eq!(sequential.content_height(), parallel.content_height()); + assert_eq!(sequential.width(), parallel.width()); + assert_eq!(sequential.height(), parallel.height()); + assert_eq!(sequential.first_line_height(), parallel.first_line_height()); + assert_eq!(sequential.spacing(), parallel.spacing()); + + match (parallel, sequential) { + (BlockItem::Paragraph(parallel), BlockItem::Paragraph(sequential)) => { + assert_eq!(sequential.width(), parallel.width()); + assert_eq!(sequential.height(), parallel.height()); + assert_eq!(sequential.first_line_height(), parallel.first_line_height()); + } + ( + BlockItem::RunnableCodeBlock { + paragraph_block: parallel, + code_block_type: parallel_type, + .. + }, + BlockItem::RunnableCodeBlock { + paragraph_block: sequential, + code_block_type: sequential_type, + .. + }, + ) => { + assert_eq!(sequential_type, parallel_type); + assert_eq!(sequential.paragraphs().len(), parallel.paragraphs().len()); + for (parallel, sequential) in parallel.paragraphs().iter().zip(sequential.paragraphs()) + { + assert_eq!(sequential.width(), parallel.width()); + assert_eq!(sequential.height(), parallel.height()); + assert_eq!(sequential.first_line_height(), parallel.first_line_height()); + } + } + (BlockItem::Hidden(parallel), BlockItem::Hidden(sequential)) => { + assert_eq!(sequential.content_length(), parallel.content_length()); + assert_eq!(sequential.line_count(), parallel.line_count()); + assert_eq!(sequential.height(), parallel.height()); + } + (parallel, sequential) => { + panic!("rendered block types differ: parallel={parallel:?}, sequential={sequential:?}") + } + } +} + #[test] fn test_layout_delta_chunk_boundary_preserves_order_hidden_collapsing_and_trailing_newline() { // Regression test for APP-5392: bounding EditDelta::layout_delta's parallel fan-out into @@ -1408,51 +1468,94 @@ fn test_layout_delta_single_chunk_matches_direct_layout() { fn test_sequential_layout_preserves_parallel_layout_results() { App::test((), |app| async move { app.read(|ctx| { - let layout_cache = LayoutCache::new(); - let text_layout = TextLayout::new( - &layout_cache, + let parallel_cache = LayoutCache::new(); + let parallel_layout = TextLayout::new( + ¶llel_cache, ctx.font_cache().text_layout_system(), &TEST_STYLES, - f32::MAX, + 160., + ); + let sequential_cache = LayoutCache::new(); + let sequential_layout = TextLayout::new( + &sequential_cache, + ctx.font_cache().text_layout_system(), + &TEST_STYLES, + 160., + ); + let code_style = BufferBlockStyle::CodeBlock { + code_block_type: CodeBlockType::Shell, + }; + let new_lines = vec![ + text_block( + "A paragraph long enough to wrap across multiple rendered lines at this width.\n", + BufferBlockStyle::PlainText, + ), + text_block( + "printf 'first line'\nprintf 'second line'\n", + code_style.clone(), + ), + text_block("hidden command\n", code_style), + text_block( + "Visible tail without a newline", + BufferBlockStyle::PlainText, + ), + ]; + let mut offset = CharOffset::from(1); + let block_starts: Vec<_> = new_lines + .iter() + .map(|block| { + let start = offset; + offset += block.content_length(); + start + }) + .collect(); + let mut hidden_ranges = RangeSet::new(); + hidden_ranges.insert( + block_starts[2]..block_starts[2] + new_lines[2].content_length(), ); let delta = EditDelta { - old_offset: CharOffset::from(1)..CharOffset::from(13), - new_lines: Arc::new(vec![ - identifiable_text_block(3), - identifiable_text_block(4), - identifiable_text_block(5), - ]), + old_offset: CharOffset::from(1)..offset, + new_lines: Arc::new(new_lines), ..Default::default() }; let parallel = delta.layout_delta( - &text_layout, + ¶llel_layout, None, &RenderLayoutOptions::default(), - None, + Some(hidden_ranges.clone()), ctx, ); let sequential = delta.layout_delta_sequential( - &text_layout, + &sequential_layout, None, &RenderLayoutOptions::default(), - None, + Some(hidden_ranges), ctx, ); assert_eq!(sequential.old_offset, parallel.old_offset); - assert_eq!( - sequential - .laid_out_line - .iter() - .map(BlockItem::content_length) - .collect::>(), - parallel - .laid_out_line - .iter() - .map(BlockItem::content_length) - .collect::>() - ); + assert_eq!(sequential.laid_out_line.len(), 4); + assert!(matches!( + parallel.laid_out_line.as_slice(), + [ + BlockItem::Paragraph(_), + BlockItem::RunnableCodeBlock { + code_block_type: CodeBlockType::Shell, + .. + }, + BlockItem::Hidden(_), + BlockItem::Paragraph(_) + ] + )); + for (parallel, sequential) in parallel + .laid_out_line + .iter() + .zip(&sequential.laid_out_line) + { + assert_same_rendered_block(parallel, sequential); + } + assert!(parallel.trailing_newline.is_none()); assert_eq!( sequential.trailing_newline.is_some(), parallel.trailing_newline.is_some()