Fix popup buffer selection and remaining newline geometry mismatches

crates/neomacs-display-protocol/src/glyph_matrix.rs

@@ -1090,7 +1090,8 @@ pub struct GlyphRow {
     ///
     /// Mirrors GNU `struct glyph_row::height`. `0.0` means unset.
     pub height_px: f32,
-    /// Additional logical descent from line spacing. Cursor height excludes it.
+    /// Additional logical descent from line spacing. Ordinary point glyphs do
+    /// not inherit it; GNU's synthetic newline cursor uses the full row.
     #[serde(default)]
     pub line_spacing_px: f32,
     /// Authoritative baseline ascent from row top in pixels.

crates/neomacs-gui-tests/fixtures/newline-geometry.el

@@ -0,0 +1,115 @@
+;;; newline-geometry.el --- Newline font and replay regressions -*- lexical-binding: t; -*-
+
+(defun neo-newline-probe-fail (err)
+  "Report ERR and terminate the isolated GUI process."
+  (princ (format "COMMIT-EDITING-FAIL %S\n" err) 'external-debugging-output)
+  (kill-emacs 1))
+
+;; Install before setup, so an unexpected input wait cannot hang the suite.
+(run-at-time 10 nil (lambda () (neo-newline-probe-fail "watchdog expired")))
+
+(defun neo-newline-probe-y (position)
+  "Return the redisplayed buffer POSITION's vertical pixel coordinate."
+  (cdr (posn-x-y (posn-at-point position))))
+
+(defun neo-newline-probe-stats-size ()
+  "Return the layout telemetry size in Neomacs, or nil for the GNU reference."
+  (when (fboundp 'neomacs--frame-snapshot)
+    (let* ((path (getenv "NEOMACS_LAYOUT_STATS_FILE"))
+           (attributes (and path (file-attributes path))))
+      (unless attributes (error "Missing layout telemetry: %S" path))
+      (file-attribute-size attributes))))
+
+(defvar neo-newline-probe-cold-cursor nil
+  "Physical cursor captured by the first mixed-height full layout.")
+
+(defun neo-newline-probe-finish ()
+  "Check physical row or cursor geometry after the queued redisplay."
+  (condition-case err
+      (progn
+        (redisplay t)
+        (let* ((probe (getenv "NEOMACS_COMMIT_PROBE"))
+               (body-y (neo-newline-probe-y 6))
+               (normal (- (neo-newline-probe-y 10) body-y)))
+          (cond
+           ((equal probe "replacement-newline")
+            ;; The replaced position need not have a buffer glyph. Measure
+            ;; the short row from the following body row instead.
+            (let ((height (- body-y (neo-newline-probe-y 1) normal)))
+              (unless (and (> height 0) (< height (/ normal 2.0)))
+                (error "Replacement newline is not short: %S/%S" height normal))))
+           ((equal probe "tall-overlay-newline")
+            (let ((height (- body-y (neo-newline-probe-y 1) (* 2 normal))))
+              (unless (> height (* normal 1.5))
+                (error "Overlay newline did not grow: %S/%S" height normal))))
+           ((member probe '("raised-text" "lowered-text"))
+            (let ((height (- (neo-newline-probe-y 5) (neo-newline-probe-y 1)))
+                  (expected (* normal 1.5)))
+              (unless (<= (abs (- height expected)) 1)
+                (error "Shifted text lost an opposite extent: %S/%S" height expected))))
+           ((string-prefix-p "mixed-" probe)
+            (let ((height (- (neo-newline-probe-y 5) (neo-newline-probe-y 1)))
+                  (cursor (window-cursor-info)))
+              (unless (and (> height 0) (= (aref cursor 4) height)
+                           (= (aref neo-newline-probe-cold-cursor 4) height)
+                           (= (aref cursor 5) (aref neo-newline-probe-cold-cursor 5)))
+                (error "Mixed newline cursor must fill the row, cold and replayed: %S/%S/%S"
+                       neo-newline-probe-cold-cursor cursor height))))
+           ((equal probe "short-newline-cursor")
+            (let ((row-height (- body-y (neo-newline-probe-y 5)))
+                  (cursor-height (aref (window-cursor-info) 4)))
+              (unless (and (> row-height 0) (= cursor-height row-height))
+                (error "Replayed cursor differs from short spaced row: %S/%S"
+                       cursor-height row-height))))
+           (t (error "Unknown newline probe: %S" probe))))
+        (princ "COMMIT-EDITING-PASS\n" 'external-debugging-output)
+        (kill-emacs 0))
+    (error (neo-newline-probe-fail err))))
+
+(switch-to-buffer (get-buffer-create "*newline-geometry*"))
+(fundamental-mode)
+(blink-cursor-mode -1)
+(insert "AAA\n\nBBB\nnext\n")
+(pcase (getenv "NEOMACS_COMMIT_PROBE")
+  ((or "raised-text" "lowered-text")
+   (put-text-property 1 4 'display
+                      (list 'raise (if (equal (getenv "NEOMACS_COMMIT_PROBE") "raised-text") 0.5 -0.5)))
+   (put-text-property 4 5 'line-height t))
+  ("replacement-newline"
+   (put-text-property 5 6 'display
+                      (propertize "\n" 'face '(:height 0.1 :inverse-video t :extend t))))
+  ("tall-overlay-newline"
+   (overlay-put (make-overlay 5 5) 'before-string
+                (propertize "\n" 'face '(:height 2.0 :inverse-video t :extend t))))
+  ("short-newline-cursor"
+   (put-text-property 5 6 'face '(:height 0.1))
+   (setq-local line-spacing 5)))
+(when (string-prefix-p "mixed-" (getenv "NEOMACS_COMMIT_PROBE"))
+  (put-text-property 1 4 'face '(:height 2.0))
+  (put-text-property 4 5 'face
+                     (list :height (if (string-match-p "small" (getenv "NEOMACS_COMMIT_PROBE")) 0.1 1.0)))
+  (setq-local line-spacing (if (string-match-p "spaced" (getenv "NEOMACS_COMMIT_PROBE")) 5 0)))
+(goto-char (if (string-prefix-p "mixed-" (getenv "NEOMACS_COMMIT_PROBE")) 4 1))
+(run-at-time
+ 0.5 nil
+ (lambda ()
+   (condition-case err
+       (progn
+         (redisplay t)
+         (when (string-suffix-p "newline-cursor" (getenv "NEOMACS_COMMIT_PROBE"))
+           (when (string-prefix-p "mixed-" (getenv "NEOMACS_COMMIT_PROBE"))
+             (setq neo-newline-probe-cold-cursor (window-cursor-info))
+             (goto-char 1)
+             (redisplay t))
+           ;; Retain rows first, then move only point onto the newline.
+           (let ((before (neo-newline-probe-stats-size)))
+             (goto-char (if neo-newline-probe-cold-cursor 4 5))
+             (redisplay t)
+             (if before
+                 (princ (format "CURSOR-REPLAY-STATS %d %d\n"
+                                before (neo-newline-probe-stats-size))
+                        'external-debugging-output)
+               (princ "CURSOR-REPLAY-REFERENCE-GEOMETRY-ONLY\n"
+                      'external-debugging-output))))
+         (run-at-time 0.5 nil #'neo-newline-probe-finish))
+     (error (neo-newline-probe-fail err)))))

crates/neomacs-gui-tests/tests/commit_editing.rs

@@ -20,6 +20,11 @@ fn commit_probe_backend(configured: Option<&str>) -> GuiBackend {
 
 /// Run a native commit-editing fixture and require its explicit success marker.
 fn run_commit_probe(probe: &str) {
+    run_fixture_probe(probe, "commit-editing.el");
+}
+
+/// Run an isolated native fixture, retaining diagnostics and checking its marker.
+fn run_fixture_probe(probe: &str, fixture: &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());
@@ -38,11 +43,18 @@ fn run_commit_probe(probe: &str) {
         &artifacts,
         GuiScenario::new(
             probe,
-            root.join("crates/neomacs-gui-tests/fixtures/commit-editing.el"),
+            root.join("crates/neomacs-gui-tests/fixtures").join(fixture),
         ),
     )
     .with_program(program)
     .with_env("NEOMACS_COMMIT_PROBE", probe)
+    .with_env(
+        "NEOMACS_LAYOUT_STATS_FILE",
+        artifacts
+            .join("layout-stats.log")
+            .to_string_lossy()
+            .into_owned(),
+    )
     .with_env("RUST_LOG", "warn");
     for (key, value) in session.env() {
         plan = plan.with_env(key, value);
@@ -57,6 +69,58 @@ fn run_commit_probe(probe: &str) {
     assert!(!result.timed_out, "{result:#?}\n{stderr}");
     assert_eq!(result.exit_code, Some(0), "{result:#?}\n{stderr}");
     assert!(stderr.contains("COMMIT-EDITING-PASS"), "{stderr}");
+    if probe.ends_with("newline-cursor")
+        && !stderr.contains("CURSOR-REPLAY-REFERENCE-GEOMETRY-ONLY")
+    {
+        let marker = stderr
+            .lines()
+            .find_map(|line| line.strip_prefix("CURSOR-REPLAY-STATS "))
+            .expect("missing replay telemetry interval");
+        let bounds: Vec<usize> = marker
+            .split_whitespace()
+            .map(|value| value.parse().unwrap())
+            .collect();
+        assert_eq!(bounds.len(), 2, "{marker}");
+        let stats = fs::read_to_string(artifacts.join("layout-stats.log")).unwrap();
+        let interval = stats
+            .get(bounds[0]..bounds[1])
+            .expect("invalid replay telemetry interval");
+        assert!(
+            cursor_replay_reused_rows(interval),
+            "point move did not reuse rows through cursor-only replay: {interval}"
+        );
+    }
+}
+
+/// Require the marked redisplay to reuse body rows rather than rebuilding them.
+fn cursor_replay_reused_rows(stats: &str) -> bool {
+    stats.lines().any(|line| {
+        let counter = |name: &str| {
+            line.split_whitespace().find_map(|entry| {
+                entry
+                    .strip_prefix(name)
+                    .and_then(|value| value.parse::<usize>().ok())
+            })
+        };
+        counter("cursor_only=").is_some_and(|value| value > 0)
+            && counter("reused=").is_some_and(|value| value > 0)
+            && counter("relaid_body=") == Some(0)
+    })
+}
+
+#[test]
+fn replay_telemetry_rejects_full_layout_and_missing_reuse() {
+    assert!(cursor_replay_reused_rows(
+        "cursor_only=1 reused=3 relaid_body=0"
+    ));
+    for stats in [
+        "",
+        "full=1 reused=3 relaid_body=0",
+        "cursor_only=1 reused=0 relaid_body=0",
+        "cursor_only=1 reused=3 relaid_body=1",
+    ] {
+        assert!(!cursor_replay_reused_rows(stats), "{stats}");
+    }
 }
 
 /// Backend aliases and platform defaults must not require a live compositor.
@@ -104,3 +168,40 @@ fn overlay_separator_uses_its_small_font_height() {
 fn log_edit_separator_uses_its_small_font_height() {
     run_commit_probe("separator");
 }
+
+#[test]
+fn replacement_newline_uses_its_own_font_height() {
+    run_fixture_probe("replacement-newline", "newline-geometry.el");
+}
+
+#[test]
+fn tall_overlay_newline_grows_the_empty_row() {
+    run_fixture_probe("tall-overlay-newline", "newline-geometry.el");
+}
+
+#[test]
+fn short_newline_cursor_replay_uses_full_row_bounds() {
+    run_fixture_probe("short-newline-cursor", "newline-geometry.el");
+}
+
+#[test]
+fn mixed_newline_cursor_matches_gnu_row_geometry() {
+    for probe in [
+        "mixed-newline-cursor",
+        "mixed-small-newline-cursor",
+        "mixed-spaced-newline-cursor",
+        "mixed-spaced-small-newline-cursor",
+    ] {
+        run_fixture_probe(probe, "newline-geometry.el");
+    }
+}
+
+#[test]
+fn raised_text_keeps_the_original_descent() {
+    run_fixture_probe("raised-text", "newline-geometry.el");
+}
+
+#[test]
+fn lowered_text_keeps_the_original_ascent() {
+    run_fixture_probe("lowered-text", "newline-geometry.el");
+}