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

[Aaron Jensen <aaronjensen@gmail.com>]

On Sat, Jun 24, 2023 at 12:18 AM Kai Ma <justksqsf@gmail.com> wrote:
>
>
>
> On Jun 19, 2023, at 23:46, Aaron Jensen <aaronjensen@gmail.com> wrote:
>
> Kai, could you try this patch out. It's a total guess, but let me know
> if it does any better for you.
>
>
> Thanks. I have been running this for several days.  It does not fix the problem completely, but it’s possible to set polling-period to a very small value now.

Hmm, do you see any of the artifacts w/o changing the polling period?

> 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");
>
>
> I haven't seen this message so far. So probably it is removing performSelectorOnMainThread that is effective.

Could you try removing the destination lock as well and see if that
impacts anything? From what I can tell, locking the destination may be
a good idea, but I'm curious if Alan has any thoughts as to why it'd
be a bad idea.

Thanks,

Aaron