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

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

[Alex Gramiak]

[-- Attachment #1: Type: text/plain, Size: 301 bytes --]

I have two questions about this:

1) Is it okay to not use reset_mouse_highlight in the generic version
(ns_update_window_end still uses it)?  See commit 60ae3d09932f for why
reset_mouse_highlight was removed in the x/w32 versions.

2) Should the #if 0 section be removed in w32_update_window_begin?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Refactor-update_window_begin-and-update_window_end-h.patch --]
[-- Type: text/x-patch, Size: 13382 bytes --]

From ab69f49d4cb1446832e8c6167888435fd71ae356 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Sat, 27 Apr 2019 15:00:13 -0600
Subject: [PATCH] Refactor update_window_begin and update_window_end hooks

* src/xdisp.c (gui_update_window_begin, gui_update_window_end): New
procedures implementing common functionality.

* src/nsterm.m: (ns_update_window_begin, ns_update_window_end): Remove
in favor of the generic version.

* src/w32term.c: (w32_update_window_begin, w32_update_window_end):
Use gui_update_window_begin and gui_update_window_end.

* src/xterm.c: (x_update_window_begin, x_update_window_end): Remove in
favor of the generic version.
---
 src/dispextern.h |  3 ++
 src/nsterm.m     | 78 +++-----------------------------------------
 src/w32term.c    | 40 ++---------------------
 src/xdisp.c      | 67 ++++++++++++++++++++++++++++++++++++++
 src/xterm.c      | 84 +++---------------------------------------------
 5 files changed, 80 insertions(+), 192 deletions(-)

diff --git a/src/dispextern.h b/src/dispextern.h
index 4d6d0371d3..827eed86f1 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -3303,6 +3303,9 @@ extern void gui_update_cursor (struct frame *, bool);
 extern void gui_clear_cursor (struct window *);
 extern void gui_draw_vertical_border (struct window *w);
 extern void gui_draw_right_divider (struct window *w);
+extern void gui_update_window_begin (struct window *w);
+extern void gui_update_window_end (struct window *w, bool cursor_on_p,
+                                   bool mouse_face_overwritten_p);
 
 extern int get_glyph_string_clip_rects (struct glyph_string *,
                                         NativeRectangle *, int);
diff --git a/src/nsterm.m b/src/nsterm.m
index cdf1916e71..2a2b8cbaba 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1106,7 +1106,7 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
 ns_update_begin (struct frame *f)
 /* --------------------------------------------------------------------------
    Prepare for a grouped sequence of drawing calls
-   external (RIF) call; whole frame, called before update_window_begin
+   external (RIF) call; whole frame, called before gui_update_window_begin
    -------------------------------------------------------------------------- */
 {
 #ifdef NS_IMPL_COCOA
@@ -1128,81 +1128,11 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
 }
 
 
-static void
-ns_update_window_begin (struct window *w)
-/* --------------------------------------------------------------------------
-   Prepare for a grouped sequence of drawing calls
-   external (RIF) call; for one window, called after update_begin
-   -------------------------------------------------------------------------- */
-{
-  struct frame *f = XFRAME (WINDOW_FRAME (w));
-  Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
-
-  NSTRACE_WHEN (NSTRACE_GROUP_UPDATES, "ns_update_window_begin");
-  w->output_cursor = w->cursor;
-
-  block_input ();
-
-  if (f == hlinfo->mouse_face_mouse_frame)
-    {
-      /* Don't do highlighting for mouse motion during the update.  */
-      hlinfo->mouse_face_defer = 1;
-
-        /* If the frame needs to be redrawn,
-           simply forget about any prior mouse highlighting.  */
-      if (FRAME_GARBAGED_P (f))
-        hlinfo->mouse_face_window = Qnil;
-
-      /* (further code for mouse faces ifdef'd out in other terms elided) */
-    }
-
-  unblock_input ();
-}
-
-
-static void
-ns_update_window_end (struct window *w, bool cursor_on_p,
-                      bool mouse_face_overwritten_p)
-/* --------------------------------------------------------------------------
-   Finished a grouped sequence of drawing calls
-   external (RIF) call; for one window called before update_end
-   -------------------------------------------------------------------------- */
-{
-  NSTRACE_WHEN (NSTRACE_GROUP_UPDATES, "ns_update_window_end");
-
-  /* note: this fn is nearly identical in all terms */
-  if (!w->pseudo_window_p)
-    {
-      block_input ();
-
-      if (cursor_on_p)
-	display_and_set_cursor (w, 1,
-				w->output_cursor.hpos, w->output_cursor.vpos,
-				w->output_cursor.x, w->output_cursor.y);
-
-      if (draw_window_fringes (w, 1))
-	{
-	  if (WINDOW_RIGHT_DIVIDER_WIDTH (w))
-	    gui_draw_right_divider (w);
-	  else
-	    gui_draw_vertical_border (w);
-	}
-
-      unblock_input ();
-    }
-
-  /* If a row with mouse-face was overwritten, arrange for
-     frame_up_to_date to redisplay the mouse highlight.  */
-  if (mouse_face_overwritten_p)
-    reset_mouse_highlight (MOUSE_HL_INFO (XFRAME (w->frame)));
-}
-
-
 static void
 ns_update_end (struct frame *f)
 /* --------------------------------------------------------------------------
    Finished a grouped sequence of drawing calls
-   external (RIF) call; for whole frame, called after update_window_end
+   external (RIF) call; for whole frame, called after gui_update_window_end
    -------------------------------------------------------------------------- */
 {
   NSTRACE_WHEN (NSTRACE_GROUP_UPDATES, "ns_update_end");
@@ -5166,8 +5096,8 @@ static Lisp_Object ns_string_to_lispmod (const char *s)
   gui_clear_end_of_line,
   ns_scroll_run,
   ns_after_update_window_line,
-  ns_update_window_begin,
-  ns_update_window_end,
+  gui_update_window_begin,
+  gui_update_window_end,
   0, /* flush_display */
   gui_clear_window_mouse_face,
   gui_get_glyph_overhangs,
diff --git a/src/w32term.c b/src/w32term.c
index 451dd54dd8..8d5f57836c 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -565,19 +565,12 @@ w32_update_window_begin (struct window *w)
 			  0, 6000, NULL);
     }
 
-  w->output_cursor = w->cursor;
+  gui_update_window_begin (w);
 
   block_input ();
 
   if (f == hlinfo->mouse_face_mouse_frame)
     {
-      /* Don't do highlighting for mouse motion during the update.  */
-      hlinfo->mouse_face_defer = true;
-
-      /* If F needs to be redrawn, simply forget about any prior mouse
-	 highlighting.  */
-      if (FRAME_GARBAGED_P (f))
-	hlinfo->mouse_face_window = Qnil;
 
 #if 0 /* Rows in a current matrix containing glyphs in mouse-face have
 	 their mouse_face_p flag set, which means that they are always
@@ -696,36 +689,7 @@ static void
 w32_update_window_end (struct window *w, bool cursor_on_p,
 		     bool mouse_face_overwritten_p)
 {
-  if (!w->pseudo_window_p)
-    {
-      block_input ();
-
-      if (cursor_on_p)
-	display_and_set_cursor (w, true,
-				w->output_cursor.hpos, w->output_cursor.vpos,
-				w->output_cursor.x, w->output_cursor.y);
-
-      if (draw_window_fringes (w, true))
-	{
-	  if (WINDOW_RIGHT_DIVIDER_WIDTH (w))
-	    gui_draw_right_divider (w);
-	  else
-	    gui_draw_vertical_border (w);
-	}
-
-      unblock_input ();
-    }
-
-  /* If a row with mouse-face was overwritten, arrange for
-     XTframe_up_to_date to redisplay the mouse highlight.  */
-  if (mouse_face_overwritten_p)
-    {
-      Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (XFRAME (w->frame));
-
-      hlinfo->mouse_face_beg_row = hlinfo->mouse_face_beg_col = -1;
-      hlinfo->mouse_face_end_row = hlinfo->mouse_face_end_col = -1;
-      hlinfo->mouse_face_window = Qnil;
-    }
+  gui_update_window_end (w, cursor_on_p, mouse_face_overwritten_p);
 
   /* Unhide the caret.  This won't actually show the cursor, unless it
      was visible before the corresponding call to HideCaret in
diff --git a/src/xdisp.c b/src/xdisp.c
index d52d1333a0..dbf34cf3bd 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -32223,6 +32223,73 @@ gui_draw_bottom_divider (struct window *w)
     }
 }
 
+/* EXPORT:
+   Start update of window W.  */
+
+void
+gui_update_window_begin (struct window *w)
+{
+  struct frame *f = XFRAME (WINDOW_FRAME (w));
+  Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
+
+  w->output_cursor = w->cursor;
+
+  block_input ();
+
+  if (f == hlinfo->mouse_face_mouse_frame)
+    {
+      /* Don't do highlighting for mouse motion during the update.  */
+      hlinfo->mouse_face_defer = true;
+
+      /* If the frame needs to be redrawn, simply forget about any
+	 prior mouse highlighting.  */
+      if (FRAME_GARBAGED_P (f))
+	hlinfo->mouse_face_window = Qnil;
+    }
+
+  unblock_input ();
+}
+
+/* EXPORT:
+   End update of window W.
+
+   Draw vertical borders between horizontally adjacent windows, and
+   display W's cursor if CURSOR_ON_P is non-zero.
+
+   MOUSE_FACE_OVERWRITTEN_P non-zero means that some row containing
+   glyphs in mouse-face were overwritten.  In that case we have to
+   make sure that the mouse-highlight is properly redrawn.  */
+void
+gui_update_window_end (struct window *w, bool cursor_on_p,
+                       bool mouse_face_overwritten_p)
+{
+  /* Pseudo windows don't have cursors, so don't display them here.  */
+  if (!w->pseudo_window_p)
+    {
+      block_input ();
+
+      if (cursor_on_p)
+	display_and_set_cursor (w, true,
+				w->output_cursor.hpos, w->output_cursor.vpos,
+				w->output_cursor.x, w->output_cursor.y);
+
+      if (draw_window_fringes (w, true))
+	{
+	  if (WINDOW_RIGHT_DIVIDER_WIDTH (w))
+	    gui_draw_right_divider (w);
+	  else
+	    gui_draw_vertical_border (w);
+	}
+
+      unblock_input ();
+    }
+
+  /* If a row with mouse-face was overwritten, arrange for
+     frame_up_to_date_hook to redisplay the mouse highlight.  */
+  if (mouse_face_overwritten_p)
+    reset_mouse_highlight (MOUSE_HL_INFO (XFRAME (w->frame)));
+}
+
 /* Redraw the part of window W intersection rectangle FR.  Pixel
    coordinates in FR are frame-relative.  Call this function with
    input blocked.  Value is true if the exposure overwrites
diff --git a/src/xterm.c b/src/xterm.c
index dd19b8bde1..b08f5f6a62 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -989,7 +989,7 @@ x_set_frame_alpha (struct frame *f)
 
 /* Start an update of frame F.  This function is installed as a hook
    for update_begin, i.e. it is called when update_begin is called.
-   This function is called prior to calls to x_update_window_begin for
+   This function is called prior to calls to gui_update_window_begin for
    each window being updated.  Currently, there is nothing to do here
    because all interesting stuff is done on a window basis.  */
 
@@ -1033,33 +1033,6 @@ x_update_begin (struct frame *f)
 #endif /* USE_CAIRO */
 }
 
-/* Start update of window W.  */
-
-static void
-x_update_window_begin (struct window *w)
-{
-  struct frame *f = XFRAME (WINDOW_FRAME (w));
-  Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
-
-  w->output_cursor = w->cursor;
-
-  block_input ();
-
-  if (f == hlinfo->mouse_face_mouse_frame)
-    {
-      /* Don't do highlighting for mouse motion during the update.  */
-      hlinfo->mouse_face_defer = true;
-
-      /* If F needs to be redrawn, simply forget about any prior mouse
-	 highlighting.  */
-      if (FRAME_GARBAGED_P (f))
-	hlinfo->mouse_face_window = Qnil;
-    }
-
-  unblock_input ();
-}
-
-
 /* Draw a vertical window border from (x,y0) to (x,y1)  */
 
 static void
@@ -1139,55 +1112,6 @@ x_draw_window_divider (struct window *w, int x0, int x1, int y0, int y1)
     }
 }
 
-/* End update of window W.
-
-   Draw vertical borders between horizontally adjacent windows, and
-   display W's cursor if CURSOR_ON_P is non-zero.
-
-   MOUSE_FACE_OVERWRITTEN_P non-zero means that some row containing
-   glyphs in mouse-face were overwritten.  In that case we have to
-   make sure that the mouse-highlight is properly redrawn.
-
-   W may be a menu bar pseudo-window in case we don't have X toolkit
-   support.  Such windows don't have a cursor, so don't display it
-   here.  */
-
-static void
-x_update_window_end (struct window *w, bool cursor_on_p,
-		     bool mouse_face_overwritten_p)
-{
-  if (!w->pseudo_window_p)
-    {
-      block_input ();
-
-      if (cursor_on_p)
-	display_and_set_cursor (w, true,
-				w->output_cursor.hpos, w->output_cursor.vpos,
-				w->output_cursor.x, w->output_cursor.y);
-
-      if (draw_window_fringes (w, true))
-	{
-	  if (WINDOW_RIGHT_DIVIDER_WIDTH (w))
-	    gui_draw_right_divider (w);
-	  else
-	    gui_draw_vertical_border (w);
-	}
-
-      unblock_input ();
-    }
-
-  /* If a row with mouse-face was overwritten, arrange for
-     XTframe_up_to_date to redisplay the mouse highlight.  */
-  if (mouse_face_overwritten_p)
-    {
-      Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (XFRAME (w->frame));
-
-      hlinfo->mouse_face_beg_row = hlinfo->mouse_face_beg_col = -1;
-      hlinfo->mouse_face_end_row = hlinfo->mouse_face_end_col = -1;
-      hlinfo->mouse_face_window = Qnil;
-    }
-}
-
 /* Show the frame back buffer.  If frame is double-buffered,
    atomically publish to the user's screen graphics updates made since
    the last call to show_back_buffer.  */
@@ -4306,7 +4230,7 @@ x_scroll_run (struct window *w, struct run *run)
 
   block_input ();
 
-  /* Cursor off.  Will be switched on again in x_update_window_end.  */
+  /* Cursor off.  Will be switched on again in gui_update_window_end.  */
   gui_clear_cursor (w);
 
 #ifdef USE_CAIRO
@@ -13145,8 +13069,8 @@ static struct redisplay_interface x_redisplay_interface =
     gui_clear_end_of_line,
     x_scroll_run,
     x_after_update_window_line,
-    x_update_window_begin,
-    x_update_window_end,
+    gui_update_window_begin,
+    gui_update_window_end,
     x_flip_and_flush,
     gui_clear_window_mouse_face,
     gui_get_glyph_overhangs,
-- 
2.21.0

bug#35463: [PATCH] Refactor update_window_begin and update_window_end hooks

[Alex Gramiak]

[-- Attachment #1: Type: text/plain, Size: 91 bytes --]

Ah, I forgot to update the patch to not use reset_mouse_highlight in the
generic version:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2 --]
[-- Type: text/x-patch, Size: 13581 bytes --]

From 380549267f0e83d284752ac90f2fdd1a8da99afb Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Sat, 27 Apr 2019 15:00:13 -0600
Subject: [PATCH] Refactor update_window_begin and update_window_end hooks

* src/xdisp.c (gui_update_window_begin, gui_update_window_end): New
procedures implementing common functionality.

* src/nsterm.m: (ns_update_window_begin, ns_update_window_end): Remove
in favor of the generic version.

* src/w32term.c: (w32_update_window_begin, w32_update_window_end):
Use gui_update_window_begin and gui_update_window_end.

* src/xterm.c: (x_update_window_begin, x_update_window_end): Remove in
favor of the generic version.
---
 src/dispextern.h |  3 ++
 src/nsterm.m     | 78 +++-----------------------------------------
 src/w32term.c    | 40 ++---------------------
 src/xdisp.c      | 73 +++++++++++++++++++++++++++++++++++++++++
 src/xterm.c      | 84 +++---------------------------------------------
 5 files changed, 86 insertions(+), 192 deletions(-)

diff --git a/src/dispextern.h b/src/dispextern.h
index 4d6d0371d3..827eed86f1 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -3303,6 +3303,9 @@ extern void gui_update_cursor (struct frame *, bool);
 extern void gui_clear_cursor (struct window *);
 extern void gui_draw_vertical_border (struct window *w);
 extern void gui_draw_right_divider (struct window *w);
+extern void gui_update_window_begin (struct window *w);
+extern void gui_update_window_end (struct window *w, bool cursor_on_p,
+                                   bool mouse_face_overwritten_p);
 
 extern int get_glyph_string_clip_rects (struct glyph_string *,
                                         NativeRectangle *, int);
diff --git a/src/nsterm.m b/src/nsterm.m
index cdf1916e71..2a2b8cbaba 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1106,7 +1106,7 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
 ns_update_begin (struct frame *f)
 /* --------------------------------------------------------------------------
    Prepare for a grouped sequence of drawing calls
-   external (RIF) call; whole frame, called before update_window_begin
+   external (RIF) call; whole frame, called before gui_update_window_begin
    -------------------------------------------------------------------------- */
 {
 #ifdef NS_IMPL_COCOA
@@ -1128,81 +1128,11 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
 }
 
 
-static void
-ns_update_window_begin (struct window *w)
-/* --------------------------------------------------------------------------
-   Prepare for a grouped sequence of drawing calls
-   external (RIF) call; for one window, called after update_begin
-   -------------------------------------------------------------------------- */
-{
-  struct frame *f = XFRAME (WINDOW_FRAME (w));
-  Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
-
-  NSTRACE_WHEN (NSTRACE_GROUP_UPDATES, "ns_update_window_begin");
-  w->output_cursor = w->cursor;
-
-  block_input ();
-
-  if (f == hlinfo->mouse_face_mouse_frame)
-    {
-      /* Don't do highlighting for mouse motion during the update.  */
-      hlinfo->mouse_face_defer = 1;
-
-        /* If the frame needs to be redrawn,
-           simply forget about any prior mouse highlighting.  */
-      if (FRAME_GARBAGED_P (f))
-        hlinfo->mouse_face_window = Qnil;
-
-      /* (further code for mouse faces ifdef'd out in other terms elided) */
-    }
-
-  unblock_input ();
-}
-
-
-static void
-ns_update_window_end (struct window *w, bool cursor_on_p,
-                      bool mouse_face_overwritten_p)
-/* --------------------------------------------------------------------------
-   Finished a grouped sequence of drawing calls
-   external (RIF) call; for one window called before update_end
-   -------------------------------------------------------------------------- */
-{
-  NSTRACE_WHEN (NSTRACE_GROUP_UPDATES, "ns_update_window_end");
-
-  /* note: this fn is nearly identical in all terms */
-  if (!w->pseudo_window_p)
-    {
-      block_input ();
-
-      if (cursor_on_p)
-	display_and_set_cursor (w, 1,
-				w->output_cursor.hpos, w->output_cursor.vpos,
-				w->output_cursor.x, w->output_cursor.y);
-
-      if (draw_window_fringes (w, 1))
-	{
-	  if (WINDOW_RIGHT_DIVIDER_WIDTH (w))
-	    gui_draw_right_divider (w);
-	  else
-	    gui_draw_vertical_border (w);
-	}
-
-      unblock_input ();
-    }
-
-  /* If a row with mouse-face was overwritten, arrange for
-     frame_up_to_date to redisplay the mouse highlight.  */
-  if (mouse_face_overwritten_p)
-    reset_mouse_highlight (MOUSE_HL_INFO (XFRAME (w->frame)));
-}
-
-
 static void
 ns_update_end (struct frame *f)
 /* --------------------------------------------------------------------------
    Finished a grouped sequence of drawing calls
-   external (RIF) call; for whole frame, called after update_window_end
+   external (RIF) call; for whole frame, called after gui_update_window_end
    -------------------------------------------------------------------------- */
 {
   NSTRACE_WHEN (NSTRACE_GROUP_UPDATES, "ns_update_end");
@@ -5166,8 +5096,8 @@ static Lisp_Object ns_string_to_lispmod (const char *s)
   gui_clear_end_of_line,
   ns_scroll_run,
   ns_after_update_window_line,
-  ns_update_window_begin,
-  ns_update_window_end,
+  gui_update_window_begin,
+  gui_update_window_end,
   0, /* flush_display */
   gui_clear_window_mouse_face,
   gui_get_glyph_overhangs,
diff --git a/src/w32term.c b/src/w32term.c
index 451dd54dd8..8d5f57836c 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -565,19 +565,12 @@ w32_update_window_begin (struct window *w)
 			  0, 6000, NULL);
     }
 
-  w->output_cursor = w->cursor;
+  gui_update_window_begin (w);
 
   block_input ();
 
   if (f == hlinfo->mouse_face_mouse_frame)
     {
-      /* Don't do highlighting for mouse motion during the update.  */
-      hlinfo->mouse_face_defer = true;
-
-      /* If F needs to be redrawn, simply forget about any prior mouse
-	 highlighting.  */
-      if (FRAME_GARBAGED_P (f))
-	hlinfo->mouse_face_window = Qnil;
 
 #if 0 /* Rows in a current matrix containing glyphs in mouse-face have
 	 their mouse_face_p flag set, which means that they are always
@@ -696,36 +689,7 @@ static void
 w32_update_window_end (struct window *w, bool cursor_on_p,
 		     bool mouse_face_overwritten_p)
 {
-  if (!w->pseudo_window_p)
-    {
-      block_input ();
-
-      if (cursor_on_p)
-	display_and_set_cursor (w, true,
-				w->output_cursor.hpos, w->output_cursor.vpos,
-				w->output_cursor.x, w->output_cursor.y);
-
-      if (draw_window_fringes (w, true))
-	{
-	  if (WINDOW_RIGHT_DIVIDER_WIDTH (w))
-	    gui_draw_right_divider (w);
-	  else
-	    gui_draw_vertical_border (w);
-	}
-
-      unblock_input ();
-    }
-
-  /* If a row with mouse-face was overwritten, arrange for
-     XTframe_up_to_date to redisplay the mouse highlight.  */
-  if (mouse_face_overwritten_p)
-    {
-      Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (XFRAME (w->frame));
-
-      hlinfo->mouse_face_beg_row = hlinfo->mouse_face_beg_col = -1;
-      hlinfo->mouse_face_end_row = hlinfo->mouse_face_end_col = -1;
-      hlinfo->mouse_face_window = Qnil;
-    }
+  gui_update_window_end (w, cursor_on_p, mouse_face_overwritten_p);
 
   /* Unhide the caret.  This won't actually show the cursor, unless it
      was visible before the corresponding call to HideCaret in
diff --git a/src/xdisp.c b/src/xdisp.c
index d52d1333a0..a3dddfa8a5 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -32223,6 +32223,79 @@ gui_draw_bottom_divider (struct window *w)
     }
 }
 
+/* EXPORT:
+   Start update of window W.  */
+
+void
+gui_update_window_begin (struct window *w)
+{
+  struct frame *f = XFRAME (WINDOW_FRAME (w));
+  Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
+
+  w->output_cursor = w->cursor;
+
+  block_input ();
+
+  if (f == hlinfo->mouse_face_mouse_frame)
+    {
+      /* Don't do highlighting for mouse motion during the update.  */
+      hlinfo->mouse_face_defer = true;
+
+      /* If the frame needs to be redrawn, simply forget about any
+	 prior mouse highlighting.  */
+      if (FRAME_GARBAGED_P (f))
+	hlinfo->mouse_face_window = Qnil;
+    }
+
+  unblock_input ();
+}
+
+/* EXPORT:
+   End update of window W.
+
+   Draw vertical borders between horizontally adjacent windows, and
+   display W's cursor if CURSOR_ON_P is non-zero.
+
+   MOUSE_FACE_OVERWRITTEN_P non-zero means that some row containing
+   glyphs in mouse-face were overwritten.  In that case we have to
+   make sure that the mouse-highlight is properly redrawn.  */
+void
+gui_update_window_end (struct window *w, bool cursor_on_p,
+                       bool mouse_face_overwritten_p)
+{
+  /* Pseudo windows don't have cursors, so don't display them here.  */
+  if (!w->pseudo_window_p)
+    {
+      block_input ();
+
+      if (cursor_on_p)
+	display_and_set_cursor (w, true,
+				w->output_cursor.hpos, w->output_cursor.vpos,
+				w->output_cursor.x, w->output_cursor.y);
+
+      if (draw_window_fringes (w, true))
+	{
+	  if (WINDOW_RIGHT_DIVIDER_WIDTH (w))
+	    gui_draw_right_divider (w);
+	  else
+	    gui_draw_vertical_border (w);
+	}
+
+      unblock_input ();
+    }
+
+  /* If a row with mouse-face was overwritten, arrange for
+     frame_up_to_date_hook to redisplay the mouse highlight.  */
+  if (mouse_face_overwritten_p)
+    {
+      Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (XFRAME (w->frame));
+
+      hlinfo->mouse_face_beg_row = hlinfo->mouse_face_beg_col = -1;
+      hlinfo->mouse_face_end_row = hlinfo->mouse_face_end_col = -1;
+      hlinfo->mouse_face_window = Qnil;
+    }
+}
+
 /* Redraw the part of window W intersection rectangle FR.  Pixel
    coordinates in FR are frame-relative.  Call this function with
    input blocked.  Value is true if the exposure overwrites
diff --git a/src/xterm.c b/src/xterm.c
index dd19b8bde1..b08f5f6a62 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -989,7 +989,7 @@ x_set_frame_alpha (struct frame *f)
 
 /* Start an update of frame F.  This function is installed as a hook
    for update_begin, i.e. it is called when update_begin is called.
-   This function is called prior to calls to x_update_window_begin for
+   This function is called prior to calls to gui_update_window_begin for
    each window being updated.  Currently, there is nothing to do here
    because all interesting stuff is done on a window basis.  */
 
@@ -1033,33 +1033,6 @@ x_update_begin (struct frame *f)
 #endif /* USE_CAIRO */
 }
 
-/* Start update of window W.  */
-
-static void
-x_update_window_begin (struct window *w)
-{
-  struct frame *f = XFRAME (WINDOW_FRAME (w));
-  Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
-
-  w->output_cursor = w->cursor;
-
-  block_input ();
-
-  if (f == hlinfo->mouse_face_mouse_frame)
-    {
-      /* Don't do highlighting for mouse motion during the update.  */
-      hlinfo->mouse_face_defer = true;
-
-      /* If F needs to be redrawn, simply forget about any prior mouse
-	 highlighting.  */
-      if (FRAME_GARBAGED_P (f))
-	hlinfo->mouse_face_window = Qnil;
-    }
-
-  unblock_input ();
-}
-
-
 /* Draw a vertical window border from (x,y0) to (x,y1)  */
 
 static void
@@ -1139,55 +1112,6 @@ x_draw_window_divider (struct window *w, int x0, int x1, int y0, int y1)
     }
 }
 
-/* End update of window W.
-
-   Draw vertical borders between horizontally adjacent windows, and
-   display W's cursor if CURSOR_ON_P is non-zero.
-
-   MOUSE_FACE_OVERWRITTEN_P non-zero means that some row containing
-   glyphs in mouse-face were overwritten.  In that case we have to
-   make sure that the mouse-highlight is properly redrawn.
-
-   W may be a menu bar pseudo-window in case we don't have X toolkit
-   support.  Such windows don't have a cursor, so don't display it
-   here.  */
-
-static void
-x_update_window_end (struct window *w, bool cursor_on_p,
-		     bool mouse_face_overwritten_p)
-{
-  if (!w->pseudo_window_p)
-    {
-      block_input ();
-
-      if (cursor_on_p)
-	display_and_set_cursor (w, true,
-				w->output_cursor.hpos, w->output_cursor.vpos,
-				w->output_cursor.x, w->output_cursor.y);
-
-      if (draw_window_fringes (w, true))
-	{
-	  if (WINDOW_RIGHT_DIVIDER_WIDTH (w))
-	    gui_draw_right_divider (w);
-	  else
-	    gui_draw_vertical_border (w);
-	}
-
-      unblock_input ();
-    }
-
-  /* If a row with mouse-face was overwritten, arrange for
-     XTframe_up_to_date to redisplay the mouse highlight.  */
-  if (mouse_face_overwritten_p)
-    {
-      Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (XFRAME (w->frame));
-
-      hlinfo->mouse_face_beg_row = hlinfo->mouse_face_beg_col = -1;
-      hlinfo->mouse_face_end_row = hlinfo->mouse_face_end_col = -1;
-      hlinfo->mouse_face_window = Qnil;
-    }
-}
-
 /* Show the frame back buffer.  If frame is double-buffered,
    atomically publish to the user's screen graphics updates made since
    the last call to show_back_buffer.  */
@@ -4306,7 +4230,7 @@ x_scroll_run (struct window *w, struct run *run)
 
   block_input ();
 
-  /* Cursor off.  Will be switched on again in x_update_window_end.  */
+  /* Cursor off.  Will be switched on again in gui_update_window_end.  */
   gui_clear_cursor (w);
 
 #ifdef USE_CAIRO
@@ -13145,8 +13069,8 @@ static struct redisplay_interface x_redisplay_interface =
     gui_clear_end_of_line,
     x_scroll_run,
     x_after_update_window_line,
-    x_update_window_begin,
-    x_update_window_end,
+    gui_update_window_begin,
+    gui_update_window_end,
     x_flip_and_flush,
     gui_clear_window_mouse_face,
     gui_get_glyph_overhangs,
-- 
2.21.0

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

[Eli Zaretskii]

> From: Alex Gramiak <agrambot@gmail.com>
> Date: Sat, 27 Apr 2019 15:28:49 -0600
> 
> 1) Is it okay to not use reset_mouse_highlight in the generic version
> (ns_update_window_end still uses it)?  See commit 60ae3d09932f for why
> reset_mouse_highlight was removed in the x/w32 versions.

Don't know, the problem was specific to X.  I think we should leave
the code there, the NS port has enough redisplay problems already.

> 2) Should the #if 0 section be removed in w32_update_window_begin?

Yes, I think we can remove that.

> @@ -13145,8 +13069,8 @@ static struct redisplay_interface x_redisplay_interface =
>      gui_clear_end_of_line,
>      x_scroll_run,
>      x_after_update_window_line,
> -    x_update_window_begin,
> -    x_update_window_end,
> +    gui_update_window_begin,
> +    gui_update_window_end,
>      x_flip_and_flush,
>      gui_clear_window_mouse_face,
>      gui_get_glyph_overhangs,

This looks like a step in the wrong direction to me: the different
implementations are all almost completely identical, except that w32
has a small quirk there.  So I'd say make a single function
window_update_begin, that will be called directly (not via a hook
pointer), and make the w32 part be an optional hook called only if
non-NULL.

Also, please don't add gui_* functions extracted from the *term.c
files in xdisp.c, as that file is already too large.  Renaming
existing functions in xdisp.c is OK, as well as adding static utility
functions.  But for new gui_* functions that were originally in
xterm.c etc., I'd prefer a new file, let's call it gui_term.c.

Thanks.

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

[Alex Gramiak]

[-- Attachment #1: Type: text/plain, Size: 1763 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

> Don't know, the problem was specific to X.  I think we should leave
> the code there, the NS port has enough redisplay problems already.

It seems that it's needed (see commit 96e78d1fb) for both X and W32, so
I wouldn't be surprised if NS needs the change as well, or, at least, if
it doesn't matter for that port.

>> @@ -13145,8 +13069,8 @@ static struct redisplay_interface x_redisplay_interface =
>>      gui_clear_end_of_line,
>>      x_scroll_run,
>>      x_after_update_window_line,
>> -    x_update_window_begin,
>> -    x_update_window_end,
>> +    gui_update_window_begin,
>> +    gui_update_window_end,
>>      x_flip_and_flush,
>>      gui_clear_window_mouse_face,
>>      gui_get_glyph_overhangs,
>
> This looks like a step in the wrong direction to me: the different
> implementations are all almost completely identical, except that w32
> has a small quirk there.  So I'd say make a single function
> window_update_begin, that will be called directly (not via a hook
> pointer), and make the w32 part be an optional hook called only if
> non-NULL.

Done.

> Also, please don't add gui_* functions extracted from the *term.c
> files in xdisp.c, as that file is already too large.  Renaming
> existing functions in xdisp.c is OK, as well as adding static utility
> functions.  But for new gui_* functions that were originally in
> xterm.c etc., I'd prefer a new file, let's call it gui_term.c.

A gui_term.c might be beneficial in the future, but I'm not sure it
should be the place for RIF procedures like these. I attached an updated
patch that currently puts them in terminal.c (beside update_begin and
update_end), but I think a better place would be to put them beside
update_window in dispnew.c. WDYT?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2 --]
[-- Type: text/x-patch, Size: 18740 bytes --]

From 6b6bae2370a378e08e50acad99007f10b541fe93 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Sat, 27 Apr 2019 15:00:13 -0600
Subject: [PATCH] Refactor update_window_begin and update_window_end hooks

* src/terminal.c (gui_update_window_begin, gui_update_window_end): New
procedures implementing common functionality.

* src/nsterm.m: (ns_update_window_begin, ns_update_window_end):
* src/xterm.c: (x_update_window_begin, x_update_window_end): Remove in
favor of only using the new generic versions.

* src/w32term.c: (w32_update_window_begin, w32_update_window_end):
Remove duplicated code.
---
 src/dispextern.h |  4 +++
 src/dispnew.c    |  4 +--
 src/nsterm.m     | 78 +++----------------------------------------
 src/terminal.c   | 86 ++++++++++++++++++++++++++++++++++++++++++++++++
 src/w32term.c    | 83 ++--------------------------------------------
 src/xdisp.c      | 12 +++----
 src/xterm.c      | 84 +++-------------------------------------------
 7 files changed, 109 insertions(+), 242 deletions(-)

diff --git a/src/dispextern.h b/src/dispextern.h
index 4d6d0371d3..dede7830ec 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -3533,6 +3533,10 @@ extern void fill_up_frame_row_with_spaces (struct glyph_row *, int);
 extern void ring_bell (struct frame *);
 extern void update_begin (struct frame *);
 extern void update_end (struct frame *);
+#ifdef HAVE_WINDOW_SYSTEM
+extern void gui_update_window_begin (struct window *);
+extern void gui_update_window_end (struct window *, bool, bool);
+#endif
 extern void set_terminal_window (struct frame *, int);
 extern void cursor_to (struct frame *, int, int);
 extern void raw_cursor_to (struct frame *, int, int);
diff --git a/src/dispnew.c b/src/dispnew.c
index 25a2d1cd38..7577760483 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -3411,7 +3411,7 @@ update_window (struct window *w, bool force_p)
       bool changed_p = 0, mouse_face_overwritten_p = 0;
       int n_updated = 0;
 
-      rif->update_window_begin_hook (w);
+      gui_update_window_begin (w);
       yb = window_text_bottom_y (w);
       row = MATRIX_ROW (desired_matrix, 0);
       end = MATRIX_MODE_LINE_ROW (desired_matrix);
@@ -3539,7 +3539,7 @@ update_window (struct window *w, bool force_p)
          paused updating the display because in this case,
          set_window_cursor_after_update hasn't been called, and
          W->output_cursor doesn't contain the cursor location.  */
-      rif->update_window_end_hook (w, !paused_p, mouse_face_overwritten_p);
+      gui_update_window_end (w, !paused_p, mouse_face_overwritten_p);
     }
   else
     paused_p = 1;
diff --git a/src/nsterm.m b/src/nsterm.m
index cdf1916e71..ffb7b7692b 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1106,7 +1106,7 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
 ns_update_begin (struct frame *f)
 /* --------------------------------------------------------------------------
    Prepare for a grouped sequence of drawing calls
-   external (RIF) call; whole frame, called before update_window_begin
+   external (RIF) call; whole frame, called before gui_update_window_begin
    -------------------------------------------------------------------------- */
 {
 #ifdef NS_IMPL_COCOA
@@ -1128,81 +1128,11 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
 }
 
 
-static void
-ns_update_window_begin (struct window *w)
-/* --------------------------------------------------------------------------
-   Prepare for a grouped sequence of drawing calls
-   external (RIF) call; for one window, called after update_begin
-   -------------------------------------------------------------------------- */
-{
-  struct frame *f = XFRAME (WINDOW_FRAME (w));
-  Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
-
-  NSTRACE_WHEN (NSTRACE_GROUP_UPDATES, "ns_update_window_begin");
-  w->output_cursor = w->cursor;
-
-  block_input ();
-
-  if (f == hlinfo->mouse_face_mouse_frame)
-    {
-      /* Don't do highlighting for mouse motion during the update.  */
-      hlinfo->mouse_face_defer = 1;
-
-        /* If the frame needs to be redrawn,
-           simply forget about any prior mouse highlighting.  */
-      if (FRAME_GARBAGED_P (f))
-        hlinfo->mouse_face_window = Qnil;
-
-      /* (further code for mouse faces ifdef'd out in other terms elided) */
-    }
-
-  unblock_input ();
-}
-
-
-static void
-ns_update_window_end (struct window *w, bool cursor_on_p,
-                      bool mouse_face_overwritten_p)
-/* --------------------------------------------------------------------------
-   Finished a grouped sequence of drawing calls
-   external (RIF) call; for one window called before update_end
-   -------------------------------------------------------------------------- */
-{
-  NSTRACE_WHEN (NSTRACE_GROUP_UPDATES, "ns_update_window_end");
-
-  /* note: this fn is nearly identical in all terms */
-  if (!w->pseudo_window_p)
-    {
-      block_input ();
-
-      if (cursor_on_p)
-	display_and_set_cursor (w, 1,
-				w->output_cursor.hpos, w->output_cursor.vpos,
-				w->output_cursor.x, w->output_cursor.y);
-
-      if (draw_window_fringes (w, 1))
-	{
-	  if (WINDOW_RIGHT_DIVIDER_WIDTH (w))
-	    gui_draw_right_divider (w);
-	  else
-	    gui_draw_vertical_border (w);
-	}
-
-      unblock_input ();
-    }
-
-  /* If a row with mouse-face was overwritten, arrange for
-     frame_up_to_date to redisplay the mouse highlight.  */
-  if (mouse_face_overwritten_p)
-    reset_mouse_highlight (MOUSE_HL_INFO (XFRAME (w->frame)));
-}
-
-
 static void
 ns_update_end (struct frame *f)
 /* --------------------------------------------------------------------------
    Finished a grouped sequence of drawing calls
-   external (RIF) call; for whole frame, called after update_window_end
+   external (RIF) call; for whole frame, called after gui_update_window_end
    -------------------------------------------------------------------------- */
 {
   NSTRACE_WHEN (NSTRACE_GROUP_UPDATES, "ns_update_end");
@@ -5166,8 +5096,8 @@ static Lisp_Object ns_string_to_lispmod (const char *s)
   gui_clear_end_of_line,
   ns_scroll_run,
   ns_after_update_window_line,
-  ns_update_window_begin,
-  ns_update_window_end,
+  NULL, /* update_window_begin */
+  NULL, /* update_window_end   */
   0, /* flush_display */
   gui_clear_window_mouse_face,
   gui_get_glyph_overhangs,
diff --git a/src/terminal.c b/src/terminal.c
index 0ee0121e35..5486905ff2 100644
--- a/src/terminal.c
+++ b/src/terminal.c
@@ -26,6 +26,7 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
 #include "termchar.h"
 #include "termhooks.h"
 #include "keyboard.h"
+#include "blockinput.h"
 
 #if HAVE_STRUCT_UNIPAIR_UNICODE
 # include <errno.h>
@@ -93,6 +94,91 @@ update_end (struct frame *f)
     (*FRAME_TERMINAL (f)->update_end_hook) (f);
 }
 
+#ifdef HAVE_WINDOW_SYSTEM
+
+/* Start update of window W.  */
+
+void
+gui_update_window_begin (struct window *w)
+{
+  struct frame *f = XFRAME (WINDOW_FRAME (w));
+  Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
+
+  block_input ();
+
+  if (FRAME_RIF (f)->update_window_begin_hook)
+    FRAME_RIF (f)->update_window_begin_hook (w);
+
+  w->output_cursor = w->cursor;
+
+  if (f == hlinfo->mouse_face_mouse_frame)
+    {
+      /* Don't do highlighting for mouse motion during the update.  */
+      hlinfo->mouse_face_defer = true;
+
+      /* If the frame needs to be redrawn, simply forget about any
+	 prior mouse highlighting.  */
+      if (FRAME_GARBAGED_P (f))
+	hlinfo->mouse_face_window = Qnil;
+    }
+
+  unblock_input ();
+}
+
+/* End update of window W.
+
+   Draw vertical borders between horizontally adjacent windows, and
+   display W's cursor if CURSOR_ON_P is non-zero.
+
+   MOUSE_FACE_OVERWRITTEN_P non-zero means that some row containing
+   glyphs in mouse-face were overwritten.  In that case we have to
+   make sure that the mouse-highlight is properly redrawn.  */
+void
+gui_update_window_end (struct window *w, bool cursor_on_p,
+                       bool mouse_face_overwritten_p)
+{
+  struct frame *f = XFRAME (WINDOW_FRAME (w));
+
+  block_input ();
+
+  /* Pseudo windows don't have cursors, so don't display them here.  */
+  if (!w->pseudo_window_p)
+    {
+
+      if (cursor_on_p)
+	display_and_set_cursor (w, true,
+				w->output_cursor.hpos, w->output_cursor.vpos,
+				w->output_cursor.x, w->output_cursor.y);
+
+      if (draw_window_fringes (w, true))
+	{
+	  if (WINDOW_RIGHT_DIVIDER_WIDTH (w))
+	    gui_draw_right_divider (w);
+	  else
+	    gui_draw_vertical_border (w);
+	}
+    }
+
+  /* If a row with mouse-face was overwritten, arrange for
+     frame_up_to_date_hook to redisplay the mouse highlight.  */
+  if (mouse_face_overwritten_p)
+    {
+      Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
+
+      hlinfo->mouse_face_beg_row = hlinfo->mouse_face_beg_col = -1;
+      hlinfo->mouse_face_end_row = hlinfo->mouse_face_end_col = -1;
+      hlinfo->mouse_face_window = Qnil;
+    }
+
+  if (FRAME_RIF (f)->update_window_end_hook)
+    FRAME_RIF (f)->update_window_end_hook (w,
+                                           cursor_on_p,
+                                           mouse_face_overwritten_p);
+  unblock_input ();
+}
+
+#endif /* HAVE_WINDOW_SYSTEM  */
+
 /* Specify how many text lines, from the top of the window,
    should be affected by insert-lines and delete-lines operations.
    This, and those operations, are used only within an update
diff --git a/src/w32term.c b/src/w32term.c
index 451dd54dd8..0abec3d92a 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -529,7 +529,7 @@ w32_display_pixel_width (struct w32_display_info *dpyinfo)
 
 /* Start an update of frame F.  This function is installed as a hook
    for update_begin, i.e. it is called when update_begin is called.
-   This function is called prior to calls to w32_update_window_begin
+   This function is called prior to calls to gui_update_window_begin
    for each window being updated.  */
 
 static void
@@ -555,58 +555,12 @@ w32_update_begin (struct frame *f)
 static void
 w32_update_window_begin (struct window *w)
 {
-  struct frame *f = XFRAME (WINDOW_FRAME (w));
-  Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
-
   /* Hide the system caret during an update.  */
   if (w32_use_visible_system_caret && w32_system_caret_hwnd)
     {
       SendMessageTimeout (w32_system_caret_hwnd, WM_EMACS_HIDE_CARET, 0, 0,
 			  0, 6000, NULL);
     }
-
-  w->output_cursor = w->cursor;
-
-  block_input ();
-
-  if (f == hlinfo->mouse_face_mouse_frame)
-    {
-      /* Don't do highlighting for mouse motion during the update.  */
-      hlinfo->mouse_face_defer = true;
-
-      /* If F needs to be redrawn, simply forget about any prior mouse
-	 highlighting.  */
-      if (FRAME_GARBAGED_P (f))
-	hlinfo->mouse_face_window = Qnil;
-
-#if 0 /* Rows in a current matrix containing glyphs in mouse-face have
-	 their mouse_face_p flag set, which means that they are always
-	 unequal to rows in a desired matrix which never have that
-	 flag set.  So, rows containing mouse-face glyphs are never
-	 scrolled, and we don't have to switch the mouse highlight off
-	 here to prevent it from being scrolled.  */
-
-      /* Can we tell that this update does not affect the window
-	 where the mouse highlight is?  If so, no need to turn off.
-	 Likewise, don't do anything if the frame is garbaged;
-	 in that case, the frame's current matrix that we would use
-	 is all wrong, and we will redisplay that line anyway.  */
-      if (!NILP (hlinfo->mouse_face_window)
-	  && w == XWINDOW (hlinfo->mouse_face_window))
-	{
-	  int i;
-
-          for (i = 0; i < w->desired_matrix->nrows; ++i)
-	    if (MATRIX_ROW_ENABLED_P (w->desired_matrix, i))
-	      break;
-
-	  if (i < w->desired_matrix->nrows)
-	    clear_mouse_face (hlinfo);
-	}
-#endif /* 0 */
-    }
-
-  unblock_input ();
 }
 
 /* Draw a vertical window border from (x,y0) to (x,y1)  */
@@ -694,39 +648,8 @@ w32_draw_window_divider (struct window *w, int x0, int x1, int y0, int y1)
 
 static void
 w32_update_window_end (struct window *w, bool cursor_on_p,
-		     bool mouse_face_overwritten_p)
+                       bool mouse_face_overwritten_p)
 {
-  if (!w->pseudo_window_p)
-    {
-      block_input ();
-
-      if (cursor_on_p)
-	display_and_set_cursor (w, true,
-				w->output_cursor.hpos, w->output_cursor.vpos,
-				w->output_cursor.x, w->output_cursor.y);
-
-      if (draw_window_fringes (w, true))
-	{
-	  if (WINDOW_RIGHT_DIVIDER_WIDTH (w))
-	    gui_draw_right_divider (w);
-	  else
-	    gui_draw_vertical_border (w);
-	}
-
-      unblock_input ();
-    }
-
-  /* If a row with mouse-face was overwritten, arrange for
-     XTframe_up_to_date to redisplay the mouse highlight.  */
-  if (mouse_face_overwritten_p)
-    {
-      Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (XFRAME (w->frame));
-
-      hlinfo->mouse_face_beg_row = hlinfo->mouse_face_beg_col = -1;
-      hlinfo->mouse_face_end_row = hlinfo->mouse_face_end_col = -1;
-      hlinfo->mouse_face_window = Qnil;
-    }
-
   /* Unhide the caret.  This won't actually show the cursor, unless it
      was visible before the corresponding call to HideCaret in
      w32_update_window_begin.  */
@@ -2859,7 +2782,7 @@ w32_scroll_run (struct window *w, struct run *run)
 
   block_input ();
 
-  /* Cursor off.  Will be switched on again in w32_update_window_end.  */
+  /* Cursor off.  Will be switched on again in gui_update_window_end.  */
   gui_clear_cursor (w);
 
   {
diff --git a/src/xdisp.c b/src/xdisp.c
index d52d1333a0..7450da59b6 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -18031,10 +18031,10 @@ try_window_reusing_current_matrix (struct window *w)
 	  if (run.height > 0 && run.current_y != run.desired_y)
 	    {
 	      update_begin (f);
-	      FRAME_RIF (f)->update_window_begin_hook (w);
+	      gui_update_window_begin (w);
 	      FRAME_RIF (f)->clear_window_mouse_face (w);
 	      FRAME_RIF (f)->scroll_run_hook (w, &run);
-	      FRAME_RIF (f)->update_window_end_hook (w, false, false);
+	      gui_update_window_end (w, false, false);
 	      update_end (f);
 	    }
 
@@ -18195,10 +18195,10 @@ try_window_reusing_current_matrix (struct window *w)
       if (run.height)
 	{
 	  update_begin (f);
-	  FRAME_RIF (f)->update_window_begin_hook (w);
+	  gui_update_window_begin (w);
 	  FRAME_RIF (f)->clear_window_mouse_face (w);
 	  FRAME_RIF (f)->scroll_run_hook (w, &run);
-	  FRAME_RIF (f)->update_window_end_hook (w, false, false);
+	  gui_update_window_end (w, false, false);
 	  update_end (f);
 	}
 
@@ -19147,10 +19147,10 @@ try_window_id (struct window *w)
 
       if (FRAME_WINDOW_P (f))
 	{
-	  FRAME_RIF (f)->update_window_begin_hook (w);
+	  gui_update_window_begin (w);
 	  FRAME_RIF (f)->clear_window_mouse_face (w);
 	  FRAME_RIF (f)->scroll_run_hook (w, &run);
-	  FRAME_RIF (f)->update_window_end_hook (w, false, false);
+	  gui_update_window_end (w, false, false);
 	}
       else
 	{
diff --git a/src/xterm.c b/src/xterm.c
index dd19b8bde1..26f74cde91 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -989,7 +989,7 @@ x_set_frame_alpha (struct frame *f)
 
 /* Start an update of frame F.  This function is installed as a hook
    for update_begin, i.e. it is called when update_begin is called.
-   This function is called prior to calls to x_update_window_begin for
+   This function is called prior to calls to gui_update_window_begin for
    each window being updated.  Currently, there is nothing to do here
    because all interesting stuff is done on a window basis.  */
 
@@ -1033,33 +1033,6 @@ x_update_begin (struct frame *f)
 #endif /* USE_CAIRO */
 }
 
-/* Start update of window W.  */
-
-static void
-x_update_window_begin (struct window *w)
-{
-  struct frame *f = XFRAME (WINDOW_FRAME (w));
-  Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
-
-  w->output_cursor = w->cursor;
-
-  block_input ();
-
-  if (f == hlinfo->mouse_face_mouse_frame)
-    {
-      /* Don't do highlighting for mouse motion during the update.  */
-      hlinfo->mouse_face_defer = true;
-
-      /* If F needs to be redrawn, simply forget about any prior mouse
-	 highlighting.  */
-      if (FRAME_GARBAGED_P (f))
-	hlinfo->mouse_face_window = Qnil;
-    }
-
-  unblock_input ();
-}
-
-
 /* Draw a vertical window border from (x,y0) to (x,y1)  */
 
 static void
@@ -1139,55 +1112,6 @@ x_draw_window_divider (struct window *w, int x0, int x1, int y0, int y1)
     }
 }
 
-/* End update of window W.
-
-   Draw vertical borders between horizontally adjacent windows, and
-   display W's cursor if CURSOR_ON_P is non-zero.
-
-   MOUSE_FACE_OVERWRITTEN_P non-zero means that some row containing
-   glyphs in mouse-face were overwritten.  In that case we have to
-   make sure that the mouse-highlight is properly redrawn.
-
-   W may be a menu bar pseudo-window in case we don't have X toolkit
-   support.  Such windows don't have a cursor, so don't display it
-   here.  */
-
-static void
-x_update_window_end (struct window *w, bool cursor_on_p,
-		     bool mouse_face_overwritten_p)
-{
-  if (!w->pseudo_window_p)
-    {
-      block_input ();
-
-      if (cursor_on_p)
-	display_and_set_cursor (w, true,
-				w->output_cursor.hpos, w->output_cursor.vpos,
-				w->output_cursor.x, w->output_cursor.y);
-
-      if (draw_window_fringes (w, true))
-	{
-	  if (WINDOW_RIGHT_DIVIDER_WIDTH (w))
-	    gui_draw_right_divider (w);
-	  else
-	    gui_draw_vertical_border (w);
-	}
-
-      unblock_input ();
-    }
-
-  /* If a row with mouse-face was overwritten, arrange for
-     XTframe_up_to_date to redisplay the mouse highlight.  */
-  if (mouse_face_overwritten_p)
-    {
-      Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (XFRAME (w->frame));
-
-      hlinfo->mouse_face_beg_row = hlinfo->mouse_face_beg_col = -1;
-      hlinfo->mouse_face_end_row = hlinfo->mouse_face_end_col = -1;
-      hlinfo->mouse_face_window = Qnil;
-    }
-}
-
 /* Show the frame back buffer.  If frame is double-buffered,
    atomically publish to the user's screen graphics updates made since
    the last call to show_back_buffer.  */
@@ -4306,7 +4230,7 @@ x_scroll_run (struct window *w, struct run *run)
 
   block_input ();
 
-  /* Cursor off.  Will be switched on again in x_update_window_end.  */
+  /* Cursor off.  Will be switched on again in gui_update_window_end.  */
   gui_clear_cursor (w);
 
 #ifdef USE_CAIRO
@@ -13145,8 +13069,8 @@ static struct redisplay_interface x_redisplay_interface =
     gui_clear_end_of_line,
     x_scroll_run,
     x_after_update_window_line,
-    x_update_window_begin,
-    x_update_window_end,
+    NULL, /* update_window_begin */
+    NULL, /* update_window_end   */
     x_flip_and_flush,
     gui_clear_window_mouse_face,
     gui_get_glyph_overhangs,
-- 
2.21.0

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

[Eli Zaretskii]

> From: Alex Gramiak <agrambot@gmail.com>
> Cc: 35464@debbugs.gnu.org
> Date: Sun, 28 Apr 2019 12:37:28 -0600
> 
> > Also, please don't add gui_* functions extracted from the *term.c
> > files in xdisp.c, as that file is already too large.  Renaming
> > existing functions in xdisp.c is OK, as well as adding static utility
> > functions.  But for new gui_* functions that were originally in
> > xterm.c etc., I'd prefer a new file, let's call it gui_term.c.
> 
> A gui_term.c might be beneficial in the future, but I'm not sure it
> should be the place for RIF procedures like these.

Bu these functions are not RIF procedures, they are just regular
functions.

> I attached an updated patch that currently puts them in terminal.c
> (beside update_begin and update_end), but I think a better place
> would be to put them beside update_window in dispnew.c. WDYT?

It isn't worth arguing about 2 short functions, so OK.  But those in
your other patch should definitely start a new file.

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

[Alex Gramiak]

[-- Attachment #1: Type: text/plain, Size: 201 bytes --]

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?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: scroll_run_hook --]
[-- Type: text/x-patch, Size: 12362 bytes --]

From 18c2ba95e5f519b44ec49c398eee9572f2ded759 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
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
 
 \f
 /************************************************************************
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

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

[Eli Zaretskii]

> From: Alex Gramiak <agrambot@gmail.com>
> Cc: 35464@debbugs.gnu.org
> Date: Thu, 02 May 2019 23:10:41 -0600
> 
> 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?

I'm not sure this is a change for the best.  scroll_run is already a
frame-specific method; just because its 3 implementations are similar
in some of their parts doesn't yet mean _all_ possible implementations
will be that way.  Suppose the NS implementation needs to change for
some reason, or the TTY case needs to use this -- what do we do then?
move the code back to *term.[cm]?

The change also splits the code's logic into 2 parts in 2 different
places, another minus in my book.

This also adds an ifdef where currently there is none, which is an
additional small disadvantage.

Bottom line: I think this does not have enough significant advantages
to change the current code.

Thanks.