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

From: Alan Third <[email-masked]>
Date: Thu, 8 Nov 2018 23:21:13 +0000

On Thu, Nov 08, 2018 at 08:28:03AM -0800, Aaron Jensen wrote:
> On November 8, 2018 at 8:17:21 AM, Alan Third
> ([email-masked](mailto:[email-masked])) wrote:
> 
> > If I could suppress the clearing action that would solve the problem.
> >
> > If expose_frame could draw the rectangle as it was before the
> > frame/window was marked garbaged, that would also solve the problem.
> >
> > I don’t believe the former is possible, and I don’t know if the latter
> > is possible.
> 
> I’m only partially following this now, so I’m sorry if this idea
> doesn’t make sense—but if there’s a spot where the repaint actually
> happens, could you dirty the rects right before the repaint. In other
> words, queue into a queue the rect dirtying in all the places it
> happens and only actually process the dirty queue once you know you
> can paint.

Yes, and I hadn’t thought of that option. It makes a lot more sense
than queueing the clears. I’ve got another patch for you to try first,
though. This one requires the previous one (i.e. you should be able to
just apply it without the removing anything).
-- 
Alan Third
>From 171713fb172911416ac4615f4bf3ae1802a4e756 Mon Sep 17 00:00:00 2001
From: Alan Third <[email-masked]>
Date: Thu, 8 Nov 2018 23:11:31 +0000
Subject: [PATCH] Further changes to NS drawing (bug#32932)

* src/nsterm.m (ns_update_begin): Get rid of the display at the start
of redisplay.
(ns_update_window_begin): Remove redundant code that never executes.
(ns_draw_window_cursor): Perform a display when not in redisplay.
([EmacsView drawRect:]): Show the rectangle being exposed.
* src/xdisp.c (expose_window_tree) [HAVE_NS]:
(expose_frame) [HAVE_NS]: Redraw even if the frame is garbaged.
---
 src/nsterm.m | 43 +++++++++----------------------------------
 src/xdisp.c  |  8 +++++++-
 2 files changed, 16 insertions(+), 35 deletions(-)

diff --git a/src/nsterm.m b/src/nsterm.m
index bb21be4a18..9e6779d4a3 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1061,17 +1061,6 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
 
   ns_update_auto_hide_menu_bar ();
 
-  /* Flush any existing changes to screen before redisplay gets going.
-     If we don't do this then it's possible for redisplay to mark
-     areas as garbaged so they won't be redrawn in the next drawRect
-     call.
-
-     Is this a bad thing to do since we're effectively calling
-     frame_expose from within redisplay?  */
-  block_input ();
-  [FRAME_NS_VIEW (f) displayIfNeeded];
-  unblock_input ();
-
   if ([view isFullscreen] && [view fsIsNative])
   {
     // Fix reappearing tool bar in fullscreen for Mac OS X 10.7
@@ -1080,29 +1069,6 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
     if (! tbar_visible != ! [toolbar isVisible])
       [toolbar setVisible: tbar_visible];
   }
-
-  /* drawRect may have been called for say the minibuffer, and then clip path
-     is for the minibuffer.  But the display engine may draw more because
-     we have set the frame as garbaged.  So reset clip path to the whole
-     view.  */
-  /* FIXME: I don't think we need to do this.  */
-  if ([NSView focusView] == FRAME_NS_VIEW (f))
-    {
-      NSBezierPath *bp;
-      NSRect r = [view frame];
-      NSRect cr = [[view window] frame];
-      /* If a large frame size is set, r may be larger than the window frame
-         before constrained.  In that case don't change the clip path, as we
-         will clear in to the tool bar and title bar.  */
-      if (r.size.height
-          + FRAME_NS_TITLEBAR_HEIGHT (f)
-          + FRAME_TOOLBAR_HEIGHT (f) <= cr.size.height)
-        {
-          bp = [[NSBezierPath bezierPathWithRect: r] retain];
-          [bp setClip];
-          [bp release];
-        }
-    }
 #endif
 }
 
@@ -3158,6 +3124,12 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
 
       ns_reset_clipping (f);
     }
+  else if (! redisplaying_p)
+    {
+      /* If this function is called outside redisplay, it probably
+         means we need an immediate update.  */
+      [FRAME_NS_VIEW (f) display];
+    }
 }

@@ -8120,6 +8092,9 @@ - (void)drawRect: (NSRect)rect
   for (int i = 0 ; i < numRects ; i++)
     {
       NSRect r = rectList[i];
+
+      NSTRACE_RECT ("r", r);
+
       expose_frame (emacsframe,
                     NSMinX (r), NSMinY (r),
                     NSWidth (r), NSHeight (r));
diff --git a/src/xdisp.c b/src/xdisp.c
index 357f0fb30c..a59a62fb93 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -32258,7 +32258,11 @@ expose_window_tree (struct window *w, XRectangle *r)
   struct frame *f = XFRAME (w->frame);
   bool mouse_face_overwritten_p = false;
 
-  while (w && !FRAME_GARBAGED_P (f))
+  while (w
+#if !defined (HAVE_NS)
+         && !FRAME_GARBAGED_P (f)
+#endif
+         )
     {
       mouse_face_overwritten_p
 	|= (WINDOWP (w->contents)
@@ -32286,12 +32290,14 @@ expose_frame (struct frame *f, int x, int y, int w, int h)
 
   TRACE ((stderr, "expose_frame "));
 
+#if !defined (HAVE_NS)
   /* No need to redraw if frame will be redrawn soon.  */
   if (FRAME_GARBAGED_P (f))
     {
       TRACE ((stderr, " garbaged\n"));
       return;
     }
+#endif
 
   /* If basic faces haven't been realized yet, there is no point in
      trying to redraw anything.  This can happen when we get an expose
-- 
2.19.1