bug#62573: 29.0.60; Cursor color not being inverted in emacs-29

["Daniel Martín via Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

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

Po Lu <luangruo@yahoo.com> writes:

> Daniel Martín <mardani29@yahoo.es> writes:
>
>> I've solved the bug by replicating that logic in the appropriate places,
>> nsterm.m and macfont.m:
>
> Nice, thanks.
>
>> +              CGColorRef colorref = get_cgcolor_from_nscolor (FRAME_CURSOR_COLOR (f), f);
>
> This row is now wider than 80 columns.  Please wrap it.
>

Alright, I have also extracted a couple of macros because there were
already some macros for similar code patterns.

>>            struct face *face = s->face;
>>            if (!face->stipple)
>> -	    {
>> -	      if (s->hl != DRAW_CURSOR)
>> -		[(NS_FACE_BACKGROUND (face) != 0
>> -		  ? [NSColor colorWithUnsignedLong:NS_FACE_BACKGROUND (face)]
>> -		  : FRAME_BACKGROUND_COLOR (s->f)) set];
>> -	      else
>> -		[FRAME_CURSOR_COLOR (s->f) set];
>> -	    }
>> +            {
>> +              if (s->hl != DRAW_CURSOR)
>> +                [(NS_FACE_BACKGROUND (face) != 0
>> +                  ? [NSColor colorWithUnsignedLong:NS_FACE_BACKGROUND (face)]
>> +                  : FRAME_BACKGROUND_COLOR (s->f)) set];
>> +              else if (face && NS_FACE_BACKGROUND (face)
>> +                       == [(NSColor*)FRAME_CURSOR_COLOR (s->f) unsignedLong])
>> +                [[NSColor colorWithUnsignedLong:NS_FACE_FOREGROUND (face)] set];
>> +              else
>> +                [FRAME_CURSOR_COLOR (s->f) set];
>> +            }
>>            else
>>              {
>>                struct ns_display_info *dpyinfo = FRAME_DISPLAY_INFO (s->f);
>>
>
> Please write:
>
>   (face && (NS_FACE_BACKGROUND (face)
>   	    == [(NSColor *) FRAME_CURSOR_COLOR (s->f)
>                   unsignedLong]))
>
> instead.

Done.

>
>> Could you give it a try on macOS and GNUstep?  Thank you.
>
> Works for me, thanks.
>
> P.S. I think what needs to be done is to make the NS port keep track of
> glyph string colors through GCs, so that the relevant code can be easily
> synchronized with X.

Yes, I agree that we should refactor the code to use GCs in order to
avoid these differences with X.  Maybe I can work on that and suggest a
patch for the master branch.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Change-cursor-color-on-NS-port-when-it-matches-the-f.patch --]
[-- Type: text/x-patch, Size: 5016 bytes --]

From f824d2e59921f14aa61ece17e8401c8a47c77d5f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=ADn?= <mardani29@yahoo.es>
Date: Sun, 2 Apr 2023 22:39:44 +0200
Subject: [PATCH] Change cursor color on NS port when it matches the face
 background

* src/macfont.m (CG_SET_FILL_COLOR_WITH_FRAME_CURSOR): New macro.
(CG_SET_FILL_COLOR_WITH_FRAME_BACKGROUND): New macro.
(macfont_draw): When the cursor's color matches the face background,
set the fill color of the cursor to the face foreground.
* src/nsterm.m (ns_maybe_dumpglyphs_background): When dumping the
background of a glyph string, apply the logic mentioned
above.  (Bug#62573)
---
 src/macfont.m | 32 ++++++++++++++++++++++++++------
 src/nsterm.m  | 20 ++++++++++++--------
 2 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/src/macfont.m b/src/macfont.m
index d0cdbcd08c7..a582bc1e38e 100644
--- a/src/macfont.m
+++ b/src/macfont.m
@@ -642,6 +642,20 @@ static void mac_font_get_glyphs_for_variants (CFDataRef, UTF32Char,
     CGContextSetFillColorWithColor (context, refcol_);                  \
     CGColorRelease (refcol_);                                           \
   } while (0)
+#define CG_SET_FILL_COLOR_WITH_FRAME_CURSOR(context, frame)             \
+  do {                                                                  \
+    CGColorRef refcol_ =                                                \
+      get_cgcolor_from_nscolor (FRAME_CURSOR_COLOR (frame), frame);     \
+    CGContextSetFillColorWithColor (context, refcol_);                  \
+    CGColorRelease (refcol_);                                           \
+  } while (0)
+#define CG_SET_FILL_COLOR_WITH_FRAME_BACKGROUND(context, frame)         \
+  do {                                                                  \
+    CGColorRef refcol_ =                                                \
+      get_cgcolor_from_nscolor (FRAME_BACKGROUND_COLOR (frame), frame); \
+    CGContextSetFillColorWithColor (context, refcol_);                  \
+    CGColorRelease (refcol_);                                           \
+  } while (0)
 #define CG_SET_STROKE_COLOR_WITH_FACE_FOREGROUND(context, face)         \
   do {                                                                  \
     CGColorRef refcol_ = get_cgcolor (NS_FACE_FOREGROUND (face));       \
@@ -2933,9 +2947,12 @@ So we use CTFontDescriptorCreateMatchingFontDescriptor (no
     {
       if (s->hl == DRAW_CURSOR)
         {
-	  CGColorRef colorref = get_cgcolor_from_nscolor (FRAME_CURSOR_COLOR (f), f);
-	  CGContextSetFillColorWithColor (context, colorref);
-	  CGColorRelease (colorref);
+          if (face && (NS_FACE_BACKGROUND (face)
+                       == [(NSColor *) FRAME_CURSOR_COLOR (f)
+                                       unsignedLong]))
+            CG_SET_FILL_COLOR_WITH_FACE_FOREGROUND (context, face);
+          else
+            CG_SET_FILL_COLOR_WITH_FRAME_CURSOR (context, f);
         }
       else
         CG_SET_FILL_COLOR_WITH_FACE_BACKGROUND (context, face);
@@ -2949,9 +2966,12 @@ So we use CTFontDescriptorCreateMatchingFontDescriptor (no
       CGContextScaleCTM (context, 1, -1);
       if (s->hl == DRAW_CURSOR)
         {
-	  CGColorRef colorref = get_cgcolor_from_nscolor (FRAME_BACKGROUND_COLOR (f), f);
-	  CGContextSetFillColorWithColor (context, colorref);
-	  CGColorRelease (colorref);
+          if (face && (NS_FACE_BACKGROUND (face)
+                       == [(NSColor *) FRAME_CURSOR_COLOR (f)
+                                       unsignedLong]))
+            CG_SET_FILL_COLOR_WITH_FACE_BACKGROUND (context, face);
+          else
+            CG_SET_FILL_COLOR_WITH_FRAME_BACKGROUND (context, f);
         }
       else
         CG_SET_FILL_COLOR_WITH_FACE_FOREGROUND (context, face);
diff --git a/src/nsterm.m b/src/nsterm.m
index 46007ec4fcb..637bc4b6419 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -3750,14 +3750,18 @@ Function modeled after x_draw_glyph_string_box ().
 	{
           struct face *face = s->face;
           if (!face->stipple)
-	    {
-	      if (s->hl != DRAW_CURSOR)
-		[(NS_FACE_BACKGROUND (face) != 0
-		  ? [NSColor colorWithUnsignedLong:NS_FACE_BACKGROUND (face)]
-		  : FRAME_BACKGROUND_COLOR (s->f)) set];
-	      else
-		[FRAME_CURSOR_COLOR (s->f) set];
-	    }
+            {
+              if (s->hl != DRAW_CURSOR)
+                [(NS_FACE_BACKGROUND (face) != 0
+                  ? [NSColor colorWithUnsignedLong:NS_FACE_BACKGROUND (face)]
+                  : FRAME_BACKGROUND_COLOR (s->f)) set];
+              else if (face && (NS_FACE_BACKGROUND (face)
+                                == [(NSColor *) FRAME_CURSOR_COLOR (s->f)
+                                                unsignedLong]))
+                [[NSColor colorWithUnsignedLong:NS_FACE_FOREGROUND (face)] set];
+              else
+                [FRAME_CURSOR_COLOR (s->f) set];
+            }
           else
             {
               struct ns_display_info *dpyinfo = FRAME_DISPLAY_INFO (s->f);
-- 
2.34.1