[PATCH] Feedback request for x_set_mouse_color on NextSTEP/MacOS

[PATCH] Feedback request for x_set_mouse_color on NextSTEP/MacOS

[Joe Matarazzo]

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

Hi, I would like feedback on a patch to enable changing the text and
non-text mouse pointer colors on MacOS, via "set-mouse-color". I'm new
to Emacs development and objective-C, so I expect I have a sub-optimal
and potentially leaky implementation.

I chose to only modify those pointers, as my implementation is a bit
heavy handed and changes the entire cursor image to the requested
color. On the I-beam (text) cursor this is ok, but the (nontext) arrow
pointer loses its thin white outline. The other cursors (the modeline
and resize pointers) don't suffer the visibility problems the
text/nontext pointers do when using a dark frame background, and are
left alone.

Thanks,
Joe

--------------

diff --git a/src/nsfns.m b/src/nsfns.m
index ca8f492..266ae5b 100644
--- a/src/nsfns.m
+++ b/src/nsfns.m
@@ -61,11 +61,15 @@ int fns_trace_num = 1;

 #ifdef HAVE_NS

+// Required for set-mouse-color
+#import <QuartzCore/CoreImage.h>
+
 extern NSArray *ns_send_types, *ns_return_types, *ns_drag_types;

 extern Lisp_Object Qforeground_color;
 extern Lisp_Object Qbackground_color;
 extern Lisp_Object Qcursor_color;
+extern Lisp_Object Qmouse_color;
 extern Lisp_Object Qinternal_border_width;
 extern Lisp_Object Qvisibility;
 extern Lisp_Object Qcursor_type;
@@ -878,15 +882,90 @@ x_set_cursor_type (struct frame *f, Lisp_Object arg,
Lisp_Object oldval)
   set_frame_cursor_types (f, arg);
 }

+
+#undef Z
+static void
+changeMouseColor(NSCursor **crs, NSColor *col)
+{
+
+  NSPoint hs    = [*crs hotSpot];
+  NSImage *img  = [*crs image];
+
+  CGContextRef myContext = [[NSGraphicsContext currentContext]
graphicsPort];
+  CIContext *context     = [CIContext contextWithCGContext:myContext
options:nil];
+
+  // Convert to CIImage input
+  NSRect  rect;
+  rect.origin = NSMakePoint(0.0, 0.0);
+  rect.size   = [img size];
+
+  CGImageRef cgImage = [img CGImageForProposedRect:&rect
+                                           context:[NSGraphicsContext
currentContext]
+                                             hints:nil];
+  CIImage *inputImage = [CIImage imageWithCGImage:cgImage];
+
+
+  // Set up the filter path
+  CIFilter *constColor = [CIFilter filterWithName:@
"CIConstantColorGenerator"];
+  CIColor  *newColor   = [CIColor colorWithCGColor:[col CGColor]];
+  [constColor setValue:newColor forKey: kCIInputColorKey];
+
+  // Needs crop to be usable
+  constColor = [CIFilter filterWithName:@"CICrop" keysAndValues:
+                           kCIInputImageKey, [constColor valueForKey:
kCIOutputImageKey],
+                         @"inputRectangle", [CIVector vectorWithX:0.0f
Y:0.0f Z:2.0f W:2.0f],
+                         nil];
+
+  // Apply a constant color map to change all colors in the image
+  CIFilter *colorMap   = [CIFilter filterWithName:@"CIColorMap"];
+  [colorMap setValue:inputImage forKey:kCIInputImageKey];
+  [colorMap setValue:[constColor valueForKey: kCIOutputImageKey]
+              forKey:kCIInputGradientImageKey];
+
+
+  // Render the output image
+  CIImage      *render = [colorMap valueForKey: kCIOutputImageKey];
+  CGRect        extent = [render extent];
+  CGImageRef  finalImg = [context createCGImage:render fromRect:extent];
+
+  NSImage *resultImage = [[NSImage alloc] initWithCGImage:finalImg
size:extent.size];
+
+  if (resultImage)
+    {
+      *crs = [[NSCursor alloc] initWithImage:resultImage hotSpot:hs];
+      [resultImage release];
+    }
+  else {
+    error("Could not change cursor");
+  }
+}
+
+
+
 /* called to set mouse pointer color, but all other terms use it to
    initialize pointer types (and don't set the color ;) */
 static void
 x_set_mouse_color (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
 {
-  /* don't think we can do this on Nextstep */
+  NSColor *col;
+  if (ns_lisp_to_color (arg, &col))
+    {
+      store_frame_param (f, Qmouse_color, oldval);
+      error ("Unknown color");
+    }
+
+  // Only change text and non-text cursor colors. The pointy hand and
resize
+  // cursors should be okay (ie. easily visible) in their appropriate
window
+  // locations.
+  if (FRAME_CURSOR(f, text))    changeMouseColor(&FRAME_CURSOR(f, text),
col);
+  if (FRAME_CURSOR(f, nontext)) changeMouseColor(&FRAME_CURSOR(f,
nontext), col);
+
+  store_frame_param(f, Qmouse_color, arg);
 }


+
+
 #define Str(x) #x
 #define Xstr(x) Str(x)

diff --git a/src/nsterm.h b/src/nsterm.h
index 00a0b54..7c26259 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -736,9 +736,10 @@ struct x_output

 #define FRAME_DEFAULT_FACE(f) FACE_FROM_ID (f, DEFAULT_FACE_ID)

-#define FRAME_NS_VIEW(f) ((f)->output_data.ns->view)
+#define FRAME_NS_VIEW(f)      ((f)->output_data.ns->view)
 #define FRAME_CURSOR_COLOR(f) ((f)->output_data.ns->cursor_color)
 #define FRAME_POINTER_TYPE(f) ((f)->output_data.ns->current_pointer)
+#define FRAME_CURSOR(f, t)    ((f)->output_data.ns->t##_cursor)

 #define FRAME_FONT(f) ((f)->output_data.ns->font)

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

Re: [PATCH] Feedback request for x_set_mouse_color on NextSTEP/MacOS

[Jan D.]

Hello.

Joe Matarazzo skrev 2014-08-12 20:32:
> Hi, I would like feedback on a patch to enable changing the text and
> non-text mouse pointer colors on MacOS, via "set-mouse-color". I'm new
> to Emacs development and objective-C, so I expect I have a sub-optimal
> and potentially leaky implementation.
>
> I chose to only modify those pointers, as my implementation is a bit
> heavy handed and changes the entire cursor image to the requested
> color. On the I-beam (text) cursor this is ok, but the (nontext) arrow
> pointer loses its thin white outline.

That would have to be fixed.

> The other cursors (the modeline
> and resize pointers) don't suffer the visibility problems the
> text/nontext pointers do when using a dark frame background, and are
> left alone.
>
> Thanks,
> Joe
>
> --------------
>
> diff --git a/src/nsfns.m b/src/nsfns.m
> index ca8f492..266ae5b 100644
> --- a/src/nsfns.m
> +++ b/src/nsfns.m
> @@ -61,11 +61,15 @@ int fns_trace_num = 1;
>   #ifdef HAVE_NS
> +// Required for set-mouse-color
> +#import <QuartzCore/CoreImage.h>
> +
>   extern NSArray *ns_send_types, *ns_return_types, *ns_drag_types;
>   extern Lisp_Object Qforeground_color;
>   extern Lisp_Object Qbackground_color;
>   extern Lisp_Object Qcursor_color;
> +extern Lisp_Object Qmouse_color;
>   extern Lisp_Object Qinternal_border_width;
>   extern Lisp_Object Qvisibility;
>   extern Lisp_Object Qcursor_type;
> @@ -878,15 +882,90 @@ x_set_cursor_type (struct frame *f, Lisp_Object
> arg, Lisp_Object oldval)
>     set_frame_cursor_types (f, arg);
>   }
> +
> +#undef Z

What is Z?

> +static void
> +changeMouseColor(NSCursor **crs, NSColor *col)

Avoid in/out parameter (crs), let the function return a NSCursor instead.

static NSCursor *
changeMouseColor(NSCursor *crs, NSColor *col)
...

> +{
> +
> +  NSPoint hs    = [*crs hotSpot];
> +  NSImage *img  = [*crs image];
> +
> +  CGContextRef myContext = [[NSGraphicsContext currentContext]
> graphicsPort];
> +  CIContext *context     = [CIContext contextWithCGContext:myContext
> options:nil];
> +
> +  // Convert to CIImage input
> +  NSRect  rect;
> +  rect.origin = NSMakePoint(0.0, 0.0);
> +  rect.size   = [img size];
> +
> +  CGImageRef cgImage = [img CGImageForProposedRect:&rect
> +                                           context:[NSGraphicsContext
> currentContext]
> +                                             hints:nil];
> +  CIImage *inputImage = [CIImage imageWithCGImage:cgImage];
> +
> +
> +  // Set up the filter path
> +  CIFilter *constColor = [CIFilter
> filterWithName:@"CIConstantColorGenerator"];
> +  CIColor  *newColor   = [CIColor colorWithCGColor:[col CGColor]];
> +  [constColor setValue:newColor forKey: kCIInputColorKey];
> +
> +  // Needs crop to be usable
> +  constColor = [CIFilter filterWithName:@"CICrop" keysAndValues:
> +                           kCIInputImageKey, [constColor valueForKey:
> kCIOutputImageKey],
> +                         @"inputRectangle", [CIVector vectorWithX:0.0f
> Y:0.0f Z:2.0f W:2.0f],
> +                         nil];
> +
> +  // Apply a constant color map to change all colors in the image
> +  CIFilter *colorMap   = [CIFilter filterWithName:@"CIColorMap"];
> +  [colorMap setValue:inputImage forKey:kCIInputImageKey];
> +  [colorMap setValue:[constColor valueForKey: kCIOutputImageKey]
> +              forKey:kCIInputGradientImageKey];
> +
> +
> +  // Render the output image
> +  CIImage      *render = [colorMap valueForKey: kCIOutputImageKey];
> +  CGRect        extent = [render extent];
> +  CGImageRef  finalImg = [context createCGImage:render fromRect:extent];
> +
> +  NSImage *resultImage = [[NSImage alloc] initWithCGImage:finalImg
> size:extent.size];
> +
> +  if (resultImage)
> +    {
> +      *crs = [[NSCursor alloc] initWithImage:resultImage hotSpot:hs];

This never gets reelased.

> +      [resultImage release];
> +    }
> +  else {
> +    error("Could not change cursor");
> +  }
> +}
> +

This whole function is very OSX specific.  It would be preferred if 
there was a common GNUStep/OSX implementation.  At the very least we 
would need a GNUStep implementation also before bringing it in Emacs.

> +
> +
>   /* called to set mouse pointer color, but all other terms use it to
>      initialize pointer types (and don't set the color ;) */
>   static void
>   x_set_mouse_color (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
>   {
> -  /* don't think we can do this on Nextstep */
> +  NSColor *col;
> +  if (ns_lisp_to_color (arg, &col))
> +    {
> +      store_frame_param (f, Qmouse_color, oldval);
> +      error ("Unknown color");
> +    }
> +
> +  // Only change text and non-text cursor colors. The pointy hand and
> resize
> +  // cursors should be okay (ie. easily visible) in their appropriate
> window
> +  // locations.
> +  if (FRAME_CURSOR(f, text))    changeMouseColor(&FRAME_CURSOR(f,
> text), col);
> +  if (FRAME_CURSOR(f, nontext)) changeMouseColor(&FRAME_CURSOR(f,
> nontext), col);
> +
> +  store_frame_param(f, Qmouse_color, arg);
>   }
> +
> +
>   #define Str(x) #x
>   #define Xstr(x) Str(x)
> diff --git a/src/nsterm.h b/src/nsterm.h
> index 00a0b54..7c26259 100644
> --- a/src/nsterm.h
> +++ b/src/nsterm.h
> @@ -736,9 +736,10 @@ struct x_output
>   #define FRAME_DEFAULT_FACE(f) FACE_FROM_ID (f, DEFAULT_FACE_ID)
> -#define FRAME_NS_VIEW(f) ((f)->output_data.ns->view)
> +#define FRAME_NS_VIEW(f)      ((f)->output_data.ns->view)

Do not make whitespace only changes.

>   #define FRAME_CURSOR_COLOR(f) ((f)->output_data.ns->cursor_color)
>   #define FRAME_POINTER_TYPE(f) ((f)->output_data.ns->current_pointer)
> +#define FRAME_CURSOR(f, t)    ((f)->output_data.ns->t##_cursor)
>   #define FRAME_FONT(f) ((f)->output_data.ns->font)

	Jan D.

Re: [PATCH] Feedback request for x_set_mouse_color on NextSTEP/MacOS

[Joe Matarazzo]

Jan D. <jan.h.d <at> swipnet.se> writes:

> 
> This whole function is very OSX specific.  It would be preferred if 
> there was a common GNUStep/OSX implementation.  At the very least we 
> would need a GNUStep implementation also before bringing it in Emacs.
> 

Thanks for the feedback Jan. Based on your input, I've decided to go back
and try a different approach that will only use NS* routines, to stay 
compliant with GNUstep. I'll post another patch when I have it working.

Joe

Re: [PATCH] Feedback request for x_set_mouse_color on NextSTEP/MacOS

[Jan Djärv]

Hello.

15 aug 2014 kl. 17:28 skrev Joe Matarazzo <joe.matarazzo@gmail.com>:

> Jan D. <jan.h.d <at> swipnet.se> writes:
> 
>> 
>> This whole function is very OSX specific.  It would be preferred if 
>> there was a common GNUStep/OSX implementation.  At the very least we 
>> would need a GNUStep implementation also before bringing it in Emacs.
>> 
> 
> Thanks for the feedback Jan. Based on your input, I've decided to go back
> and try a different approach that will only use NS* routines, to stay 
> compliant with GNUstep. I'll post another patch when I have it working.

It is not requred that the same code works on both OSX and GNUStep, but that there is an implementation for GNUStep and OSX.  They might be quite different #ifdef... blocks of code.

	Jan D.

Re: [PATCH] Feedback request for x_set_mouse_color on NextSTEP/MacOS

[Stefan Monnier]

> It is not requred that the same code works on both OSX and GNUStep, but that
> there is an implementation for GNUStep and OSX.  They might be quite
> different #ifdef... blocks of code.

But if it can be "the same code", it's clearly a better choice.


        Stefan

Re: [PATCH] Feedback request for x_set_mouse_color on NextSTEP/MacOS

[Jan Djärv]

Hi.

15 aug 2014 kl. 20:46 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

>> It is not requred that the same code works on both OSX and GNUStep, but that
>> there is an implementation for GNUStep and OSX.  They might be quite
>> different #ifdef... blocks of code.
> 
> But if it can be "the same code", it's clearly a better choice.
> 

Yes, but a fast look at the API:s suggests that this is not possible.  I would like to be shown wrong though.

	Jan D.

Re: [PATCH] Feedback request for x_set_mouse_color on NextSTEP/MacOS

[Joe Matarazzo]

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

On Sat, Aug 16, 2014 at 1:52 AM, Jan Djärv <jan.h.d@swipnet.se> wrote:

> Hi.
>
> 15 aug 2014 kl. 20:46 skrev Stefan Monnier <monnier@iro.umontreal.ca>:
>
> >> It is not requred that the same code works on both OSX and GNUStep, but
> that
> >> there is an implementation for GNUStep and OSX.  They might be quite
> >> different #ifdef... blocks of code.
> >
> > But if it can be "the same code", it's clearly a better choice.
> >
>
> Yes, but a fast look at the API:s suggests that this is not possible.  I
> would like to be shown wrong though.
>
>


I've been having some early success creating a fascimile of the system
cursors by drawing into
a custom image rep and then tying it to a new cursor via
-initWithImage:hotSpot:. I can duplicate
the vector graphics and fill/stroke the paths to the requested colors.

What do you think of this approach?

Joe

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

Re: [PATCH] Feedback request for x_set_mouse_color on NextSTEP/MacOS

[Jan Djärv]

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

Hi. 

> 17 aug 2014 kl. 05:06 skrev Joe Matarazzo <joe.matarazzo@gmail.com>:
> 
> 
>> On Sat, Aug 16, 2014 at 1:52 AM, Jan Djärv <jan.h.d@swipnet.se> wrote:
>> Hi.
>> 
>> 15 aug 2014 kl. 20:46 skrev Stefan Monnier <monnier@iro.umontreal.ca>:
>> 
>> >> It is not requred that the same code works on both OSX and GNUStep, but that
>> >> there is an implementation for GNUStep and OSX.  They might be quite
>> >> different #ifdef... blocks of code.
>> >
>> > But if it can be "the same code", it's clearly a better choice.
>> >
>> 
>> Yes, but a fast look at the API:s suggests that this is not possible.  I would like to be shown wrong though.
>  
> I've been having some early success creating a fascimile of the system cursors by drawing into
> a custom image rep and then tying it to a new cursor via -initWithImage:hotSpot:. I can duplicate
> the vector graphics and fill/stroke the paths to the requested colors.
> 
> What do you think of this approach?

Sounds like a good plan. 


     Jan D

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

Re: [PATCH] Feedback request for x_set_mouse_color on NextSTEP/MacOS

[Joe Matarazzo]

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

Some implementation questions:

1. The source tree includes a "left pointer" xbm that I was able to
leverage for the arrow cursor. However,
it doesn't visually match the Mac OS system arrow cursor. Also, it doesn't
scale well if the Accessibility panel
mouse cursor size slider is used to grow the cursor size. I've found that
you need to provide an NSImage
with multiple higher resolution NSImageRep's underneath to get nice scaled
cursors.

Would it be preferable to use the included src/bitmaps/leftptr.xbm and
adhere to more of an X display, or
adhere more to the style of the GNU/NeXTStep cursors and roll my own
bitmap? (This is for the "nontext_cursor"
in nsfns.m)

2. The source tree doesn't have an equivalent i-beam cursor that MacOS uses
for the text_cursor, that I could
see. If I end up creating one to match the system style and also provide
color control, it influences the answer to
#1, IMO. What guidance can the dev-list offer on this point? Add a new XBM
to src/bitmaps or make it
NS-specific, with the added scaling goodness that follows?

Thanks,
Joe

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

Re: [PATCH] Feedback request for x_set_mouse_color on NextSTEP/MacOS

[Jan Djärv]

Hello.

26 aug 2014 kl. 07:48 skrev Joe Matarazzo <joe.matarazzo@gmail.com>:

> Some implementation questions:
> 
> 1. The source tree includes a "left pointer" xbm that I was able to leverage for the arrow cursor. However,
> it doesn't visually match the Mac OS system arrow cursor. Also, it doesn't scale well if the Accessibility panel
> mouse cursor size slider is used to grow the cursor size. I've found that you need to provide an NSImage
> with multiple higher resolution NSImageRep's underneath to get nice scaled cursors.
> 
> Would it be preferable to use the included src/bitmaps/leftptr.xbm and adhere to more of an X display, or
> adhere more to the style of the GNU/NeXTStep cursors and roll my own bitmap? (This is for the "nontext_cursor"
> in nsfns.m)

leftptr.xbm is only used for the old (in practice never used) menu implementation.  Normal is to  use platform cursors.  If you can make a good image for NS go for it.

> 
> 2. The source tree doesn't have an equivalent i-beam cursor that MacOS uses for the text_cursor, that I could
> see. If I end up creating one to match the system style and also provide color control, it influences the answer to
> #1, IMO. What guidance can the dev-list offer on this point? Add a new XBM to src/bitmaps or make it 
> NS-specific, with the added scaling goodness that follows?

See above.

	Jan D.