Re: [hober0@gmail.com: Re: mode-line redisplay bug]

[storm@cua.dk (Kim F. Storm)]

YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> writes:

>>>>>> On Mon, 10 Oct 2005 20:40:18 +0100, Jason Rumney <jasonr@gnu.org> said:
>
>> "Jan D." <jan.h.d@swipnet.se> writes:
>>> AFAIK co-ordinates on W32 behave the same as on X, so your patch
>>> should be OK.  It is easy to confirm anyway.
>
>> OK, I've installed the patch, but as YAMAMOTO Mitsuharu pointed out,
>> it may not deal with the left fringe width - which might be an
>> explanation for the gx calculation being off by one, and possibly
>> also the explanation for why this patch does work as I expected.
>
> A simple debug code below shows the incorrectness of the calculated
> rectangle :-).  And if it is corrected, the problem I said in
> http://lists.gnu.org/archive/html/emacs-pretest-bug/2005-06/msg00148.html
> will appear.


I think this should be solved by making remember_mouse_glyph generic
for all platforms and handle all window parts sensibly (the old code
in glyph_rect had many problems).

Below is a patch which does this, but I have only tested it on X.
Could somebody test it on W32 and MAC?

But the MAC version doesn't actually call "remember_mouse_glyph"
anywhere (it uses pixel_to_glyph_coords as the other versions used to
do) so I don't expect this to have any effect on the Mac...?


*** dispextern.h	07 Oct 2005 23:39:34 +0200	1.210
--- dispextern.h	11 Oct 2005 12:01:06 +0200
***************
*** 2615,2620 ****
--- 2615,2622 ----
  void pixel_to_glyph_coords P_ ((struct frame *, int, int, int *, int *,
  				NativeRectangle *, int));
  int glyph_to_pixel_coords P_ ((struct window *, int, int, int *, int *));
+ void remember_mouse_glyph P_ ((struct frame *, int, int, NativeRectangle *));
+
  void mark_window_display_accurate P_ ((Lisp_Object, int));
  void redisplay_preserve_echo_area P_ ((int));
  void set_cursor_from_row P_ ((struct window *, struct glyph_row *,

*** xdisp.c	07 Oct 2005 10:53:12 +0200	1.1058
--- xdisp.c	11 Oct 2005 12:05:41 +0200
***************
*** 2027,2032 ****
--- 2027,2199 ----
    return WINDOW_TO_FRAME_PIXEL_Y (w, y);
  }

+ /*
+  * Remember which glyph the mouse is over.
+  */
+
+ void
+ remember_mouse_glyph (f, gx, gy, rect)
+      struct frame *f;
+      int gx, gy;
+      NativeRectangle *rect;
+ {
+   Lisp_Object window;
+   struct window *w;
+   struct glyph_row *r, *gr, *end_row;
+   enum window_part part;
+   enum glyph_row_area area;
+   int x, y, width, height;
+
+   /* Try to determine frame pixel position and size of the glyph under
+      frame pixel coordinates X/Y on frame F.  */
+
+   width = FRAME_SMALLEST_CHAR_WIDTH (f);
+   height = FRAME_SMALLEST_FONT_HEIGHT (f);
+
+   window = window_from_coordinates (f, gx, gy, &part, &x, &y, 0);
+   if (NILP (window))
+     goto virtual_glyph;
+
+   w = XWINDOW (window);
+   width = WINDOW_FRAME_COLUMN_WIDTH (w);
+   height = WINDOW_FRAME_LINE_HEIGHT (w);
+
+   r = MATRIX_FIRST_TEXT_ROW (w->current_matrix);
+   end_row = r + w->current_matrix->nrows - 1;
+
+   if (w->pseudo_window_p)
+     {
+       area = TEXT_AREA;
+       part = ON_MODE_LINE; /* Don't adjust margin. */
+       goto text_glyph;
+     }
+
+   switch (part)
+     {
+     case ON_LEFT_MARGIN:
+       area = LEFT_MARGIN_AREA;
+       goto text_glyph;
+
+     case ON_RIGHT_MARGIN:
+       area = RIGHT_MARGIN_AREA;
+       goto text_glyph;
+
+     case ON_TEXT:
+     case ON_MODE_LINE:
+     case ON_HEADER_LINE:
+       area = TEXT_AREA;
+       goto text_glyph;
+
+     case ON_LEFT_FRINGE:
+       x = (WINDOW_HAS_FRINGES_OUTSIDE_MARGINS (w)
+ 	   ? WINDOW_LEFT_SCROLL_BAR_AREA_WIDTH (w)
+ 	   : window_box_right_offset (w, LEFT_MARGIN_AREA));
+       width = WINDOW_LEFT_FRINGE_WIDTH (w);
+       goto row_glyph;
+
+     case ON_RIGHT_FRINGE:
+       x = (WINDOW_HAS_FRINGES_OUTSIDE_MARGINS (w)
+ 	   ? window_box_right_offset (w, RIGHT_MARGIN_AREA)
+ 	   : window_box_right_offset (w, TEXT_AREA));
+       width = WINDOW_RIGHT_FRINGE_WIDTH (w);
+       goto row_glyph;
+
+     case ON_SCROLL_BAR:
+       x = (WINDOW_HAS_VERTICAL_SCROLL_BAR_ON_LEFT (w)
+ 	   ? 0
+ 	   : (window_box_right_offset (w, RIGHT_MARGIN_AREA)
+ 	      + (WINDOW_HAS_FRINGES_OUTSIDE_MARGINS (w)
+ 		 ? WINDOW_RIGHT_FRINGE_WIDTH (w)
+ 		 : 0)));
+       width = WINDOW_SCROLL_BAR_AREA_WIDTH (w);
+       goto row_glyph;
+
+     default:
+       goto virtual_glyph;
+     }
+
+  text_glyph:
+   gr = 0; gy = 0;
+   for (; r < end_row && r->enabled_p && r->y + r->height < y; ++r)
+     gr = r; gy = r->y;
+
+   if (gr && gy <= y && gy + gr->height > y)
+     {
+       struct glyph *g = gr->glyphs[area];
+       struct glyph *end = g + gr->used[area];
+
+       height = gr->height;
+       gx = gr->x;
+       while (g < end && gx < x)
+ 	gx += g->pixel_width, ++g;
+
+       if (g < end)
+ 	width = g->pixel_width;
+       else
+ 	{
+ 	  /* Use nominal char spacing at end of line.  */
+ 	  x -= gx;
+ 	  gx += (x / width) * width;
+ 	}
+
+       if (part != ON_MODE_LINE && part != ON_HEADER_LINE)
+ 	gx += window_box_left_offset (w, area);
+     }
+   else
+     {
+       /* Use nominal line height at end of window.  */
+       gx = (x / width) * width;
+       y -= gy;
+       gy += (y / height) * height;
+     }
+
+   STORE_NATIVE_RECT (*rect,
+ 		     WINDOW_TO_FRAME_PIXEL_X (w, gx),
+ 		     WINDOW_TO_FRAME_PIXEL_Y (w, gy),
+ 		     width,
+ 		     height);
+   return;
+
+  row_glyph:
+   gr = 0, gy = 0;
+   for (; r < end_row && r->enabled_p && r->y + r->height < y; ++r)
+     gr = r, gy = r->y;
+
+   if (gr && gy <= y && gy + gr->height > y)
+     height = gr->height;
+   else
+     {
+       /* Use nominal line height at end of window.  */
+       y -= gy;
+       gy += (y / height) * height;
+     }
+
+   STORE_NATIVE_RECT (*rect,
+ 		     WINDOW_TO_FRAME_PIXEL_X (w, gx),
+ 		     WINDOW_TO_FRAME_PIXEL_Y (w, gy),
+ 		     width,
+ 		     height);
+   return;
+
+
+  virtual_glyph:
+   /* If there is no glyph under the mouse, then we divide the screen
+      into a grid of the smallest glyph in the frame, and use that
+      as our "glyph".  */
+
+   /* Arrange for the division in FRAME_PIXEL_X_TO_COL etc. to
+      round down even for negative values.  */
+   if (gx < 0)
+     gx -= width - 1;
+   if (gy < 0)
+     gy -= height - 1;
+
+   gx = (gx / width) * width;
+   gy = (gy / width) * width;
+
+   STORE_NATIVE_RECT (*rect, gx, gy, width, height);
+ }
+

  #endif /* HAVE_WINDOW_SYSTEM */

*** xterm.c	11 Oct 2005 00:27:24 +0200	1.879
--- xterm.c	11 Oct 2005 12:00:11 +0200
***************
*** 3582,3589 ****
  static XMotionEvent last_mouse_motion_event;
  static Lisp_Object last_mouse_motion_frame;

- static void remember_mouse_glyph P_ ((struct frame *, int, int));
-
  static void
  note_mouse_movement (frame, event)
       FRAME_PTR frame;
--- 3582,3587 ----
***************
*** 3610,3616 ****
        last_mouse_scroll_bar = Qnil;
        note_mouse_highlight (frame, event->x, event->y);
        /* Remember which glyph we're now on.  */
!       remember_mouse_glyph (frame, event->x, event->y);
      }
  }

--- 3608,3614 ----
        last_mouse_scroll_bar = Qnil;
        note_mouse_highlight (frame, event->x, event->y);
        /* Remember which glyph we're now on.  */
!       remember_mouse_glyph (frame, event->x, event->y, &last_mouse_glyph);
      }
  }

***************
*** 3630,3727 ****
  }


- static int glyph_rect P_ ((struct frame *f, int, int, XRectangle *));
-
-
- /* Try to determine frame pixel position and size of the glyph under
-    frame pixel coordinates X/Y on frame F .  Return the position and
-    size in *RECT.  Value is non-zero if we could compute these
-    values.  */
-
- static int
- glyph_rect (f, x, y, rect)
-      struct frame *f;
-      int x, y;
-      XRectangle *rect;
- {
-   Lisp_Object window;
-   struct window *w;
-   struct glyph_row *r, *end_row;
-   enum window_part part;
-
-   window = window_from_coordinates (f, x, y, &part, &x, &y, 0);
-   if (NILP (window))
-     return 0;
-
-   w = XWINDOW (window);
-   r = MATRIX_FIRST_TEXT_ROW (w->current_matrix);
-   end_row = r + w->current_matrix->nrows - 1;
-
-   if (part != ON_TEXT)
-     return 0;
-
-   for (; r < end_row && r->enabled_p; ++r)
-     {
-       if (r->y >= y)
- 	{
- 	  struct glyph *g = r->glyphs[TEXT_AREA];
- 	  struct glyph *end = g + r->used[TEXT_AREA];
- 	  int gx = r->x;
- 	  while (g < end && gx < x)
- 	    gx += g->pixel_width, ++g;
- 	  if (g < end)
- 	    {
- 	      rect->width = g->pixel_width;
- 	      rect->height = r->height;
- 	      rect->x = WINDOW_TO_FRAME_PIXEL_X (w, gx);
- 	      rect->y = WINDOW_TO_FRAME_PIXEL_Y (w, r->y);
- 	      return 1;
- 	    }
- 	  break;
- 	}
-     }
-
-   return 0;
- }
-
-
- /* Remember which glyph the mouse is over.
-  */
- static void
- remember_mouse_glyph (f1, win_x, win_y)
-      FRAME_PTR f1;
-      int win_x, win_y;
- {
-   int width, height, gx, gy;
-
-   /* Try getting the rectangle of the actual glyph.  */
-   if (!glyph_rect (f1, win_x, win_y, &last_mouse_glyph))
-     {
-       /* If there is no glyph under the mouse, then we divide the screen
- 	 into a grid of the smallest glyph in the frame, and use that
- 	 as our "glyph".  */
-       width = FRAME_SMALLEST_CHAR_WIDTH (f1);
-       height = FRAME_SMALLEST_FONT_HEIGHT (f1);
-       gx = win_x;
-       gy = win_y;
-
-       /* Arrange for the division in FRAME_PIXEL_X_TO_COL etc. to
- 	 round down even for negative values.  */
-       if (gx < 0)
- 	gx -= width - 1;
-       if (gy < 0)
- 	gy -= height - 1;
-
-       gx = gx / width * width;
-       gy = gy / width * width;
-
-       last_mouse_glyph.width  = width;
-       last_mouse_glyph.height = height;
-       last_mouse_glyph.x = gx;
-       last_mouse_glyph.y = gy;
-     }
- }
-

  /* Return the current position of the mouse.
     *FP should be a frame which indicates which display to ask about.
--- 3628,3633 ----
***************
*** 3909,3915 ****
  	       on it, i.e. into the same rectangles that matrices on
  	       the frame are divided into.  */

! 	    remember_mouse_glyph (f1, win_x, win_y);

  	    *bar_window = Qnil;
  	    *part = 0;
--- 3815,3821 ----
  	       on it, i.e. into the same rectangles that matrices on
  	       the frame are divided into.  */

! 	    remember_mouse_glyph (f1, win_x, win_y, &last_mouse_glyph);

  	    *bar_window = Qnil;
  	    *part = 0;

*** w32term.c	07 Oct 2005 10:53:11 +0200	1.232
--- w32term.c	11 Oct 2005 12:11:18 +0200
***************
*** 3204,3211 ****
  static MSG last_mouse_motion_event;
  static Lisp_Object last_mouse_motion_frame;

- static void remember_mouse_glyph P_ ((struct frame *, int, int));
-
  static void
  note_mouse_movement (frame, msg)
       FRAME_PTR frame;
--- 3204,3209 ----
***************
*** 3227,3235 ****

    /* Has the mouse moved off the glyph it was on at the last sighting?  */
    else if (mouse_x < last_mouse_glyph.left
! 	   || mouse_x > last_mouse_glyph.right
  	   || mouse_y < last_mouse_glyph.top
! 	   || mouse_y > last_mouse_glyph.bottom)
      {
        frame->mouse_moved = 1;
        last_mouse_scroll_bar = Qnil;
--- 3225,3233 ----

    /* Has the mouse moved off the glyph it was on at the last sighting?  */
    else if (mouse_x < last_mouse_glyph.left
! 	   || mouse_x >= last_mouse_glyph.right
  	   || mouse_y < last_mouse_glyph.top
! 	   || mouse_y >= last_mouse_glyph.bottom)
      {
        frame->mouse_moved = 1;
        last_mouse_scroll_bar = Qnil;
***************
*** 3238,3244 ****
  	 gets called when mouse tracking is enabled but we also need
  	 to keep track of the mouse for help_echo and highlighting at
  	 other times.  */
!       remember_mouse_glyph (frame, mouse_x, mouse_y);
      }
  }

--- 3236,3242 ----
  	 gets called when mouse tracking is enabled but we also need
  	 to keep track of the mouse for help_echo and highlighting at
  	 other times.  */
!       remember_mouse_glyph (frame, mouse_x, mouse_y, &last_mouse_glyph);
      }
  }

***************
*** 3250,3257 ****
  static struct scroll_bar *x_window_to_scroll_bar ();
  static void x_scroll_bar_report_motion ();
  static void x_check_fullscreen P_ ((struct frame *));
- static int glyph_rect P_ ((struct frame *f, int, int, RECT *));
-

  static void
  redo_mouse_highlight ()
--- 3248,3253 ----
***************
*** 3270,3377 ****
  {
    PostMessage (window, WM_EMACS_SETCURSOR, (WPARAM) cursor, 0);
  }
-
- /* Try to determine frame pixel position and size of the glyph under
-    frame pixel coordinates X/Y on frame F .  Return the position and
-    size in *RECT.  Value is non-zero if we could compute these
-    values.  */
-
- static int
- glyph_rect (f, x, y, rect)
-      struct frame *f;
-      int x, y;
-      RECT *rect;
- {
-   Lisp_Object window;
-
-   window = window_from_coordinates (f, x, y, 0, &x, &y, 0);
-
-   if (!NILP (window))
-     {
-       struct window *w = XWINDOW (window);
-       struct glyph_row *r = MATRIX_FIRST_TEXT_ROW (w->current_matrix);
-       struct glyph_row *end = r + w->current_matrix->nrows - 1;
-
-       for (; r < end && r->enabled_p; ++r)
- 	if (r->y <= y && r->y + r->height > y)
- 	  {
- 	    /* Found the row at y.  */
- 	    struct glyph *g = r->glyphs[TEXT_AREA];
- 	    struct glyph *end = g + r->used[TEXT_AREA];
- 	    int gx;
-
- 	    rect->top = WINDOW_TO_FRAME_PIXEL_Y (w, r->y);
- 	    rect->bottom = rect->top + r->height;
-
- 	    if (x < r->x)
- 	      {
- 		/* x is to the left of the first glyph in the row.  */
- 		/* Shouldn't this be a pixel value?
- 		   WINDOW_LEFT_EDGE_X (w) seems to be the right value.
- 		   ++KFS */
- 		rect->left = WINDOW_LEFT_EDGE_COL (w);
- 		rect->right = WINDOW_TO_FRAME_PIXEL_X (w, r->x);
- 		return 1;
- 	      }
-
- 	    for (gx = r->x; g < end; gx += g->pixel_width, ++g)
- 	      if (gx <= x && gx + g->pixel_width > x)
- 		{
- 		  /* x is on a glyph.  */
- 		  rect->left = WINDOW_TO_FRAME_PIXEL_X (w, gx);
- 		  rect->right = rect->left + g->pixel_width;
- 		  return 1;
- 		}
-
- 	    /* x is to the right of the last glyph in the row.  */
- 	    rect->left = WINDOW_TO_FRAME_PIXEL_X (w, gx);
- 	    /* Shouldn't this be a pixel value?
- 	       WINDOW_RIGHT_EDGE_X (w) seems to be the right value.
- 	       ++KFS */
- 	    rect->right = WINDOW_RIGHT_EDGE_COL (w);
- 	    return 1;
- 	  }
-     }
-
-   /* The y is not on any row.  */
-   return 0;
- }
-
- /* Record the position of the mouse in last_mouse_glyph.  */
- static void
- remember_mouse_glyph (f1, gx, gy)
-      struct frame * f1;
-      int gx, gy;
- {
-   if (!glyph_rect (f1, gx, gy, &last_mouse_glyph))
-     {
-       int width = FRAME_SMALLEST_CHAR_WIDTH (f1);
-       int height = FRAME_SMALLEST_FONT_HEIGHT (f1);
-
-       /* Arrange for the division in FRAME_PIXEL_X_TO_COL etc. to
- 	 round down even for negative values.  */
-       if (gx < 0)
- 	gx -= width - 1;
-       if (gy < 0)
- 	gy -= height - 1;
- #if 0
-       /* This was the original code from XTmouse_position, but it seems
- 	 to give the position of the glyph diagonally next to the one
- 	 the mouse is over.  */
-       gx = (gx + width - 1) / width * width;
-       gy = (gy + height - 1) / height * height;
- #else
-       gx = gx / width * width;
-       gy = gy / height * height;
- #endif
-
-       last_mouse_glyph.left = gx;
-       last_mouse_glyph.top = gy;
-       last_mouse_glyph.right  = gx + width;
-       last_mouse_glyph.bottom = gy + height;
-     }
- }
-
  /* Return the current position of the mouse.
     *fp should be a frame which indicates which display to ask about.

--- 3266,3271 ----
***************
*** 3474,3480 ****
  				   || insist);
  #else
  	    ScreenToClient (FRAME_W32_WINDOW (f1), &pt);
! 	    remember_mouse_glyph (f1, pt.x, pt.y);
  #endif

  	    *bar_window = Qnil;
--- 3368,3374 ----
  				   || insist);
  #else
  	    ScreenToClient (FRAME_W32_WINDOW (f1), &pt);
! 	    remember_mouse_glyph (f1, pt.x, pt.y, &last_mouse_glyph);
  #endif

  	    *bar_window = Qnil;

*** macterm.c	11 Oct 2005 09:46:53 +0200	1.134
--- macterm.c	11 Oct 2005 12:17:44 +0200
***************
*** 4198,4206 ****
  			      Mouse Face
   ************************************************************************/

- static int glyph_rect P_ ((struct frame *f, int, int, Rect *));
-
-
  /* MAC TODO:  This should be called from somewhere (or removed)  ++KFS */

  static void
--- 4198,4203 ----
***************
*** 4214,4323 ****
  }


- /* Try to determine frame pixel position and size of the glyph under
-    frame pixel coordinates X/Y on frame F .  Return the position and
-    size in *RECT.  Value is non-zero if we could compute these
-    values.  */
-
- static int
- glyph_rect (f, x, y, rect)
-      struct frame *f;
-      int x, y;
-      Rect *rect;
- {
-   Lisp_Object window;
-
-   window = window_from_coordinates (f, x, y, 0, &x, &y, 0);
-
-   if (!NILP (window))
-     {
-       struct window *w = XWINDOW (window);
-       struct glyph_row *r = MATRIX_FIRST_TEXT_ROW (w->current_matrix);
-       struct glyph_row *end = r + w->current_matrix->nrows - 1;
-
-       for (; r < end && r->enabled_p; ++r)
- 	if (r->y <= y && r->y + r->height > y)
- 	  {
- 	    /* Found the row at y.  */
- 	    struct glyph *g = r->glyphs[TEXT_AREA];
- 	    struct glyph *end = g + r->used[TEXT_AREA];
- 	    int gx;
-
- 	    rect->top = WINDOW_TO_FRAME_PIXEL_Y (w, r->y);
- 	    rect->bottom = rect->top + r->height;
-
- 	    if (x < r->x)
- 	      {
- 		/* x is to the left of the first glyph in the row.  */
- 		/* Shouldn't this be a pixel value?
- 		   WINDOW_LEFT_EDGE_X (w) seems to be the right value.
- 		   ++KFS */
- 		rect->left = WINDOW_LEFT_EDGE_COL (w);
- 		rect->right = WINDOW_TO_FRAME_PIXEL_X (w, r->x);
- 		return 1;
- 	      }
-
- 	    for (gx = r->x; g < end; gx += g->pixel_width, ++g)
- 	      if (gx <= x && gx + g->pixel_width > x)
- 		{
- 		  /* x is on a glyph.  */
- 		  rect->left = WINDOW_TO_FRAME_PIXEL_X (w, gx);
- 		  rect->right = rect->left + g->pixel_width;
- 		  return 1;
- 		}
-
- 	    /* x is to the right of the last glyph in the row.  */
- 	    rect->left = WINDOW_TO_FRAME_PIXEL_X (w, gx);
- 	    /* Shouldn't this be a pixel value?
- 	       WINDOW_RIGHT_EDGE_X (w) seems to be the right value.
- 	       ++KFS */
- 	    rect->right = WINDOW_RIGHT_EDGE_COL (w);
- 	    return 1;
- 	  }
-     }
-
-   /* The y is not on any row.  */
-   return 0;
- }
-
- /* MAC TODO:  This should be called from somewhere (or removed)  ++KFS */
-
- /* Record the position of the mouse in last_mouse_glyph.  */
- static void
- remember_mouse_glyph (f1, gx, gy)
-      struct frame * f1;
-      int gx, gy;
- {
-   if (!glyph_rect (f1, gx, gy, &last_mouse_glyph))
-     {
-       int width = FRAME_SMALLEST_CHAR_WIDTH (f1);
-       int height = FRAME_SMALLEST_FONT_HEIGHT (f1);
-
-       /* Arrange for the division in FRAME_PIXEL_X_TO_COL etc. to
- 	 round down even for negative values.  */
-       if (gx < 0)
- 	gx -= width - 1;
-       if (gy < 0)
- 	gy -= height - 1;
- #if 0
-       /* This was the original code from XTmouse_position, but it seems
- 	 to give the position of the glyph diagonally next to the one
- 	 the mouse is over.  */
-       gx = (gx + width - 1) / width * width;
-       gy = (gy + height - 1) / height * height;
- #else
-       gx = gx / width * width;
-       gy = gy / height * height;
- #endif
-
-       last_mouse_glyph.left = gx;
-       last_mouse_glyph.top = gy;
-       last_mouse_glyph.right  = gx + width;
-       last_mouse_glyph.bottom = gy + height;
-     }
- }
-
-
  static struct frame *
  mac_focus_frame (dpyinfo)
       struct mac_display_info *dpyinfo;
--- 4211,4216 ----

--
Kim F. Storm <storm@cua.dk> http://www.cua.dk