bug#16856: 24.3.50; Cursor leaves garbage in fringe

bug#16856: 24.3.50; Cursor leaves garbage in fringe

[David Reitter]

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

I don’t think that bea0f95 (May 21, nsterm.m) fully fixed this problem.  I’ve had several “appearances” of the ominous garbage in the right fringe yesterday.

This was after applying your patch (and removing my workaround).

The workaround is shown below, but at the time it only worked with cursor-type (bar . 2), not (bar . 3).  I haven’t checked with your change.








diff --git a/src/xdisp.c b/src/xdisp.c
index b43ce61..389fea7 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -2018,6 +2018,13 @@ get_glyph_string_clip_rects (struct glyph_string *s, NativeRectangle *rects, int
       /* This is a text line that may be partially visible.  */
       r.x = window_box_left (s->w, s->area);
       r.width = window_box_width (s->w, s->area);
+
+      /* Aquamacs workaround - Because the cursor is drawn without limiting focus to the
+        window box, but it is removed by writing glyph and nothing into the right margin,
+        while focus is applied to the window box, parts of the cursor may remain visible.
+        This is a stop-gap measure that fails if the cursor is (bar . 3) or wider. */
+      r.width += 1;
+
       r.height = s->row->visible_height;
     }


[-- Attachment #2.1: Type: text/html, Size: 2445 bytes --]

[-- Attachment #2.2: Screen Shot 2016-07-16 at 6.35.30 PM.png --]
[-- Type: image/png, Size: 67955 bytes --]

bug#16856: 24.3.50; Cursor leaves garbage in fringe

[Alan Third]

On Sun, Jul 17, 2016 at 03:33:03PM +0900, David Reitter wrote:
> I don’t think that bea0f95 (May 21, nsterm.m) fully fixed this
> problem. I’ve had several “appearances” of the ominous garbage in
> the right fringe yesterday.
> 
> This was after applying your patch (and removing my workaround).

Hi David, I'm not entirely sure what's going on in your screenshot. Is
the garbage definitely appearing in the fringe rather than in the main
text area? If so, is it on the left or the right (or the middle, I
guess) of the fringe?
-- 
Alan Third

bug#16856: 24.3.50; Cursor leaves garbage in fringe

[David Reitter]

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

Good point, I can't tell.  It happens only in some situations (font size, text extent, window size)

---Sent from my phone

On Sun, Jul 17, 2016 at 03:33:03PM +0900, David Reitter wrote:

> I don’t think that bea0f95 (May 21, nsterm.m) fully fixed this

> problem. I’ve had several “appearances” of the ominous garbage in

> the right fringe yesterday.

>  
> This was after applying your patch (and removing my workaround).



Hi David, I'm not entirely sure what's going on in your screenshot. Is

the garbage definitely appearing in the fringe rather than in the main

text area? If so, is it on the left or the right (or the middle, I

guess) of the fringe?

--  
Alan Third


[-- Attachment #2: Type: text/html, Size: 965 bytes --]

bug#16856: 24.3.50; Cursor leaves garbage in fringe

[Eli Zaretskii]

> Date: Sun, 17 Jul 2016 09:42:32 +0100
> From: Alan Third <alan@idiocy.org>
> Cc: 16856@debbugs.gnu.org
> 
> On Sun, Jul 17, 2016 at 03:33:03PM +0900, David Reitter wrote:
> > I don’t think that bea0f95 (May 21, nsterm.m) fully fixed this
> > problem. I’ve had several “appearances” of the ominous garbage in
> > the right fringe yesterday.
> > 
> > This was after applying your patch (and removing my workaround).
> 
> Hi David, I'm not entirely sure what's going on in your screenshot. Is
> the garbage definitely appearing in the fringe rather than in the main
> text area? If so, is it on the left or the right (or the middle, I
> guess) of the fringe?

I actually am puzzled by more than that: it looks like the "garbage"
is some text drawn on the fringe, which seems to point to incorrect
coordinates of some screen write.  If that screen write is the one
that draws or erases the cursor, then I don't understand this comment:

  "Because the cursor is drawn without limiting focus to the window
   box, but it is removed by writing glyph and nothing into the right
   margin, while focus is applied to the window box, parts of the
   cursor may remain visible."

It seems to imply that drawing cursor and erasing it are implemented
in Aquamacs by two very different code fragments?  (That's not what
happens on other platforms, AFAIR.)  And if that's true, I understand
the workaround even less: it limits the _width_ of the cursor, whereas
the problem is clearly with its coordinates.

What am I missing?

bug#16856: 24.3.50; Cursor leaves garbage in fringe

[David Reitter]

On Jul 17, 2016, at 9:09 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>  "Because the cursor is drawn without limiting focus to the window
>   box, but it is removed by writing glyph and nothing into the right
>   margin, while focus is applied to the window box, parts of the
>   cursor may remain visible."
> 
> It seems to imply that drawing cursor and erasing it are implemented
> in Aquamacs by two very different code fragments?

I don’t think they’re that different.
I do not know the code there very well, otherwise I would have just fixed the problem.

Is it possible that when drawing the glyph, we call ns_focus with the rectangle returned by ns_get_glyph_string_clip_rect()?
ns_draw_window_cursor() on the other hand focuses via ns_clip_to_row().  It might do so before deleting the cursor (if that’s where the cursor is erased), but that doesn’t matter, because clipping in ns_focus probably isn’t incremental from what it looks like.

>   And if that's true, I understand
> the workaround even less: it limits the _width_ of the cursor, whereas
> the problem is clearly with its coordinates.

No, it increases the width of the clipped rectangle so that the cursor can be erased from wherever it was drawn.
But again, if the cursor type is wider than 2px (on the right hand side of the underlying glyph), we would have to widen the clip box even further.

I’m not sure what the correct solution is here.  If the bar cursor is drawn to the right of the glyph, it’s going to go into the margin or fringe.  If you prevent that even at reasonable cursors like (bar . 2), the cursor is going to look funny on the right hand side.  So, I think we have to  (A)  widen the clipping rectangle to something like max(row_width, glyph_x+glyph_w+min(2, cursor_width)), and (B)  also clip when drawing the cursor so that wide cursors don’t go into the fringe.  I think Alan’s change may have done B already.  Haven’t tested that.  (As for A, I can’t work on it right now and probably don’t know the code well enough to do this right anyway.)

bug#16856: 24.3.50; Cursor leaves garbage in fringe

[Alan Third]

On Sun, Jul 17, 2016 at 03:09:34PM +0300, Eli Zaretskii wrote:
> I actually am puzzled by more than that: it looks like the "garbage"
> is some text drawn on the fringe, which seems to point to incorrect
> coordinates of some screen write.  If that screen write is the one
> that draws or erases the cursor, then I don't understand this comment:

I think the screenshot shows Emacs running in front of some other (OS)
window and we can see a little bit of that window on either side of
the Emacs frame.

-- 
Alan Third

bug#16856: 24.3.50; Cursor leaves garbage in fringe

[Alan Third]

On Sun, Jul 17, 2016 at 07:41:42PM +0900, David Reitter wrote:
> Good point, I can't tell.  It happens only in some situations (font
> size, text extent, window size)

I think I’ve got a reproducible case:

Emacs -Q

(visual-line-mode 1)
(variable-pitch-mode 1)
(setq-default cursor-type '(bar . 4))

SPC SPC SPC C-b C-b C-b

It doesn’t even have to be spaces, it looks like it's any case where
the bar cursor is wider than the underlying glyph.

Interestingly the Mac port doesn’t seem to ever draw the bar cursor
wider than the glyph whereas the NS port always draws it at the
configured width. Perhaps that’s where the fault is.
-- 
Alan Third

bug#16856: [PATCH] Prevent bar cursor overwriting next glyph (bug#16856)

[Alan Third]

* src/nsterm.m (ns_draw_window_cursor): Test glyph width vs cursor width
before setting final size.
---
 src/nsterm.m | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/nsterm.m b/src/nsterm.m
index a6160ed..8da2ffe 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -2861,7 +2861,10 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
     {
       if (cursor_width < 1)
 	cursor_width = max (FRAME_CURSOR_WIDTH (f), 1);
-      w->phys_cursor_width = cursor_width;
+
+      /* The bar cursor should never be wider than the glyph. */
+      if (cursor_width < w->phys_cursor_width)
+        w->phys_cursor_width = cursor_width;
     }
   /* If we have an HBAR, "cursor_width" MAY specify height. */
   else if (cursor_type == HBAR_CURSOR)
-- 

And here's a patch to prevent the bar cursor straying into the next glyph.
-- 
Alan Third

bug#16856: [PATCH] Prevent bar cursor overwriting next glyph (bug#16856)

[David Reitter]

No ill effects with that.  What is the glyph at the end of the line?
Also, about your patch, it seems like w->phys_cursor_width will then just be whatever it was before.


> On Jul 17, 2016, at 10:51 PM, Alan Third <alan@idiocy.org> wrote:
> 
> * src/nsterm.m (ns_draw_window_cursor): Test glyph width vs cursor width
> before setting final size.
> ---
> src/nsterm.m | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/nsterm.m b/src/nsterm.m
> index a6160ed..8da2ffe 100644
> --- a/src/nsterm.m
> +++ b/src/nsterm.m
> @@ -2861,7 +2861,10 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
>     {
>       if (cursor_width < 1)
> 	cursor_width = max (FRAME_CURSOR_WIDTH (f), 1);
> -      w->phys_cursor_width = cursor_width;
> +
> +      /* The bar cursor should never be wider than the glyph. */
> +      if (cursor_width < w->phys_cursor_width)
> +        w->phys_cursor_width = cursor_width;
>     }
>   /* If we have an HBAR, "cursor_width" MAY specify height. */
>   else if (cursor_type == HBAR_CURSOR)
> -- 
> 
> And here's a patch to prevent the bar cursor straying into the next glyph.
> -- 
> Alan Third

bug#16856: [PATCH] Prevent bar cursor overwriting next glyph (bug#16856)

[Alan Third]

On 17 July 2016 at 23:54, David Reitter <david.reitter@gmail.com> wrote:
> No ill effects with that.  What is the glyph at the end of the line?

I don't know how the end-of-line is displayed. On the Windows Emacs
I've got here it's a narrow glyph (same as space, I think), so the bar
isn't displayed at it's full width if it's set to be wide. I expect
it'll be the same on the Mac, I can check later if you want.

> Also, about your patch, it seems like w->phys_cursor_width will then just be whatever it was before.

No, w->phys_cursor_width appears to hold the glyph width by default,
so we should end up with the smaller of cursor_width or the glyph
width.

I realise it might be more desirable to have the bar cursor be a
consistent width, but that would make the NS port different from all
the others, afaics.

-- 
Alan Third