bug#73384: [PATCH] Draw coloured stipples on NS

bug#73384: [PATCH] Draw coloured stipples on NS

[Ben Simms]

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

Hello, I've been advised to submit my patch adding support for drawing
coloured stipples on NS Emacs here:

From 87c143b1b77ae02a08b9e1bbe27da57859e28a8d Mon Sep 17 00:00:00 2001
From: Ben Simms <ben@bensimms.moe>
Date: Fri, 20 Sep 2024 09:50:47 +0200
Subject: [PATCH] Support drawing coloured stipples

This makes use of CoreGraphics, while this works fine on macos systems,
I'm unsure how to test this on a GNUStep or other supported NS
implementation.
---
 src/nsimage.m | 17 +++++++++++++---
 src/nsterm.h  |  4 ++--
 src/nsterm.m  | 56 +++++++++++++++++++++++++++++++++++++++++++++------
 3 files changed, 66 insertions(+), 11 deletions(-)

diff --git a/src/nsimage.m b/src/nsimage.m
index ee72d6e0ea..100af5c3e9 100644
--- a/src/nsimage.m
+++ b/src/nsimage.m
@@ -28,6 +28,7 @@ Updated by Christian Limpach (chris@nice.ch)
 /* This should be the first include, as it may set up #defines affecting
    interpretation of even the system includes.  */
 #include <config.h>
+#include <CoreGraphics/CoreGraphics.h>

 #include "lisp.h"
 #include "dispextern.h"
@@ -510,10 +511,20 @@ - (void) setAlphaAtX: (int) x Y: (int) y to:
(unsigned char) a
 }

 /* Returns a pattern color, which is cached here.  */
-- (NSColor *)stippleMask
+- (CGImageRef)stippleMask
 {
-  if (stippleMask == nil)
-      stippleMask = [[NSColor colorWithPatternImage: self] retain];
+  if (stippleMask == nil) {
+    CGDataProviderRef provider = CGDataProviderCreateWithData (NULL,
[bmRep bitmapData],
+                                                             [self
sizeInBytes], NULL);
+    id mask = (id)CGImageMaskCreate(
+                                          [self size].width,
+                                          [self size].height,
+                                          8, 8, [self size].width,
+                                          provider, NULL, 0);
+
+    CGDataProviderRelease(provider);
+    stippleMask = (CGImageRef)[mask retain];
+  }
   return stippleMask;
 }

diff --git a/src/nsterm.h b/src/nsterm.h
index 3a713f8e8c..7ec851966f 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -670,7 +670,7 @@ #define NSTRACE_UNSILENCE()
 {
   NSBitmapImageRep *bmRep; /* used for accessing pixel data */
   unsigned char *pixmapData[5]; /* shortcut to access pixel data */
-  NSColor *stippleMask;
+  CGImageRef stippleMask;
 @public
   NSAffineTransform *transform;
   BOOL smoothing;
@@ -687,7 +687,7 @@ #define NSTRACE_UNSILENCE()
                green: (unsigned char)g blue: (unsigned char)b
               alpha:(unsigned char)a;
 - (void)setAlphaAtX: (int)x Y: (int)y to: (unsigned char)a;
-- (NSColor *)stippleMask;
+- (CGImageRef)stippleMask;
 - (Lisp_Object)getMetadata;
 - (BOOL)setFrame: (unsigned int) index;
 - (void)setTransform: (double[3][3]) m;
diff --git a/src/nsterm.m b/src/nsterm.m
index 794630de1c..84084f12a4 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -3826,9 +3826,28 @@ Function modeled after x_draw_glyph_string_box ().

       if (s->stippled_p)
  {
+  [[NSColor colorWithUnsignedLong:face->background] set];
+  r = NSMakeRect (s->x, s->y + box_line_width,
+  s->background_width,
+  s->height - 2 * box_line_width);
+  NSRectFill (r);
+  s->background_filled_p = 1;
   struct ns_display_info *dpyinfo = FRAME_DISPLAY_INFO (s->f);
-  [[dpyinfo->bitmaps[face->stipple-1].img stippleMask] set];
-  goto fill;
+  CGImageRef mask =
+    [dpyinfo->bitmaps[face->stipple - 1].img stippleMask];
+  CGRect bounds = CGRectMake (s->x, s->y + box_line_width,
+      s->background_width,
+      s->height - 2 * box_line_width);
+  NSGraphicsContext *ctx = [NSGraphicsContext currentContext];
+  [ctx saveGraphicsState];
+  CGContextRef context = [ctx CGContext];
+  CGContextClipToRect (context, bounds);
+  CGContextScaleCTM (context, 1, -1);
+  [[NSColor colorWithUnsignedLong:face->foreground] set];
+  CGRect imageSize = CGRectMake (0, 0, CGImageGetWidth (mask),
+ CGImageGetHeight (mask));
+  CGContextDrawTiledImage (context, imageSize, mask);
+  [[NSGraphicsContext currentContext] restoreGraphicsState];
  }
       else if (FONT_HEIGHT (s->font) < s->height - 2 * box_line_width
        /* When xdisp.c ignores FONT_HEIGHT, we cannot trust font
@@ -3851,7 +3870,6 @@ Function modeled after x_draw_glyph_string_box ().
   else
     [FRAME_CURSOR_COLOR (s->f) set];

- fill:
   r = NSMakeRect (s->x, s->y + box_line_width,
   s->background_width,
   s->height - 2 * box_line_width);
@@ -4175,10 +4193,36 @@ Function modeled after x_draw_glyph_string_box ().
   dpyinfo = FRAME_DISPLAY_INFO (s->f);
   if (s->hl == DRAW_CURSOR)
     [FRAME_CURSOR_COLOR (s->f) set];
-  else if (s->stippled_p)
-    [[dpyinfo->bitmaps[s->face->stipple - 1].img stippleMask] set];
-  else
+  else if (s->stippled_p) {
+      [[NSColor colorWithUnsignedLong:s->face->background]
+ set];
+      NSRectFill (
+ NSMakeRect (x, s->y, background_width, s->height));
+      CGImageRef mask =
+ [dpyinfo->bitmaps[s->face->stipple - 1]
+    .img stippleMask];
+      CGRect bounds
+ = CGRectMake (s->x, s->y, s->background_width,
+      s->height);
+      NSGraphicsContext *ctx =
+ [NSGraphicsContext currentContext];
+      [ctx saveGraphicsState];
+      CGContextRef context = [ctx CGContext];
+      CGContextClipToRect(context, bounds);
+      CGContextScaleCTM (context, 1, -1);
+      [[NSColor colorWithUnsignedLong:s->face->foreground]
+ set];
+      CGRect imageSize
+ = CGRectMake (0, 0, CGImageGetWidth (mask),
+      CGImageGetHeight (mask));
+      CGContextDrawTiledImage (context, imageSize, mask);
+      [[NSGraphicsContext currentContext]
+ restoreGraphicsState];
+    }
+  else {
     [[NSColor colorWithUnsignedLong: s->face->background] set];
+    NSRectFill (NSMakeRect (x, s->y, background_width, s->height));
+  }

   NSRectFill (NSMakeRect (x, s->y, background_width, s->height));
  }
-- 
2.45.2

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

bug#73384: [PATCH] Draw coloured stipples on NS

[Stefan Kangas]

Ben Simms <bsimms.simms@gmail.com> writes:

> Hello, I've been advised to submit my patch adding support for drawing
> coloured stipples on NS Emacs here:

Thanks for the patch.

Could you please also provide a simple recipe for testing this change?

bug#73384: [PATCH] Draw coloured stipples on NS

[Arash Esbati]

Stefan Kangas <stefankangas@gmail.com> writes:

> Could you please also provide a simple recipe for testing this change?

You could try the one provided here:

  https://github.com/jdtsmith/indent-bars?tab=readme-ov-file#testing-stipples

Here is how it currently looks:

  https://lists.gnu.org/archive/html/bug-gnu-emacs/2024-09/msg00304.html

Best, Arash

bug#73384: [PATCH] Draw coloured stipples on NS

[JD Smith]

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



> On Sep 21, 2024, at 11:08 AM, Arash Esbati <arash@gnu.org> wrote:
> 
> Stefan Kangas <stefankangas@gmail.com> writes:
> 
>> Could you please also provide a simple recipe for testing this change?

It would be good to move quickly to assess and integrate this patch in time for Emacs 30, since it fixes an existing bug in the display of :stipple face attributes on the NS port, which will likely confuse users.  Po I believe implemented the partial fix to NS stipples in ef6ffbdc79.  

See bug#73082 for more (and a test recipe).

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

bug#73384: [PATCH] Draw coloured stipples on NS

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

JD Smith <jdtsmith@gmail.com> writes:

>  On Sep 21, 2024, at 11:08 AM, Arash Esbati <arash@gnu.org>
>  wrote:
>
>  Stefan Kangas <stefankangas@gmail.com> writes:
>
>  Could you please also provide a simple recipe for testing this
>  change?
>
> It would be good to move quickly to assess and integrate this patch in
> time for Emacs 30, since it fixes an existing bug in the display of :stipple
> face attributes on the NS port, which will likely confuse users.  Po I
> believe implemented the partial fix to NS stipples in ef6ffbdc79.  
>
> See bug#73082 for more (and a test recipe).

We cannot install changes for Mac OS that use Mac-specific graphics
features, and in consequence, break the GNUstep build.

bug#73384: [PATCH] Draw coloured stipples on NS

[JD Smith]

> On Sep 28, 2024, at 7:49 PM, Po Lu <luangruo@yahoo.com> wrote:
> 
> JD Smith <jdtsmith@gmail.com> writes:
> 
>> On Sep 21, 2024, at 11:08 AM, Arash Esbati <arash@gnu.org>
>> wrote:
>> 
>> Stefan Kangas <stefankangas@gmail.com> writes:
>> 
>> Could you please also provide a simple recipe for testing this
>> change?
>> 
>> It would be good to move quickly to assess and integrate this patch in
>> time for Emacs 30, since it fixes an existing bug in the display of :stipple
>> face attributes on the NS port, which will likely confuse users.  Po I
>> believe implemented the partial fix to NS stipples in ef6ffbdc79.  
>> 
>> See bug#73082 for more (and a test recipe).
> 
> We cannot install changes for Mac OS that use Mac-specific graphics
> features, and in consequence, break the GNUstep build.

I see.  Are there generic GNUstep analogs that could be substituted?

bug#73384: [PATCH] Draw coloured stipples on NS

[Stefan Kangas]

Po Lu <luangruo@yahoo.com> writes:

> JD Smith <jdtsmith@gmail.com> writes:
>
>>  On Sep 21, 2024, at 11:08 AM, Arash Esbati <arash@gnu.org>
>>  wrote:
>>
>>  Stefan Kangas <stefankangas@gmail.com> writes:
>>
>>  Could you please also provide a simple recipe for testing this
>>  change?
>>
>> It would be good to move quickly to assess and integrate this patch in
>> time for Emacs 30, since it fixes an existing bug in the display of :stipple
>> face attributes on the NS port, which will likely confuse users.  Po I
>> believe implemented the partial fix to NS stipples in ef6ffbdc79.
>>
>> See bug#73082 for more (and a test recipe).
>
> We cannot install changes for Mac OS that use Mac-specific graphics
> features, and in consequence, break the GNUstep build.

We can make them conditional on macOS though, right?

bug#73384: [PATCH] Draw coloured stipples on NS

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stefan Kangas <stefankangas@gmail.com> writes:

>> We cannot install changes for Mac OS that use Mac-specific graphics
>> features, and in consequence, break the GNUstep build.
>
> We can make them conditional on macOS though, right?

I'll be very unsatisfied with such a solution, because it implies more
NS code that is impossible to test for Emacs developers in the Free
world.

bug#73384: [PATCH] Draw coloured stipples on NS

[Stefan Kangas]

Po Lu <luangruo@yahoo.com> writes:

> Stefan Kangas <stefankangas@gmail.com> writes:
>
>>> We cannot install changes for Mac OS that use Mac-specific graphics
>>> features, and in consequence, break the GNUstep build.
>>
>> We can make them conditional on macOS though, right?
>
> I'll be very unsatisfied with such a solution, because it implies more
> NS code that is impossible to test for Emacs developers in the Free
> world.

That would be the drawback, indeed.

Can anyone see a way to fix this bug using APIs that are also
implemented by GNUstep?

bug#73384: [PATCH] Draw coloured stipples on NS

[Eli Zaretskii]

> Cc: Ben Simms <bsimms.simms@gmail.com>, 73384@debbugs.gnu.org,
>  Arash Esbati <arash@gnu.org>, JD Smith <jdtsmith@gmail.com>
> Date: Mon, 30 Sep 2024 07:31:31 +0800
> From:  Po Lu via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Stefan Kangas <stefankangas@gmail.com> writes:
> 
> >> We cannot install changes for Mac OS that use Mac-specific graphics
> >> features, and in consequence, break the GNUstep build.
> >
> > We can make them conditional on macOS though, right?
> 
> I'll be very unsatisfied with such a solution, because it implies more
> NS code that is impossible to test for Emacs developers in the Free
> world.

The macOS port is full of those, so I don't think one more should make
a difference.