Re: "Final" version of tty child frames

[Jared Finder <jared@finder.org>]

[-- Attachment #1: Type: text/plain, Size: 2371 bytes --]

On 2025-01-06 05:30, Eli Zaretskii wrote:
>> Date: Sun, 05 Jan 2025 16:05:59 -0800
>> From: Jared Finder <jared@finder.org>
>> Cc: gerd.moellmann@gmail.com, emacs-devel@gnu.org, rudalics@gmx.at
>> 
>> Style comments addressed, thank you! Someday, I will internalize 
>> Emacs'
>> unique C style conventions.  Probably once I only code for Emacs and 
>> not
>> other projects. :)
> 
> FTR, they are not Emacs conventions, they are GNU conventions.

Ah, good point.  This gave me hope I could just run GNU indent on the 
files I edited, but sadly that doesn't work.  I got a lot of unrelated 
changes, some of which seem to be fine either way, and some of which are 
conflicts with the DEFUN() doc: parameter.  This is sad, it would be 
really really nice if GNU style conventions could be automatically 
applied.  Then such conventions could be automatically checked or fixed 
when commits are made.

Does anyone here have a way to automatically check or fix the Emacs 
style convention?  It would be soooo helpful for occasional committers 
like myself.

>> > Does this mean child frames on xterm will have the selected-frame set
>> > to a child frame, and thus do not need this trick with
>> > last_mouse_frame?  Or what other difference between xterm and the
>> > Linux console hides behind this "fallback"?
>> 
>> This is just keeping the behavior that already exists today.  I was
>> surprised by the current behavior: that term_mouse_position which only
>> works for a GPM enabled mouse gets called even when GPM could not be
>> used.  It could make sense to change this behavior however it would be 
>> a
>> bigger change as gpm-mouse-mode is enabled by default even for non-GPM
>> supporting terminals.  I suspect that GPM support is equivalent to
>> TERM=linux, but I may be wrong.
>> 
>> This works when xterm-mouse-mode is enabled because 
>> mouse_position_hook
>> (set to term_mouse_position) is called first, followed by
>> mouse-position-function (set to xterm-mouse-position-function).  On 
>> ttys
>> with xterm-mouse-mode enabled the return value from 
>> term_mouse_position
>> is ignored.  See existing definitions in Fmouse_pixel_position and
>> mouse_position in frame.c.
> 
> Maybe term_mouse_position should return immediately if GPM is not
> enabled?

Sure, that was easy enough.  Other comments addressed as well.  New 
patch attached.

   -- MJF

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Support-TTY-child-frames-with-GPM-mouse.patch --]
[-- Type: text/x-diff; name=0001-Support-TTY-child-frames-with-GPM-mouse.patch, Size: 7236 bytes --]

From 73545bf94102d60719c5dbb22259a791ddb3c0d9 Mon Sep 17 00:00:00 2001
From: Jared Finder <jared@finder.org>
Date: Mon, 6 Jan 2025 20:52:11 -0800
Subject: [PATCH] Support TTY child frames with GPM mouse

* lisp/frame.el (x-list-fonts): Delete `frame-at', to move to
C implementation.
* lisp/xt-mouse.el (xterm-mouse-event): Call new `tty-frame-at'.
* src/term.c (tty_frame_at, Ftty_frame_at): New C function,
replacing `frame-at' only for TTYs.
(term_mouse_position): Use last_mouse_frame when it is set.
(handle_one_term_event): Call tty_frame_at to get frame under
mouse, store it in last_mouse_frame.  Alter event coordinates
based on mouse frame.
(syms_of_term): Add tty-frame-at, last_mouse_frame.
* src/termhooks.h: Make Gpm_Event parameter const.
---
 lisp/frame.el    |  7 -----
 lisp/xt-mouse.el |  2 +-
 src/term.c       | 70 +++++++++++++++++++++++++++++++++++++++---------
 src/termhooks.h  |  2 +-
 4 files changed, 60 insertions(+), 21 deletions(-)

diff --git a/lisp/frame.el b/lisp/frame.el
index 7da6bce697a..091e84f09c4 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -1506,13 +1506,6 @@ frame-outer-height
   (let ((edges (frame-edges frame 'outer-edges)))
     (- (nth 3 edges) (nth 1 edges))))
 
-(defun frame-at (x y)
-  "Return frame containing pixel position X, Y."
-  (cl-loop for frame in (frame-list-z-order)
-           as (x0 y0 x1 y1) = (frame-edges frame)
-           when (and (<= x0 x (1- x1)) (<= y0 y (1- y1)))
-           return frame))
-
 (declare-function x-list-fonts "xfaces.c"
                   (pattern &optional face frame maximum width))
 
diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
index ccb585ce631..19d688e4d1e 100644
--- a/lisp/xt-mouse.el
+++ b/lisp/xt-mouse.el
@@ -298,7 +298,7 @@ xterm-mouse-event
              ;; FIXME: The test for running in batch mode is here solely
              ;; for the sake of xt-mouse-tests where the only frame is
              ;; the initial frame.
-             (frame (unless noninteractive (frame-at x y)))
+             (frame (unless noninteractive (tty-frame-at x y)))
              ;;(_ (message (format "*** %S" frame)))
              (frame-pos (frame-position frame))
              ;;(_ (message (format "*** %S" frame-pos)))
diff --git a/src/term.c b/src/term.c
index 368e20803e1..10380eba4b9 100644
--- a/src/term.c
+++ b/src/term.c
@@ -141,6 +141,7 @@ #define OUTPUT1_IF(tty, a) \
 struct tty_display_info *gpm_tty = NULL;
 
 /* Last recorded mouse coordinates.  */
+static Lisp_Object last_mouse_frame;
 static int last_mouse_x, last_mouse_y;
 #endif /* HAVE_GPM */
 
@@ -2593,6 +2594,35 @@ tty_draw_row_with_mouse_face (struct window *w, struct glyph_row *row,
 
 #endif
 
+static Lisp_Object
+tty_frame_at (int x, int y)
+{
+  for (Lisp_Object frames = Ftty_frame_list_z_order (Qnil); frames != Qnil;
+       frames = Fcdr (frames))
+    {
+      Lisp_Object frame = Fcar (frames);
+      struct frame *f = XFRAME (frame);
+
+      if (f->left_pos <= x && x < f->left_pos + f->pixel_width &&
+	  f->top_pos <= y && y < f->top_pos + f->pixel_height)
+	return frame;
+    }
+
+  return Qnil;
+}
+
+DEFUN ("tty-frame-at", Ftty_frame_at, Stty_frame_at,
+       2, 2, 0,
+       doc: /* Return tty frame containing pixel position X, Y.  */)
+  (Lisp_Object x, Lisp_Object y)
+{
+  if (! FIXNUMP (x) || ! FIXNUMP (y))
+    /* Coordinates this big can not correspond to any frame.  */
+    return Qnil;
+
+  return tty_frame_at (XFIXNUM (x), XFIXNUM (y));
+}
+
 #ifdef HAVE_GPM
 
 void
@@ -2638,7 +2668,12 @@ term_mouse_position (struct frame **fp, int insist, Lisp_Object *bar_window,
 		     enum scroll_bar_part *part, Lisp_Object *x,
 		     Lisp_Object *y, Time *timeptr)
 {
-  *fp = SELECTED_FRAME ();
+  /* If we've gotten no GPM mouse events yet, last_mouse_frame won't be
+     set.  Perhaps `gpm-mouse-mode' was never active.  */
+  if (! FRAMEP (last_mouse_frame))
+    return;
+
+  *fp = XFRAME (last_mouse_frame);
   (*fp)->mouse_moved = 0;
 
   *bar_window = Qnil;
@@ -2713,9 +2748,14 @@ term_mouse_click (struct input_event *result, Gpm_Event *event,
 }
 
 int
-handle_one_term_event (struct tty_display_info *tty, Gpm_Event *event)
+handle_one_term_event (struct tty_display_info *tty, const Gpm_Event *event_in)
 {
-  struct frame *f = XFRAME (tty->top_frame);
+  Lisp_Object frame = tty_frame_at (event_in->x, event_in->y);
+  struct frame *f = decode_live_frame (frame);
+  Gpm_Event event = *event_in;
+  event.x -= f->left_pos;
+  event.y -= f->top_pos;
+
   struct input_event ie;
   int count = 0;
 
@@ -2723,30 +2763,34 @@ handle_one_term_event (struct tty_display_info *tty, Gpm_Event *event)
   ie.kind = NO_EVENT;
   ie.arg = Qnil;
 
-  if (event->type & (GPM_MOVE | GPM_DRAG))
+  if (event.type & (GPM_MOVE | GPM_DRAG))
     {
-      Gpm_DrawPointer (event->x, event->y, fileno (tty->output));
+      /* The pointer must be drawn using screen coordinates (x,y), not
+	 frame coordinates.  Use event_in which has an unmodified event
+	 directly from GPM.  */
+      Gpm_DrawPointer (event_in->x, event_in->y, fileno (tty->output));
 
       /* Has the mouse moved off the glyph it was on at the last
          sighting?  */
-      if (event->x != last_mouse_x || event->y != last_mouse_y)
+      if (event.x != last_mouse_x || event.y != last_mouse_y)
         {
-          /* FIXME: These three lines can not be moved into
+          /* FIXME: These four lines can not be moved into
              update_mouse_position unless xterm-mouse gets updated to
              generate mouse events via C code.  See
              https://lists.gnu.org/archive/html/emacs-devel/2020-11/msg00163.html */
-          last_mouse_x = event->x;
-          last_mouse_y = event->y;
+          last_mouse_frame = frame;
+          last_mouse_x = event.x;
+          last_mouse_y = event.y;
           f->mouse_moved = 1;
 
-          count += update_mouse_position (f, event->x, event->y);
+          count += update_mouse_position (f, event.x, event.y);
         }
     }
   else
     {
       f->mouse_moved = 0;
-      term_mouse_click (&ie, event, f);
-      ie.arg = tty_handle_tab_bar_click (f, event->x, event->y,
+      term_mouse_click (&ie, &event, f);
+      ie.arg = tty_handle_tab_bar_click (f, event.x, event.y,
 					 (ie.modifiers & down_modifier) != 0, &ie);
       kbd_buffer_store_event (&ie);
       count++;
@@ -4967,9 +5011,11 @@ syms_of_term (void)
   defsubr (&Stty__set_output_buffer_size);
   defsubr (&Stty__output_buffer_size);
 #endif /* !HAVE_ANDROID */
+  defsubr (&Stty_frame_at);
 #ifdef HAVE_GPM
   defsubr (&Sgpm_mouse_start);
   defsubr (&Sgpm_mouse_stop);
+  staticpro (&last_mouse_frame);
 #endif /* HAVE_GPM */
 
   defsubr (&Stty_frame_geometry);
diff --git a/src/termhooks.h b/src/termhooks.h
index b32804a57b3..0795148f1af 100644
--- a/src/termhooks.h
+++ b/src/termhooks.h
@@ -458,7 +458,7 @@ #define EVENT_INIT(event) (memset (&(event), 0, sizeof (struct input_event)), \
 
 #ifdef HAVE_GPM
 #include <gpm.h>
-extern int handle_one_term_event (struct tty_display_info *, Gpm_Event *);
+extern int handle_one_term_event (struct tty_display_info *, const Gpm_Event *);
 extern void term_mouse_moveto (int, int);
 
 /* The device for which we have enabled gpm support.  */
-- 
2.39.5