Re: macOS: Cursor leaving traces when scrolling

[Alan Third <alan@idiocy.org>]

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

On Sat, Oct 06, 2018 at 08:06:29AM -0400, David Reitter wrote:
> On Oct 4, 2018, at 5:58 PM, Alan Third <alan@idiocy.org> wrote:
> > I believe it’s because we’re copying the contents of the frame on
> > scroll without redrawing it without the cursor first. I think that
> > means we need to stick a call to NSView displayIfNeeded into
> > ns_copy_bits or ns_scroll_run.
> 
> It seems to be limited to scrolling, not all cursor movement, so yes.  
> Any news on this?

It seems this is more complicated that I thought. Adding a call to
displayIfNeeded causes the whole frame to flicker and doesn’t always
get rid of the cursor. I’ve no idea why.

The attached patch fixes it, but it marks the whole area dirty instead
of copying it, so it will be redrawn at the next expose event. I can’t
see any difference in simple testing, but it may slow down scrolling
on very complex buffers.
-- 
Alan Third

[-- Attachment #2: 0001-Fix-NS-redraw-errors.patch --]
[-- Type: text/plain, Size: 3593 bytes --]

From e828710d75d1271957375b8b132bef0c4e62a964 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Thu, 4 Oct 2018 22:47:23 +0100
Subject: [PATCH] Fix NS redraw errors

* src/nsterm.m (ns_clip_to_rect, ns_reset_clipping): Always pop the
graphics state.
(ns_copy_bits): Remove function.
(ns_scroll_run):
(ns_shift_glyphs_for_insert): Remove references to ns_copy_bits and
mark changed areas for redraw.
---
 src/nsterm.m | 39 ++++++++++-----------------------------
 1 file changed, 10 insertions(+), 29 deletions(-)

diff --git a/src/nsterm.m b/src/nsterm.m
index d92d6c3244..e4958543eb 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -277,7 +277,6 @@ - (NSColor *)colorUsingDefaultColorSpace
 
 /* display update */
 static int ns_window_num = 0;
-static BOOL gsaved = NO;
 static BOOL ns_fake_keydown = NO;
 #ifdef NS_IMPL_COCOA
 static BOOL ns_menu_bar_is_hidden = NO;
@@ -1180,7 +1179,6 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
             NSRectClipList (r, 2);
           else
             NSRectClip (*r);
-          gsaved = YES;
 
           return YES;
         }
@@ -1204,11 +1202,7 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
 {
   NSTRACE_WHEN (NSTRACE_GROUP_FOCUS, "ns_reset_clipping");
 
-  if (gsaved)
-    {
-      [[NSGraphicsContext currentContext] restoreGraphicsState];
-      gsaved = NO;
-    }
+  [[NSGraphicsContext currentContext] restoreGraphicsState];
 }
 
 
@@ -2707,23 +2701,6 @@ so some key presses (TAB) are swallowed by the system. */
     }
 }
 
-static void
-ns_copy_bits (struct frame *f, NSRect src, NSRect dest)
-{
-  NSTRACE ("ns_copy_bits");
-
-  if (FRAME_NS_VIEW (f))
-    {
-      hide_bell();              // Ensure the bell image isn't scrolled.
-
-      /* FIXME: scrollRect:by: is deprecated in macOS 10.14.  There is
-         no obvious replacement so we may have to come up with our own.  */
-      [FRAME_NS_VIEW (f) scrollRect: src
-                                 by: NSMakeSize (dest.origin.x - src.origin.x,
-                                                 dest.origin.y - src.origin.y)];
-    }
-}
-
 static void
 ns_scroll_run (struct window *w, struct run *run)
 /* --------------------------------------------------------------------------
@@ -2773,10 +2750,12 @@ so some key presses (TAB) are swallowed by the system. */
   x_clear_cursor (w);
 
   {
-    NSRect srcRect = NSMakeRect (x, from_y, width, height);
+    /* We should do this by copying the contents of the NSView,
+       however x_clear_cursor, above, seems to leave detritus since we
+       changed to the mark-dirty/expose method, and simply redrawing
+       the whole thing seems to have no performance issues.  */
     NSRect dstRect = NSMakeRect (x, to_y, width, height);
-
-    ns_copy_bits (f, srcRect , dstRect);
+    [FRAME_NS_VIEW (f) setNeedsDisplayInRect:dstRect];
   }
 
   unblock_input ();
@@ -2830,12 +2809,14 @@ so some key presses (TAB) are swallowed by the system. */
     External (RIF): copy an area horizontally, don't worry about clearing src
    -------------------------------------------------------------------------- */
 {
-  NSRect srcRect = NSMakeRect (x, y, width, height);
+  /* This should be done by copying the contents of the screen,
+     however we can get away with just marking the destination as
+     needing redrawn.  */
   NSRect dstRect = NSMakeRect (x+shift_by, y, width, height);
 
   NSTRACE ("ns_shift_glyphs_for_insert");
 
-  ns_copy_bits (f, srcRect, dstRect);
+  [FRAME_NS_VIEW (f) setNeedsDisplayInRect:dstRect];
 }
 
 
-- 
2.18.0