Subject: Re: bug#32932: 27.0.50; render bugs on macOS Mojave

From: Alan Third <[email-masked]>
Date: Sun, 14 Oct 2018 19:20:12 +0100

On Sun, Oct 14, 2018 at 01:19:50AM -0700, Aaron Jensen wrote:
> On October 10, 2018 at 8:40:18 PM, Aaron Jensen
> ([email-masked](mailto:[email-masked])) wrote:
> 
> > On October 10, 2018 at 11:27:54 AM, Alan Third ([email-masked](mailto:[email-masked])) wrote:
> >
> > > On Tue, Oct 09, 2018 at 10:15:18AM +0300, Boris Buliga wrote:
> > > > Usually, it happens during resizing. But I've seen it several times without
> > > > resizing.
> > >
> > > I doubt this will make any difference, but can one of you try removing
> > > the called to [window display] in windowWillResize in nsterm.m
> >
> > I can still repro with this change made. Also, it’s not just on resizing, it happens often just while using it w/ a fixed window size.
> 
> On a whim, I commented out:
> 
>   [FRAME_NS_VIEW (f) displayIfNeeded];
> 
> In ns_flush_display. I cannot reproduce the problem with that
> commented out. I don’t know what ill effects it will have, but so far
> it seems like things draw properly.

Hmm, could’ve sworn we needed that there... This could all be down to
me misunderstanding something.

*checks*

Oh dammit. Yes. Looks like that flush display is not needed at all. I
could’ve sworn it was, but perhaps some other change fixed that...

Attached is a patch with this and a couple of other small graphics
fixes (I think this breaks GNUstep as is, but I’ll look at that
later). Can you please give it a go and see if there are any problems?
-- 
Alan Third
>From 4758870a4a7ebee44264cceeeeb0687002dccb57 Mon Sep 17 00:00:00 2001
From: Alan Third <[email-masked]>
Date: Sun, 14 Oct 2018 19:12:00 +0100
Subject: [PATCH] Fix some NS drawing issues (bug#32932)

* src/nsterm.m (ns_clip_to_rect):
(ns_reset_clipping): Remove gsaved variable and associated code.
(ns_flush_display): Remove function.
(ns_copy_bits): use translateRectsNeedingDisplayInRect:by: to copy any
pending drawing actions along with the image.
([EmacsView windowWillResize:toSize:]): Remove unneeded call.
---
 src/nsterm.m | 41 +++++++++++++++--------------------------
 1 file changed, 15 insertions(+), 26 deletions(-)

diff --git a/src/nsterm.m b/src/nsterm.m
index 8c355a89f8..cff77037d4 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];
 }

@@ -1234,19 +1228,6 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
   return ns_clip_to_rect (f, &clip_rect, 1);
 }
 
-
-static void
-ns_flush_display (struct frame *f)
-/* Force the frame to redisplay.  If areas have previously been marked
-   dirty by setNeedsDisplayInRect (in ns_clip_to_rect), then this will call
-   draw_rect: which will "expose" those areas.  */
-{
-  block_input ();
-  [FRAME_NS_VIEW (f) displayIfNeeded];
-  unblock_input ();
-}
-
-
 /* ==========================================================================
 
     Visible bell and beep.
@@ -2710,6 +2691,8 @@ so some key presses (TAB) are swallowed by the system. */
 static void
 ns_copy_bits (struct frame *f, NSRect src, NSRect dest)
 {
+  NSSize delta = NSMakeSize (dest.origin.x - src.origin.x,
+                             dest.origin.y - src.origin.y)
   NSTRACE ("ns_copy_bits");
 
   if (FRAME_NS_VIEW (f))
@@ -2718,10 +2701,17 @@ so some key presses (TAB) are swallowed by the system. */
 
       /* 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)];
-      [FRAME_NS_VIEW (f) setNeedsDisplay:YES];
+      [FRAME_NS_VIEW (f) scrollRect: src by: delta];
+
+      /* As far as I can tell from the documentation, scrollRect:by:,
+         above, should copy the dirty rectangles from our source
+         rectangle to our destination, however it appears it clips the
+         operation to src.  As a result we need to use
+         translateRectsNeedingDisplayInRect:by: below, and we have to
+         union src and dest so it can pick up the dirty rectangles,
+         and place them, as it also clips to the rectangle.  */
+      [FRAME_NS_VIEW (f) translateRectsNeedingDisplayInRect:NSUnionRect (src, dest)
+                                                         by:delta];
     }
 }
 
@@ -4977,7 +4967,7 @@ static Lisp_Object ns_string_to_lispmod (const char *s)
   ns_after_update_window_line,
   ns_update_window_begin,
   ns_update_window_end,
-  ns_flush_display, /* flush_display */
+  0, /* flush_display */
   x_clear_window_mouse_face,
   x_get_glyph_overhangs,
   x_fix_overlapping_area,
@@ -7046,7 +7036,6 @@ - (NSSize)windowWillResize: (NSWindow *)sender toSize: (NSSize)frameSize
         size_title = xmalloc (strlen (old_title) + 40);
 	esprintf (size_title, "%s  —  (%d x %d)", old_title, cols, rows);
         [window setTitle: [NSString stringWithUTF8String: size_title]];
-        [window display];
         xfree (size_title);
       }
   }
-- 
2.18.0