Subject: bug#72230: 29.4; Cursor Disappears with Hebrew Text

From: Gerd Möllmann <[email-masked]>
Date: Mon, 22 Jul 2024 08:47:30 +0200

Po Lu <[email-masked]> writes:

> Stefan Kangas <[email-masked]> writes:
>
>> Eli Zaretskii <[email-masked]> writes:
>>
>>> So maybe the problem is elsewhere.  One thing is almost certain: it's
>>> a problem specific to the macOS build of Emacs.  People who can debug
>>> on that platform are encouraged to suggest insights and changes.
>>
>> This seems to have something to with the clipping path, and is fixed
>> here by the below patch. I don't think it's suitable for installing
>> though.
>>
>> Po Lu, since you added the below NSRectClip call, do you have any
>> thoughts here?
>
> Yes, this will cause the overhangs of a cursor glyph to overwrite its
> surroundings.  There is certain to be some other factor that has not
> been accounted for, while mindlessly removing clipping operations is not
> the solution.
>
>> diff --git a/src/nsterm.m b/src/nsterm.m
>> index 794630de1c1..54b15bc851d 100644
>> --- a/src/nsterm.m
>> +++ b/src/nsterm.m
>> @@ -3102,7 +3102,7 @@ Note that CURSOR_WIDTH is meaningful only for
>> (h)bar cursors.
>>  #ifdef NS_IMPL_GNUSTEP
>>    GSRectClipList (ctx, &r, 1);
>>  #else
>> -  NSRectClip (r);
>> +  // NSRectClip (r);
>>  #endif
>>
>>    [FRAME_CURSOR_COLOR (f) set];
>> @@ -3135,7 +3135,7 @@ Note that CURSOR_WIDTH is meaningful only for
>> (h)bar cursors.
>>           on the right of its glyph, rather than on the left.  */
>>        cursor_glyph = get_phys_cursor_glyph (w);
>>        if ((cursor_glyph->resolved_level & 1) != 0)
>> -        s.origin.x += cursor_glyph->pixel_width - s.size.width;
>> +        s.origin.x += cursor_glyph->pixel_width - cursor_width;
>
> I don't recollect what `s' is supposed to represent.

I propose the following as a fix. This computes the right rectangle to
draw before that rectangle is set via NSClipRect.

1 file changed, 8 insertions(+), 12 deletions(-)
src/nsterm.m | 20 ++++++++------------

modified   src/nsterm.m
@@ -3026,7 +3026,7 @@ External call (RIF): draw cursor.
      Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
    -------------------------------------------------------------------------- */
 {
-  NSRect r, s;
+  NSRect r;
   int fx, fy, h, cursor_height;
   struct frame *f = WINDOW_XFRAME (w);
   struct glyph *phys_cursor_glyph;
@@ -3076,6 +3076,12 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
       /* The bar cursor should never be wider than the glyph.  */
       if (cursor_width < w->phys_cursor_width)
         w->phys_cursor_width = cursor_width;
+
+      /* If the character under cursor is R2L, draw the bar cursor
+         on the right of its glyph, rather than on the left.  */
+      cursor_glyph = get_phys_cursor_glyph (w);
+      if ((cursor_glyph->resolved_level & 1) != 0)
+        fx += cursor_glyph->pixel_width - w->phys_cursor_width;
     }
   /* If we have an HBAR, "cursor_width" MAY specify height.  */
   else if (cursor_type == HBAR_CURSOR)
@@ -3126,18 +3132,8 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
       [ctx restoreGraphicsState];
       break;
     case HBAR_CURSOR:
-      NSRectFill (r);
-      [ctx restoreGraphicsState];
-      break;
     case BAR_CURSOR:
-      s = r;
-      /* If the character under cursor is R2L, draw the bar cursor
-         on the right of its glyph, rather than on the left.  */
-      cursor_glyph = get_phys_cursor_glyph (w);
-      if ((cursor_glyph->resolved_level & 1) != 0)
-        s.origin.x += cursor_glyph->pixel_width - s.size.width;
-
-      NSRectFill (s);
+      NSRectFill (r);
       [ctx restoreGraphicsState];
       break;
     }