Re: [Emacs.app dev]: ghost cursor problem is still there

[David Reitter <david.reitter@gmail.com>]

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

On 22 Jul 2008, at 08:39, Adrian Robert wrote:
>
> Thanks, great stuff!  But could you please summarize the changes?   
> It shows like every line was changed (maybe because of indentation?)  
> but it doesn't look like that was actually the case.  Could you  
> regenerate the patch w/o indentation/tab changes?

Okay, have a look at what's below.

The main changes are that we check cursor_type instead of cursorType  
and draw the text glyph rather than the cursor when erasing anything  
(`hl' variable).  There's a range of steps that we do to ensure that  
the cursor area is actually visible; I'm not sure if they are really  
needed, but the corresponding X code does it, too.
There is a good bit of guess-work involved, but I'm sure that testing  
will take care of any problems.

>> I don't understand why internal-show-cursor  and the w- 
>> >cursor_off_p (that it sets) do not work in the NS port - they are  
>> checked by redisplay_internal.  Is there any commentary regarding  
>> stuff like that?
>
> This is the way it is..  ;-/  Usually you just have to get your  
> hands dirty and try to trace it back -- where does w->cursor_off_p  
> get set, when/how are other terms calling or triggering functions  
> that set it or trigger calling of draw_window_cursor(), etc..

Yeah, that's how I've been working... it doesn't really feel like  
2008. (And not even like 1999.)

> Did you have any sort of a start on using the emacs common code  
> (even though it didn't work)?  Maybe if you post a patch for that  
> others could take a stab at moving it further.

Well, if someone could explain why redisplay noticed a change in - 
 >cursor_off_p in Carbon or X, but not in NS, then that would already  
be helpful.  I don't think enabling the original blink-cursor-mode  
will be difficult, and you get a lot of functionality for free that way.

I'm trying again with a few extra TRACE steps enabled.

cc'ing emacs-devel for some advice regarding the patch.

- D



===================================================================
RCS file: /sources/emacs/emacs/src/nsterm.m,v
retrieving revision 1.11
diff -c -w -r1.11 nsterm.m
*** nsterm.m	21 Jul 2008 17:47:24 -0000	1.11
--- nsterm.m	22 Jul 2008 13:30:21 -0000
***************
*** 2274,2279 ****
--- 2274,2285 ----
                          int on_p, int active_p)
   /*  
--------------------------------------------------------------------------
        External call (RIF): draw cursor
+      (modeled after x_draw_window_cursor and erase_phys_cursor.
+      FIXME: erase_phys_cursor is called from display_and_set_cursor,
+      called from update_window_cursor/x_update_window_end/...
+      Why do we have to duplicate this code?
+      Also, why doesn't cursor_off_p (internal_show_cursor) work?
+      This prevents the original blink-cursor-mode from working.
       
-------------------------------------------------------------------------- */
   {
     NSRect r, s;
***************
*** 2282,2295 ****
     struct glyph *phys_cursor_glyph;
     int overspill;
     unsigned char drawGlyph = 0, cursorType, oldCursorType;

     NSTRACE (dumpcursor);

!   if (!on_p)
         return;

     w->phys_cursor_type = cursor_type;
!   w->phys_cursor_on_p = 1;

     if (cursor_type == NO_CURSOR)
       {
--- 2288,2310 ----
     struct glyph *phys_cursor_glyph;
     int overspill;
     unsigned char drawGlyph = 0, cursorType, oldCursorType;
+   int new_cursor_type;
+   int new_cursor_width;
+   int active_cursor;
+   enum draw_glyphs_face hl;
+   struct glyph_matrix *active_glyphs = w->current_matrix;
+   Display_Info *dpyinfo = FRAME_X_DISPLAY_INFO (f);
+   int hpos = w->phys_cursor.hpos;
+   int vpos = w->phys_cursor.vpos;
+   struct glyph_row *cursor_row;

     NSTRACE (dumpcursor);

!   if (!on_p) // check this?    && !w->phys_cursor_on_p)
         return;

     w->phys_cursor_type = cursor_type;
!   w->phys_cursor_on_p = on_p;

     if (cursor_type == NO_CURSOR)
       {
***************
*** 2322,2333 ****
--- 2337,2350 ----
     if (overspill > 0)
       r.size.width -= overspill;

+
     /* PENDING: 23: use emacs stored f->cursor_type instead of ns- 
specific */
     oldCursorType = FRAME_CURSOR (f);
     cursorType = FRAME_CURSOR (f) = FRAME_NEW_CURSOR (f);
     f->output_data.ns->current_cursor_color
       = f->output_data.ns->desired_cursor_color;

+
     /* PENDING: only needed in rare cases with last-resort font in  
HELLO..
        should we do this more efficiently? */
     ns_clip_to_row (w, glyph_row, -1, NULL);
***************
*** 2346,2357 ****
     if (cursorType == no_highlight || cursor_type == NO_CURSOR)
       {
         /* clearing for blink: erase the cursor itself */
         [FRAME_BACKGROUND_COLOR (f) set];
!       cursorType = oldCursorType; /* just clear what we had before */
       }
     else
         [FRAME_CURSOR_COLOR (f) set];

     if (!active_p)
       {
         /* inactive window: ignore what we just set and use a hollow  
box */
--- 2363,2441 ----
     if (cursorType == no_highlight || cursor_type == NO_CURSOR)
       {
         /* clearing for blink: erase the cursor itself */
+
+       /* No cursor displayed or row invalidated => nothing to do on  
the
+ 	 screen.  */
+       if (w->phys_cursor_type == NO_CURSOR)
+ 	return;
+
+       /* VPOS >= active_glyphs->nrows means that window has been  
resized.
+ 	 Don't bother to erase the cursor.  */
+       if (vpos >= active_glyphs->nrows)
+ 	return;
+
+       /* If row containing cursor is marked invalid, there is  
nothing we
+ 	 can do.  */
+       cursor_row = MATRIX_ROW (active_glyphs, vpos);
+       if (!cursor_row->enabled_p)
+ 	return;
+
+       /* If line spacing is > 0, old cursor may only be partially  
visible in
+ 	 window after split-window.  So adjust visible height.  */
+       cursor_row->visible_height = min (cursor_row->visible_height,
+ 					window_text_bottom_y (w) - cursor_row->y);
+
+       /* If row is completely invisible, don't attempt to delete a  
cursor which
+ 	 isn't there.  This can happen if cursor is at top of a window, and
+ 	 we switch to a buffer with a header line in that window.  */
+       if (cursor_row->visible_height <= 0)
+ 	return;
+
+       /* If cursor is in the fringe, erase by drawing actual bitmap  
there.  */
+       if (cursor_row->cursor_in_fringe_p)
+ 	{
+ 	  cursor_row->cursor_in_fringe_p = 0;
+ 	  draw_fringe_bitmap (w, cursor_row, 0);
+ 	  return;
+ 	}
+
+       /* This can happen when the new row is shorter than the old one.
+ 	 In this case, either draw_glyphs or clear_end_of_line
+ 	 should have cleared the cursor.  Note that we wouldn't be
+ 	 able to erase the cursor in this case because we don't have a
+ 	 cursor glyph at hand.  */
+       if (w->phys_cursor.hpos >= cursor_row->used[TEXT_AREA])
+ 	return;
+
+       /* If the cursor is in the mouse face area, redisplay that when
+ 	 we clear the cursor.  */
+       if (! NILP (dpyinfo->mouse_face_window)
+ 	  && w == XWINDOW (dpyinfo->mouse_face_window)
+ 	  && (vpos > dpyinfo->mouse_face_beg_row
+ 	      || (vpos == dpyinfo->mouse_face_beg_row
+ 		  && hpos >= dpyinfo->mouse_face_beg_col))
+ 	  && (vpos < dpyinfo->mouse_face_end_row
+ 	      || (vpos == dpyinfo->mouse_face_end_row
+ 		  && hpos < dpyinfo->mouse_face_end_col))
+ 	  /* Don't redraw the cursor's spot in mouse face if it is at the
+ 	     end of a line (on a newline).  The cursor appears there, but
+ 	     mouse highlighting does not.  */
+ 	  && cursor_row->used[TEXT_AREA] > hpos)
+ 	hl = DRAW_MOUSE_FACE;
+       else
+ 	hl = DRAW_NORMAL_TEXT;
+       drawGlyph = 1; // just draw the Glyph
         [FRAME_BACKGROUND_COLOR (f) set];
!
!       NSDisableScreenUpdates ();
       }
     else
+     {
+       cursorType = cursor_type;
+       hl = DRAW_CURSOR;
         [FRAME_CURSOR_COLOR (f) set];

+
         if (!active_p)
   	{
   	  /* inactive window: ignore what we just set and use a hollow box  
*/
***************
*** 2359,2396 ****
         [FRAME_CURSOR_COLOR (f) set];
       }

     switch (cursorType)
       {
!     case no_highlight:
         break;
!     case filled_box:
         NSRectFill (r);
         drawGlyph = 1;
         break;
!     case hollow_box:
         NSRectFill (r);
         [FRAME_BACKGROUND_COLOR (f) set];
         NSRectFill (NSInsetRect (r, 1, 1));
         [FRAME_CURSOR_COLOR (f) set];
         drawGlyph = 1;
         break;
!     case underscore:
         s = r;
         s.origin.y += lrint (0.75 * s.size.height);
!       s.size.height = lrint (s.size.height * 0.25);
         NSRectFill (s);
         break;
!     case bar:
         s = r;
!       s.size.width = 1;
         NSRectFill (s);
         break;
       }
     ns_unfocus (f);

     /* if needed, draw the character under the cursor */
     if (drawGlyph)
!     draw_phys_cursor_glyph (w, glyph_row, DRAW_CURSOR);
   }


--- 2443,2487 ----
   	  [FRAME_CURSOR_COLOR (f) set];
   	}

+       NSDisableScreenUpdates ();
+
         switch (cursorType)
   	{
! 	case NO_CURSOR: // no_highlight:
   	  break;
! 	case FILLED_BOX_CURSOR: //filled_box:
   	  NSRectFill (r);
   	  drawGlyph = 1;
   	  break;
! 	case HOLLOW_BOX_CURSOR: //hollow_box:
   	  NSRectFill (r);
   	  [FRAME_BACKGROUND_COLOR (f) set];
   	  NSRectFill (NSInsetRect (r, 1, 1));
   	  [FRAME_CURSOR_COLOR (f) set];
   	  drawGlyph = 1;
   	  break;
! 	case HBAR_CURSOR: // underscore:
   	  s = r;
   	  s.origin.y += lrint (0.75 * s.size.height);
! 	  s.size.height = cursor_width; //lrint (s.size.height * 0.25);
   	  NSRectFill (s);
   	  break;
! 	case BAR_CURSOR: //bar:
   	  s = r;
! 	  s.size.width = cursor_width;
   	  NSRectFill (s);
+ 	  drawGlyph = 1;
   	  break;
   	}
+     }
     ns_unfocus (f);

     /* if needed, draw the character under the cursor */
     if (drawGlyph)
!     draw_phys_cursor_glyph (w, glyph_row, hl);
!
!   NSEnableScreenUpdates ();
!
   }


~/Projects/Aquamacs/emacs/src$


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]