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

From: Alan Third <[email-masked]>
Date: Mon, 29 Oct 2018 16:09:43 +0000

On Sun, Oct 28, 2018 at 07:18:04PM -0700, Aaron Jensen wrote:
> On October 24, 2018 at 3:42:47 AM, Alan Third
> ([email-masked](mailto:[email-masked])) wrote:
> 
> > I’ve pushed a slight variant of this change to emacs-26. I’ve
> > witnessed a very rare flicker of the modeline and the line containing
> > the cursor in other windows while scrolling, but I can’t replicate it
> > consistently.
> 
> I am definitely seeing the flicker on the current line. It happens
> occasionally when my emacs is idle (probably not truly idle). I see it
> sometimes when I tell emacs to do something that is a thread blocking
> operation (like loading some autoloaded lisp for the first time). It’s
> as if it clears the current line and then immediately blocks the
> rendering thread before it gets a chance to rerender the line. As soon
> as Emacs is done doing whatever it was doing, the line rerenders.
> 
> No clue if that helps, but if there’s anything you want to try, I’ll
> be able to let you know if it fixes it :)

One more go. I don’t think I’ve seen the cursor line flicker after
installing this.

Simply, all I’ve done is stop making it redraw the entire line the
cursor is on. That should stop any flicker of the line text caused by
redrawing the cursor, but won’t stop any flicker of the cursor itself,
although I’ve not seen it flicker.

There’s a bit more in here to do with drawing the fringe bitmaps,
because that was the only other place ns_clip_to_row was being used,
so I’ve removed it from there too.
-- 
Alan Third
>From 95160c34b4f41e867761f29239db02e8e7232bbe Mon Sep 17 00:00:00 2001
From: Alan Third <[email-masked]>
Date: Mon, 29 Oct 2018 15:37:35 +0000
Subject: [PATCH] Fix more drawing bugs in NS port (bug#32932)

* src/nsterm.m (ns_row_rect): New function.
(ns_clip_to_row): Remove function.
(ns_copy_bits): Fix mistake.
(ns_draw_fringe_bitmap): Stop using ns_clip_to_row.
(ns_draw_window_cursor): Stop using ns_clip_to_row and move
ns_reset_clipping to try and force cursor drawing to be atomic.
---
 src/nsterm.m | 105 +++++++++++++++++++++++++++++----------------------
 1 file changed, 59 insertions(+), 46 deletions(-)

diff --git a/src/nsterm.m b/src/nsterm.m
index 4b5d025ee3..39a7b25f60 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -796,6 +796,27 @@ Free a pool and temporary objects it refers to (callable from C)
 }

+static NSRect
+ns_row_rect (struct window *w, struct glyph_row *row,
+               enum glyph_row_area area)
+/* Get the row as an NSRect.  */
+{
+  struct frame *f = XFRAME (WINDOW_FRAME (w));
+  NSRect rect;
+  int window_x, window_y, window_width;
+
+  window_box (w, area, &window_x, &window_y, &window_width, 0);
+
+  rect.origin.x = window_x;
+  rect.origin.y = WINDOW_TO_FRAME_PIXEL_Y (w, max (0, row->y));
+  rect.origin.y = max (rect.origin.y, window_y);
+  rect.size.width = window_width;
+  rect.size.height = row->visible_height;
+
+  return rect;
+}
+
+
 /* ==========================================================================
 
     Focus (clipping) and screen update
@@ -1206,28 +1227,6 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
 }

-static BOOL
-ns_clip_to_row (struct window *w, struct glyph_row *row,
-		enum glyph_row_area area, BOOL gc)
-/* --------------------------------------------------------------------------
-     Internal (but parallels other terms): Focus drawing on given row
-   -------------------------------------------------------------------------- */
-{
-  struct frame *f = XFRAME (WINDOW_FRAME (w));
-  NSRect clip_rect;
-  int window_x, window_y, window_width;
-
-  window_box (w, area, &window_x, &window_y, &window_width, 0);
-
-  clip_rect.origin.x = window_x;
-  clip_rect.origin.y = WINDOW_TO_FRAME_PIXEL_Y (w, max (0, row->y));
-  clip_rect.origin.y = max (clip_rect.origin.y, window_y);
-  clip_rect.size.width = window_width;
-  clip_rect.size.height = row->visible_height;
-
-  return ns_clip_to_rect (f, &clip_rect, 1);
-}
-
 /* ==========================================================================
 
     Visible bell and beep.
@@ -2692,7 +2691,7 @@ so some key presses (TAB) are swallowed by the system. */
 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)
+                             dest.origin.y - src.origin.y);
   NSTRACE ("ns_copy_bits");
 
   if (FRAME_NS_VIEW (f))
@@ -2911,6 +2910,9 @@ so some key presses (TAB) are swallowed by the system. */
   struct face *face = p->face;
   static EmacsImage **bimgs = NULL;
   static int nBimgs = 0;
+  NSRect clearRect = NSZeroRect;
+  NSRect imageRect = NSZeroRect;
+  NSRect rowRect = ns_row_rect (w, row, ANY_AREA);
 
   NSTRACE_WHEN (NSTRACE_GROUP_FRINGE, "ns_draw_fringe_bitmap");
   NSTRACE_MSG ("which:%d cursor:%d overlay:%d width:%d height:%d period:%d",
@@ -2925,25 +2927,40 @@ so some key presses (TAB) are swallowed by the system. */
       nBimgs = max_used_fringe_bitmap;
     }
 
-  /* Must clip because of partially visible lines.  */
-  if (ns_clip_to_row (w, row, ANY_AREA, YES))
+  /* Work out the rectangle we will composite into.  */
+  if (p->which)
+    imageRect = NSMakeRect (p->x, p->y, p->wd, p->h);
+
+  /* Work out the rectangle we will need to clear.  Because we're
+     compositing rather than blitting, we need to clear the area under
+     the image regardless of anything else.  */
+  if (!p->overlay_p)
+    {
+      clearRect = NSMakeRect (p->bx, p->by, p->nx, p->ny);
+      clearRect = NSUnionRect (clearRect, imageRect);
+    }
+  else
     {
-      if (!p->overlay_p)
+      clearRect = imageRect;
+    }
+
+  /* Handle partially visible rows.  */
+  clearRect = NSIntersectionRect (clearRect, rowRect);
+
+  /* The visible portion of imageRect will always be contained within
+     clearRect.  */
+  if (ns_clip_to_rect (f, &clearRect, 1))
+    {
+      if (! NSIsEmptyRect (clearRect))
         {
-          int bx = p->bx, by = p->by, nx = p->nx, ny = p->ny;
+          NSTRACE_RECT ("clearRect", clearRect);
 
-          if (bx >= 0 && nx > 0)
-            {
-              NSRect r = NSMakeRect (bx, by, nx, ny);
-              NSRectClip (r);
-              [ns_lookup_indexed_color (face->background, f) set];
-              NSRectFill (r);
-            }
+          [ns_lookup_indexed_color(face->background, f) set];
+          NSRectFill (clearRect);
         }
 
       if (p->which)
         {
-          NSRect r = NSMakeRect (p->x, p->y, p->wd, p->h);
           EmacsImage *img = bimgs[p->which - 1];
 
           if (!img)
@@ -2964,13 +2981,6 @@ so some key presses (TAB) are swallowed by the system. */
               xfree (cbits);
             }
 
-          NSTRACE_RECT ("r", r);
-
-          NSRectClip (r);
-          /* Since we composite the bitmap instead of just blitting it, we need
-             to erase the whole background.  */
-          [ns_lookup_indexed_color(face->background, f) set];
-          NSRectFill (r);
 
           {
             NSColor *bm_color;
@@ -2990,7 +3000,7 @@ so some key presses (TAB) are swallowed by the system. */
 
           NSTRACE_RECT ("fromRect", fromRect);
 
-          [img drawInRect: r
+          [img drawInRect: imageRect
                  fromRect: fromRect
                 operation: NSCompositingOperationSourceOver
                  fraction: 1.0
@@ -2998,7 +3008,7 @@ so some key presses (TAB) are swallowed by the system. */
                     hints: nil];
 #else
           {
-            NSPoint pt = r.origin;
+            NSPoint pt = imageRect.origin;
             pt.y += p->h;
             [img compositeToPoint: pt operation: NSCompositingOperationSourceOver];
           }
@@ -3088,7 +3098,9 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
   r.size.width = w->phys_cursor_width;
 
   /* Prevent the cursor from being drawn outside the text area.  */
-  if (ns_clip_to_row (w, glyph_row, TEXT_AREA, NO))
+  r = NSIntersectionRect (r, ns_row_rect (w, glyph_row, TEXT_AREA));
+
+  if (ns_clip_to_rect (f, &r, 1))
     {
       face = FACE_FROM_ID_OR_NULL (f, phys_cursor_glyph->face_id);
       if (face && NS_FACE_BACKGROUND (face)
@@ -3128,11 +3140,12 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
           NSRectFill (s);
           break;
         }
-      ns_reset_clipping (f);
 
       /* draw the character under the cursor */
       if (cursor_type != NO_CURSOR)
         draw_phys_cursor_glyph (w, glyph_row, DRAW_CURSOR);
+
+      ns_reset_clipping (f);
     }
 }
 
-- 
2.18.0