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

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

[Richard M. Stallman]

His test case is very clear, but it does not fail when I try it.
Can anyone else observe this failure?

------- Start of forwarded message -------
To: emacs-pretest-bug@gnu.org
From: Edward O'Connor <hober0@gmail.com>
Date: Thu, 11 Aug 2005 09:17:55 -0700
Organization: Church of Emacs
Mime-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Subject: Re: mode-line redisplay bug
X-Spam-Status: No, hits=0.0 required=5.0 tests=none autolearn=no version=2.63

- --=-=-=

Several months ago, I wrote:

> Every so often for a while I've occasionally seen this odd redisplay
> bug, and I now think I know about it to report it. Anyway, here's what
> happens: I set `mode-line-format' to nil in my eshell buffers. I
> switch to an ERC buffer with the command `erc-track-switch-buffer'
> (which is just a cute wrapper around `switch-to-buffer'). My mode-line
> in the ERC buffer upon such a buffer switch will often be garbled,
> with only part of it appearing, as you can see in this screenshot:
>
> http://edward.oconnor.cx/tmp/emacs/bug1.png
>
> I thought that the bug might have something to do with my custom
> `mode-line' face, but as you can see in this other screenshot, it
> occurs with default Emacs faces as well:
>
> http://edward.oconnor.cx/tmp/emacs/bug2.png
>
> (For completeness sake, here's what a normal ERC buffer looks like:
> http://edward.oconnor.cx/tmp/emacs/normal.png)
>
> I imagine the redisplay code is trying to only redraw those parts of the
> mode-line that have changed, or something like that. Nevertheless, it
> does seem that the redisplay engine doesn't realize that, when switching
> from a buffer without a mode-line, *everything in the mode-line* needs
> to be redisplayed.

(For the record, I've seen this bug under X11, Mac OS X, and w32.)

Richard Stallman replied:

> Can you please send a precise self-contained test case for this bug?

And I've finally written a self-contained test case (attached):


- --=-=-=
Content-Type: application/emacs-lisp
Content-Disposition: inline; filename=redisplay-bug.el
Content-Description: test case for redisplay bug

;; Step 0: Launch an Emacs on some variety of window-system.

;; Step 1: Ensure that there's something in the mode-line that changes
;;         periodically.

(defvar frob " hello")

(add-to-list 'minor-mode-alist '(t frob))

(defun frob-it ()
  "Change `frob' to a random string, and force a mode-line update."
  (setq frob (concat " " (make-string (+ 2 (random 6))
				      (+ 97 (random 26)))))
  (force-mode-line-update t))

(run-with-timer 5 5 'frob-it)

;; Step 2: Ensure that there's a buffer with no mode-line.

(with-current-buffer (get-buffer "*scratch*")
  (setq header-line-format mode-line-format
	mode-line-format   nil))

;; Step 3: Make a key binding for switching between the buffer with no
;;         mode-line and a buffer with a mode-line which uses
;;         `switch-to-buffer'.

(global-set-key (kbd "C-c c")
		(lambda ()
		  (interactive)
		  (if (eq (current-buffer) (get-buffer "*Messages*"))
		      (switch-to-buffer (get-buffer "*scratch*"))
		    (switch-to-buffer (get-buffer "*Messages*")))))

;; Step 4: To observe the bug, switch to *scratch*, wait for the
;;         header-line to change, and hit C-c c. More often than not,
;;         the mode-line in *Messages* will be only partially
;;         displayed. (Try this several times, by repeated use of the
;;         C-c c key binding, if you don't observe the effect right
;;         away.) Typing C-l (unsurprisingly) fixes the display.

- --=-=-=
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
Emacs-pretest-bug mailing list
Emacs-pretest-bug@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-pretest-bug

- --=-=-=--
------- End of forwarded message -------

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

[Eli Zaretskii]

> From: "Richard M. Stallman" <rms@gnu.org>
> Date: Fri, 12 Aug 2005 10:59:21 -0400
> 
> His test case is very clear, but it does not fail when I try it.

FWIW, it doesn't fail for me, either, with today's CVS on w32.

Since he says he saw this on several different platforms, while you
don't see this on GNU/Linux, my crystal ball says it has something to
do with customizations.

Edward, can you please recheck if this problem happens with "emacs -Q"?

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

[Edward O'Connor]

> Edward, can you please recheck if this problem happens with "emacs -Q"?

It still happens with emacs -q --no-site-file.


Ted

-- 
Edward O'Connor
hober0@gmail.com

Ense petit placidam sub libertate quietem.

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

[Edward O'Connor]

>> Edward, can you please recheck if this problem happens with "emacs -Q"?
>
> It still happens with emacs -q --no-site-file.

And emacs -Q (just checked).


Ted

-- 
Edward O'Connor
hober0@gmail.com

Ense petit placidam sub libertate quietem.

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

[Robert J. Chassell]

Today's GNU Emacs CVS snapshot, Fri, 2005 Aug 12  15:50 UTC
GNU Emacs 22.0.50.68 (i686-pc-linux-gnu, GTK+ Version 2.6.8)
started with 

    emacs/src/emacs -q --no-site-file

> His test case is very clear, but it does not fail when I try it.

Good test case, but I do not see the mode-line redisplay bug, either.

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

[Henrik Enberg]

> From: Edward O'Connor <hober0@gmail.com>
> Date: Fri, 12 Aug 2005 10:19:09 -0700
> 
> > Edward, can you please recheck if this problem happens with "emacs -Q"?
> 
> It still happens with emacs -q --no-site-file.

FWIW, it happens for me too on GNU/Linux with emacs -q --no-site-file

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

[Jason Rumney]

"Richard M. Stallman" <rms@gnu.org> writes:

> His test case is very clear, but it does not fail when I try it.
> Can anyone else observe this failure?

I don't see this failure, but while trying to replicate it, I did see
some other display problems. I tried GNU/Linux and W32, I saw problems
on both when the mouse was over mouse-sensitive areas of the modeline,
but the symptoms were different.

If I hold the mouse above a mouse-sensitive part of the modeline while
switching buffers with C-c c as described in the original bug report,
I see the following:

On W32:

The highlighted portion of the modeline stays when the buffer
switches, overwriting any text that from the buffer that should appear
there. If I do the same in the header line of the *scratch* buffer, 
the entire header line is erased as it should be.

On GNU/Linux:

The highlit area flickers. On W32 it flickers once when the tooltip
pops up, but on X, it flickers constantly. I remember fixing something
like this on W32 years ago, it was in the code that detected mouse
movement - movement events were being sent for zero movement when
inside track-mouse forms. But my recollection is that the same bug did
not appear on X at the time, so even though the code appeared to be
the same, I did not try to apply my fix to the X code.

If C-c c is pressed before the tooltip pops up, nothing happens until
the tooltip-delay expires, then the tooltip flashes up for a brief
instant and the buffer switch takes place.

Both problems I saw on GNU/Linux appear in both the header-line and
mode-line.

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

[Richard M. Stallman]

    On GNU/Linux:

    The highlit area flickers. On W32 it flickers once when the tooltip
    pops up, but on X, it flickers constantly. I remember fixing something
    like this on W32 years ago, it was in the code that detected mouse
    movement - movement events were being sent for zero movement when
    inside track-mouse forms. But my recollection is that the same bug did
    not appear on X at the time, so even though the code appeared to be
    the same, I did not try to apply my fix to the X code.

    If C-c c is pressed before the tooltip pops up, nothing happens until
    the tooltip-delay expires, then the tooltip flashes up for a brief
    instant and the buffer switch takes place.

    Both problems I saw on GNU/Linux appear in both the header-line and
    mode-line.

Can anyone else reproduce this?

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

[Jason Rumney]

Jason Rumney <jasonr@gnu.org> writes:

> "Richard M. Stallman" <rms@gnu.org> writes:
>
>> His test case is very clear, but it does not fail when I try it.
>> Can anyone else observe this failure?
>
> I don't see this failure, but while trying to replicate it, I did see
> some other display problems. I tried GNU/Linux and W32, I saw problems
> on both when the mouse was over mouse-sensitive areas of the modeline,
> but the symptoms were different.
>
> If I hold the mouse above a mouse-sensitive part of the modeline while
> switching buffers with C-c c as described in the original bug report,
> I see the following:
>
> On W32:
>
> The highlighted portion of the modeline stays when the buffer
> switches, overwriting any text that from the buffer that should appear
> there. If I do the same in the header line of the *scratch* buffer, 
> the entire header line is erased as it should be.
>
> On GNU/Linux:
>
> The highlit area flickers. On W32 it flickers once when the tooltip
> pops up, but on X, it flickers constantly. I remember fixing something
> like this on W32 years ago,

I think the change I am recalling is the following one. Unfortunately
I checked it in along with a load of catch-up changes ported from
xterm.c, so the CVS diff will need a bit of filtering. Unless someone
else picks this up first, I hope to get a chance to look at it
sometime in the next couple of weeks.

2001-10-21  Jason Rumney  <jasonr@gnu.org>

	* w32term.c (remember_mouse_glyph): New function.
	(w32_mouse_position): Use it.
	(note_mouse_movement): If the mouse moved off the glyph, remember
	its new position.

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

[Kim F. Storm]

Henrik Enberg <henrik.enberg@telia.com> writes:

>> From: Edward O'Connor <hober0@gmail.com>
>> Date: Fri, 12 Aug 2005 10:19:09 -0700
>> 
>> > Edward, can you please recheck if this problem happens with "emacs -Q"?
>> 
>> It still happens with emacs -q --no-site-file.
>
> FWIW, it happens for me too on GNU/Linux with emacs -q --no-site-file

It didn't happen when I just executed the code manually in *scratch*, but
it does happen for me if I save Edward's instructions in a file xx.el,
and run emacs like this:

      emacs -Q -D -l xx.el

---- xx.el:

;; Step 0: Launch an Emacs on some variety of window-system.

;; Step 1: Ensure that there's something in the mode-line that changes
;;         periodically.

(defvar frob " hello")

(add-to-list 'minor-mode-alist '(t frob))

(defun frob-it ()
  "Change `frob' to a random string, and force a mode-line update."
  (setq frob (concat " " (make-string (+ 2 (random 6))
				      (+ 97 (random 26)))))
  (force-mode-line-update t))

(run-with-timer 5 5 'frob-it)

;; Step 2: Ensure that there's a buffer with no mode-line.

(with-current-buffer (get-buffer "*scratch*")
  (setq header-line-format mode-line-format
	mode-line-format   nil))

;; Step 3: Make a key binding for switching between the buffer with no
;;         mode-line and a buffer with a mode-line which uses
;;         `switch-to-buffer'.

(global-set-key (kbd "C-c c")
		(lambda ()
		  (interactive)
		  (if (eq (current-buffer) (get-buffer "*Messages*"))
		      (switch-to-buffer (get-buffer "*scratch*"))
		    (switch-to-buffer (get-buffer "*Messages*")))))

;; Step 4: To observe the bug, switch to *scratch*, wait for the
;;         header-line to change, and hit C-c c. More often than not,
;;         the mode-line in *Messages* will be only partially
;;         displayed. (Try this several times, by repeated use of the
;;         C-c c key binding, if you don't observe the effect right
;;         away.) Typing C-l (unsurprisingly) fixes the display.

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

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

[Jason Rumney]

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

Jason Rumney <jasonr@gnu.org> writes:

> On GNU/Linux:
>
> The highlit area flickers. On W32 it flickers once when the tooltip
> pops up, but on X, it flickers constantly. I remember fixing something
> like this on W32 years ago, it was in the code that detected mouse
> movement - movement events were being sent for zero movement when
> inside track-mouse forms. But my recollection is that the same bug did
> not appear on X at the time, so even though the code appeared to be
> the same, I did not try to apply my fix to the X code.

I have looked at this and narrowed down the changes I made to
w32term.c. Looking at the code again, I am sure that my changes should
be applied to xterm.c as well, but applying them does not seem to make
any difference to the flickering. It probably needs an X expert to
look at it as I may have some misunderstanding here.

In note_mouse_movement, which is called from handle_one_xevent in
response to several types of event, we check the event's mouse position
against last_mouse_glyph, to limit the number of events we pass
through to lisp.

last_mouse_glyph is updated in XTmouse_position, nowhere else as far
as I can tell. XTmouse_position is installed as mouse_position_hook.
As far as I can tell, it is called only when do_mouse_tracking is
non-nil (in keyboard.c), or when the functions mouse-position and
mouse-pixel-position (both in frame.c) are explicitly called.

This means that last_mouse_glyph normally won't get updated, so it
doesn't serve the purpose that we use it for.

I fixed this in w32term.c by factoring out the code that updates
last_mouse_glyph from w32_mouse_position (the equivalent of
XTmouse_position) into a new function remember_mouse_glyph, and call
it from note_mouse_movement when we notice the glyph has changed, to
ensure it is always up to date.

Having done that, I also noticed the following code from
XTmouse_position:

      /* 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 - 1) / width * width;
      gy = (gy + height - 1) / height * height;

      last_mouse_glyph.width  = width;
      last_mouse_glyph.height = height;
      last_mouse_glyph.x = gx;
      last_mouse_glyph.y = gy;

gx and gy are initially the position of the mouse. We are trying to
find the rectangle around the glyph under the mouse here.

On Windows, I found that this code returns the rectangle of a glyph
diagonally adjacent to the glyph we want. I am not sure if I am
missing something about the way X co-ordinates work, but I think the
two lines that round gx and gy should be simply:

      gx = gx / width * width;
      gy = gy / height * height;



Here is the full patch adapted to xterm.c. As I said, it does not seem
to have any effect, so it needs someone more familiar with X to look
at it further:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: xterm.patch --]
[-- Type: text/x-patch, Size: 2180 bytes --]

3584a3585,3586
> static void remember_mouse_glyph P_ ((struct frame *, int, int));
> 
3609a3612,3613
>       /* Remember which glyph we're now on.  */
>       remember_mouse_glyph (frame, event->x, event->y);
3679a3684,3724
> /* 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;
> #if 0
>       gx = (gx + width - 1) / width * width;
>       gy = (gy + height - 1) / height * height;
> #else
>       gx = gx / width * width;
>       gy = gy / width * width;
> #endif
>       last_mouse_glyph.width  = width;
>       last_mouse_glyph.height = height;
>       last_mouse_glyph.x = gx;
>       last_mouse_glyph.y = gy;
>     }
> }
> 
> 
3866,3891c3911
< 	    int width, height, gx, gy;
< 	    XRectangle rect;
< 
< 	    if (glyph_rect (f1, win_x, win_y, &rect))
< 	      last_mouse_glyph = rect;
< 	    else
< 	      {
< 		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 - 1) / width * width;
< 		gy = (gy + height - 1) / height * height;
< 
< 		last_mouse_glyph.width  = width;
< 		last_mouse_glyph.height = height;
< 		last_mouse_glyph.x = gx;
< 		last_mouse_glyph.y = gy;
< 	      }
---
> 	    remember_mouse_glyph (f1, win_x, win_y);

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

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

[mituharu]

> Jason Rumney <jasonr@gnu.org> writes:

> I fixed this in w32term.c by factoring out the code that updates
> last_mouse_glyph from w32_mouse_position (the equivalent of
> XTmouse_position) into a new function remember_mouse_glyph, and call
> it from note_mouse_movement when we notice the glyph has changed, to
> ensure it is always up to date.

To use last_mouse_glyph properly, screen update at that rectangle
should also be detected before using the rectangle recorded on
the last mouse movement.

  http://lists.gnu.org/archive/html/emacs-pretest-bug/2005-06/msg00148.html

Do you have any ideas about that?

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

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

[Jan D.]

>
>       /* 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 - 1) / width * width;
>       gy = (gy + height - 1) / height * height;
>
>       last_mouse_glyph.width  = width;
>       last_mouse_glyph.height = height;
>       last_mouse_glyph.x = gx;
>       last_mouse_glyph.y = gy;
>
> gx and gy are initially the position of the mouse. We are trying to
> find the rectangle around the glyph under the mouse here.
>
> On Windows, I found that this code returns the rectangle of a glyph
> diagonally adjacent to the glyph we want. I am not sure if I am
> missing something about the way X co-ordinates work, but I think the
> two lines that round gx and gy should be simply:
>
>       gx = gx / width * width;
>       gy = gy / height * height;
>

AFAIK co-ordinates on W32 behave the same as on X, so your patch  
should be OK.  It is easy to confirm anyway.

     Jan D.

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

[Jason Rumney]

"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.

As a reminder of what I expected it to fix, when investigating the
originally reported bug, I observed a lot of flickering of
mouse-highlight area on the modeline when the mouse was held over it.

Having investigated a similar problem on Windows, I remembered that I
had traced that problem to excessive mouse-move events being generated.

I fixed the problem on Windows and had investigated whether the same
problem existed on X since the code was the same, but at the time I
did not see the flickering so concluded that something was different
about X that I did not understand.

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

[YAMAMOTO Mitsuharu]

>>>>> 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.

>>>>> On Tue, 07 Jun 2005 18:45:48 +0900, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> said:

>      2.1 Sometimes a tooltip is not shown.  I hope the first
>          fragment of the patch below fix this.

>      2.2 The value of last_mouse_glyph may become invalid.  For
>          example, after clicking the image on the splash screen,
>          dragging the area where the image was displayed does not
>          issue mouse-movement events.  I think last_mouse_glyph
>          should be cleared in some appropriate timing, but I'm not
>          sure when it is.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Index: src/xterm.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/xterm.c,v
retrieving revision 1.879
diff -c -r1.879 xterm.c
*** src/xterm.c	10 Oct 2005 22:54:19 -0000	1.879
--- src/xterm.c	11 Oct 2005 01:10:58 -0000
***************
*** 3675,3680 ****
--- 3677,3688 ----
  	      rect->height = r->height;
  	      rect->x = WINDOW_TO_FRAME_PIXEL_X (w, gx);
  	      rect->y = WINDOW_TO_FRAME_PIXEL_Y (w, r->y);
+ 
+ 	      XDrawRectangle (FRAME_X_DISPLAY (f), FRAME_X_WINDOW (f),
+ 			      f->output_data.x->normal_gc,
+ 			      rect->x - 1, rect->y - 1,
+ 			      rect->width + 2, rect->height + 2);
+ 
  	      return 1;
  	    }
  	  break;

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

[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

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

[Jason Rumney]

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

> A simple debug code below shows the incorrectness of the calculated
> rectangle :-).

The debugging code you posted was for glyph_rect, which was not
affected by my patch. But this does show that there are problems with
all this code to detect mouse motion and throttle the movement events.

You debugging code appears to highlight the glyph below the one the
mouse is currently over.

This if statement seems to be wrong:

      if (r->y >= y)

Where y is the mouse position and r->y is the top of the glyph.

If r->y > y, then we have already reached the glyph below the one we
want. The only case where this works correctly is when the mouse
pointer is over the top edge of the glyph so r->y == y.

I think this needs to be:

      if (r->y + r->height > y)

I will do some more testing, and check in the changes when I think I
have got it right.

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

[Kim F. Storm]

Jason Rumney <jasonr@gnu.org> writes:

> I think this needs to be:
>
>       if (r->y + r->height > y)
>
> I will do some more testing, and check in the changes when I think I
> have got it right.

This change is part of the generic fix that I posted a few hours ago.

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

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

[YAMAMOTO Mitsuharu]

>>>>> On Tue, 11 Oct 2005 12:21:50 +0200, storm@cua.dk (Kim F. Storm) said:

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

I think it still has some off-by-one errors.

*** xdisp.c.bak	Tue Oct 11 20:52:32 2005
--- xdisp.c	Tue Oct 11 21:20:35 2005
***************
*** 2085,2110 ****
        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;
  
--- 2085,2110 ----
        goto text_glyph;
  
      case ON_LEFT_FRINGE:
!       gx = (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:
!       gx = (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:
!       gx = (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;
  
***************
*** 2114,2131 ****
  
   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;
--- 2114,2135 ----
  
   text_glyph:
    gr = 0; gy = 0;
!   for (; r < end_row && r->enabled_p; ++r)
!     if (r->y + r->height > y)
!       {
! 	gr = r; gy = r->y;
! 	break;
!       }
  
!   if (gr && gy <= y)
      {
        struct glyph *g = gr->glyphs[area];
        struct glyph *end = g + gr->used[area];
  
        height = gr->height;
!       for (gx = gr->x; g < end; gx += g->pixel_width, ++g)
! 	if (gx + g->pixel_width > x)
! 	  break;
  
        if (g < end)
  	width = g->pixel_width;
***************
*** 2156,2165 ****
  
   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
      {
--- 2160,2173 ----
  
   row_glyph:
    gr = 0, gy = 0;
!   for (; r < end_row && r->enabled_p; ++r)
!     if (r->y + r->height > y)
!       {
! 	gr = r; gy = r->y;
! 	break;
!       }
  
!   if (gr && gy <= y)
      height = gr->height;
    else
      {


(The ON_SCROLL_BAR case still does not work well.)

And again, if it is corrected, the problems 2.1 and 2.2 I said in
http://lists.gnu.org/archive/html/emacs-pretest-bug/2005-06/msg00148.html
will appear.

> 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...?

The patch below adds "remember_mouse_glyph" calls.  Actually, a part
of this patch is included in the patch in the above URL.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

*** macterm.c.bak	Tue Oct 11 20:23:08 2005
--- macterm.c	Tue Oct 11 21:25:15 2005
***************
*** 4190,4195 ****
--- 4190,4197 ----
        frame->mouse_moved = 1;
        last_mouse_scroll_bar = Qnil;
        note_mouse_highlight (frame, pos->h, pos->v);
+       /* Remember which glyph we're now on.  */
+       remember_mouse_glyph (frame, pos->h, pos->v, &last_mouse_glyph);
      }
  }
  
***************
*** 4226,4243 ****
  
  
  /* Return the current position of the mouse.
!    *fp should be a frame which indicates which display to ask about.
  
!    If the mouse movement started in a scroll bar, set *fp, *bar_window,
!    and *part to the frame, window, and scroll bar part that the mouse
!    is over.  Set *x and *y to the portion and whole of the mouse's
     position on the scroll bar.
  
!    If the mouse movement started elsewhere, set *fp to the frame the
!    mouse is on, *bar_window to nil, and *x and *y to the character cell
     the mouse is over.
  
!    Set *time to the server time-stamp for the time at which the mouse
     was at this position.
  
     Don't store anything if we don't have a valid set of values to report.
--- 4228,4245 ----
  
  
  /* Return the current position of the mouse.
!    *FP should be a frame which indicates which display to ask about.
  
!    If the mouse movement started in a scroll bar, set *FP, *BAR_WINDOW,
!    and *PART to the frame, window, and scroll bar part that the mouse
!    is over.  Set *X and *Y to the portion and whole of the mouse's
     position on the scroll bar.
  
!    If the mouse movement started elsewhere, set *FP to the frame the
!    mouse is on, *BAR_WINDOW to nil, and *X and *Y to the character cell
     the mouse is over.
  
!    Set *TIME to the server time-stamp for the time at which the mouse
     was at this position.
  
     Don't store anything if we don't have a valid set of values to report.
***************
*** 4254,4264 ****
       Lisp_Object *x, *y;
       unsigned long *time;
  {
!   Point mouse_pos;
!   int ignore1, ignore2;
!   struct frame *f = mac_focus_frame (FRAME_MAC_DISPLAY_INFO (*fp));
!   WindowPtr wp = FRAME_MAC_WINDOW (f);
!   Lisp_Object frame, tail;
  
    BLOCK_INPUT;
  
--- 4256,4262 ----
       Lisp_Object *x, *y;
       unsigned long *time;
  {
!   FRAME_PTR f1;
  
    BLOCK_INPUT;
  
***************
*** 4266,4290 ****
      x_scroll_bar_report_motion (fp, bar_window, part, x, y, time);
    else
      {
        /* Clear the mouse-moved flag for every frame on this display.  */
        FOR_EACH_FRAME (tail, frame)
!         XFRAME (frame)->mouse_moved = 0;
  
        last_mouse_scroll_bar = Qnil;
  
!       SetPortWindowPort (wp);
! 
!       GetMouse (&mouse_pos);
! 
!       pixel_to_glyph_coords (f, mouse_pos.h, mouse_pos.v, &ignore1, &ignore2,
!                              &last_mouse_glyph, insist);
! 
!       *bar_window = Qnil;
!       *part = scroll_bar_handle;
!       *fp = f;
!       XSETINT (*x, mouse_pos.h);
!       XSETINT (*y, mouse_pos.v);
!       *time = last_mouse_movement_time;
      }
  
    UNBLOCK_INPUT;
--- 4264,4306 ----
      x_scroll_bar_report_motion (fp, bar_window, part, x, y, time);
    else
      {
+       Lisp_Object frame, tail;
+ 
        /* Clear the mouse-moved flag for every frame on this display.  */
        FOR_EACH_FRAME (tail, frame)
! 	XFRAME (frame)->mouse_moved = 0;
  
        last_mouse_scroll_bar = Qnil;
  
!       if (FRAME_MAC_DISPLAY_INFO (*fp)->grabbed && last_mouse_frame
! 	  && FRAME_LIVE_P (last_mouse_frame))
! 	f1 = last_mouse_frame;
!       else
! 	f1 = mac_focus_frame (FRAME_MAC_DISPLAY_INFO (*fp));
! 
!       if (f1)
! 	{
! 	  /* Ok, we found a frame.  Store all the values.
! 	     last_mouse_glyph is a rectangle used to reduce the
! 	     generation of mouse events.  To not miss any motion
! 	     events, we must divide the frame into rectangles of the
! 	     size of the smallest character that could be displayed
! 	     on it, i.e. into the same rectangles that matrices on
! 	     the frame are divided into.  */
! 	  Point mouse_pos;
! 
! 	  SetPortWindowPort (FRAME_MAC_WINDOW (f1));
! 	  GetMouse (&mouse_pos);
! 	  remember_mouse_glyph (f1, mouse_pos.h, mouse_pos.v,
! 				&last_mouse_glyph);
! 
! 	  *bar_window = Qnil;
! 	  *part = 0;
! 	  *fp = f1;
! 	  XSETINT (*x, mouse_pos.h);
! 	  XSETINT (*y, mouse_pos.v);
! 	  *time = last_mouse_movement_time;
! 	}
      }
  
    UNBLOCK_INPUT;

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

[Jason Rumney]

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

storm@cua.dk (Kim F. Storm) writes:

> 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).

It would be an improvement in maintainability, as the code is
basically the same.

I tried your patch on Windows. It compiles and runs, but something is
still not right. Using the trick of drawing last_glyph_rect on the
screen, I see for example that the splash screen image's rect gets
drawn below the splash screen (the top of the rect is aligned with the
bottom of the actual image) when the mouse reaches the bottom left
corner of the image.


[-- Attachment #2: Emacs cursor position bug --]
[-- Type: image/png, Size: 17495 bytes --]

[-- Attachment #3: Type: text/plain, Size: 142 bytes --]

_______________________________________________
Emacs-devel mailing list
Emacs-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/emacs-devel

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

[Kim F. Storm]

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

>>>>>> On Tue, 11 Oct 2005 12:21:50 +0200, storm@cua.dk (Kim F. Storm) said:
>
>> Below is a patch which does this, but I have only tested it on X.
>> Could somebody test it on W32 and MAC?
>
> I think it still has some off-by-one errors.

Thanks for finding the bugs in my code.
Does it still have errors with your changes?

In any case, I have committed changes to fix buffer positions in mouse
click events and mouse movement events in the margins, fringes and
scroll bars.  

Now, marking text with the mouse seems to work alright when you drag
outside left/right borders of the text area.

>
> (The ON_SCROLL_BAR case still does not work well.)

Does it work better now?

>
> And again, if it is corrected, the problems 2.1 and 2.2 I said in
> http://lists.gnu.org/archive/html/emacs-pretest-bug/2005-06/msg00148.html
> will appear.
>
>> 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...?
>
> The patch below adds "remember_mouse_glyph" calls.  Actually, a part
> of this patch is included in the patch in the above URL.

I think we should commit these patches -- and then address the 2.1 and
2.2 problems subsequently -- it is too hard to fix everything at once
(and keep patches synchronized).

I'll commit the current set of changes (but just the Mac-related
change to macterm.c that you included in your mail) tomorrow unless
anyone objects before then...

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

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

[Kim F. Storm]

Jason Rumney <jasonr@gnu.org> writes:

> storm@cua.dk (Kim F. Storm) writes:
>
>> 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).
>
> It would be an improvement in maintainability, as the code is
> basically the same.
>
> I tried your patch on Windows. It compiles and runs, but something is
> still not right. Using the trick of drawing last_glyph_rect on the
> screen, I see for example that the splash screen image's rect gets
> drawn below the splash screen (the top of the rect is aligned with the
> bottom of the actual image) when the mouse reaches the bottom left
> corner of the image.

I tried this as well, and indeed there were quite some problems also on X.

I have now fixed some more bugs in the window_from_coordinates
function which caused some of those problems, and using the "draw box"
trick, I think I've ironed out the remaning problems -- at least for
X, so I have installed (a revised version) of my changes, including
Mitsuharu's changes to macterm.c.

I left the debugging code (for X) in remember_mouse_glyph (in xdisp.c),
so if you have problems, pls. try to identify what's going wrong.
Feel free to add W32 and MAC specific versions of the debugging code
(look for XDrawRectangle).

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

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

[YAMAMOTO Mitsuharu]

>>>>> On Wed, 12 Oct 2005 00:43:44 +0200, storm@cua.dk (Kim F. Storm) said:

> I have now fixed some more bugs in the window_from_coordinates
> function which caused some of those problems, and using the "draw
> box" trick, I think I've ironed out the remaning problems -- at
> least for X, so I have installed (a revised version) of my changes,
> including Mitsuharu's changes to macterm.c.

I tried it on Mac, and found that the calculated rectangle on a
mode-line was not correct when it was displayed with a variable-width
font.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp


*** xdisp.c.~1.1059.~	Wed Oct 12 09:31:47 2005
--- xdisp.c	Wed Oct 12 10:19:31 2005
***************
*** 2059,2067 ****
    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;
--- 2059,2064 ----
***************
*** 2085,2093 ****
        area = TEXT_AREA;
  
      text_glyph:
        gr = 0; gy = 0;
!       for (; r < end_row && r->enabled_p; ++r)
! 	if (r->y + r->height > y)
  	  {
  	    gr = r; gy = r->y;
  	    break;
--- 2082,2093 ----
        area = TEXT_AREA;
  
      text_glyph:
+       r = MATRIX_HEADER_LINE_ROW (w->current_matrix);
+       end_row = MATRIX_MODE_LINE_ROW (w->current_matrix);
+ 
        gr = 0; gy = 0;
!       for (; r <= end_row; ++r)
! 	if (r->enabled_p && r->y + r->height > y)
  	  {
  	    gr = r; gy = r->y;
  	    break;
***************
*** 2148,2156 ****
        width = WINDOW_SCROLL_BAR_AREA_WIDTH (w);
  
      row_glyph:
        gr = 0, gy = 0;
!       for (; r < end_row && r->enabled_p; ++r)
! 	if (r->y + r->height > y)
  	  {
  	    gr = r; gy = r->y;
  	    break;
--- 2148,2159 ----
        width = WINDOW_SCROLL_BAR_AREA_WIDTH (w);
  
      row_glyph:
+       r = MATRIX_FIRST_TEXT_ROW (w->current_matrix);
+       end_row = MATRIX_BOTTOM_TEXT_ROW (w->current_matrix, w);
+ 
        gr = 0, gy = 0;
!       for (; r <= end_row; ++r)
! 	if (r->enabled_p && r->y + r->height > y)
  	  {
  	    gr = r; gy = r->y;
  	    break;

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

[Kim F. Storm]

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

> I tried it on Mac, and found that the calculated rectangle on a
> mode-line was not correct when it was displayed with a variable-width
> font.
>

It's a tricky piece of code indeed :-)

Your changes look good -- please install them.  Thanks.

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

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

[YAMAMOTO Mitsuharu]

>>>>> On Wed, 12 Oct 2005 12:15:37 +0900, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> said:

> I tried it on Mac, and found that the calculated rectangle on a
> mode-line was not correct when it was displayed with a
> variable-width font.

Sorry for the noise.  If enabled rows except the mode-line in the
current matrix are gathered to the lower indices, the following one
would be better.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

*** xdisp.c.~1.1059.~	Wed Oct 12 09:31:47 2005
--- xdisp.c	Wed Oct 12 14:59:59 2005
***************
*** 2059,2067 ****
    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;
--- 2059,2064 ----
***************
*** 2085,2097 ****
        area = TEXT_AREA;
  
      text_glyph:
        gr = 0; gy = 0;
!       for (; r < end_row && r->enabled_p; ++r)
! 	if (r->y + r->height > y)
! 	  {
! 	    gr = r; gy = r->y;
! 	    break;
! 	  }
  
        if (gr && gy <= y)
  	{
--- 2082,2104 ----
        area = TEXT_AREA;
  
      text_glyph:
+       r = MATRIX_HEADER_LINE_ROW (w->current_matrix);
+       end_row = MATRIX_MODE_LINE_ROW (w->current_matrix);
+ 
        gr = 0; gy = 0;
!       for (; r <= end_row; ++r)
! 	{
! 	  if (!r->enabled_p)
! 	    if (end_row->enabled_p)
! 	      r = end_row;
! 	    else
! 	      break;
! 	  if (r->y + r->height > y)
! 	    {
! 	      gr = r; gy = r->y;
! 	      break;
! 	    }
! 	}
  
        if (gr && gy <= y)
  	{
***************
*** 2148,2155 ****
        width = WINDOW_SCROLL_BAR_AREA_WIDTH (w);
  
      row_glyph:
        gr = 0, gy = 0;
!       for (; r < end_row && r->enabled_p; ++r)
  	if (r->y + r->height > y)
  	  {
  	    gr = r; gy = r->y;
--- 2155,2165 ----
        width = WINDOW_SCROLL_BAR_AREA_WIDTH (w);
  
      row_glyph:
+       r = MATRIX_FIRST_TEXT_ROW (w->current_matrix);
+       end_row = MATRIX_BOTTOM_TEXT_ROW (w->current_matrix, w);
+ 
        gr = 0, gy = 0;
!       for (; r <= end_row && r->enabled_p; ++r)
  	if (r->y + r->height > y)
  	  {
  	    gr = r; gy = r->y;

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

[Kim F. Storm]

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

>>>>>> On Wed, 12 Oct 2005 12:15:37 +0900, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> said:
>
>> I tried it on Mac, and found that the calculated rectangle on a
>> mode-line was not correct when it was displayed with a
>> variable-width font.
>
> Sorry for the noise.  If enabled rows except the mode-line in the
> current matrix are gathered to the lower indices, the following one
> would be better.

I don't quite remember, but most often, all rows above the modeline
are enabled, so I doubt it makes a big difference in practice.

Your previous code made fewer assumptions, and was still correct,
so I think I like that one better.

BTW, how does your code work for a window without a modeline
(mode-line-format = nil)?

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

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

[YAMAMOTO Mitsuharu]

>>>>> On Wed, 12 Oct 2005 11:29:57 +0200, storm@cua.dk (Kim F. Storm) said:

> I don't quite remember, but most often, all rows above the modeline
> are enabled, so I doubt it makes a big difference in practice.

> Your previous code made fewer assumptions, and was still correct, so
> I think I like that one better.

> BTW, how does your code work for a window without a modeline
> (mode-line-format = nil)?

I installed yet another one, which does not change the original code
so much, but just directly obtains header-line/mode-line rows.  So it
should work like the original one for no mode-line case.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

Index: src/xdisp.c
===================================================================
RCS file: /cvsroot/emacs/emacs/src/xdisp.c,v
retrieving revision 1.1059
diff -c -r1.1059 xdisp.c
*** src/xdisp.c	11 Oct 2005 22:36:46 -0000	1.1059
--- src/xdisp.c	12 Oct 2005 09:40:14 -0000
***************
*** 2060,2066 ****
    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)
      {
--- 2060,2066 ----
    height = WINDOW_FRAME_LINE_HEIGHT (w);
  
    r = MATRIX_FIRST_TEXT_ROW (w->current_matrix);
!   end_row = MATRIX_BOTTOM_TEXT_ROW (w->current_matrix, w);
  
    if (w->pseudo_window_p)
      {
***************
*** 2079,2098 ****
        area = RIGHT_MARGIN_AREA;
        goto text_glyph;
  
-     case ON_TEXT:
-     case ON_MODE_LINE:
      case ON_HEADER_LINE:
        area = TEXT_AREA;
  
      text_glyph:
        gr = 0; gy = 0;
!       for (; r < end_row && r->enabled_p; ++r)
  	if (r->y + r->height > y)
  	  {
  	    gr = r; gy = r->y;
  	    break;
  	  }
  
        if (gr && gy <= y)
  	{
  	  struct glyph *g = gr->glyphs[area];
--- 2079,2106 ----
        area = RIGHT_MARGIN_AREA;
        goto text_glyph;
  
      case ON_HEADER_LINE:
+     case ON_MODE_LINE:
+       gr = (part == ON_HEADER_LINE
+ 	    ? MATRIX_HEADER_LINE_ROW (w->current_matrix)
+ 	    : MATRIX_MODE_LINE_ROW (w->current_matrix));
+       gy = gr->y;
+       area = TEXT_AREA;
+       goto text_glyph_row_found;
+ 
+     case ON_TEXT:
        area = TEXT_AREA;
  
      text_glyph:
        gr = 0; gy = 0;
!       for (; r <= end_row && r->enabled_p; ++r)
  	if (r->y + r->height > y)
  	  {
  	    gr = r; gy = r->y;
  	    break;
  	  }
  
+     text_glyph_row_found:
        if (gr && gy <= y)
  	{
  	  struct glyph *g = gr->glyphs[area];
***************
*** 2149,2155 ****
  
      row_glyph:
        gr = 0, gy = 0;
!       for (; r < end_row && r->enabled_p; ++r)
  	if (r->y + r->height > y)
  	  {
  	    gr = r; gy = r->y;
--- 2157,2163 ----
  
      row_glyph:
        gr = 0, gy = 0;
!       for (; r <= end_row && r->enabled_p; ++r)
  	if (r->y + r->height > y)
  	  {
  	    gr = r; gy = r->y;