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.

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

[Arash Esbati]

Eli Zaretskii <eliz@gnu.org> writes:

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

Ben, do you see a chance to update your patch after Eli's comment?  TIA.

Best, Arash

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

[Rudolf Adamkovič]

Arash Esbati <arash@gnu.org> writes:

> Here is how it currently looks:
>
>   https://lists.gnu.org/archive/html/bug-gnu-emacs/2024-09/msg00304.html

OMG, this explains my recent troubles with Indent Bars on macOS.

+1 for merging!

Rudy
-- 
"The whole science is nothing more than a refinement of everyday
thinking."  --- Albert Einstein, 1879-1955

Rudolf Adamkovič <rudolf@adamkovic.org> [he/him]
http://adamkovic.org

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

[Ben Simms]

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

Hey all, I've gone ahead and put my changes behind NS_IMPL_COCA ifdefs:

From fb1ceddc6385bcefc3fb31f2c96652448298df77 Mon Sep 17 00:00:00 2001
From: Ben Simms <ben@bensimms.moe>
Date: Mon, 14 Oct 2024 19:32:53 +0100
Subject: [PATCH] Use masked coregraphics images on cocoa NS

---
 src/nsimage.m | 31 +++++++++++++++++++++++++++++++
 src/nsterm.h  |  8 ++++++++
 src/nsterm.m  | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/src/nsimage.m b/src/nsimage.m
index ee72d6e0ea1..e36cbe5dc87 100644
--- a/src/nsimage.m
+++ b/src/nsimage.m
@@ -35,6 +35,9 @@ Updated by Christian Limpach (chris@nice.ch)
 #include "frame.h"
 #include "coding.h"

+#ifdef NS_IMPL_COCOA
+#include <CoreGraphics/CoreGraphics.h>
+#endif

 #if defined (NS_IMPL_GNUSTEP) || MAC_OS_X_VERSION_MAX_ALLOWED < 1070
 # define COLORSPACE_NAME NSCalibratedRGBColorSpace
@@ -289,7 +292,11 @@ + (instancetype)allocInitFromFile: (Lisp_Object)file

 - (void)dealloc
 {
+#ifdef NS_IMPL_COCOA
+  CGImageRelease(stippleMask);
+#else
   [stippleMask release];
+#endif
   [bmRep release];
   [transform release];
   [super dealloc];
@@ -300,7 +307,11 @@ - (id)copyWithZone:(NSZone *)zone
 {
   EmacsImage *copy = [super copyWithZone:zone];

+#ifdef NS_IMPL_COCOA
+  copy->stippleMask = CGImageCreateCopy(stippleMask);
+#else
   copy->stippleMask = [stippleMask copyWithZone:zone];
+#endif /* NS_IMPL_COCOA */
   copy->bmRep = [bmRep copyWithZone:zone];
   copy->transform = [transform copyWithZone:zone];

@@ -509,6 +520,25 @@ - (void) setAlphaAtX: (int) x Y: (int) y to: (unsigned
char) a
     }
 }

+#ifdef NS_IMPL_COCOA
+/* Returns a cached CGImageMask of the stipple pattern */
+- (CGImageRef)stippleMask
+{
+  if (stippleMask == nil) {
+    CGDataProviderRef provider = CGDataProviderCreateWithData (NULL,
[bmRep bitmapData],
+                                                             [self
sizeInBytes], NULL);
+    CGImageRef mask = CGImageMaskCreate(
+                                        [self size].width,
+                                        [self size].height,
+                                        8, 8, [self size].width,
+                                        provider, NULL, 0);
+
+    CGDataProviderRelease(provider);
+    stippleMask = CGImageRetain(mask);
+  }
+  return stippleMask;
+}
+#else
 /* Returns a pattern color, which is cached here.  */
 - (NSColor *)stippleMask
 {
@@ -516,6 +546,7 @@ - (NSColor *)stippleMask
       stippleMask = [[NSColor colorWithPatternImage: self] retain];
   return stippleMask;
 }
+#endif /* NS_IMPL_COCOA */

 /* Find the first NSBitmapImageRep which has multiple frames.  */
 - (NSBitmapImageRep *)getAnimatedBitmapImageRep
diff --git a/src/nsterm.h b/src/nsterm.h
index 6c67653705e..2370f6ea3fb 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -671,7 +671,11 @@ #define NSTRACE_UNSILENCE()
 {
   NSBitmapImageRep *bmRep; /* used for accessing pixel data */
   unsigned char *pixmapData[5]; /* shortcut to access pixel data */
+#ifdef NS_IMPL_COCOA
+  CGImageRef stippleMask;
+#else
   NSColor *stippleMask;
+#endif // NS_IMPL_COCOA
 @public
   NSAffineTransform *transform;
   BOOL smoothing;
@@ -688,7 +692,11 @@ #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;
+#ifdef NS_IMPL_COCOA
+- (CGImageRef)stippleMask;
+#else
 - (NSColor *)stippleMask;
+#endif // NS_IMPL_COCOA
 - (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 f68a22d9fbc..a617669cb4d 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -3823,8 +3823,31 @@ Function modeled after x_draw_glyph_string_box ().
       if (s->stippled_p)
  {
   struct ns_display_info *dpyinfo = FRAME_DISPLAY_INFO (s->f);
+#ifdef NS_IMPL_COCOA
+    [[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;
+    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
   [[dpyinfo->bitmaps[face->stipple-1].img stippleMask] set];
   goto fill;
+#endif /* NS_IMPL_COCOA */
+
  }
       else if (FONT_HEIGHT (s->font) < s->height - 2 * box_line_width
        /* When xdisp.c ignores FONT_HEIGHT, we cannot trust font
@@ -3847,7 +3870,9 @@ Function modeled after x_draw_glyph_string_box ().
   else
     [FRAME_CURSOR_COLOR (s->f) set];

+#ifndef NS_IMPL_COCOA
  fill:
+#endif /* !NS_IMPL_COCOA */
   r = NSMakeRect (s->x, s->y + box_line_width,
   s->background_width,
   s->height - 2 * box_line_width);
@@ -4171,8 +4196,26 @@ 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 if (s->stippled_p) {
+#ifdef NS_IMPL_COCOA
+      [[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
+      [[dpyinfo->bitmaps[s->face->stipple - 1].img stippleMask] set];
+#endif /* NS_IMPL_COCOA */
+    }
   else
     [[NSColor colorWithUnsignedLong: s->face->background] set];

-- 
2.46.0


On Mon, 14 Oct 2024 at 02:29, Rudolf Adamkovič <rudolf@adamkovic.org> wrote:

> Arash Esbati <arash@gnu.org> writes:
>
> > Here is how it currently looks:
> >
> >   https://lists.gnu.org/archive/html/bug-gnu-emacs/2024-09/msg00304.html
>
> OMG, this explains my recent troubles with Indent Bars on macOS.
>
> +1 for merging!
>
> Rudy
> --
> "The whole science is nothing more than a refinement of everyday
> thinking."  --- Albert Einstein, 1879-1955
>
> Rudolf Adamkovič <rudolf@adamkovic.org> [he/him]
> http://adamkovic.org
>

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

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

[Arash Esbati]

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

> Hey all, I've gone ahead and put my changes behind NS_IMPL_COCA ifdefs:

Thanks.  I tried to test this but the patch doesn't apply:

$ git apply --check 00stipple.patch
error: patch failed: src/nsterm.m:3823
error: src/nsterm.m: patch does not apply

This is with Emacs from master 6213ca44.  Do you have the chance to
check and resend?

Best, Arash

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

[Ben Simms]

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

Sure, this is the patch from 6213ca44

From 1c4b8efb82bd0e35c91d98f2759217702f3a7c65 Mon Sep 17 00:00:00 2001
From: Ben Simms <ben@bensimms.moe>
Date: Mon, 14 Oct 2024 19:32:53 +0100
Subject: [PATCH] Use masked coregraphics images on cocoa NS

---
 src/nsimage.m | 31 +++++++++++++++++++++++++++++++
 src/nsterm.h  |  8 ++++++++
 src/nsterm.m  | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/src/nsimage.m b/src/nsimage.m
index ee72d6e0ea1..e36cbe5dc87 100644
--- a/src/nsimage.m
+++ b/src/nsimage.m
@@ -35,6 +35,9 @@ Updated by Christian Limpach (chris@nice.ch)
 #include "frame.h"
 #include "coding.h"

+#ifdef NS_IMPL_COCOA
+#include <CoreGraphics/CoreGraphics.h>
+#endif

 #if defined (NS_IMPL_GNUSTEP) || MAC_OS_X_VERSION_MAX_ALLOWED < 1070
 # define COLORSPACE_NAME NSCalibratedRGBColorSpace
@@ -289,7 +292,11 @@ + (instancetype)allocInitFromFile: (Lisp_Object)file

 - (void)dealloc
 {
+#ifdef NS_IMPL_COCOA
+  CGImageRelease(stippleMask);
+#else
   [stippleMask release];
+#endif
   [bmRep release];
   [transform release];
   [super dealloc];
@@ -300,7 +307,11 @@ - (id)copyWithZone:(NSZone *)zone
 {
   EmacsImage *copy = [super copyWithZone:zone];

+#ifdef NS_IMPL_COCOA
+  copy->stippleMask = CGImageCreateCopy(stippleMask);
+#else
   copy->stippleMask = [stippleMask copyWithZone:zone];
+#endif /* NS_IMPL_COCOA */
   copy->bmRep = [bmRep copyWithZone:zone];
   copy->transform = [transform copyWithZone:zone];

@@ -509,6 +520,25 @@ - (void) setAlphaAtX: (int) x Y: (int) y to: (unsigned
char) a
     }
 }

+#ifdef NS_IMPL_COCOA
+/* Returns a cached CGImageMask of the stipple pattern */
+- (CGImageRef)stippleMask
+{
+  if (stippleMask == nil) {
+    CGDataProviderRef provider = CGDataProviderCreateWithData (NULL,
[bmRep bitmapData],
+                                                             [self
sizeInBytes], NULL);
+    CGImageRef mask = CGImageMaskCreate(
+                                        [self size].width,
+                                        [self size].height,
+                                        8, 8, [self size].width,
+                                        provider, NULL, 0);
+
+    CGDataProviderRelease(provider);
+    stippleMask = CGImageRetain(mask);
+  }
+  return stippleMask;
+}
+#else
 /* Returns a pattern color, which is cached here.  */
 - (NSColor *)stippleMask
 {
@@ -516,6 +546,7 @@ - (NSColor *)stippleMask
       stippleMask = [[NSColor colorWithPatternImage: self] retain];
   return stippleMask;
 }
+#endif /* NS_IMPL_COCOA */

 /* Find the first NSBitmapImageRep which has multiple frames.  */
 - (NSBitmapImageRep *)getAnimatedBitmapImageRep
diff --git a/src/nsterm.h b/src/nsterm.h
index 6c67653705e..2370f6ea3fb 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -671,7 +671,11 @@ #define NSTRACE_UNSILENCE()
 {
   NSBitmapImageRep *bmRep; /* used for accessing pixel data */
   unsigned char *pixmapData[5]; /* shortcut to access pixel data */
+#ifdef NS_IMPL_COCOA
+  CGImageRef stippleMask;
+#else
   NSColor *stippleMask;
+#endif // NS_IMPL_COCOA
 @public
   NSAffineTransform *transform;
   BOOL smoothing;
@@ -688,7 +692,11 @@ #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;
+#ifdef NS_IMPL_COCOA
+- (CGImageRef)stippleMask;
+#else
 - (NSColor *)stippleMask;
+#endif // NS_IMPL_COCOA
 - (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 f68a22d9fbc..a617669cb4d 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -3823,8 +3823,31 @@ Function modeled after x_draw_glyph_string_box ().
       if (s->stippled_p)
  {
   struct ns_display_info *dpyinfo = FRAME_DISPLAY_INFO (s->f);
+#ifdef NS_IMPL_COCOA
+    [[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;
+    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
   [[dpyinfo->bitmaps[face->stipple-1].img stippleMask] set];
   goto fill;
+#endif /* NS_IMPL_COCOA */
+
  }
       else if (FONT_HEIGHT (s->font) < s->height - 2 * box_line_width
        /* When xdisp.c ignores FONT_HEIGHT, we cannot trust font
@@ -3847,7 +3870,9 @@ Function modeled after x_draw_glyph_string_box ().
   else
     [FRAME_CURSOR_COLOR (s->f) set];

+#ifndef NS_IMPL_COCOA
  fill:
+#endif /* !NS_IMPL_COCOA */
   r = NSMakeRect (s->x, s->y + box_line_width,
   s->background_width,
   s->height - 2 * box_line_width);
@@ -4171,8 +4196,26 @@ 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 if (s->stippled_p) {
+#ifdef NS_IMPL_COCOA
+      [[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
+      [[dpyinfo->bitmaps[s->face->stipple - 1].img stippleMask] set];
+#endif /* NS_IMPL_COCOA */
+    }
   else
     [[NSColor colorWithUnsignedLong: s->face->background] set];

-- 
2.46.0


On Thu, 17 Oct 2024 at 12:43, Arash Esbati <arash@gnu.org> wrote:

> Ben Simms <bsimms.simms@gmail.com> writes:
>
> > Hey all, I've gone ahead and put my changes behind NS_IMPL_COCA ifdefs:
>
> Thanks.  I tried to test this but the patch doesn't apply:
>
> $ git apply --check 00stipple.patch
> error: patch failed: src/nsterm.m:3823
> error: src/nsterm.m: patch does not apply
>
> This is with Emacs from master 6213ca44.  Do you have the chance to
> check and resend?
>
> Best, Arash
>

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

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

[Arash Esbati]

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

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

> Sure, this is the patch from 6213ca44

Thanks.  I copied the patch into the file attached and tried it again,
but still no avail:

$ git apply 00stipple.patch
00stipple.patch:179: trailing whitespace.
#else
error: patch failed: src/nsterm.m:3823
error: src/nsterm.m: patch does not apply

What am I missing?

Best, Arash

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

From 1c4b8efb82bd0e35c91d98f2759217702f3a7c65 Mon Sep 17 00:00:00 2001
From: Ben Simms <ben@bensimms.moe>
Date: Mon, 14 Oct 2024 19:32:53 +0100
Subject: [PATCH] Use masked coregraphics images on cocoa NS

---
 src/nsimage.m | 31 +++++++++++++++++++++++++++++++
 src/nsterm.h  |  8 ++++++++
 src/nsterm.m  | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/src/nsimage.m b/src/nsimage.m
index ee72d6e0ea1..e36cbe5dc87 100644
--- a/src/nsimage.m
+++ b/src/nsimage.m
@@ -35,6 +35,9 @@ Updated by Christian Limpach (chris@nice.ch)
 #include "frame.h"
 #include "coding.h"

+#ifdef NS_IMPL_COCOA
+#include <CoreGraphics/CoreGraphics.h>
+#endif

 #if defined (NS_IMPL_GNUSTEP) || MAC_OS_X_VERSION_MAX_ALLOWED < 1070
 # define COLORSPACE_NAME NSCalibratedRGBColorSpace
@@ -289,7 +292,11 @@ + (instancetype)allocInitFromFile: (Lisp_Object)file

 - (void)dealloc
 {
+#ifdef NS_IMPL_COCOA
+  CGImageRelease(stippleMask);
+#else
   [stippleMask release];
+#endif
   [bmRep release];
   [transform release];
   [super dealloc];
@@ -300,7 +307,11 @@ - (id)copyWithZone:(NSZone *)zone
 {
   EmacsImage *copy = [super copyWithZone:zone];

+#ifdef NS_IMPL_COCOA
+  copy->stippleMask = CGImageCreateCopy(stippleMask);
+#else
   copy->stippleMask = [stippleMask copyWithZone:zone];
+#endif /* NS_IMPL_COCOA */
   copy->bmRep = [bmRep copyWithZone:zone];
   copy->transform = [transform copyWithZone:zone];

@@ -509,6 +520,25 @@ - (void) setAlphaAtX: (int) x Y: (int) y to: (unsigned char) a
     }
 }

+#ifdef NS_IMPL_COCOA
+/* Returns a cached CGImageMask of the stipple pattern */
+- (CGImageRef)stippleMask
+{
+  if (stippleMask == nil) {
+    CGDataProviderRef provider = CGDataProviderCreateWithData (NULL, [bmRep bitmapData],
+                                                             [self sizeInBytes], NULL);
+    CGImageRef mask = CGImageMaskCreate(
+                                        [self size].width,
+                                        [self size].height,
+                                        8, 8, [self size].width,
+                                        provider, NULL, 0);
+
+    CGDataProviderRelease(provider);
+    stippleMask = CGImageRetain(mask);
+  }
+  return stippleMask;
+}
+#else
 /* Returns a pattern color, which is cached here.  */
 - (NSColor *)stippleMask
 {
@@ -516,6 +546,7 @@ - (NSColor *)stippleMask
       stippleMask = [[NSColor colorWithPatternImage: self] retain];
   return stippleMask;
 }
+#endif /* NS_IMPL_COCOA */

 /* Find the first NSBitmapImageRep which has multiple frames.  */
 - (NSBitmapImageRep *)getAnimatedBitmapImageRep
diff --git a/src/nsterm.h b/src/nsterm.h
index 6c67653705e..2370f6ea3fb 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -671,7 +671,11 @@ #define NSTRACE_UNSILENCE()
 {
   NSBitmapImageRep *bmRep; /* used for accessing pixel data */
   unsigned char *pixmapData[5]; /* shortcut to access pixel data */
+#ifdef NS_IMPL_COCOA
+  CGImageRef stippleMask;
+#else
   NSColor *stippleMask;
+#endif // NS_IMPL_COCOA
 @public
   NSAffineTransform *transform;
   BOOL smoothing;
@@ -688,7 +692,11 @@ #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;
+#ifdef NS_IMPL_COCOA
+- (CGImageRef)stippleMask;
+#else
 - (NSColor *)stippleMask;
+#endif // NS_IMPL_COCOA
 - (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 f68a22d9fbc..a617669cb4d 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -3823,8 +3823,31 @@ Function modeled after x_draw_glyph_string_box ().
       if (s->stippled_p)
  {
   struct ns_display_info *dpyinfo = FRAME_DISPLAY_INFO (s->f);
+#ifdef NS_IMPL_COCOA
+    [[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;
+    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
   [[dpyinfo->bitmaps[face->stipple-1].img stippleMask] set];
   goto fill;
+#endif /* NS_IMPL_COCOA */
+
  }
       else if (FONT_HEIGHT (s->font) < s->height - 2 * box_line_width
        /* When xdisp.c ignores FONT_HEIGHT, we cannot trust font
@@ -3847,7 +3870,9 @@ Function modeled after x_draw_glyph_string_box ().
   else
     [FRAME_CURSOR_COLOR (s->f) set];

+#ifndef NS_IMPL_COCOA
  fill:
+#endif /* !NS_IMPL_COCOA */
   r = NSMakeRect (s->x, s->y + box_line_width,
   s->background_width,
   s->height - 2 * box_line_width);
@@ -4171,8 +4196,26 @@ 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 if (s->stippled_p) {
+#ifdef NS_IMPL_COCOA
+      [[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 
+      [[dpyinfo->bitmaps[s->face->stipple - 1].img stippleMask] set];
+#endif /* NS_IMPL_COCOA */
+    }
   else
     [[NSColor colorWithUnsignedLong: s->face->background] set];

-- 
2.46.0

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

[Arash Esbati]

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

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

> I'm not sure, I've attached the patch instead of including it in the
> email body, perhaps this is better.

Yes, thanks, now it works.  I think it was due to a trailing whitespace
after else in this part:

+      [[NSGraphicsContext currentContext] restoreGraphicsState];
+#else
+      [[dpyinfo->bitmaps[s->face->stipple - 1].img stippleMask] set];

which was removed in the body.  At any rate, I applied your change,
built emacs anew from scratch, and it looks like this:

[-- Attachment #2: stipples-patch.png --]
[-- Type: image/png, Size: 14116 bytes --]

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


I'd say it works.  What do others think?  Many thanks for this.

Best, Arash

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

[Arash Esbati]

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

Arash Esbati <arash@gnu.org> writes:

> I'd say it works.  What do others think?  Many thanks for this.

Stefan, did you have a chance to look at this again?  I'm attaching the
patch Ben sent me off-list.  Maybe you can advise how to proceed and get
this installed.  TIA.

Best, Arash

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-masked-coregraphics-images-on-cocoa-NS.patch --]
[-- Type: text/x-patch, Size: 6731 bytes --]

From 1c4b8efb82bd0e35c91d98f2759217702f3a7c65 Mon Sep 17 00:00:00 2001
From: Ben Simms <ben@bensimms.moe>
Date: Mon, 14 Oct 2024 19:32:53 +0100
Subject: [PATCH] Use masked coregraphics images on cocoa NS

---
 src/nsimage.m | 31 +++++++++++++++++++++++++++++++
 src/nsterm.h  |  8 ++++++++
 src/nsterm.m  | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/src/nsimage.m b/src/nsimage.m
index ee72d6e0ea1..e36cbe5dc87 100644
--- a/src/nsimage.m
+++ b/src/nsimage.m
@@ -35,6 +35,9 @@ Updated by Christian Limpach (chris@nice.ch)
 #include "frame.h"
 #include "coding.h"
 
+#ifdef NS_IMPL_COCOA
+#include <CoreGraphics/CoreGraphics.h>
+#endif
 
 #if defined (NS_IMPL_GNUSTEP) || MAC_OS_X_VERSION_MAX_ALLOWED < 1070
 # define COLORSPACE_NAME NSCalibratedRGBColorSpace
@@ -289,7 +292,11 @@ + (instancetype)allocInitFromFile: (Lisp_Object)file
 
 - (void)dealloc
 {
+#ifdef NS_IMPL_COCOA
+  CGImageRelease(stippleMask);
+#else
   [stippleMask release];
+#endif
   [bmRep release];
   [transform release];
   [super dealloc];
@@ -300,7 +307,11 @@ - (id)copyWithZone:(NSZone *)zone
 {
   EmacsImage *copy = [super copyWithZone:zone];
 
+#ifdef NS_IMPL_COCOA
+  copy->stippleMask = CGImageCreateCopy(stippleMask);
+#else
   copy->stippleMask = [stippleMask copyWithZone:zone];
+#endif /* NS_IMPL_COCOA */
   copy->bmRep = [bmRep copyWithZone:zone];
   copy->transform = [transform copyWithZone:zone];
 
@@ -509,6 +520,25 @@ - (void) setAlphaAtX: (int) x Y: (int) y to: (unsigned char) a
     }
 }
 
+#ifdef NS_IMPL_COCOA
+/* Returns a cached CGImageMask of the stipple pattern */
+- (CGImageRef)stippleMask
+{
+  if (stippleMask == nil) {
+    CGDataProviderRef provider = CGDataProviderCreateWithData (NULL, [bmRep bitmapData],
+                                                             [self sizeInBytes], NULL);
+    CGImageRef mask = CGImageMaskCreate(
+                                        [self size].width,
+                                        [self size].height,
+                                        8, 8, [self size].width,
+                                        provider, NULL, 0);
+
+    CGDataProviderRelease(provider);
+    stippleMask = CGImageRetain(mask);
+  }
+  return stippleMask;
+}
+#else
 /* Returns a pattern color, which is cached here.  */
 - (NSColor *)stippleMask
 {
@@ -516,6 +546,7 @@ - (NSColor *)stippleMask
       stippleMask = [[NSColor colorWithPatternImage: self] retain];
   return stippleMask;
 }
+#endif /* NS_IMPL_COCOA */
 
 /* Find the first NSBitmapImageRep which has multiple frames.  */
 - (NSBitmapImageRep *)getAnimatedBitmapImageRep
diff --git a/src/nsterm.h b/src/nsterm.h
index 6c67653705e..2370f6ea3fb 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -671,7 +671,11 @@ #define NSTRACE_UNSILENCE()
 {
   NSBitmapImageRep *bmRep; /* used for accessing pixel data */
   unsigned char *pixmapData[5]; /* shortcut to access pixel data */
+#ifdef NS_IMPL_COCOA
+  CGImageRef stippleMask;
+#else
   NSColor *stippleMask;
+#endif // NS_IMPL_COCOA
 @public
   NSAffineTransform *transform;
   BOOL smoothing;
@@ -688,7 +692,11 @@ #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;
+#ifdef NS_IMPL_COCOA
+- (CGImageRef)stippleMask;
+#else
 - (NSColor *)stippleMask;
+#endif // NS_IMPL_COCOA
 - (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 f68a22d9fbc..a617669cb4d 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -3823,8 +3823,31 @@ Function modeled after x_draw_glyph_string_box ().
       if (s->stippled_p)
 	{
 	  struct ns_display_info *dpyinfo = FRAME_DISPLAY_INFO (s->f);
+#ifdef NS_IMPL_COCOA
+    [[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;
+    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
 	  [[dpyinfo->bitmaps[face->stipple-1].img stippleMask] set];
 	  goto fill;
+#endif /* NS_IMPL_COCOA */
+
 	}
       else if (FONT_HEIGHT (s->font) < s->height - 2 * box_line_width
 	       /* When xdisp.c ignores FONT_HEIGHT, we cannot trust font
@@ -3847,7 +3870,9 @@ Function modeled after x_draw_glyph_string_box ().
 	  else
 	    [FRAME_CURSOR_COLOR (s->f) set];
 
+#ifndef NS_IMPL_COCOA
 	fill:
+#endif /* !NS_IMPL_COCOA */
 	  r = NSMakeRect (s->x, s->y + box_line_width,
 			  s->background_width,
 			  s->height - 2 * box_line_width);
@@ -4171,8 +4196,26 @@ 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 if (s->stippled_p) {
+#ifdef NS_IMPL_COCOA
+      [[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 
+      [[dpyinfo->bitmaps[s->face->stipple - 1].img stippleMask] set];
+#endif /* NS_IMPL_COCOA */
+    }
 	  else
 	    [[NSColor colorWithUnsignedLong: s->face->background] set];
 
-- 
2.46.0

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

[Arash Esbati]

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

Arash Esbati <arash@gnu.org> writes:

> Stefan, did you have a chance to look at this again?  I'm attaching the
> patch Ben sent me off-list.  Maybe you can advise how to proceed and get
> this installed.  TIA.

Ping!

Do you see a chance to install this change?  Or should I ask Alan Third
if he can kindly have a look?

Best, Arash

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-masked-coregraphics-images-on-cocoa-NS.patch --]
[-- Type: text/x-patch, Size: 6731 bytes --]

From 1c4b8efb82bd0e35c91d98f2759217702f3a7c65 Mon Sep 17 00:00:00 2001
From: Ben Simms <ben@bensimms.moe>
Date: Mon, 14 Oct 2024 19:32:53 +0100
Subject: [PATCH] Use masked coregraphics images on cocoa NS

---
 src/nsimage.m | 31 +++++++++++++++++++++++++++++++
 src/nsterm.h  |  8 ++++++++
 src/nsterm.m  | 47 +++++++++++++++++++++++++++++++++++++++++++++--
 3 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/src/nsimage.m b/src/nsimage.m
index ee72d6e0ea1..e36cbe5dc87 100644
--- a/src/nsimage.m
+++ b/src/nsimage.m
@@ -35,6 +35,9 @@ Updated by Christian Limpach (chris@nice.ch)
 #include "frame.h"
 #include "coding.h"
 
+#ifdef NS_IMPL_COCOA
+#include <CoreGraphics/CoreGraphics.h>
+#endif
 
 #if defined (NS_IMPL_GNUSTEP) || MAC_OS_X_VERSION_MAX_ALLOWED < 1070
 # define COLORSPACE_NAME NSCalibratedRGBColorSpace
@@ -289,7 +292,11 @@ + (instancetype)allocInitFromFile: (Lisp_Object)file
 
 - (void)dealloc
 {
+#ifdef NS_IMPL_COCOA
+  CGImageRelease(stippleMask);
+#else
   [stippleMask release];
+#endif
   [bmRep release];
   [transform release];
   [super dealloc];
@@ -300,7 +307,11 @@ - (id)copyWithZone:(NSZone *)zone
 {
   EmacsImage *copy = [super copyWithZone:zone];
 
+#ifdef NS_IMPL_COCOA
+  copy->stippleMask = CGImageCreateCopy(stippleMask);
+#else
   copy->stippleMask = [stippleMask copyWithZone:zone];
+#endif /* NS_IMPL_COCOA */
   copy->bmRep = [bmRep copyWithZone:zone];
   copy->transform = [transform copyWithZone:zone];
 
@@ -509,6 +520,25 @@ - (void) setAlphaAtX: (int) x Y: (int) y to: (unsigned char) a
     }
 }
 
+#ifdef NS_IMPL_COCOA
+/* Returns a cached CGImageMask of the stipple pattern */
+- (CGImageRef)stippleMask
+{
+  if (stippleMask == nil) {
+    CGDataProviderRef provider = CGDataProviderCreateWithData (NULL, [bmRep bitmapData],
+                                                             [self sizeInBytes], NULL);
+    CGImageRef mask = CGImageMaskCreate(
+                                        [self size].width,
+                                        [self size].height,
+                                        8, 8, [self size].width,
+                                        provider, NULL, 0);
+
+    CGDataProviderRelease(provider);
+    stippleMask = CGImageRetain(mask);
+  }
+  return stippleMask;
+}
+#else
 /* Returns a pattern color, which is cached here.  */
 - (NSColor *)stippleMask
 {
@@ -516,6 +546,7 @@ - (NSColor *)stippleMask
       stippleMask = [[NSColor colorWithPatternImage: self] retain];
   return stippleMask;
 }
+#endif /* NS_IMPL_COCOA */
 
 /* Find the first NSBitmapImageRep which has multiple frames.  */
 - (NSBitmapImageRep *)getAnimatedBitmapImageRep
diff --git a/src/nsterm.h b/src/nsterm.h
index 6c67653705e..2370f6ea3fb 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -671,7 +671,11 @@ #define NSTRACE_UNSILENCE()
 {
   NSBitmapImageRep *bmRep; /* used for accessing pixel data */
   unsigned char *pixmapData[5]; /* shortcut to access pixel data */
+#ifdef NS_IMPL_COCOA
+  CGImageRef stippleMask;
+#else
   NSColor *stippleMask;
+#endif // NS_IMPL_COCOA
 @public
   NSAffineTransform *transform;
   BOOL smoothing;
@@ -688,7 +692,11 @@ #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;
+#ifdef NS_IMPL_COCOA
+- (CGImageRef)stippleMask;
+#else
 - (NSColor *)stippleMask;
+#endif // NS_IMPL_COCOA
 - (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 f68a22d9fbc..a617669cb4d 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -3823,8 +3823,31 @@ Function modeled after x_draw_glyph_string_box ().
       if (s->stippled_p)
 	{
 	  struct ns_display_info *dpyinfo = FRAME_DISPLAY_INFO (s->f);
+#ifdef NS_IMPL_COCOA
+    [[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;
+    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
 	  [[dpyinfo->bitmaps[face->stipple-1].img stippleMask] set];
 	  goto fill;
+#endif /* NS_IMPL_COCOA */
+
 	}
       else if (FONT_HEIGHT (s->font) < s->height - 2 * box_line_width
 	       /* When xdisp.c ignores FONT_HEIGHT, we cannot trust font
@@ -3847,7 +3870,9 @@ Function modeled after x_draw_glyph_string_box ().
 	  else
 	    [FRAME_CURSOR_COLOR (s->f) set];
 
+#ifndef NS_IMPL_COCOA
 	fill:
+#endif /* !NS_IMPL_COCOA */
 	  r = NSMakeRect (s->x, s->y + box_line_width,
 			  s->background_width,
 			  s->height - 2 * box_line_width);
@@ -4171,8 +4196,26 @@ 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 if (s->stippled_p) {
+#ifdef NS_IMPL_COCOA
+      [[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 
+      [[dpyinfo->bitmaps[s->face->stipple - 1].img stippleMask] set];
+#endif /* NS_IMPL_COCOA */
+    }
 	  else
 	    [[NSColor colorWithUnsignedLong: s->face->background] set];
 
-- 
2.46.0

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

[Stefan Kangas]

Alan, do you have any comments on the below patch?  Thanks in advance.

Arash Esbati <arash@gnu.org> writes:

> Arash Esbati <arash@gnu.org> writes:
>
>> Stefan, did you have a chance to look at this again?  I'm attaching the
>> patch Ben sent me off-list.  Maybe you can advise how to proceed and get
>> this installed.  TIA.
>
> Ping!
>
> Do you see a chance to install this change?  Or should I ask Alan Third
> if he can kindly have a look?
>
> From 1c4b8efb82bd0e35c91d98f2759217702f3a7c65 Mon Sep 17 00:00:00 2001
> From: Ben Simms <ben@bensimms.moe>
> Date: Mon, 14 Oct 2024 19:32:53 +0100
> Subject: [PATCH] Use masked coregraphics images on cocoa NS
>
> ---
>  src/nsimage.m | 31 +++++++++++++++++++++++++++++++
>  src/nsterm.h  |  8 ++++++++
>  src/nsterm.m  | 47 +++++++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 84 insertions(+), 2 deletions(-)
>
> diff --git a/src/nsimage.m b/src/nsimage.m
> index ee72d6e0ea1..e36cbe5dc87 100644
> --- a/src/nsimage.m
> +++ b/src/nsimage.m
> @@ -35,6 +35,9 @@ Updated by Christian Limpach (chris@nice.ch)
>  #include "frame.h"
>  #include "coding.h"
>
> +#ifdef NS_IMPL_COCOA
> +#include <CoreGraphics/CoreGraphics.h>
> +#endif
>
>  #if defined (NS_IMPL_GNUSTEP) || MAC_OS_X_VERSION_MAX_ALLOWED < 1070
>  # define COLORSPACE_NAME NSCalibratedRGBColorSpace
> @@ -289,7 +292,11 @@ + (instancetype)allocInitFromFile: (Lisp_Object)file
>
>  - (void)dealloc
>  {
> +#ifdef NS_IMPL_COCOA
> +  CGImageRelease(stippleMask);
> +#else
>    [stippleMask release];
> +#endif
>    [bmRep release];
>    [transform release];
>    [super dealloc];
> @@ -300,7 +307,11 @@ - (id)copyWithZone:(NSZone *)zone
>  {
>    EmacsImage *copy = [super copyWithZone:zone];
>
> +#ifdef NS_IMPL_COCOA
> +  copy->stippleMask = CGImageCreateCopy(stippleMask);
> +#else
>    copy->stippleMask = [stippleMask copyWithZone:zone];
> +#endif /* NS_IMPL_COCOA */
>    copy->bmRep = [bmRep copyWithZone:zone];
>    copy->transform = [transform copyWithZone:zone];
>
> @@ -509,6 +520,25 @@ - (void) setAlphaAtX: (int) x Y: (int) y to: (unsigned char) a
>      }
>  }
>
> +#ifdef NS_IMPL_COCOA
> +/* Returns a cached CGImageMask of the stipple pattern */
> +- (CGImageRef)stippleMask
> +{
> +  if (stippleMask == nil) {
> +    CGDataProviderRef provider = CGDataProviderCreateWithData (NULL, [bmRep bitmapData],
> +                                                             [self sizeInBytes], NULL);
> +    CGImageRef mask = CGImageMaskCreate(
> +                                        [self size].width,
> +                                        [self size].height,
> +                                        8, 8, [self size].width,
> +                                        provider, NULL, 0);
> +
> +    CGDataProviderRelease(provider);
> +    stippleMask = CGImageRetain(mask);
> +  }
> +  return stippleMask;
> +}
> +#else
>  /* Returns a pattern color, which is cached here.  */
>  - (NSColor *)stippleMask
>  {
> @@ -516,6 +546,7 @@ - (NSColor *)stippleMask
>        stippleMask = [[NSColor colorWithPatternImage: self] retain];
>    return stippleMask;
>  }
> +#endif /* NS_IMPL_COCOA */
>
>  /* Find the first NSBitmapImageRep which has multiple frames.  */
>  - (NSBitmapImageRep *)getAnimatedBitmapImageRep
> diff --git a/src/nsterm.h b/src/nsterm.h
> index 6c67653705e..2370f6ea3fb 100644
> --- a/src/nsterm.h
> +++ b/src/nsterm.h
> @@ -671,7 +671,11 @@ #define NSTRACE_UNSILENCE()
>  {
>    NSBitmapImageRep *bmRep; /* used for accessing pixel data */
>    unsigned char *pixmapData[5]; /* shortcut to access pixel data */
> +#ifdef NS_IMPL_COCOA
> +  CGImageRef stippleMask;
> +#else
>    NSColor *stippleMask;
> +#endif // NS_IMPL_COCOA
>  @public
>    NSAffineTransform *transform;
>    BOOL smoothing;
> @@ -688,7 +692,11 @@ #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;
> +#ifdef NS_IMPL_COCOA
> +- (CGImageRef)stippleMask;
> +#else
>  - (NSColor *)stippleMask;
> +#endif // NS_IMPL_COCOA
>  - (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 f68a22d9fbc..a617669cb4d 100644
> --- a/src/nsterm.m
> +++ b/src/nsterm.m
> @@ -3823,8 +3823,31 @@ Function modeled after x_draw_glyph_string_box ().
>        if (s->stippled_p)
>  	{
>  	  struct ns_display_info *dpyinfo = FRAME_DISPLAY_INFO (s->f);
> +#ifdef NS_IMPL_COCOA
> +    [[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;
> +    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
>  	  [[dpyinfo->bitmaps[face->stipple-1].img stippleMask] set];
>  	  goto fill;
> +#endif /* NS_IMPL_COCOA */
> +
>  	}
>        else if (FONT_HEIGHT (s->font) < s->height - 2 * box_line_width
>  	       /* When xdisp.c ignores FONT_HEIGHT, we cannot trust font
> @@ -3847,7 +3870,9 @@ Function modeled after x_draw_glyph_string_box ().
>  	  else
>  	    [FRAME_CURSOR_COLOR (s->f) set];
>
> +#ifndef NS_IMPL_COCOA
>  	fill:
> +#endif /* !NS_IMPL_COCOA */
>  	  r = NSMakeRect (s->x, s->y + box_line_width,
>  			  s->background_width,
>  			  s->height - 2 * box_line_width);
> @@ -4171,8 +4196,26 @@ 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 if (s->stippled_p) {
> +#ifdef NS_IMPL_COCOA
> +      [[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
> +      [[dpyinfo->bitmaps[s->face->stipple - 1].img stippleMask] set];
> +#endif /* NS_IMPL_COCOA */
> +    }
>  	  else
>  	    [[NSColor colorWithUnsignedLong: s->face->background] set];
>
> --
> 2.46.0

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

[Alan Third]

> +#ifdef NS_IMPL_COCOA
> +/* Returns a cached CGImageMask of the stipple pattern */
> +- (CGImageRef)stippleMask
> +{
> +  if (stippleMask == nil) {
> +    CGDataProviderRef provider = CGDataProviderCreateWithData (NULL, [bmRep bitmapData],
> +                                                             [self sizeInBytes], NULL);
> +    CGImageRef mask = CGImageMaskCreate(
> +                                        [self size].width,
> +                                        [self size].height,
> +                                        8, 8, [self size].width,
> +                                        provider, NULL, 0);

There's some weird formatting in this patch. Some of it looks like
it's perhaps due to email, but other bits, like the above, just look
wrong.

Other things I've noticed include C++ comments, //, instead of C
comments, /* */. Large blocks of code with no whitespace that is a bit
hard to follow. It would be nicer if it was broken up into logical
blocks.


> +    r = NSMakeRect (s->x, s->y + box_line_width,
> +        s->background_width,
> +        s->height - 2 * box_line_width);
<snip>
> +    NSRectFill (r);
> +    s->background_filled_p = 1;
> +    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);

NSRect and CGRect are the same thing, so here "r" and "bounds" are
identical. It might be worth just having one variable.

> +	  else if (s->stippled_p) {

Opening braces go on new lines.

Really that's it, Just some polishing required and a proper commit
message. Otherwise it looks OK to me.

I take it this doesn't require the addition of any extra build flags
to bring in CoreGraphics?
-- 
Alan Third