bug#63187: 30.0.50; Tail of longer lines painted after end of nearby lines on macOS

[Aaron Jensen]

On Thu, Jun 15, 2023 at 10:17 PM Aaron Jensen <aaronjensen@gmail.com> wrote:
>
> On Mon, Jun 12, 2023 at 9:04 AM Aaron Jensen <aaronjensen@gmail.com> wrote:
> >
> > On Fri, Jun 9, 2023 at 4:00 PM Alan Third <alan@idiocy.org> wrote:
> > >
> > > On Fri, Jun 09, 2023 at 02:46:29PM -0400, Aaron Jensen wrote:
> > > > On Fri, Jun 9, 2023 at 2:27 PM Alan Third <alan@idiocy.org> wrote:
> > > > > It seems to me that removing the call to performSelectorOnMainThread
> > > > > should be done. That may even fix Aaron's original issue too, given
> > > > > that I don't know why calling setNeedsDisplayInRect twice in a row
> > > > > should help, especially given it's not actually used anywhere else in
> > > > > the display code.
> > > >
> > > > How is it called twice? Does copyRect mark for redisplay?
> > >
> > > Sorry, I had misunderstood your change.
> > >
> > > Either way, I don't see how it's made a difference.
> > > setNeedsDisplayInRect is telling the system which parts it needs to
> > > call drawRect on, but we don't use drawRect any more, so I would think
> > > all it can be doing is setting the needsDisplay boolean to true.
> > >
> > > copyRect definitely doesn't do anything with the rectangle. It deals
> > > with the bitmap's pixel data directly, so there's no clipping or
> > > anything else affecting it and it's changes don't need to be
> > > committed to some backing store.
> > >
> > > Even when it comes to actually displaying the view on the screen, we
> > > pass in the entire bitmap to the graphics subsystem and it
> > > (supposedly) displays it in it's entirety.
> > >
> > > So as I understand it the rectangle passed into setNeedsDisplayInRect
> > > doesn't do anything. I think that call in ns_scroll_run was left there
> > > by mistake. It's literally the only call to it in the entire nsterm.m
> > > file.
> > >
> > > But you report that it has fixed your problem. I can't explain that
> > > because it runs counter to my understanding of how macOS draws.
> > >
> > > But then again, none of this is documented in any in-depth way by
> > > Apple, so who knows what's REALLY going on.
> > >
> > > Patch attached, but it's untested. It may even make things worse. I'm
> > > happy to leave it up to you to decide what to do since you're in a
> > > better position to tell if any given change actually helps.
> >
> > Thanks, I'm trying it out now. I'll report back in a bit. If it's fine
> > I'll try removing the setNeedsDisplay as well and try again.
>
> I saw a paint issue today. The "<" to the left of the indented (and
> redacted) lines 65-68 was an artifact. It kept painting there even
> while scrolling until I resized the window, then they all disappeared.
> They appeared one at a time while scrolling, as if the painting of one
> the one on line 63 was "fixed" in the window position as I was
> scrolling (likely it just didn't get cleared as necessary).

Kai, could you try this patch out. It's a total guess, but let me know
if it does any better for you.

diff --git a/src/nsterm.h b/src/nsterm.h
index b6e5a813a6d..4f6c6f7c28b 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -1384,3 +1384,11 @@ #define NSButtonTypeMomentaryPushIn
NSMomentaryPushInButton
 extern void mark_nsterm (void);

 #endif /* HAVE_NS */
+
+#define AJTRACE(...)                                          \
+  do                                                                        \
+    {                                                                       \
+        fprintf (stderr, __VA_ARGS__);                                    \
+        putc ('\n', stderr);     \
+    }                                                                       \
+  while(0)
diff --git a/src/nsterm.m b/src/nsterm.m
index 3e089cc1ff1..5a92f4cda0b 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -2708,9 +2708,9 @@ Hide the window (X11 semantics)
     EmacsView *view = FRAME_NS_VIEW (f);

     [view copyRect:srcRect to:dest];
-#ifdef NS_IMPL_COCOA
-    [view setNeedsDisplayInRect:destRect];
-#endif
+// #ifdef NS_IMPL_COCOA
+//     [view setNeedsDisplayInRect:destRect];
+// #endif
   }

   unblock_input ();
@@ -10636,9 +10636,9 @@ - (void) display

       /* Schedule a run of getContext so that if Emacs is idle it will
          perform the buffer copy, etc.  */
-      [self performSelectorOnMainThread:@selector (getContext)
-                             withObject:nil
-                          waitUntilDone:NO];
+      // [self performSelectorOnMainThread:@selector (getContext)
+      //                        withObject:nil
+      //                     waitUntilDone:NO];
     }
 }

@@ -10651,6 +10651,7 @@ - (void) copyContentsTo: (IOSurfaceRef) destination
   IOReturn lockStatus;
   IOSurfaceRef source = (IOSurfaceRef)[self contents];
   void *sourceData, *destinationData;
+  int seed1 = 0, seed2 = 1;
   int numBytes = IOSurfaceGetAllocSize (destination);

   NSTRACE_WHEN (NSTRACE_GROUP_FOCUS, "[EmacsLayer copyContentsTo:]");
@@ -10662,14 +10663,31 @@ - (void) copyContentsTo: (IOSurfaceRef) destination
   if (lockStatus != kIOReturnSuccess)
     NSLog (@"Failed to lock source surface: %x", lockStatus);

+  lockStatus = IOSurfaceLock (destination, kIOSurfaceLockAvoidSync, nil);
+  if (lockStatus != kIOReturnSuccess)
+    NSLog (@"Failed to lock destination surface: %x", lockStatus);
+
   sourceData = IOSurfaceGetBaseAddress (source);
   destinationData = IOSurfaceGetBaseAddress (destination);

-  /* Since every IOSurface should have the exact same settings, a
-     memcpy seems like the fastest way to copy the data from one to
-     the other.  */
-  memcpy (destinationData, sourceData, numBytes);
+  while (seed1 != seed2)
+    {
+      seed1 = IOSurfaceGetSeed (source);
+
+      /* Since every IOSurface should have the exact same settings, a
+        memcpy seems like the fastest way to copy the data from one to
+        the other.  */
+      memcpy (destinationData, sourceData, numBytes);

+      seed2 = IOSurfaceGetSeed (source);
+      if (seed1 != seed2) {
+        AJTRACE ("SEED DO NOT MATCH");
+      }
+    }
+
+  lockStatus = IOSurfaceUnlock (destination, kIOSurfaceLockAvoidSync, nil);
+  if (lockStatus != kIOReturnSuccess)
+    NSLog (@"Failed to unlock destination surface: %x", lockStatus);
   lockStatus = IOSurfaceUnlock (source, kIOSurfaceLockReadOnly, nil);
   if (lockStatus != kIOReturnSuccess)
     NSLog (@"Failed to unlock source surface: %x", lockStatus);