diff --git a/crates/neomacs-gui-tests/fixtures/commit-editing.el b/crates/neomacs-gui-tests/fixtures/commit-editing.el new file mode 100644 index 0000000000..237e776dca --- /dev/null +++ b/crates/neomacs-gui-tests/fixtures/commit-editing.el @@ -0,0 +1,91 @@ +;;; commit-editing.el --- Commit-buffer input and separator regressions -*- lexical-binding: t; -*- + +(defvar neo-commit-old-map-count 0) +(defconst neo-commit-async-probe + (member (getenv "NEOMACS_COMMIT_PROBE") '("async-map" "async-map-delayed")) + "Non-nil when this fixture checks asynchronous command dispatch.") +(defconst neo-commit-overlay-probe + (equal (getenv "NEOMACS_COMMIT_PROBE") "overlay-separator") + "Non-nil when the small newline belongs to an overlay string.") + +(defun neo-commit-probe-finish () + "Check the completed input sequence or the redisplayed separator geometry." + (interactive) + (condition-case err + (progn + (if neo-commit-async-probe + (progn + (unless (= neo-commit-old-map-count 0) + (error "First commit key used the previous buffer's map")) + (unless (equal (with-current-buffer "*commit-message*" (buffer-string)) "aa") + (error "Commit buffer did not receive both characters"))) + (redisplay t) + (let* ((summary-y (cdr (posn-x-y (posn-at-point 1)))) + (separator-y (cdr (posn-x-y (posn-at-point 11)))) + (body-y (cdr (posn-x-y (posn-at-point 12)))) + (normal-height + (if neo-commit-overlay-probe + (- (cdr (posn-x-y (posn-at-point 17))) body-y) + (- separator-y summary-y))) + (separator-height + (if neo-commit-overlay-probe + (- body-y summary-y (* 2 normal-height)) + (- body-y separator-y)))) + (unless (and (> separator-height 0) + (< separator-height (/ normal-height 2.0))) + (error "Separator is not short: normal=%S separator=%S" + normal-height separator-height)))) + (princ "COMMIT-EDITING-PASS\n" 'external-debugging-output) + (kill-emacs 0)) + (error + (princ (format "COMMIT-EDITING-FAIL %S\n" err) 'external-debugging-output) + (kill-emacs 1)))) + +;; Install the watchdog before setup can enter any input wait. +(run-at-time 10 nil + (lambda () + (princ "COMMIT-EDITING-FAIL watchdog expired\n" 'external-debugging-output) + (kill-emacs 1))) + +(if neo-commit-async-probe + (progn + ;; This sentinel is dispatched only after both queued characters, even + ;; when slow startup makes multiple timers due in one service pass. + (global-set-key [neo-commit-probe-finish] #'neo-commit-probe-finish) + (switch-to-buffer (get-buffer-create "*commit-status*")) + (use-local-map (make-sparse-keymap)) + (local-set-key "a" (lambda () (interactive) + (setq neo-commit-old-map-count (1+ neo-commit-old-map-count)))) + (with-current-buffer (get-buffer-create "*commit-message*") + (fundamental-mode)) + ;; Like a with-editor process filter: display the new buffer, but + ;; restore the old current buffer while the command loop awaits input. + (run-at-time 0.5 nil + (lambda () + (save-current-buffer (switch-to-buffer "*commit-message*")) + (setq unread-command-events + (list ?a ?a 'neo-commit-probe-finish))))) + (require 'log-edit) + (switch-to-buffer (get-buffer-create "*commit-separator*")) + (if neo-commit-overlay-probe + (progn + (fundamental-mode) + (insert "Summary: \n\nbody\nnext\n") + (overlay-put (make-overlay 11 11) 'before-string + (propertize "\n" 'face + '(:height 0.1 :inverse-video t :extend t)))) + ;; Checkout-local settings must not prompt during an isolated GUI probe. + (let ((enable-dir-local-variables nil)) + (log-edit-mode)) + (insert "Summary: \n\nbody\n") + (font-lock-ensure)) + (goto-char 10)) + +(unless neo-commit-async-probe + (run-at-time 2 nil #'neo-commit-probe-finish)) + +;; Reproduce startup slow enough to expire both timers in the old fixture, +;; before the command loop could dispatch either queued character. +(when (equal (getenv "NEOMACS_COMMIT_PROBE") "async-map-delayed") + (let ((deadline (+ (float-time) 3))) + (while (< (float-time) deadline)))) diff --git a/crates/neomacs-gui-tests/tests/commit_editing.rs b/crates/neomacs-gui-tests/tests/commit_editing.rs new file mode 100644 index 0000000000..53cd3b6802 --- /dev/null +++ b/crates/neomacs-gui-tests/tests/commit_editing.rs @@ -0,0 +1,106 @@ +//! Real command-input and small-font newline regressions, including macOS. +#![cfg(any(target_os = "linux", target_os = "macos"))] + +use neomacs_gui_tests::{ + DisplayHarness, GuiBackend, GuiRunOptions, GuiScenario, GuiTestPlan, ProcessGuiCommandRunner, +}; +use std::{fs, path::PathBuf, time::Duration}; + +/// Honor explicit compositor selection, defaulting to the host's native backend. +fn commit_probe_backend(configured: Option<&str>) -> GuiBackend { + match configured { + Some("wayland" | "linux-wayland") => GuiBackend::LinuxWayland, + Some("x11" | "linux-x11") => GuiBackend::LinuxX11, + Some("macos") => GuiBackend::Macos, + None if cfg!(target_os = "macos") => GuiBackend::Macos, + None => GuiBackend::LinuxX11, + Some(other) => panic!("unsupported NEOMACS_GUI_TEST_BACKEND={other:?}"), + } +} + +/// Run a native commit-editing fixture and require its explicit success marker. +fn run_commit_probe(probe: &str) { + let root = neomacs_infra::crate_root!().join("../.."); + let configured = std::env::var("NEOMACS_GUI_TEST_BACKEND").ok(); + let backend = commit_probe_backend(configured.as_deref()); + let artifacts = root + .join("target/neomacs-gui-tests") + .join(format!("commit-{probe}")); + let session = DisplayHarness::for_backend(backend) + .start_session(&artifacts) + .unwrap(); + let program = std::env::var_os("NEOMACS_GUI_TEST_BINARY") + .map(PathBuf::from) + .unwrap_or_else(|| root.join("target/release/neomacs")); + let mut plan = GuiTestPlan::new( + backend, + &root, + &artifacts, + GuiScenario::new( + probe, + root.join("crates/neomacs-gui-tests/fixtures/commit-editing.el"), + ), + ) + .with_program(program) + .with_env("NEOMACS_COMMIT_PROBE", probe) + .with_env("RUST_LOG", "warn"); + for (key, value) in session.env() { + plan = plan.with_env(key, value); + } + let result = plan + .run_with( + &mut ProcessGuiCommandRunner, + GuiRunOptions::with_timeout(Duration::from_secs(20)), + ) + .unwrap(); + let stderr = fs::read_to_string(&result.artifacts.stderr).unwrap_or_default(); + assert!(!result.timed_out, "{result:#?}\n{stderr}"); + assert_eq!(result.exit_code, Some(0), "{result:#?}\n{stderr}"); + assert!(stderr.contains("COMMIT-EDITING-PASS"), "{stderr}"); +} + +/// Backend aliases and platform defaults must not require a live compositor. +#[test] +fn commit_probe_respects_requested_backend() { + for name in ["wayland", "linux-wayland"] { + assert_eq!(commit_probe_backend(Some(name)), GuiBackend::LinuxWayland); + } + for name in ["x11", "linux-x11"] { + assert_eq!(commit_probe_backend(Some(name)), GuiBackend::LinuxX11); + } + assert_eq!(commit_probe_backend(Some("macos")), GuiBackend::Macos); + assert_eq!( + commit_probe_backend(None), + if cfg!(target_os = "macos") { + GuiBackend::Macos + } else { + GuiBackend::LinuxX11 + } + ); +} + +#[test] +#[should_panic(expected = "unsupported NEOMACS_GUI_TEST_BACKEND")] +fn commit_probe_rejects_unknown_backend() { + commit_probe_backend(Some("unknown")); +} + +#[test] +fn first_commit_character_uses_the_asynchronously_selected_buffer() { + run_commit_probe("async-map"); +} + +#[test] +fn delayed_timer_service_waits_for_commit_commands() { + run_commit_probe("async-map-delayed"); +} + +#[test] +fn overlay_separator_uses_its_small_font_height() { + run_commit_probe("overlay-separator"); +} + +#[test] +fn log_edit_separator_uses_its_small_font_height() { + run_commit_probe("separator"); +} diff --git a/crates/neomacs-gui-tests/tests/gui.rs b/crates/neomacs-gui-tests/tests/gui.rs index 3b80291a61..1f004e0d94 100644 --- a/crates/neomacs-gui-tests/tests/gui.rs +++ b/crates/neomacs-gui-tests/tests/gui.rs @@ -31,6 +31,9 @@ mod coding_save; #[path = "colrv1_emoji.rs"] mod colrv1_emoji; +#[path = "commit_editing.rs"] +mod commit_editing; + #[path = "compact_frame_geometry.rs"] mod compact_frame_geometry; diff --git a/crates/neomacs-layout-engine/src/buffer_source/render_plan.rs b/crates/neomacs-layout-engine/src/buffer_source/render_plan.rs index 485f405f09..a3af0820c8 100644 --- a/crates/neomacs-layout-engine/src/buffer_source/render_plan.rs +++ b/crates/neomacs-layout-engine/src/buffer_source/render_plan.rs @@ -760,6 +760,8 @@ impl BufferSourceOutputSetup { } #[allow(clippy::too_many_arguments)] + /// Render a window body using eligible retained-row replay or source layout, + /// returning the resulting installation or retry outcome. pub(crate) fn render_body_attempt< 'a, 'surface, @@ -945,7 +947,9 @@ impl BufferSourceOutputSetup { cursor_row_pixel_y, face_height, face_ascent, - cursor_height, + // Fresh capture clamps against the + // whole row, including spacing. + row.height_px, cursor_ascent, geometry.char_height, row.ends_at_zv, diff --git a/crates/neomacs-layout-engine/src/buffer_source/row_lifecycle.rs b/crates/neomacs-layout-engine/src/buffer_source/row_lifecycle.rs index 4367b959f7..dce0d02610 100644 --- a/crates/neomacs-layout-engine/src/buffer_source/row_lifecycle.rs +++ b/crates/neomacs-layout-engine/src/buffer_source/row_lifecycle.rs @@ -1941,6 +1941,8 @@ impl<'a> BufferSourceLineBreakRenderRequest<'a> { self } + /// Resolve source-newline font geometry and line-end effects before + /// advancing the row transition, including intercepted display newlines. pub(crate) fn render_and_apply( self, source_walk: &mut BufferSourceWalk<'_, B>, @@ -1961,13 +1963,19 @@ impl<'a> BufferSourceLineBreakRenderRequest<'a> { let mut source_render = source_render; let context = self.context; - let resolved_height = if self.line_height - != crate::display_item::DisplayLineHeightPolicy::Default - && source_render - .measurement_mode() - .uses_concrete_font_geometry() - { - let defaults = context.row_geometry_defaults; + // The frame default is a fallback, not a minimum for a small font. + // Share this decision with worker-prepared rows; ordinary default-font + // rows keep their accumulated geometry without scanning glyphs. + let defaults = context.row_geometry_defaults; + let face = context.active_face_state.metrics(); + let resolved_height = if source_render + .measurement_mode() + .uses_concrete_font_geometry() + && crate::display_row::metrics::newline_requires_height_resolution( + self.line_height, + (face.row_height(), face.ascent()), + (defaults.height, defaults.ascent), + ) { let fallback = crate::display_row::metrics::DisplayRowFallbackMetrics::from_default_face_extents( context.default_char_width, @@ -1975,7 +1983,6 @@ impl<'a> BufferSourceLineBreakRenderRequest<'a> { defaults.ascent, ); let content = source_render.current_row_visible_content_metrics(face_ids, fallback); - let face = context.active_face_state.metrics(); let metrics = crate::display_row::metrics::resolve_line_height( self.line_height, (content.height_px(), content.ascent_px()), diff --git a/crates/neomacs-layout-engine/src/display_row/finalizer.rs b/crates/neomacs-layout-engine/src/display_row/finalizer.rs index 80e2827db3..cfdca8d763 100644 --- a/crates/neomacs-layout-engine/src/display_row/finalizer.rs +++ b/crates/neomacs-layout-engine/src/display_row/finalizer.rs @@ -8,11 +8,12 @@ use crate::display_cursor::cursor_window_matches_current; use crate::display_item::{DisplayLineHeightPolicy, DisplayRowBreak}; +use crate::display_row::builder::DisplayRowVerticalMetrics; use crate::display_row::face_state::{DisplayRowFace, DisplayRowMeasurementMode}; use crate::display_row::line_end::{ LineEndContext, LineEndExtend, LineEndFillGeometry, NoNamedLineEndFaces, plan, }; -use crate::display_row::metrics::DisplayRowFallbackMetrics; +use crate::display_row::metrics::{DisplayLineHeightMetrics, DisplayRowFallbackMetrics}; use crate::glyph_row_writer::push_stretch_to_area; use neomacs_display_protocol::face::BoxVerticalEdges; use neomacs_display_protocol::frame_glyphs::PhysCursor; @@ -30,6 +31,7 @@ use neomacs_display_protocol::types::{Color, FaceId, Rect}; pub(crate) struct DisplayRowLineEndFinalizer { row_break: DisplayRowBreak, row_break_face_id: FaceId, + newline_font_metrics: Option, remaining_width_px: f32, fallback_metrics: DisplayRowFallbackMetrics, base_background: Color, @@ -61,6 +63,7 @@ impl DisplayRowLineEndFinalizer { Self { row_break, row_break_face_id, + newline_font_metrics: None, remaining_width_px, fallback_metrics, base_background, @@ -70,11 +73,51 @@ impl DisplayRowLineEndFinalizer { } } - pub(crate) fn finalize(self, row: &mut GlyphRow, faces: &[DisplayRowFace]) { - if self.row_break.line_height != DisplayLineHeightPolicy::Default + /// Preserve captured font extents rather than reconstructing them from + /// render faces, whose descent is rounded to an integer. + pub(crate) fn with_newline_font_metrics(mut self, metrics: DisplayRowVerticalMetrics) -> Self { + self.newline_font_metrics = Some(metrics); + self + } + + /// Resolve applicable line height and append newline/extension glyphs. + /// Return authoritative metrics only when height resolution ran. + pub(crate) fn finalize( + self, + row: &mut GlyphRow, + faces: &[DisplayRowFace], + ) -> Option { + let default = ( + self.fallback_metrics.row_height(), + self.fallback_metrics.ascent(), + ); + let face = self + .newline_font_metrics + .map(|metrics| (metrics.height_px(), metrics.ascent_px())) + .or_else(|| { + faces + .iter() + .find(|face| face.face_id == self.row_break_face_id) + .map(|face| (face.metrics.line_height_px(), face.metrics.ascent_px())) + }) + .unwrap_or(default); + let mut resolved = DisplayLineHeightMetrics { + height: row.height_px, + ascent: row.ascent_px, + newline_height: face.0, + newline_ascent: face.1, + }; + let resolve_height = self.row_break.line_height != DisplayLineHeightPolicy::Default + || self.newline_font_metrics.is_some() + && crate::display_row::metrics::newline_requires_height_resolution( + self.row_break.line_height, + face, + default, + ); + let resolve_height = resolve_height && (self.measurement_mode.uses_concrete_font_geometry() - || self.row_break.line_height == DisplayLineHeightPolicy::ContentOnly) - { + || self.row_break.line_height == DisplayLineHeightPolicy::ContentOnly); + if resolve_height { let (height, ascent) = display_row_visible_content_metrics( row, self.fallback_metrics.row_height(), @@ -86,25 +129,14 @@ impl DisplayRowLineEndFinalizer { .map(|face| (face.metrics.line_height_px(), face.metrics.ascent_px())) }, ); - let face = faces - .iter() - .find(|face| face.face_id == self.row_break_face_id) - .map(|face| (face.metrics.line_height_px(), face.metrics.ascent_px())) - .unwrap_or(( - self.fallback_metrics.row_height(), - self.fallback_metrics.ascent(), - )); - let metrics = crate::display_row::metrics::resolve_line_height( + resolved = crate::display_row::metrics::resolve_line_height( self.row_break.line_height, (height, ascent), face, - ( - self.fallback_metrics.row_height(), - self.fallback_metrics.ascent(), - ), + default, ); - row.height_px = metrics.height; - row.ascent_px = metrics.ascent; + row.height_px = resolved.height; + row.ascent_px = resolved.ascent; } // GNU order at a line end -- append_space_for_newline first, then @@ -147,11 +179,15 @@ impl DisplayRowLineEndFinalizer { // own metrics; a prepared program starts from the frame fallback, so // reading the row here made a rich line's fill one pixel taller than // fresh layout. Use the line-break face's metrics in both paths. - let break_face_metrics = faces - .iter() - .find(|face| face.face_id == self.row_break_face_id) - .map(|face| (face.metrics.line_height_px(), face.metrics.ascent_px())); - let (height_px, ascent_px) = break_face_metrics.unwrap_or((row.height_px, row.ascent_px)); + let (height_px, ascent_px) = if self.newline_font_metrics.is_some() { + (resolved.newline_height, resolved.newline_ascent) + } else { + faces + .iter() + .find(|face| face.face_id == self.row_break_face_id) + .map(|face| (face.metrics.line_height_px(), face.metrics.ascent_px())) + .unwrap_or((row.height_px, row.ascent_px)) + }; let height_px = height_px.max(1.0); let geometry = LineEndFillGeometry { content_x: 0.0, @@ -168,6 +204,7 @@ impl DisplayRowLineEndFinalizer { plan(&ctx) .resolve(&ctx, geometry, &mut NoNamedLineEndFaces) .apply_to(row); + resolve_height.then_some(resolved) } } diff --git a/crates/neomacs-layout-engine/src/display_row/metrics.rs b/crates/neomacs-layout-engine/src/display_row/metrics.rs index eb92306140..da507daf39 100644 --- a/crates/neomacs-layout-engine/src/display_row/metrics.rs +++ b/crates/neomacs-layout-engine/src/display_row/metrics.rs @@ -105,6 +105,19 @@ pub(crate) struct DisplayLineHeightMetrics { pub newline_ascent: f32, } +/// The seeded default is redundant only if the newline covers both extents. +/// Ordinary default-font rows can keep their accumulated geometry without +/// scanning glyphs; smaller newline fonts must resolve from actual content. +pub(crate) fn newline_requires_height_resolution( + policy: crate::display_item::DisplayLineHeightPolicy, + face: (f32, f32), + default: (f32, f32), +) -> bool { + policy != crate::display_item::DisplayLineHeightPolicy::Default + || face.1 < default.1 + || face.0 - face.1 < default.0 - default.1 +} + pub(crate) fn resolve_line_height( policy: crate::display_item::DisplayLineHeightPolicy, content: (f32, f32), diff --git a/crates/neomacs-layout-engine/src/display_row/mod.rs b/crates/neomacs-layout-engine/src/display_row/mod.rs index 5cc380149f..e44b9d7abd 100644 --- a/crates/neomacs-layout-engine/src/display_row/mod.rs +++ b/crates/neomacs-layout-engine/src/display_row/mod.rs @@ -1108,6 +1108,8 @@ impl<'metrics> DisplayRowRenderer<'metrics> { ) } + /// Render the next source fragment into a row under its acquisition policy. + /// Return `None` when the source state is already finished. fn render_display_item_source_row_fragment_step_into_row_with_policy< S: DisplayItemSource, P: DisplayRowRenderPolicy, @@ -1383,12 +1385,12 @@ impl<'metrics> DisplayRowRenderer<'metrics> { } } }; - if let ( + let resolved_row_metrics = if let ( DisplayRowRenderStop::RowBreak(row_break), Some((row_break_face_id, box_edges, box_membership)), ) = (stop, row_break_face) { - DisplayRowLineEndFinalizer::new( + let mut finalizer = DisplayRowLineEndFinalizer::new( row_break, row_break_face_id, line_end_right_edge_x - position.x_px(), @@ -1397,9 +1399,31 @@ impl<'metrics> DisplayRowRenderer<'metrics> { self.measurement_mode, box_edges, box_membership, - ) - .finalize(row, &row_faces); - } + ); + // Measure the primary newline font, not a newline glyph or the + // row's seeded default extents. Logical-cell rows stay untouched. + if self.measurement_mode.uses_concrete_font_geometry() + && let Some(face) = row_faces + .iter() + .find(|face| face.face_id == row_break_face_id) + && let Some(service) = face_realizer.font_metrics_service_mut() + { + let metrics = service.font_metrics( + &face.font_family, + face.font_weight, + face.italic, + face.font_size, + ); + finalizer = finalizer.with_newline_font_metrics( + builder::DisplayRowVerticalMetrics::new(metrics.line_height, metrics.ascent), + ); + } + finalizer.finalize(row, &row_faces).map(|metrics| { + builder::DisplayRowVerticalMetrics::new(metrics.height, metrics.ascent) + }) + } else { + None + }; let progress_height = if row.height_px > 0.0 { row.height_px } else { @@ -1413,12 +1437,10 @@ impl<'metrics> DisplayRowRenderer<'metrics> { .prepare_face_into_output(face.render_face(), &mut faces) .expect("row rendering must preserve its realized face identity"); } - Some(DisplayRowRenderIntoRowResult::new( - progress, - source_slots, - faces, - stop, - )) + Some( + DisplayRowRenderIntoRowResult::new(progress, source_slots, faces, stop) + .with_resolved_row_metrics(resolved_row_metrics), + ) } } diff --git a/crates/neomacs-layout-engine/src/display_row/render_state.rs b/crates/neomacs-layout-engine/src/display_row/render_state.rs index 6cc6ae4321..63278e50d4 100644 --- a/crates/neomacs-layout-engine/src/display_row/render_state.rs +++ b/crates/neomacs-layout-engine/src/display_row/render_state.rs @@ -1,5 +1,4 @@ use crate::display_item::{DisplayLineHeightPolicy, DisplayRowBreak}; -#[cfg(test)] use crate::display_row::builder::DisplayRowVerticalMetrics; use crate::display_row::builder::{ DisplayRowAppendProgress, DisplayRowAppendStatus, DisplayRowGlyphSlot, DisplayRowPosition, @@ -264,6 +263,7 @@ pub(crate) enum DisplayRowRenderStop { } pub(crate) struct DisplayRowRenderIntoRowResult { + resolved_row_metrics: Option, progress: DisplayRowOutputProgress, source_slots: Vec, faces: Vec, @@ -278,6 +278,7 @@ impl DisplayRowRenderIntoRowResult { stop: DisplayRowRenderStop, ) -> Self { Self { + resolved_row_metrics: None, progress, source_slots, faces, @@ -285,6 +286,19 @@ impl DisplayRowRenderIntoRowResult { } } + /// Carry authoritative newline extents across the source-render boundary. + pub(crate) fn with_resolved_row_metrics( + mut self, + metrics: Option, + ) -> Self { + self.resolved_row_metrics = metrics; + self + } + + pub(crate) fn resolved_row_metrics(&self) -> Option { + self.resolved_row_metrics + } + pub(crate) fn faces(&self) -> &[RealizedFrameFace] { &self.faces } @@ -381,6 +395,7 @@ impl DisplayRowRenderBounds { } pub(crate) struct CurrentTextRowRenderOutcome { + resolved_row_metrics: Option, stop: DisplayRowRenderStop, source_slots: Vec, end: DisplayRowPosition, @@ -397,6 +412,7 @@ impl CurrentTextRowRenderOutcome { row_ascent_px: f32, ) -> Self { Self { + resolved_row_metrics: None, stop, source_slots, end, @@ -405,6 +421,15 @@ impl CurrentTextRowRenderOutcome { } } + /// Preserve whether row extents replace, rather than only enlarge, geometry. + pub(crate) fn with_resolved_row_metrics( + mut self, + metrics: Option, + ) -> Self { + self.resolved_row_metrics = metrics; + self + } + pub(crate) fn stop(&self) -> DisplayRowRenderStop { self.stop } @@ -417,8 +442,11 @@ impl CurrentTextRowRenderOutcome { self.end } + /// Apply authoritative newline metrics even for the default height policy. pub(crate) fn include_vertical_metrics(&self, geometry: &mut DisplayRowGeometryState) { - if matches!( + if let Some(metrics) = self.resolved_row_metrics { + geometry.replace_current_row_metrics(metrics.height_px(), metrics.ascent_px()); + } else if matches!( self.stop, DisplayRowRenderStop::RowBreak(row_break) if row_break.line_height != DisplayLineHeightPolicy::Default diff --git a/crates/neomacs-layout-engine/src/display_row/source_render.rs b/crates/neomacs-layout-engine/src/display_row/source_render.rs index 231afc6fdf..f6da8edb85 100644 --- a/crates/neomacs-layout-engine/src/display_row/source_render.rs +++ b/crates/neomacs-layout-engine/src/display_row/source_render.rs @@ -730,6 +730,7 @@ impl<'face, 'emit, FS: crate::neovm_bridge::FaceServices> impl DisplayRowCurrentTextSourceStepResult { fn into_measure_outcome(self) -> CurrentTextRowRenderOutcome { + let resolved_row_metrics = self.result.resolved_row_metrics(); let (progress, source_slots, _faces, stop) = self.result.into_current_row_parts(); let end = display_row_output_end_position(progress); CurrentTextRowRenderOutcome::new( @@ -739,6 +740,7 @@ impl DisplayRowCurrentTextSourceStepResult { self.row_height_px, self.row_ascent_px, ) + .with_resolved_row_metrics(resolved_row_metrics) } } @@ -970,6 +972,7 @@ impl<'a> TextRowOutputRenderState<'a> { row_height_px, row_ascent_px, } = result; + let resolved_row_metrics = result.resolved_row_metrics(); let (progress, source_slots, faces, stop) = result.into_current_row_parts(); let end = display_row_output_end_position(progress); self.output.install_rendered_fragment_assets(&faces); @@ -977,6 +980,7 @@ impl<'a> TextRowOutputRenderState<'a> { self.output_emitter .emit_text_output_spans(self.evaluator, output, output_spans, end); CurrentTextRowRenderOutcome::new(stop, source_slots, end, row_height_px, row_ascent_px) + .with_resolved_row_metrics(resolved_row_metrics) } } diff --git a/crates/neomacs-layout-engine/src/engine/scroll_coverage.rs b/crates/neomacs-layout-engine/src/engine/scroll_coverage.rs index 2b7592625c..09042000cd 100644 --- a/crates/neomacs-layout-engine/src/engine/scroll_coverage.rs +++ b/crates/neomacs-layout-engine/src/engine/scroll_coverage.rs @@ -205,6 +205,8 @@ impl ScrollCoverage { }) } + /// Validate completed worker coverage and publish bounded prepared rows. + /// Reject stale admissions and do not publish incomplete bridge coverage. pub(super) fn drain( &mut self, destination: &mut PreparedViewports, @@ -275,9 +277,20 @@ impl ScrollCoverage { }; admission.retained.key.window_start = char_pos.get() as i64; } + // Small-font rows can reach the matrix's row limit before its pixel + // bottom. Match both bounds of the canonical viewport walk. + let capacity = admission + .retained + .matrix + .rows + .iter() + .skip(admission.row_base) + .filter(|row| !RetainedWindowMatrix::is_chrome_role(row.role)) + .count(); let mut height = 0.0; let visible = rows .iter() + .take(capacity) .take_while(|row| { let take = height < regions.text_body.height; if take { @@ -289,9 +302,7 @@ impl ScrollCoverage { let fills_viewport = height >= regions.text_body.height; // A suffix can start in a visual continuation: it certifies scroll // coverage, not a fresh viewport starting at that buffer character. - let complete_viewport = !bridge - && (fills_viewport - || rows.len() == admission.retained.matrix.rows.len() - admission.row_base); + let complete_viewport = !bridge && (fills_viewport || visible == capacity); if !bridge { rows.truncate(visible); } diff --git a/crates/neomacs-layout-engine/src/row_layout/program.rs b/crates/neomacs-layout-engine/src/row_layout/program.rs index b0f706688a..9339aee6a4 100644 --- a/crates/neomacs-layout-engine/src/row_layout/program.rs +++ b/crates/neomacs-layout-engine/src/row_layout/program.rs @@ -743,6 +743,8 @@ impl RowProgram { self.compute_visual_rows_inner(row_limit, cancelled, ResumedRunFonts::Worker(fonts)) } + /// Execute captured operations within the row budget using resumed fonts. + /// Preserve source/newline geometry and propagate cancellation or layout errors. fn compute_visual_rows_inner( mut self, row_limit: usize, @@ -1229,7 +1231,7 @@ impl RowProgram { DisplayRowMeasurementMode::LogicalCells } }; - DisplayRowLineEndFinalizer::new( + let finalizer = DisplayRowLineEndFinalizer::new( value, face, self.geometry.right_edge() - position.x_px(), @@ -1238,18 +1240,14 @@ impl RowProgram { mode, edges, membership, - ) - .finalize(&mut row, &self.faces); - if value.line_height != DisplayLineHeightPolicy::Default { - let resolved = crate::display_row::metrics::resolve_line_height( - value.line_height, - (row.height_px, row.ascent_px), - (face_metrics.line_height_px(), face_metrics.ascent_px()), - ( - self.geometry.metrics.row_height(), - self.geometry.metrics.ascent(), - ), - ); + ); + let finalizer = if mode.uses_concrete_font_geometry() { + terminator_height = minimum.height_px(); + finalizer.with_newline_font_metrics(minimum) + } else { + finalizer + }; + if let Some(resolved) = finalizer.finalize(&mut row, &self.faces) { terminator_height = resolved.newline_height; } // As in the canonical row transition, line spacing contributes diff --git a/crates/neomacs-layout-engine/src/row_layout/tests/program_test.rs b/crates/neomacs-layout-engine/src/row_layout/tests/program_test.rs index 373da23783..2c29547f02 100644 --- a/crates/neomacs-layout-engine/src/row_layout/tests/program_test.rs +++ b/crates/neomacs-layout-engine/src/row_layout/tests/program_test.rs @@ -241,6 +241,7 @@ fn wrapped_program_supports_tab_after_continuation_and_rejects_cancellation() { )); } +/// Worker reconstruction must preserve the captured fonts' exact row geometry. #[test] fn complete_row_program_uses_exact_captured_fonts_on_worker() { fn require_send() {} @@ -299,7 +300,7 @@ fn complete_row_program_uses_exact_captured_fonts_on_worker() { position = progress.end(); slots.extend(progress.slots().iter().cloned()); if let Some(newline) = newline { - DisplayRowLineEndFinalizer::new( + let _ = DisplayRowLineEndFinalizer::new( newline, FaceId::new(1), geometry.width - position.x_px(), diff --git a/crates/neomacs-layout-engine/src/tests/engine_line_spacing_test.rs b/crates/neomacs-layout-engine/src/tests/engine_line_spacing_test.rs index 0f3632d1d3..8ab4ed7f75 100644 --- a/crates/neomacs-layout-engine/src/tests/engine_line_spacing_test.rs +++ b/crates/neomacs-layout-engine/src/tests/engine_line_spacing_test.rs @@ -1,6 +1,164 @@ //! Line spacing is row geometry, independent of point and region decoration. use super::*; +/// A small buffer newline changes only its own row, independently of point. +#[test] +fn small_newline_face_shrinks_only_its_own_row() { + for extend in ["nil", "t"] { + let (mut eval, frame, _buffer, window) = + incr_editing_frame("Summary: \n\nbody\n", 800, 240); + realize_test_gui_frame(&mut eval, frame); + eval.eval_str(&format!( + "(put-text-property 11 12 'face '(:height 0.1 :inverse-video t :extend {extend}))" + )) + .unwrap(); + let mut engine = crate::font::metrics::test_catalog::layout_engine(); + crate::font::metrics::test_catalog::enable_metrics(&mut engine); + let rows = |engine: &mut LayoutEngine, eval: &mut Context| { + let FrameLayoutAttempt::Prepared(output) = engine.redisplay_frame_attempt(eval, frame) + else { + panic!("separator frame should prepare"); + }; + output + .window_matrices + .iter() + .find(|entry| entry.window_id.get() == window.0 as i64) + .unwrap() + .matrix + .rows + .iter() + .take(3) + .map(|row| (row.pixel_y, row.height_px)) + .collect::>() + }; + let before = rows(&mut engine, &mut eval); + assert!( + before[1].1 < before[0].1 / 2.0, + "small newline must not inherit the default row height: {before:?}" + ); + assert_eq!(before[2].1, before[0].1); + assert_eq!(before[2].0 - before[1].0, before[1].1); + eval.eval_str("(set-window-point (selected-window) 11)") + .unwrap(); + assert_eq!( + rows(&mut engine, &mut eval), + before, + "point must not enlarge the separator" + ); + } +} + +/// Normal-size visible content still determines mixed-font row extents. +#[test] +fn small_newline_keeps_normal_sized_content_in_the_same_row() { + let (mut eval, frame, _buffer, window) = incr_editing_frame("Summary: \nx\nbody\n", 800, 240); + realize_test_gui_frame(&mut eval, frame); + eval.eval_str("(put-text-property 12 13 'face '(:height 0.1 :extend t))") + .unwrap(); + let mut engine = crate::font::metrics::test_catalog::layout_engine(); + crate::font::metrics::test_catalog::enable_metrics(&mut engine); + let FrameLayoutAttempt::Prepared(output) = engine.redisplay_frame_attempt(&mut eval, frame) + else { + panic!("mixed-font frame should prepare"); + }; + let rows = &output + .window_matrices + .iter() + .find(|entry| entry.window_id.get() == window.0 as i64) + .unwrap() + .matrix + .rows; + assert_eq!(rows[1].height_px, rows[0].height_px); + assert_eq!(rows[2].height_px, rows[0].height_px); +} + +/// Font height properties cannot shrink a terminal's logical row cells. +#[test] +fn small_newline_face_does_not_shrink_terminal_rows() { + let (mut eval, frame, _buffer, window) = incr_editing_frame("Summary: \n\nbody\n", 800, 240); + eval.eval_str("(put-text-property 11 12 'face '(:height 0.1 :extend t))") + .unwrap(); + let mut engine = crate::font::metrics::test_catalog::layout_engine(); + let FrameLayoutAttempt::Prepared(output) = engine.redisplay_frame_attempt(&mut eval, frame) + else { + panic!("terminal frame should prepare"); + }; + let rows = &output + .window_matrices + .iter() + .find(|entry| entry.window_id.get() == window.0 as i64) + .unwrap() + .matrix + .rows; + assert_eq!(rows[1].height_px, rows[0].height_px); + assert_eq!(rows[2].height_px, rows[0].height_px); +} + +/// Overlay newlines use their own GUI font, but never shrink terminal cells. +#[test] +fn small_overlay_newline_uses_its_own_font_geometry() { + for graphical in [false, true] { + for property in ["before-string", "after-string"] { + for extend in ["nil", "t"] { + for prefix in ["", "x"] { + let (mut eval, frame, _buffer, window) = + incr_editing_frame("Summary: \nbody\n", 800, 240); + if graphical { + realize_test_gui_frame(&mut eval, frame); + } + eval.eval_str(&format!( + "(let ((overlay (make-overlay 11 11))) + (overlay-put overlay '{property} + (concat \"{prefix}\" (propertize \"\\n\" 'face + '(:height 0.1 :inverse-video t :extend {extend})))))" + )) + .unwrap(); + let mut engine = crate::font::metrics::test_catalog::layout_engine(); + if graphical { + crate::font::metrics::test_catalog::enable_metrics(&mut engine); + } + let rows = |engine: &mut LayoutEngine, eval: &mut Context| { + let FrameLayoutAttempt::Prepared(output) = + engine.redisplay_frame_attempt(eval, frame) + else { + panic!("overlay frame should prepare"); + }; + output + .window_matrices + .iter() + .find(|entry| entry.window_id.get() == window.0 as i64) + .unwrap() + .matrix + .rows + .iter() + .take(3) + .map(|row| (row.pixel_y, row.height_px)) + .collect::>() + }; + let before = rows(&mut engine, &mut eval); + if graphical && prefix.is_empty() { + assert!( + before[1].1 < before[0].1 / 2.0, + "small {property} newline must shrink its row: {before:?}" + ); + } else { + assert_eq!(before[1].1, before[0].1); + } + assert_eq!(before[2].1, before[0].1); + assert_eq!(before[2].0 - before[1].0, before[1].1); + eval.eval_str("(set-window-point (selected-window) 11)") + .unwrap(); + assert_eq!( + rows(&mut engine, &mut eval), + before, + "point must not change overlay row geometry" + ); + } + } + } + } +} + #[test] fn text_cursor_keeps_its_metrics_when_line_spacing_enlarges_the_row() { let (mut eval, frame, buffer, _window) = incr_editing_frame("alpha\nbeta\n", 800, 240); diff --git a/crates/neovm-core/src/emacs_core/runtime/eval/command_loop.rs b/crates/neovm-core/src/emacs_core/runtime/eval/command_loop.rs index 821a144593..eabf7e6f5c 100644 --- a/crates/neovm-core/src/emacs_core/runtime/eval/command_loop.rs +++ b/crates/neovm-core/src/emacs_core/runtime/eval/command_loop.rs @@ -858,7 +858,7 @@ impl Context { Value::T, )?; let read_result = self.read_command_key_sequence_with_options( - crate::keyboard::ReadKeySequenceOptions::new(Value::NIL, false, false, true), + crate::keyboard::ReadKeySequenceOptions::for_command_loop(), ); // The read result itself is not an EvalResult, but it can carry diff --git a/crates/neovm-core/src/emacs_core/runtime/eval/tests/input_wait.rs b/crates/neovm-core/src/emacs_core/runtime/eval/tests/input_wait.rs index bbee4082f9..f15225f1b2 100644 --- a/crates/neovm-core/src/emacs_core/runtime/eval/tests/input_wait.rs +++ b/crates/neovm-core/src/emacs_core/runtime/eval/tests/input_wait.rs @@ -117,6 +117,86 @@ fn input_after_idle_timer_wakes_wait(read: InputRead) { ); } +/// Callback-driven display changes must affect the very first command key. +#[test] +fn command_loop_uses_selected_buffers_map_after_async_switch() { + let _policy = legacy_redisplay_fixture_policy(); + let mut ev = runtime_startup_context(); + let scratch = ev.buffers.create_buffer("*async-old*"); + ev.buffers.set_current(scratch); + let frame = ev.frames.create_frame("F1", 80, 24, scratch); + assert!(ev.frames.select_frame(frame)); + let _input_owner = InputGateOwner::install(&mut ev); + fn stop(ctx: &mut Context, _args: Vec) -> EvalResult { + ctx.command_loop.running = false; + Ok(Value::NIL) + } + ev.register_subr(SubrSpec::new( + "neo-stop-async-map-test", + NativeFn::ContextVec(stop), + SubrArity::new(0, Some(0)), + )); + ev.eval_str( + r#"(progn + (setq timer-list nil timer-idle-list nil neo-old-map-count 0) + (switch-to-buffer "*async-old*") + (use-local-map (make-sparse-keymap)) + (local-set-key "a" (lambda () (interactive) + (setq neo-old-map-count (1+ neo-old-map-count)))) + (with-current-buffer (get-buffer-create "*async-new*") + (fundamental-mode)) + (global-set-key "b" (lambda () (interactive) (neo-stop-async-map-test))) + (run-at-time 0.01 nil + (lambda () + (save-current-buffer (switch-to-buffer "*async-new*")) + (setq unread-command-events (list ?a ?a ?b)))))"#, + ) + .unwrap(); + ev.command_loop.running = true; + ev.recursive_edit_inner().unwrap(); + assert_eq!( + ev.eval_symbol("neo-old-map-count").unwrap(), + Value::fixnum(0) + ); + assert_eq!( + ev.eval_str("(with-current-buffer \"*async-new*\" (buffer-string))") + .unwrap(), + Value::string("aa") + ); +} + +/// Lisp key readers retain their caller's buffer despite asynchronous display. +#[test] +fn lisp_key_reader_preserves_current_buffer_after_async_display() { + let _policy = legacy_redisplay_fixture_policy(); + let mut ev = runtime_startup_context(); + let old = ev.buffers.create_buffer("*reader-old*"); + ev.buffers.set_current(old); + let frame = ev.frames.create_frame("F1", 80, 24, old); + assert!(ev.frames.select_frame(frame)); + let _input_owner = InputGateOwner::install(&mut ev); + ev.eval_str( + r#"(progn + (setq timer-list nil timer-idle-list nil) + (use-local-map (make-sparse-keymap)) + (fset 'neo-old-key (lambda () (interactive))) + (local-set-key "a" 'neo-old-key) + (with-current-buffer (get-buffer-create "*reader-new*") + (fundamental-mode)) + (run-at-time 0.01 nil + (lambda () + (save-current-buffer (switch-to-buffer "*reader-new*")) + (setq unread-command-events (list ?a)))))"#, + ) + .unwrap(); + let (keys, binding) = ev + .read_key_sequence_with_options(ReadKeySequenceOptions::default()) + .unwrap(); + assert_eq!(keys, vec![Value::fixnum('a' as i64)]); + assert_eq!(binding.original(), Value::symbol("neo-old-key")); + assert_eq!(ev.buffers.current_buffer_id(), Some(old)); +} + #[test] fn read_char_wakes_for_input_published_at_poll_entry_after_idle_timer() { input_after_idle_timer_wakes_wait(InputRead::Character); diff --git a/crates/neovm-core/src/emacs_core/runtime/eval/vm_shared.rs b/crates/neovm-core/src/emacs_core/runtime/eval/vm_shared.rs index f1b8529f79..74543ae344 100644 --- a/crates/neovm-core/src/emacs_core/runtime/eval/vm_shared.rs +++ b/crates/neovm-core/src/emacs_core/runtime/eval/vm_shared.rs @@ -326,7 +326,9 @@ impl Context { self.interactive_minibuffer_read_count.saturating_add(1); } - pub(super) fn sync_current_buffer_to_selected_window(&mut self) { + /// Remember selected-window point and synchronize its buffer/runtime state. + /// With no selected frame, leave the caller's current buffer unchanged. + pub(crate) fn sync_current_buffer_to_selected_window(&mut self) { let Some(frame_id) = self.frames.selected_frame().map(|frame| frame.id) else { return; }; diff --git a/crates/neovm-core/src/keyboard.rs b/crates/neovm-core/src/keyboard.rs index 48f2814717..feb023db17 100644 --- a/crates/neovm-core/src/keyboard.rs +++ b/crates/neovm-core/src/keyboard.rs @@ -882,8 +882,18 @@ enum InputMethodEvent { Translated(Value), } +/// GNU `read_key_sequence`'s `fix_current_buffer` distinction: the command +/// loop follows the selected window, while Lisp key readers retain the caller's +/// current buffer even when another buffer is displayed. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum KeySequenceBufferSelection { + CurrentBuffer, + SelectedWindow, +} + #[derive(Clone, Copy, Debug, PartialEq)] pub(crate) struct ReadKeySequenceOptions { + pub buffer_selection: KeySequenceBufferSelection, pub prompt: Value, pub dont_downcase_last: bool, pub can_return_switch_frame: bool, @@ -977,6 +987,14 @@ pub(crate) enum CommandKeySequenceEnd { } impl ReadKeySequenceOptions { + pub(crate) fn for_command_loop() -> Self { + Self { + buffer_selection: KeySequenceBufferSelection::SelectedWindow, + can_return_switch_frame: true, + ..Self::default() + } + } + pub(crate) fn new( prompt: Value, continue_echo: bool, @@ -984,6 +1002,7 @@ impl ReadKeySequenceOptions { can_return_switch_frame: bool, ) -> Self { Self { + buffer_selection: KeySequenceBufferSelection::CurrentBuffer, prompt, dont_downcase_last, can_return_switch_frame, @@ -995,6 +1014,7 @@ impl ReadKeySequenceOptions { impl Default for ReadKeySequenceOptions { fn default() -> Self { Self { + buffer_selection: KeySequenceBufferSelection::CurrentBuffer, prompt: Value::NIL, dont_downcase_last: false, can_return_switch_frame: false, @@ -4953,6 +4973,22 @@ impl crate::emacs_core::eval::Context { } } + // Timers/process filters can display another buffer during + // the input wait while restoring the previously current one. + // GNU reconciles it at the first event, before consulting maps + // (keyboard.c, `if (NILP (first_event))`). Do not do this for + // Lisp-facing reads or after a mouse event selected its buffer. + if options.buffer_selection == KeySequenceBufferSelection::SelectedWindow + && self + .command_loop + .keyboard + .kboard + .current_key_sequence + .raw_events() + .is_empty() + { + self.sync_current_buffer_to_selected_window(); + } self.command_loop .keyboard .push_key_sequence_input_event(emacs_event);