Subject: Re: bug#35464: [PATCH] Refactor update_window_begin and update_window_end hooks

From: Alex Gramiak <[email-masked]>
Date: Thu, 02 May 2019 23:10:41 -0600

close 35464
quit

I pushed the change as 9ae94ebdf.

I have another change that I don't think is worth a new bug report,
which is refactoring scroll_run_hook in a similar way. Is it okay to
apply it?

>From 18c2ba95e5f519b44ec49c398eee9572f2ded759 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <[email-masked]>
Date: Thu, 2 May 2019 22:58:11 -0600
Subject: [PATCH] Refactor scroll_run_hook

* src/dispnew.c (gui_scroll_run): New procedure implementing common
functionality. All callers of scroll_run_hook changed.

* src/dispextern.h (redisplay_interface): Change signature of
scroll_run_hook to use common calculated boundaries.

* src/nsterm.m (ns_scroll_run):
* src/w32term.c (w32_scroll_run):
* src/xterm.c (x_scroll_run): Rename to ns_scroll_run_hook,
w32_scroll_run_hook, and w32_scroll_run_hook respectively, and remove
now duplicated code.
---
 src/dispextern.h |  7 ++++---
 src/dispnew.c    | 51 +++++++++++++++++++++++++++++++++++++++++++++++-
 src/nsterm.m     | 47 +++++---------------------------------------
 src/w32term.c    | 33 +++----------------------------
 src/xdisp.c      | 11 +++++------
 src/xterm.c      | 42 ++++-----------------------------------
 6 files changed, 71 insertions(+), 120 deletions(-)

diff --git a/src/dispextern.h b/src/dispextern.h
index bb981f83fc..d45b6ba3cd 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -2833,9 +2833,9 @@ struct redisplay_interface
   void (*clear_end_of_line) (struct window *w, struct glyph_row *row,
 			     enum glyph_row_area area, int x);
 
-  /* Function to call to scroll the display as described by RUN on
-     window W.  */
-  void (*scroll_run_hook) (struct window *w, struct run *run);
+  /* Function to call to scroll the display on window W.  */
+  void (*scroll_run_hook) (struct window *w, int x, int y, int bottom_y,
+                           int from_y, int to_y, int width, int height);
 
   /* Function to call after a line in a display has been completely
      updated.  Used to draw truncation marks and alike.  DESIRED_ROW
@@ -3524,6 +3524,7 @@ void update_single_window (struct window *);
 #ifdef HAVE_WINDOW_SYSTEM
 extern void gui_update_window_begin (struct window *);
 extern void gui_update_window_end (struct window *, bool, bool);
+extern void gui_scroll_run (struct window *, struct run *);
 #endif
 void do_pending_window_change (bool);
 void change_frame_size (struct frame *, int, int, bool, bool, bool, bool);
diff --git a/src/dispnew.c b/src/dispnew.c
index 52a7b6d6ee..909c26787c 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -4452,7 +4452,9 @@ scrolling_window (struct window *w, bool header_line_p)
 	if (r->current_y != r->desired_y)
 	  {
 	    rif->clear_window_mouse_face (w);
-	    rif->scroll_run_hook (w, r);
+#ifdef HAVE_WINDOW_SYSTEM
+	    gui_scroll_run (w, r);
+#endif
 	  }
 
 	/* Truncate runs that copy to where we copied to, and
@@ -4550,6 +4552,53 @@ scrolling_window (struct window *w, bool header_line_p)
   return nruns > 0;
 }
 
+#ifdef HAVE_WINDOW_SYSTEM
+/* Scroll part of the display as described by RUN.  */
+
+void
+gui_scroll_run (struct window *w, struct run *run)
+{
+  int x, y, width, height, from_y, to_y, bottom_y;
+
+  /* Get frame-relative bounding box of the text display area of W,
+     without mode lines.  Include in this box the left and right
+     fringe of W.  */
+  window_box (w, ANY_AREA, &x, &y, &width, &height);
+
+  from_y = WINDOW_TO_FRAME_PIXEL_Y (w, run->current_y);
+  to_y = WINDOW_TO_FRAME_PIXEL_Y (w, run->desired_y);
+  bottom_y = y + height;
+
+  if (to_y < from_y)
+    {
+      /* Scrolling up.  Make sure we don't copy part of the mode
+	 line at the bottom.  */
+      if (from_y + run->height > bottom_y)
+	height = bottom_y - from_y;
+      else
+	height = run->height;
+    }
+  else
+    {
+      /* Scrolling down.  Make sure we don't copy over the mode line.
+	 at the bottom.  */
+      if (to_y + run->height > bottom_y)
+	height = bottom_y - to_y;
+      else
+	height = run->height;
+    }
+
+  block_input ();
+
+  /* Cursor off.  Will be switched on again in gui_update_window_end.  */
+  gui_clear_cursor (w);
+
+  FRAME_RIF (XFRAME (w->frame))->scroll_run_hook
+    (w, x, y, bottom_y, from_y, to_y, width, height);
+
+  unblock_input ();
+}
+#endif
 
 
 /************************************************************************
diff --git a/src/nsterm.m b/src/nsterm.m
index ffb7b7692b..34a1838e46 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -2823,61 +2823,24 @@ so some key presses (TAB) are swallowed by the system.  */
 }
 
 static void
-ns_scroll_run (struct window *w, struct run *run)
+ns_scroll_run_hook (struct window *w, int x, int _y, int _bottom_y,
+                    int from_y, int to_y, int width, int height)
 /* --------------------------------------------------------------------------
     External (RIF):  Insert or delete n lines at line vpos.
    -------------------------------------------------------------------------- */
 {
-  struct frame *f = XFRAME (w->frame);
-  int x, y, width, height, from_y, to_y, bottom_y;
-
-  NSTRACE ("ns_scroll_run");
-
-  /* begin copy from other terms */
-  /* Get frame-relative bounding box of the text display area of W,
-     without mode lines.  Include in this box the left and right
-     fringe of W.  */
-  window_box (w, ANY_AREA, &x, &y, &width, &height);
-
-  from_y = WINDOW_TO_FRAME_PIXEL_Y (w, run->current_y);
-  to_y = WINDOW_TO_FRAME_PIXEL_Y (w, run->desired_y);
-  bottom_y = y + height;
-
-  if (to_y < from_y)
-    {
-      /* Scrolling up.  Make sure we don't copy part of the mode
-	 line at the bottom.  */
-      if (from_y + run->height > bottom_y)
-	height = bottom_y - from_y;
-      else
-	height = run->height;
-    }
-  else
-    {
-      /* Scrolling down.  Make sure we don't copy over the mode line.
-	 at the bottom.  */
-      if (to_y + run->height > bottom_y)
-	height = bottom_y - to_y;
-      else
-	height = run->height;
-    }
-  /* end copy from other terms */
+  NSTRACE ("ns_scroll_run_hook");
 
   if (height == 0)
       return;
 
-  block_input ();
-
-  gui_clear_cursor (w);
-
   {
+    struct frame *f = XFRAME (w->frame);
     NSRect srcRect = NSMakeRect (x, from_y, width, height);
     NSRect dstRect = NSMakeRect (x, to_y, width, height);
 
     ns_copy_bits (f, srcRect , dstRect);
   }
-
-  unblock_input ();
 }

@@ -5094,7 +5057,7 @@ static Lisp_Object ns_string_to_lispmod (const char *s)
   gui_write_glyphs,
   gui_insert_glyphs,
   gui_clear_end_of_line,
-  ns_scroll_run,
+  ns_scroll_run_hook,
   ns_after_update_window_line,
   NULL, /* update_window_begin */
   NULL, /* update_window_end   */
diff --git a/src/w32term.c b/src/w32term.c
index 0abec3d92a..634132c515 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -2743,48 +2743,22 @@ w32_ins_del_lines (struct frame *f, int vpos, int n)
 /* Scroll part of the display as described by RUN.  */
 
 static void
-w32_scroll_run (struct window *w, struct run *run)
+w32_scroll_run_hook (struct window *w, int x, int y, int bottom_y,
+                     int from_y, int to_y, int width, int height)
 {
   struct frame *f = XFRAME (w->frame);
-  int x, y, width, height, from_y, to_y, bottom_y;
   HWND hwnd = FRAME_W32_WINDOW (f);
   HRGN expect_dirty;
 
-  /* Get frame-relative bounding box of the text display area of W,
-     without mode lines.  Include in this box the left and right
-     fringes of W.  */
-  window_box (w, ANY_AREA, &x, &y, &width, &height);
-
-  from_y = WINDOW_TO_FRAME_PIXEL_Y (w, run->current_y);
-  to_y = WINDOW_TO_FRAME_PIXEL_Y (w, run->desired_y);
-  bottom_y = y + height;
-
   if (to_y < from_y)
     {
-      /* Scrolling up.  Make sure we don't copy part of the mode
-	 line at the bottom.  */
-      if (from_y + run->height > bottom_y)
-	height = bottom_y - from_y;
-      else
-	height = run->height;
       expect_dirty = CreateRectRgn (x, y + height, x + width, bottom_y);
     }
   else
     {
-      /* Scrolling down.  Make sure we don't copy over the mode line.
-	 at the bottom.  */
-      if (to_y + run->height > bottom_y)
-	height = bottom_y - to_y;
-      else
-	height = run->height;
       expect_dirty = CreateRectRgn (x, y, x + width, to_y);
     }
 
-  block_input ();
-
-  /* Cursor off.  Will be switched on again in gui_update_window_end.  */
-  gui_clear_cursor (w);
-
   {
     RECT from;
     RECT to;
@@ -2813,7 +2787,6 @@ w32_scroll_run (struct window *w, struct run *run)
     DeleteObject (combined);
   }
 
-  unblock_input ();
   DeleteObject (expect_dirty);
 }
 
@@ -7043,7 +7016,7 @@ static struct redisplay_interface w32_redisplay_interface =
   gui_write_glyphs,
   gui_insert_glyphs,
   gui_clear_end_of_line,
-  w32_scroll_run,
+  w32_scroll_run_hook,
   w32_after_update_window_line,
   w32_update_window_begin,
   w32_update_window_end,
diff --git a/src/xdisp.c b/src/xdisp.c
index 3bdb8ea1b0..410247869d 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -18021,9 +18021,8 @@ try_window_reusing_current_matrix (struct window *w)
 	  /* Scroll the display.  Do it before the current matrix is
 	     changed.  The problem here is that update has not yet
 	     run, i.e. part of the current matrix is not up to date.
-	     scroll_run_hook will clear the cursor, and use the
-	     current matrix to get the height of the row the cursor is
-	     in.  */
+	     gui_scroll_run will clear the cursor, and use the current
+	     matrix to get the height of the row the cursor is in.  */
 	  run.current_y = start_row->y;
 	  run.desired_y = it.current_y;
 	  run.height = it.last_visible_y - it.current_y;
@@ -18034,7 +18033,7 @@ try_window_reusing_current_matrix (struct window *w)
 	      update_begin (f);
 	      gui_update_window_begin (w);
 	      FRAME_RIF (f)->clear_window_mouse_face (w);
-	      FRAME_RIF (f)->scroll_run_hook (w, &run);
+	      gui_scroll_run (w, &run);
 	      gui_update_window_end (w, false, false);
 	      update_end (f);
 #endif
@@ -18200,7 +18199,7 @@ try_window_reusing_current_matrix (struct window *w)
 	  update_begin (f);
 	  gui_update_window_begin (w);
 	  FRAME_RIF (f)->clear_window_mouse_face (w);
-	  FRAME_RIF (f)->scroll_run_hook (w, &run);
+	  gui_scroll_run (w, &run);
 	  gui_update_window_end (w, false, false);
 	  update_end (f);
 #endif
@@ -19154,7 +19153,7 @@ try_window_id (struct window *w)
 #ifdef HAVE_WINDOW_SYSTEM
 	  gui_update_window_begin (w);
 	  FRAME_RIF (f)->clear_window_mouse_face (w);
-	  FRAME_RIF (f)->scroll_run_hook (w, &run);
+	  gui_scroll_run (w, &run);
 	  gui_update_window_end (w, false, false);
 #endif
 	}
diff --git a/src/xterm.c b/src/xterm.c
index 26f74cde91..48bfb8fdbf 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -4192,46 +4192,13 @@ x_ins_del_lines (struct frame *f, int vpos, int n)
 }

-/* Scroll part of the display as described by RUN.  */
+/* Scroll part of the display.  */
 
 static void
-x_scroll_run (struct window *w, struct run *run)
+x_scroll_run_hook (struct window *w, int x, int _y, int _bottom_y,
+                   int from_y, int to_y, int width, int height)
 {
   struct frame *f = XFRAME (w->frame);
-  int x, y, width, height, from_y, to_y, bottom_y;
-
-  /* Get frame-relative bounding box of the text display area of W,
-     without mode lines.  Include in this box the left and right
-     fringe of W.  */
-  window_box (w, ANY_AREA, &x, &y, &width, &height);
-
-  from_y = WINDOW_TO_FRAME_PIXEL_Y (w, run->current_y);
-  to_y = WINDOW_TO_FRAME_PIXEL_Y (w, run->desired_y);
-  bottom_y = y + height;
-
-  if (to_y < from_y)
-    {
-      /* Scrolling up.  Make sure we don't copy part of the mode
-	 line at the bottom.  */
-      if (from_y + run->height > bottom_y)
-	height = bottom_y - from_y;
-      else
-	height = run->height;
-    }
-  else
-    {
-      /* Scrolling down.  Make sure we don't copy over the mode line.
-	 at the bottom.  */
-      if (to_y + run->height > bottom_y)
-	height = bottom_y - to_y;
-      else
-	height = run->height;
-    }
-
-  block_input ();
-
-  /* Cursor off.  Will be switched on again in gui_update_window_end.  */
-  gui_clear_cursor (w);
 
 #ifdef USE_CAIRO
   if (FRAME_CR_CONTEXT (f))
@@ -4265,7 +4232,6 @@ x_scroll_run (struct window *w, struct run *run)
 	     x, to_y);
 #endif
 
-  unblock_input ();
 }

@@ -13067,7 +13033,7 @@ static struct redisplay_interface x_redisplay_interface =
     gui_write_glyphs,
     gui_insert_glyphs,
     gui_clear_end_of_line,
-    x_scroll_run,
+    x_scroll_run_hook,
     x_after_update_window_line,
     NULL, /* update_window_begin */
     NULL, /* update_window_end   */
-- 
2.21.0