Re: Patch to ensure that the hbar cursor is drawn correctly on Mac OS X

Re: Patch to ensure that the hbar cursor is drawn correctly on Mac OS X

[Ben Key]

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

Adrian Robert writes:
> Thanks a lot for this patch.  Could you try this one instead?

I tried your patch and it has the desired effect.  I agree that this patch
is better because it does have the benefit of keeping the "width"-handling
in one place.  As a result, I actually prefer your patch to mine.

Thanks.

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

Re: Patch to ensure that the hbar cursor is drawn correctly on Mac OS X

[Adrian Robert]

Ben Key <bkey76 <at> gmail.com> writes:

> 
> I tried your patch and it has the desired effect.  I agree that this patch is
> better because it does have the benefit of keeping the "width"-handling in one
> place.  As a result, I actually prefer your patch to mine.

OK, thanks, I've commited it to trunk. 

-Adrian

Patch to ensure that the hbar cursor is drawn correctly on Mac OS X

[Ben Key]

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

Hello,

The following patch fixes problems with the hbar cursor on Mac OS X that
were caused by recent changes to ns_draw_window_cursor.  As I mentioned in
an earlier message, when the user sets the cursor-type variable to the form
(hbar . HEIGHT), the user specified value for HEIGHT is being used as the
cursor width even though the expected behavior is for the cursor to be as
wide as the glyph at point.  This patch also fixes a long standing problem
that caused the user specified cursor height to be ignored in this case (the
cursor height was set to 25% of the height of the glyph at point instead).
This problem goes back to at least Emacs 23.2.

<patch>
=== modified file 'src/nsterm.m'
--- src/nsterm.m    2011-03-05 23:55:43 +0000
+++ src/nsterm.m    2011-03-06 03:58:47 +0000
@@ -2232,6 +2232,9 @@
 /*
--------------------------------------------------------------------------
      External call (RIF): draw cursor.
      Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
+     Despite the parameter name, cursor_width is set to the user
+     specified cursor width for a bar cursor and the user specified
+     cursor height for a hbar cursor.

--------------------------------------------------------------------------
*/
 {
   NSRect r, s;
@@ -2280,7 +2283,7 @@

   /* The above get_phys_cursor_geometry call set w->phys_cursor_width
      to the glyph width; replace with CURSOR_WIDTH for bar cursors. */
-  if (cursor_type == BAR_CURSOR || cursor_type == HBAR_CURSOR)
+  if (cursor_type == BAR_CURSOR)
     {
       if (cursor_width < 1)
     cursor_width = max (FRAME_CURSOR_WIDTH (f), 1);
@@ -2338,8 +2341,29 @@
       break;
     case HBAR_CURSOR:
       s = r;
-      s.origin.y += lrint (0.75 * s.size.height);
-      s.size.height = lrint (s.size.height * 0.25);
+      /*
+      When the user sets the cursor-type variable to the form
+      (hbar . HEIGHT), the cursor_width parameter is equal to the user
+      specified value for height when this function is called.
+      Previously Emacs was not honoring this value.  The following if
+      statement is designed to resolve this problem.  Note that at
+      this point, s.origin.y is equal to the pixel position of the top
+      of the character at point.  The expectation is for the cursor to
+      be placed at the bottom of the character at point.  It is
+      important to keep this in mind when modifying s here.  At this
+      point, s.size.height is equal to the height of the glyph at
+      point.
+      */
+      if (cursor_width > 1)
+        {
+          s.origin.y += s.size.height - cursor_width;
+          s.size.height = cursor_width;
+        }
+      else
+        {
+          s.origin.y += lrint (0.75 * s.size.height);
+          s.size.height = lrint (s.size.height * 0.25);
+        }
       NSRectFill (s);
       break;
     case BAR_CURSOR:

</patch>

With this change, the cursor is drawn correctly on Mac OS X when the
cursor-type variable is set to a value with the form (bar . WIDTH) or the
form (hbar . HEIGHT).

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

Re: Patch to ensure that the hbar cursor is drawn correctly on Mac OS X

[Adrian Robert]

Ben Key <bkey76 <at> gmail.com> writes:

> 
> Hello,The following patch fixes problems with the hbar cursor on
> Mac OS X that were caused by recent changes to ns_draw_window_cursor.


Hi,

Thanks a lot for this patch.  Could you try this one instead?
It's based on yours, but I'd rather keep the "width"-handling in one place.
Also, a height of 1 seems acceptable.

thanks,
Adrian




=== modified file 'src/nsterm.m'
--- src/nsterm.m	2011-03-05 23:55:43 +0000
+++ src/nsterm.m	2011-03-06 08:46:02 +0000
@@ -2235,7 +2235,7 @@ ns_draw_window_cursor (struct window *w,
    ---------------------------------------------------------
----------------- */
 {
   NSRect r, s;
-  int fx, fy, h;
+  int fx, fy, h, cursor_height;
   struct frame *f = WINDOW_XFRAME (w);
   struct glyph *phys_cursor_glyph;
   int overspill;
@@ -2279,13 +2279,20 @@ ns_draw_window_cursor (struct window *w,
   get_phys_cursor_geometry (w, glyph_row, phys_cursor_glyph, &fx, &fy, &h);
 
   /* The above get_phys_cursor_geometry call set w->phys_cursor_width
-     to the glyph width; replace with CURSOR_WIDTH for bar cursors. */
-  if (cursor_type == BAR_CURSOR || cursor_type == HBAR_CURSOR)
+     to the glyph width; replace with CURSOR_WIDTH for (V)BAR cursors. */
+  if (cursor_type == BAR_CURSOR)
     {
       if (cursor_width < 1)
 	cursor_width = max (FRAME_CURSOR_WIDTH (f), 1);
       w->phys_cursor_width = cursor_width;
     }
+  /* If we have an HBAR, "cursor_width" MAY specify height. */
+  else if (cursor_type == HBAR_CURSOR)
+    {
+      cursor_height = (cursor_width < 1) ? lrint (0.25 * h) : cursor_width;
+      fy += h - cursor_height;
+      h = cursor_height;
+    }
 
   r.origin.x = fx, r.origin.y = fy;
   r.size.height = h;
@@ -2337,10 +2344,7 @@ ns_draw_window_cursor (struct window *w,
       [FRAME_CURSOR_COLOR (f) set];
       break;
     case HBAR_CURSOR:
-      s = r;
-      s.origin.y += lrint (0.75 * s.size.height);
-      s.size.height = lrint (s.size.height * 0.25);
-      NSRectFill (s);
+      NSRectFill (r);
       break;
     case BAR_CURSOR:
       s = r;