Subject: Re: bug#75056: 31.0.50; tty-child-frames with server / multiple clients possible hangs

From: Gerd Möllmann <[email-masked]>
Date: Wed, 29 Jan 2025 14:04:10 +0100

martin rudalics <[email-masked]> writes:

>> But it seemed to work on the release branch.  Anyway, with your patches
>> I don't see the assertion violation any more so you should check them
>> in.
>
> Spoke to early.  Now setting the cursor in a C-l child frame doesn't
> work any more.

Back from the drawing board, please find now 3 patches attached.

Idea was right, only that something was missing: the selected frame is
insofar related to the update that if it is part of the z-order on the
root being updated, it determines which frame in the z-order has the
cursor. Sounds a bit complicated, but fixes the problem :-).

I have rebased to include a comment fix you mentioned in one of the old
commits.

>From 026fce0c4d74f98d3cb1bc7648a77e7996fe6719 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gerd=20M=C3=B6llmann?= <[email-masked]>
Date: Wed, 29 Jan 2025 08:28:58 +0100
Subject: [PATCH 1/3] Don't use selected frame in tty_set_cursor

* src/dispnew.c (tty_set_cursor): Add frmae parameter.
(write_matrix): Pass to the updated frame to tty_set_cursor.
---
 src/dispnew.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/dispnew.c b/src/dispnew.c
index 1b4c5eb37cf..7242849509d 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -5618,10 +5618,8 @@ scrolling_window (struct window *w, int tab_line_p)
  ************************************************************************/
 
 static void
-tty_set_cursor (void)
+tty_set_cursor (struct frame *f)
 {
-  struct frame *f = SELECTED_FRAME ();
-
   if ((cursor_in_echo_area
        /* If we are showing a message instead of the mini-buffer,
 	  show the cursor for the message instead of for the
@@ -5683,7 +5681,7 @@ tty_set_cursor (void)
   else
     {
       /* We have only one cursor on terminal frames.  Use it to
-	 display the cursor of the selected window.  */
+	 display the cursor of the selected window of the frame.  */
       struct window *w = XWINDOW (FRAME_SELECTED_WINDOW (f));
       if (w->cursor.vpos >= 0
 	  /* The cursor vpos may be temporarily out of bounds
@@ -5736,7 +5734,7 @@ write_matrix (struct frame *f, bool inhibit_id_p,
 
   /* Now just clean up termcap drivers and set cursor, etc.  */
   if (set_cursor_p)
-    tty_set_cursor ();
+    tty_set_cursor (f);
 }
 
 /* Do line insertions/deletions on frame F for frame-based redisplay.  */
-- 
2.48.1

>From 524e4e060c13d1b9d3fe24b3d8ca56903233e84e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gerd=20M=C3=B6llmann?= <[email-masked]>
Date: Wed, 29 Jan 2025 08:34:43 +0100
Subject: [PATCH 2/3] Replace two macros with functions in dispnew.c

* src/dispnew.c (WINDOW_TO_FRAME_VPOS, WINDOW_TO_FRAME_VPOS): Macros
removed.
(window_to_frame_vpos, window_to_frame_vpos): Always define, and use
them instead of the removed macros.
---
 src/dispnew.c | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/src/dispnew.c b/src/dispnew.c
index 7242849509d..0eba863fea3 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -140,11 +140,6 @@ check_rows (struct frame *f)
 
 #ifdef GLYPH_DEBUG
 
-static int window_to_frame_vpos (struct window *, int);
-static int window_to_frame_hpos (struct window *, int);
-#define WINDOW_TO_FRAME_VPOS(W, VPOS) window_to_frame_vpos (W, VPOS)
-#define WINDOW_TO_FRAME_HPOS(W, HPOS) window_to_frame_hpos (W, HPOS)
-
 /* One element of the ring buffer containing redisplay history
    information.  */
 
@@ -236,11 +231,6 @@ DEFUN ("dump-redisplay-history", Fdump_redisplay_history,
 }

-#else /* not GLYPH_DEBUG */
-
-#define WINDOW_TO_FRAME_VPOS(W, VPOS) ((VPOS) + WINDOW_TOP_EDGE_LINE (W))
-#define WINDOW_TO_FRAME_HPOS(W, HPOS) ((HPOS) + WINDOW_LEFT_EDGE_COL (W))
-
 #endif /* GLYPH_DEBUG */

@@ -3194,8 +3184,6 @@ check_matrix_pointers (struct glyph_matrix *window_matrix,
 		      VPOS and HPOS translations
  **********************************************************************/
 
-#ifdef GLYPH_DEBUG
-
 /* Translate vertical position VPOS which is relative to window W to a
    vertical position relative to W's frame.  */
 
@@ -3222,9 +3210,6 @@ window_to_frame_hpos (struct window *w, int hpos)
   return hpos;
 }
 
-#endif /* GLYPH_DEBUG */
-
-
 
 /**********************************************************************
 			    Redrawing Frames
@@ -3906,8 +3891,8 @@ abs_cursor_pos (struct frame *f, int *x, int *y)
 	 a new cursor position has been computed.  */
       && w->cursor.vpos < WINDOW_TOTAL_LINES (w))
     {
-      int wx = WINDOW_TO_FRAME_HPOS (w, w->cursor.hpos);
-      int wy = WINDOW_TO_FRAME_VPOS (w, w->cursor.vpos);
+      int wx = window_to_frame_hpos (w, w->cursor.hpos);
+      int wy = window_to_frame_vpos (w, w->cursor.vpos);
 
       wx += max (0, w->left_margin_cols);
 
@@ -5691,8 +5676,8 @@ tty_set_cursor (struct frame *f)
 	     a new cursor position has been computed.  */
 	  && w->cursor.vpos < WINDOW_TOTAL_LINES (w))
 	{
-	  int x = WINDOW_TO_FRAME_HPOS (w, w->cursor.hpos);
-	  int y = WINDOW_TO_FRAME_VPOS (w, w->cursor.vpos);
+	  int x = window_to_frame_hpos (w, w->cursor.hpos);
+	  int y = window_to_frame_vpos (w, w->cursor.vpos);
 
 	  x += max (0, w->left_margin_cols);
 	  cursor_to (f, y, x);
-- 
2.48.1

>From cd04f99c823122fe1ca6daf520a8f33c45114b35 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gerd=20M=C3=B6llmann?= <[email-masked]>
Date: Wed, 29 Jan 2025 13:42:40 +0100
Subject: [PATCH 3/3] Further fixes for cursor positioning

* src/dispnew.c (write_matrix): Don't set the cursor here. Parameter to
set the cursor or not removed.
(combine_updates_for_frame): Call tty_set_cursor here, depending on
whether or not the selected frame is related to the updated frame.
(update_frame_with_menu): Adapt to change of write_matrix.
---
 src/dispnew.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/src/dispnew.c b/src/dispnew.c
index 0eba863fea3..4b1a8b2e1f2 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -96,12 +96,14 @@ Copyright (C) 1985-1988, 1993-1995, 1997-2025 Free Software Foundation,
 static void mirror_line_dance (struct window *, int, int, int *, char *);
 static void update_window_tree (struct window *);
 static void update_window (struct window *);
-static void write_matrix (struct frame *, bool, bool, bool);
+static void write_matrix (struct frame *, bool, bool);
 static void scrolling (struct frame *);
 static void set_window_cursor_after_update (struct window *);
 static void adjust_frame_glyphs_for_window_redisplay (struct frame *);
 static void adjust_frame_glyphs_for_frame_redisplay (struct frame *);
 static void set_window_update_flags (struct window *w, bool on_p);
+static void tty_set_cursor (struct frame *f);
+
 
 #if 0 /* Please leave this in as a debugging aid.  */
 static void
@@ -4021,10 +4023,19 @@ combine_updates_for_frame (struct frame *f, bool inhibit_scrolling)
     }
 
   update_begin (root);
-  write_matrix (root, inhibit_scrolling, 1, false);
+  write_matrix (root, inhibit_scrolling, false);
   make_matrix_current (root);
   update_end (root);
 
+  /* The selected frame determines where the cursor on ttys goes, except
+     when it is a frame that is completely unrelated to the frame being
+     displayed.  This can happen with multi-tty, when the selected frame
+     can be a window-system frame.  */
+  if (frame_ancestor_p (root, SELECTED_FRAME ()))
+    tty_set_cursor (SELECTED_FRAME ());
+  else
+    tty_set_cursor (root);
+
   /* If a child is displayed, and the cursor is displayed in another
      frame, the child might lay above the cursor, so that it appears to
      "shine through" the child.  Avoid that because it's confusing.  */
@@ -4093,22 +4104,20 @@ update_frame (struct frame *f, bool inhibit_scrolling)
 update_frame_with_menu (struct frame *f, int row, int col)
 {
   struct window *root_window = XWINDOW (f->root_window);
-  bool cursor_at_point_p;
 
   eassert (FRAME_TERMCAP_P (f));
 
   /* Update the display.  */
   update_begin (f);
-  cursor_at_point_p = !(row >= 0 && col >= 0);
-  /* Do not stop due to pending input, and do not try scrolling.  This
-     means that write_glyphs will always return false.  */
-  write_matrix (f, 1, cursor_at_point_p, true);
+  write_matrix (f, true, true);
   make_matrix_current (f);
   clear_desired_matrices (f);
   /* ROW and COL tell us where in the menu to position the cursor, so
      that screen readers know the active region on the screen.  */
-  if (!cursor_at_point_p)
+  if (row >= 0 && col >= 0)
     cursor_to (f, row, col);
+  else
+    tty_set_cursor (f);
   update_end (f);
   flush_terminal (f);
 
@@ -5685,13 +5694,12 @@ tty_set_cursor (struct frame *f)
     }
 }
 
-/* Write desired matix of tty frame F and make it current.
+/* Write desired matrix of tty frame F and make it current.
    INHIBIT_ID_P means that scrolling by insert/delete should not be tried.
-   SET_CURSOR_P false means do not set cursor at point in selected window.  */
+   UPDATING_MENU_P true means we are called for updating a tty menu.  */
 
 static void
-write_matrix (struct frame *f, bool inhibit_id_p,
-	      bool set_cursor_p, bool updating_menu_p)
+write_matrix (struct frame *f, bool inhibit_id_p, bool updating_menu_p)
 {
   /* If we cannot insert/delete lines, it's no use trying it.  */
   if (!FRAME_LINE_INS_DEL_OK (f))
@@ -5716,10 +5724,6 @@ write_matrix (struct frame *f, bool inhibit_id_p,
     for (int i = first_row; i < last_row; ++i)
       if (MATRIX_ROW_ENABLED_P (f->desired_matrix, i))
 	write_row (f, i, updating_menu_p);
-
-  /* Now just clean up termcap drivers and set cursor, etc.  */
-  if (set_cursor_p)
-    tty_set_cursor (f);
 }
 
 /* Do line insertions/deletions on frame F for frame-based redisplay.  */
-- 
2.48.1