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

[Aaron Jensen]

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