MS Windows double buffering

MS Windows double buffering

[Po Lu]

I took a stab at implementing double buffering on MS Windows without any
prior knowledge of programming for that platform.  It naturally follows
that the code is nowhere near complete, but it works with some major
problems:

  - It doesn't work on Windows XP or earlier.

  - Upon startup, part of the entire display is overwritten by the Emacs
    toolbar, until the first frame is resized.

  - w32_scroll_run doesn't work.

  - Paint buffers and their DCs are not released upon frame deletion.

  - BeginBufferedPaint is probably the wrong function to use to create
    the back buffer, since EndBufferedPaint isn't used.

I hope someone will finish implementing double buffering on MS Windows
based on this code.  Even in its present state, it already eliminates
all the flicker that was discussed earlier.

diff --git a/configure.ac b/configure.ac
index 7c8638a471..d42a6dd180 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2209,7 +2209,7 @@ AC_DEFUN
       W32_OBJ="$W32_OBJ w32image.o"
     fi
     W32_LIBS="$W32_LIBS -lwinmm -lusp10 -lgdi32 -lcomdlg32"
-    W32_LIBS="$W32_LIBS -lmpr -lwinspool -lole32 -lcomctl32"
+    W32_LIBS="$W32_LIBS -lmpr -lwinspool -lole32 -lcomctl32 -luxtheme"
     W32_RES_LINK="\$(EMACSRES)"
     CLIENTRES="emacsclient.res"
     CLIENTW="emacsclientw\$(EXEEXT)"
diff --git a/src/w32gui.h b/src/w32gui.h
index 4e8de84854..689346111a 100644
--- a/src/w32gui.h
+++ b/src/w32gui.h
@@ -19,6 +19,7 @@
 #ifndef EMACS_W32GUI_H
 #define EMACS_W32GUI_H
 #include <windows.h>
+#include <uxtheme.h>
 
 #include "systime.h" /* for Time */
 
diff --git a/src/w32term.c b/src/w32term.c
index 7837032304..e55910c9e8 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -275,6 +275,36 @@ XGetGCValues (void *ignore, XGCValues *gc,
 }
 #endif
 
+static void
+w32_show_back_buffer (struct frame *f, BOOL update_frame)
+{
+  struct w32_output *output;
+
+  output = FRAME_OUTPUT_DATA (f);
+
+  if (output->paint_buffer)
+    {
+      if (!update_frame)
+	{
+	  EndBufferedPaint (output->paint_buffer, update_frame);
+	  deselect_palette (f, output->paint_dc);
+	  ReleaseDC (output->window_desc, output->paint_dc);
+
+	  output->paint_buffer = NULL;
+	  output->paint_buffer_desc = NULL;
+	  output->paint_dc = NULL;
+	}
+      else
+	{
+	  BitBlt (output->paint_dc, 0, 0,
+		  FRAME_PIXEL_WIDTH (f),
+		  FRAME_PIXEL_HEIGHT (f),
+		  output->paint_buffer_desc,
+		  0, 0, SRCCOPY);
+	}
+    }
+}
+
 static void
 w32_get_mouse_wheel_vertical_delta (void)
 {
@@ -706,8 +736,16 @@ w32_frame_up_to_date (struct frame *f)
 {
   if (FRAME_W32_P (f))
     FRAME_MOUSE_UPDATE (f);
+
+  if (!buffer_flipping_blocked_p ())
+    w32_show_back_buffer (f, TRUE);
 }
 
+static void
+w32_buffer_flipping_unblocked_hook (struct frame *f)
+{
+  w32_show_back_buffer (f, TRUE);
+}
 
 /* Draw truncation mark bitmaps, continuation mark bitmaps, overlay
    arrow bitmaps, or clear the fringes if no bitmaps are required
@@ -2872,8 +2910,7 @@ w32_scroll_run (struct window *w, struct run *run)
 {
   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;
+  HDC hdc;
 
   /* Get frame-relative bounding box of the text display area of W,
      without mode lines.  Include in this box the left and right
@@ -2892,7 +2929,6 @@ w32_scroll_run (struct window *w, struct run *run)
 	height = bottom_y - from_y;
       else
 	height = run->height;
-      expect_dirty = CreateRectRgn (x, y + height, x + width, bottom_y);
     }
   else
     {
@@ -2902,44 +2938,16 @@ w32_scroll_run (struct window *w, struct run *run)
 	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;
-    HRGN dirty = CreateRectRgn (0, 0, 0, 0);
-    HRGN combined = CreateRectRgn (0, 0, 0, 0);
-
-    from.left = to.left = x;
-    from.right = to.right = x + width;
-    from.top = from_y;
-    from.bottom = from_y + height;
-    to.top = y;
-    to.bottom = bottom_y;
-
-    ScrollWindowEx (hwnd, 0, to_y - from_y, &from, &to, dirty,
-		    NULL, SW_INVALIDATE);
-
-    /* Combine this with what we expect to be dirty. This covers the
-       case where not all of the region we expect is actually dirty.  */
-    CombineRgn (combined, dirty, expect_dirty, RGN_OR);
-
-    /* If the dirty region is not what we expected, redraw the entire frame.  */
-    if (!EqualRgn (combined, expect_dirty))
-      SET_FRAME_GARBAGED (f);
-
-    DeleteObject (dirty);
-    DeleteObject (combined);
-  }
-
+  hdc = get_frame_dc (f);
+  BitBlt (hdc, x, from_y, width, height,
+	  hdc, x, to_y, SRCCOPY);
+  release_frame_dc (f, hdc);
   unblock_input ();
-  DeleteObject (expect_dirty);
 }
 
 
@@ -5659,6 +5667,10 @@ w32_read_socket (struct terminal *terminal,
 		  if (width != FRAME_PIXEL_WIDTH (f)
 		      || height != FRAME_PIXEL_HEIGHT (f))
 		    {
+		      /* Delete the back buffer so it gets created
+			 again the next time we ask for the DC.  */
+		      w32_show_back_buffer (f, FALSE);
+
 		      change_frame_size
 			(f, width, height, false, true, false);
 		      SET_FRAME_GARBAGED (f);
@@ -7350,6 +7362,7 @@ w32_create_terminal (struct w32_display_info *dpyinfo)
   terminal->update_end_hook = w32_update_end;
   terminal->read_socket_hook = w32_read_socket;
   terminal->frame_up_to_date_hook = w32_frame_up_to_date;
+  terminal->buffer_flipping_unblocked_hook = w32_buffer_flipping_unblocked_hook;
   terminal->defined_color_hook = w32_defined_color;
   terminal->query_frame_background_color = w32_query_frame_background_color;
   terminal->query_colors = w32_query_colors;
@@ -7430,6 +7443,8 @@ w32_term_init (Lisp_Object display_name, char *xrm_option, char *resource_name)
       w32_initialized = 1;
     }
 
+  BufferedPaintInit ();
+
   w32_initialize_display_info (display_name);
 
   dpyinfo = &one_w32_display_info;
@@ -7505,6 +7520,8 @@ w32_delete_display (struct w32_display_info *dpyinfo)
     if (dpyinfo->palette)
       DeleteObject (dpyinfo->palette);
   }
+
+  BufferedPaintUnInit ();
   w32_reset_fringes ();
 }
 
diff --git a/src/w32term.h b/src/w32term.h
index 6c48323651..5a4b6e627f 100644
--- a/src/w32term.h
+++ b/src/w32term.h
@@ -412,6 +412,13 @@ #define PIX_TYPE COLORREF
      geometry when 'fullscreen' is reset to nil.  */
   WINDOWPLACEMENT normal_placement;
   int prev_fsmode;
+
+  /* The paint buffer if there is an ongoing double-buffered drawing
+     operation.  */
+  HPAINTBUFFER paint_buffer;
+
+  /* The handle of the paint buffer.  */
+  HDC paint_buffer_desc, paint_dc;
 };
 
 extern struct w32_output w32term_display;
diff --git a/src/w32xfns.c b/src/w32xfns.c
index d5974b906e..320b8619b2 100644
--- a/src/w32xfns.c
+++ b/src/w32xfns.c
@@ -136,13 +136,13 @@ select_palette (struct frame *f, HDC hdc)
     f->output_data.w32->old_palette = NULL;
 
   if (RealizePalette (hdc) != GDI_ERROR)
-  {
-    Lisp_Object frame, framelist;
-    FOR_EACH_FRAME (framelist, frame)
     {
-      SET_FRAME_GARBAGED (XFRAME (frame));
+      Lisp_Object frame, framelist;
+      FOR_EACH_FRAME (framelist, frame)
+	{
+	  SET_FRAME_GARBAGED (XFRAME (frame));
+	}
     }
-  }
 }
 
 void
@@ -157,19 +157,47 @@ deselect_palette (struct frame *f, HDC hdc)
 HDC
 get_frame_dc (struct frame *f)
 {
-  HDC hdc;
+  HDC hdc, dest;
+  HPAINTBUFFER paint_buffer;
+  RECT r;
 
   if (f->output_method != output_w32)
     emacs_abort ();
 
   enter_crit ();
 
-  hdc = GetDC (f->output_data.w32->window_desc);
+  if (FRAME_OUTPUT_DATA (f)->paint_buffer_desc)
+    return FRAME_OUTPUT_DATA (f)->paint_buffer_desc;
+
+  hdc = GetDC (FRAME_OUTPUT_DATA (f)->window_desc);
 
   /* If this gets called during startup before the frame is valid,
      there is a chance of corrupting random data or crashing. */
   if (hdc)
-    select_palette (f, hdc);
+    {
+      select_palette (f, hdc);
+
+      r.left = 0;
+      r.top = 0;
+      r.right = FRAME_PIXEL_WIDTH (f) - 1;
+      r.bottom = FRAME_PIXEL_HEIGHT (f) - 1;
+
+      paint_buffer = BeginBufferedPaint (hdc, &r,
+					 BPBF_COMPATIBLEBITMAP,
+					 NULL, &dest);
+
+      if (paint_buffer)
+	{
+	  FRAME_OUTPUT_DATA (f)->paint_buffer = paint_buffer;
+	  FRAME_OUTPUT_DATA (f)->paint_buffer_desc = dest;
+	  FRAME_OUTPUT_DATA (f)->paint_dc = hdc;
+
+	  BitBlt (dest, 0, 0, FRAME_PIXEL_WIDTH (f),
+		  FRAME_PIXEL_HEIGHT (f), hdc, 0, 0, SRCCOPY);
+
+	  return dest;
+	}
+    }
 
   return hdc;
 }
@@ -179,8 +207,15 @@ release_frame_dc (struct frame *f, HDC hdc)
 {
   int ret;
 
-  deselect_palette (f, hdc);
-  ret = ReleaseDC (f->output_data.w32->window_desc, hdc);
+  /* Avoid releasing the double-buffered DC here, since it'll be
+     released upon the next buffer flip instead.  */
+  if (hdc != FRAME_OUTPUT_DATA (f)->paint_dc)
+    {
+      deselect_palette (f, hdc);
+      ret = ReleaseDC (f->output_data.w32->window_desc, hdc);
+    }
+  else
+    ret = 0;
 
   leave_crit ();
 

Re: MS Windows double buffering

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Date: Thu, 28 Apr 2022 16:51:06 +0800
> 
> I took a stab at implementing double buffering on MS Windows without any
> prior knowledge of programming for that platform.  It naturally follows
> that the code is nowhere near complete, but it works with some major
> problems:

Thanks.

> I hope someone will finish implementing double buffering on MS Windows
> based on this code.

Seconded.

Re: MS Windows double buffering

[Po Lu]

> Seconded.

Right... since the best way to get something done fast is to do it
yourself, I tried my best to fix all the issues mentioned above.  It
should work on old versions of MS Windows now as well.  If this works
for you, I'll install this after implementing the
`inhibit-double-buffering' frame parameter.  Thanks.

diff --git a/src/w32term.c b/src/w32term.c
index 7837032304..245d46a499 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -275,6 +275,57 @@ XGetGCValues (void *ignore, XGCValues *gc,
 }
 #endif
 
+static void
+w32_show_back_buffer (struct frame *f)
+{
+  struct w32_output *output;
+  HDC raw_dc;
+
+  output = FRAME_OUTPUT_DATA (f);
+
+  enter_crit ();
+
+  if (output->paint_buffer)
+    {
+      raw_dc = GetDC (output->window_desc);
+
+      if (!raw_dc)
+	emacs_abort ();
+
+      BitBlt (raw_dc, 0, 0, FRAME_PIXEL_WIDTH (f),
+	      FRAME_PIXEL_HEIGHT (f),
+	      output->paint_dc, 0, 0, SRCCOPY);
+      ReleaseDC (output->window_desc, raw_dc);
+
+      output->paint_buffer_dirty = 0;
+    }
+
+  leave_crit ();
+}
+
+void
+w32_release_paint_buffer (struct frame *f)
+{
+  /* Delete the back buffer so it gets created
+     again the next time we ask for the DC.  */
+
+  enter_crit ();
+  if (FRAME_OUTPUT_DATA (f)->paint_buffer)
+    {
+      SelectObject (FRAME_OUTPUT_DATA (f)->paint_dc,
+		    FRAME_OUTPUT_DATA (f)->paint_dc_object);
+      ReleaseDC (FRAME_OUTPUT_DATA (f)->window_desc,
+		 FRAME_OUTPUT_DATA (f)->paint_buffer_handle);
+      DeleteDC (FRAME_OUTPUT_DATA (f)->paint_dc);
+      DeleteObject (FRAME_OUTPUT_DATA (f)->paint_buffer);
+
+      FRAME_OUTPUT_DATA (f)->paint_buffer = NULL;
+      FRAME_OUTPUT_DATA (f)->paint_dc = NULL;
+      FRAME_OUTPUT_DATA (f)->paint_buffer_handle = NULL;
+    }
+  leave_crit ();
+}
+
 static void
 w32_get_mouse_wheel_vertical_delta (void)
 {
@@ -704,10 +755,18 @@ w32_update_end (struct frame *f)
 static void
 w32_frame_up_to_date (struct frame *f)
 {
-  if (FRAME_W32_P (f))
-    FRAME_MOUSE_UPDATE (f);
+  FRAME_MOUSE_UPDATE (f);
+
+  if (!buffer_flipping_blocked_p ()
+      && FRAME_OUTPUT_DATA (f)->paint_buffer_dirty)
+    w32_show_back_buffer (f);
 }
 
+static void
+w32_buffer_flipping_unblocked_hook (struct frame *f)
+{
+  w32_show_back_buffer (f);
+}
 
 /* Draw truncation mark bitmaps, continuation mark bitmaps, overlay
    arrow bitmaps, or clear the fringes if no bitmaps are required
@@ -2872,8 +2931,7 @@ w32_scroll_run (struct window *w, struct run *run)
 {
   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;
+  HDC hdc;
 
   /* Get frame-relative bounding box of the text display area of W,
      without mode lines.  Include in this box the left and right
@@ -2892,7 +2950,6 @@ w32_scroll_run (struct window *w, struct run *run)
 	height = bottom_y - from_y;
       else
 	height = run->height;
-      expect_dirty = CreateRectRgn (x, y + height, x + width, bottom_y);
     }
   else
     {
@@ -2902,44 +2959,15 @@ w32_scroll_run (struct window *w, struct run *run)
 	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;
-    HRGN dirty = CreateRectRgn (0, 0, 0, 0);
-    HRGN combined = CreateRectRgn (0, 0, 0, 0);
-
-    from.left = to.left = x;
-    from.right = to.right = x + width;
-    from.top = from_y;
-    from.bottom = from_y + height;
-    to.top = y;
-    to.bottom = bottom_y;
-
-    ScrollWindowEx (hwnd, 0, to_y - from_y, &from, &to, dirty,
-		    NULL, SW_INVALIDATE);
-
-    /* Combine this with what we expect to be dirty. This covers the
-       case where not all of the region we expect is actually dirty.  */
-    CombineRgn (combined, dirty, expect_dirty, RGN_OR);
-
-    /* If the dirty region is not what we expected, redraw the entire frame.  */
-    if (!EqualRgn (combined, expect_dirty))
-      SET_FRAME_GARBAGED (f);
-
-    DeleteObject (dirty);
-    DeleteObject (combined);
-  }
-
+  hdc = get_frame_dc (f);
+  BitBlt (hdc, x, to_y, width, height, hdc, x, from_y, SRCCOPY);
+  release_frame_dc (f, hdc);
   unblock_input ();
-  DeleteObject (expect_dirty);
 }
 
 
@@ -4928,6 +4956,8 @@ w32_read_socket (struct terminal *terminal,
       /*            w32_name_of_message (msg.msg.message), */
       /*            msg.msg.time)); */
 
+      f = NULL;
+
       EVENT_INIT (inev);
       inev.kind = NO_EVENT;
       inev.arg = Qnil;
@@ -5659,6 +5689,8 @@ w32_read_socket (struct terminal *terminal,
 		  if (width != FRAME_PIXEL_WIDTH (f)
 		      || height != FRAME_PIXEL_HEIGHT (f))
 		    {
+		      w32_release_paint_buffer (f);
+
 		      change_frame_size
 			(f, width, height, false, true, false);
 		      SET_FRAME_GARBAGED (f);
@@ -5840,6 +5872,14 @@ w32_read_socket (struct terminal *terminal,
 	    }
 	  count++;
 	}
+
+      /* Event processing might have drawn to F outside redisplay.  If
+         that is the case, flush any changes that have been made to
+         the front buffer.  */
+
+      if (f && FRAME_OUTPUT_DATA (f)->paint_buffer_dirty
+	  && !f->garbaged)
+	w32_show_back_buffer (f);
     }
 
   /* If the focus was just given to an autoraising frame,
@@ -7054,6 +7094,9 @@ w32_free_frame_resources (struct frame *f)
      face.  */
   free_frame_faces (f);
 
+  /* Now release the back buffer if any exists.  */
+  w32_release_paint_buffer (f);
+
   if (FRAME_W32_WINDOW (f))
     my_destroy_window (f, FRAME_W32_WINDOW (f));
 
@@ -7350,6 +7393,7 @@ w32_create_terminal (struct w32_display_info *dpyinfo)
   terminal->update_end_hook = w32_update_end;
   terminal->read_socket_hook = w32_read_socket;
   terminal->frame_up_to_date_hook = w32_frame_up_to_date;
+  terminal->buffer_flipping_unblocked_hook = w32_buffer_flipping_unblocked_hook;
   terminal->defined_color_hook = w32_defined_color;
   terminal->query_frame_background_color = w32_query_frame_background_color;
   terminal->query_colors = w32_query_colors;
@@ -7505,6 +7549,7 @@ w32_delete_display (struct w32_display_info *dpyinfo)
     if (dpyinfo->palette)
       DeleteObject (dpyinfo->palette);
   }
+
   w32_reset_fringes ();
 }
 
diff --git a/src/w32term.h b/src/w32term.h
index 6c48323651..7df270eb5d 100644
--- a/src/w32term.h
+++ b/src/w32term.h
@@ -412,6 +412,24 @@ #define PIX_TYPE COLORREF
      geometry when 'fullscreen' is reset to nil.  */
   WINDOWPLACEMENT normal_placement;
   int prev_fsmode;
+
+  /* The back buffer if there is an ongoing double-buffered drawing
+     operation.  */
+  HBITMAP paint_buffer;
+
+  /* The handle of the back buffer and a DC that ought to be released
+     alongside the back buffer.  */
+  HDC paint_dc, paint_buffer_handle;
+
+  /* The object previously selected into `paint_dc'.  */
+  HGDIOBJ paint_dc_object;
+
+  /* The width and height of `paint_buffer'.  */
+  int paint_buffer_width, paint_buffer_height;
+
+  /* Whether or not some painting was done to this window that has not
+     yet been drawn.  */
+  unsigned paint_buffer_dirty : 1;
 };
 
 extern struct w32_output w32term_display;
@@ -876,6 +894,7 @@ #define GUI_SDATA(x) ((guichar_t*) SDATA (x))
 
 extern Lisp_Object w32_popup_dialog (struct frame *, Lisp_Object, Lisp_Object);
 extern void w32_arrow_cursor (void);
+extern void w32_release_paint_buffer (struct frame *);
 
 extern void syms_of_w32term (void);
 extern void syms_of_w32menu (void);
diff --git a/src/w32xfns.c b/src/w32xfns.c
index d5974b906e..e08b3519d3 100644
--- a/src/w32xfns.c
+++ b/src/w32xfns.c
@@ -136,13 +136,13 @@ select_palette (struct frame *f, HDC hdc)
     f->output_data.w32->old_palette = NULL;
 
   if (RealizePalette (hdc) != GDI_ERROR)
-  {
-    Lisp_Object frame, framelist;
-    FOR_EACH_FRAME (framelist, frame)
     {
-      SET_FRAME_GARBAGED (XFRAME (frame));
+      Lisp_Object frame, framelist;
+      FOR_EACH_FRAME (framelist, frame)
+	{
+	  SET_FRAME_GARBAGED (XFRAME (frame));
+	}
     }
-  }
 }
 
 void
@@ -157,19 +157,65 @@ deselect_palette (struct frame *f, HDC hdc)
 HDC
 get_frame_dc (struct frame *f)
 {
-  HDC hdc;
+  HDC hdc, paint_dc;
+  HBITMAP back_buffer;
+  HGDIOBJ obj;
+  struct w32_output *output;
 
   if (f->output_method != output_w32)
     emacs_abort ();
 
   enter_crit ();
+  output = FRAME_OUTPUT_DATA (f);
+
+  if (output->paint_dc)
+    {
+      if (output->paint_buffer_width != FRAME_PIXEL_WIDTH (f)
+	  || output->paint_buffer_height != FRAME_PIXEL_HEIGHT (f))
+	w32_release_paint_buffer (f);
+      else
+	{
+	  output->paint_buffer_dirty = 1;
+	  return output->paint_dc;
+	}
+    }
 
-  hdc = GetDC (f->output_data.w32->window_desc);
+  hdc = GetDC (output->window_desc);
 
   /* If this gets called during startup before the frame is valid,
      there is a chance of corrupting random data or crashing. */
   if (hdc)
-    select_palette (f, hdc);
+    {
+      select_palette (f, hdc);
+
+      back_buffer
+	= CreateCompatibleBitmap (hdc, FRAME_PIXEL_WIDTH (f),
+				  FRAME_PIXEL_HEIGHT (f));
+
+      if (back_buffer)
+	{
+	  paint_dc = CreateCompatibleDC (hdc);
+
+	  if (!paint_dc)
+	    DeleteObject (back_buffer);
+	  else
+	    {
+	      obj = SelectObject (paint_dc, back_buffer);
+
+	      output->paint_dc_object = obj;
+	      output->paint_dc = paint_dc;
+	      output->paint_buffer_handle = hdc;
+	      output->paint_buffer = back_buffer;
+	      output->paint_buffer_width = FRAME_PIXEL_WIDTH (f);
+	      output->paint_buffer_height = FRAME_PIXEL_HEIGHT (f);
+	      output->paint_buffer_dirty = 1;
+
+	      SET_FRAME_GARBAGED (f);
+
+	      return paint_dc;
+	    }
+	}
+    }
 
   return hdc;
 }
@@ -179,8 +225,15 @@ release_frame_dc (struct frame *f, HDC hdc)
 {
   int ret;
 
-  deselect_palette (f, hdc);
-  ret = ReleaseDC (f->output_data.w32->window_desc, hdc);
+  /* Avoid releasing the double-buffered DC here, since it'll be
+     released upon the next buffer flip instead.  */
+  if (hdc != FRAME_OUTPUT_DATA (f)->paint_dc)
+    {
+      deselect_palette (f, hdc);
+      ret = ReleaseDC (f->output_data.w32->window_desc, hdc);
+    }
+  else
+    ret = 0;
 
   leave_crit ();
 

Re: MS Windows double buffering

[Po Lu]

Po Lu <luangruo@yahoo.com> writes:

> Right... since the best way to get something done fast is to do it
> yourself, I tried my best to fix all the issues mentioned above.  It
> should work on old versions of MS Windows now as well.  If this works
> for you, I'll install this after implementing the
> `inhibit-double-buffering' frame parameter.  Thanks.

Here's the patch with that implemented as well:

From ee8cc91559c04a4f21a1ad14ff5461453638145e Mon Sep 17 00:00:00 2001
From: Po Lu <luangruo@yahoo.com>
Date: Fri, 29 Apr 2022 11:33:41 +0800
Subject: [PATCH] Implement double buffering on MS Windows

* etc/NEWS: Announce changes.
* src/w32fns.c (w32_set_inhibit_double_buffering): New function.
(w32_wnd_proc):
(Fx_create_frame):
(w32_create_tip_frame): Set `inhibit-double-buffering' parameter.
(w32_frame_parm_handlers): Add new handler.

* src/w32term.c (w32_show_back_buffer):
(w32_release_paint_buffer): New functions.
(w32_frame_up_to_date): Show back buffer if applicable.
(w32_buffer_flipping_unblocked_hook): New hook.
(w32_scroll_run): Use BitBlt to scroll instead of window
scrolling functions.
(w32_scroll_bar_clear): Don't clear scroll bars when double
buffered.
(w32_read_socket): Flip buffers after reading input events in
some cases.
(w32_free_frame_resources): Free back buffer.
(w32_create_terminal): Add new hook.

* src/w32term.h (struct w32_output): New fields for handling
back buffers.
* src/w32xfns.c (select_palette): Fix indentation.
(get_frame_dc, release_frame_dc): Return back buffer when
appropriate and set dirty flag.
---
 etc/NEWS      |   7 +++
 src/w32fns.c  |  33 +++++++++-
 src/w32term.c | 167 ++++++++++++++++++++++++++++++++++----------------
 src/w32term.h |  22 +++++++
 src/w32xfns.c |  76 ++++++++++++++++++++---
 5 files changed, 240 insertions(+), 65 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 70087f2629..fe8697e28f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -2134,6 +2134,13 @@ to preserve the old behavior, apply
 
 ** MS-Windows
 
+---
+*** Emacs now supports double buffering on MS Windows to reduce flicker.
+This leads to a noticable reduction in the amount of graphics flicker
+during redisplay on many systems, but can also make painting slower.
+If that happens, it can be disabled by setting the
+'inhibit-double-buffering' frame parameter.
+
 +++
 *** Emacs now supports system dark mode.
 On Windows 10 (version 1809 and higher) and Windows 11, Emacs will now
diff --git a/src/w32fns.c b/src/w32fns.c
index a880136d0a..d4e4b2a30b 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -1802,6 +1802,25 @@ w32_set_tool_bar_lines (struct frame *f, Lisp_Object value, Lisp_Object oldval)
   w32_change_tool_bar_height (f, nlines * FRAME_LINE_HEIGHT (f));
 }
 
+static void
+w32_set_inhibit_double_buffering (struct frame *f,
+				  Lisp_Object new_value,
+				  Lisp_Object old_value)
+{
+  block_input ();
+
+  if (NILP (new_value))
+    FRAME_OUTPUT_DATA (f)->want_paint_buffer = 1;
+  else
+    {
+      FRAME_OUTPUT_DATA (f)->want_paint_buffer = 0;
+      w32_release_paint_buffer (f);
+
+      SET_FRAME_GARBAGED (f);
+    }
+
+  unblock_input ();
+}
 
 /* Set the pixel height of the tool bar of frame F to HEIGHT.  */
 void
@@ -4093,7 +4112,9 @@ w32_wnd_proc (HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
     {
     case WM_ERASEBKGND:
       f = w32_window_to_frame (dpyinfo, hwnd);
-      if (f)
+
+      enter_crit ();
+      if (f && !FRAME_OUTPUT_DATA (f)->paint_buffer)
 	{
 	  HDC hdc = get_frame_dc (f);
 	  GetUpdateRect (hwnd, &wmsg.rect, FALSE);
@@ -4107,6 +4128,7 @@ w32_wnd_proc (HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
 		     wmsg.rect.right, wmsg.rect.bottom));
 #endif /* W32_DEBUG_DISPLAY */
 	}
+      leave_crit ();
       return 1;
     case WM_PALETTECHANGED:
       /* ignore our own changes */
@@ -6080,6 +6102,10 @@ DEFUN ("x-create-frame", Fx_create_frame, Sx_create_frame,
                          ? make_fixnum (0) : make_fixnum (1),
                          NULL, NULL, RES_TYPE_NUMBER);
 
+  gui_default_parameter (f, parameters, Qinhibit_double_buffering, Qnil,
+                         "inhibitDoubleBuffering", "InhibitDoubleBuffering",
+                         RES_TYPE_BOOLEAN);
+
   gui_default_parameter (f, parameters, Qbuffer_predicate, Qnil,
                          "bufferPredicate", "BufferPredicate", RES_TYPE_SYMBOL);
   gui_default_parameter (f, parameters, Qtitle, Qnil,
@@ -7096,6 +7122,9 @@ w32_create_tip_frame (struct w32_display_info *dpyinfo, Lisp_Object parms)
                          "alpha", "Alpha", RES_TYPE_NUMBER);
   gui_default_parameter (f, parms, Qalpha_background, Qnil,
                          "alphaBackground", "AlphaBackground", RES_TYPE_NUMBER);
+  gui_default_parameter (f, parms, Qinhibit_double_buffering, Qnil,
+                         "inhibitDoubleBuffering", "InhibitDoubleBuffering",
+                         RES_TYPE_BOOLEAN);
 
   /* Add `tooltip' frame parameter's default value. */
   if (NILP (Fframe_parameter (frame, Qtooltip)))
@@ -10432,7 +10461,7 @@ w32_get_resource (const char *key, const char *name, LPDWORD lpdwtype)
   gui_set_alpha,
   0, /* x_set_sticky */
   0, /* x_set_tool_bar_position */
-  0, /* x_set_inhibit_double_buffering */
+  w32_set_inhibit_double_buffering,
   w32_set_undecorated,
   w32_set_parent_frame,
   w32_set_skip_taskbar,
diff --git a/src/w32term.c b/src/w32term.c
index 7837032304..ca96320a5e 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -275,6 +275,57 @@ XGetGCValues (void *ignore, XGCValues *gc,
 }
 #endif
 
+static void
+w32_show_back_buffer (struct frame *f)
+{
+  struct w32_output *output;
+  HDC raw_dc;
+
+  output = FRAME_OUTPUT_DATA (f);
+
+  enter_crit ();
+
+  if (output->paint_buffer)
+    {
+      raw_dc = GetDC (output->window_desc);
+
+      if (!raw_dc)
+	emacs_abort ();
+
+      BitBlt (raw_dc, 0, 0, FRAME_PIXEL_WIDTH (f),
+	      FRAME_PIXEL_HEIGHT (f),
+	      output->paint_dc, 0, 0, SRCCOPY);
+      ReleaseDC (output->window_desc, raw_dc);
+
+      output->paint_buffer_dirty = 0;
+    }
+
+  leave_crit ();
+}
+
+void
+w32_release_paint_buffer (struct frame *f)
+{
+  /* Delete the back buffer so it gets created
+     again the next time we ask for the DC.  */
+
+  enter_crit ();
+  if (FRAME_OUTPUT_DATA (f)->paint_buffer)
+    {
+      SelectObject (FRAME_OUTPUT_DATA (f)->paint_dc,
+		    FRAME_OUTPUT_DATA (f)->paint_dc_object);
+      ReleaseDC (FRAME_OUTPUT_DATA (f)->window_desc,
+		 FRAME_OUTPUT_DATA (f)->paint_buffer_handle);
+      DeleteDC (FRAME_OUTPUT_DATA (f)->paint_dc);
+      DeleteObject (FRAME_OUTPUT_DATA (f)->paint_buffer);
+
+      FRAME_OUTPUT_DATA (f)->paint_buffer = NULL;
+      FRAME_OUTPUT_DATA (f)->paint_dc = NULL;
+      FRAME_OUTPUT_DATA (f)->paint_buffer_handle = NULL;
+    }
+  leave_crit ();
+}
+
 static void
 w32_get_mouse_wheel_vertical_delta (void)
 {
@@ -704,10 +755,19 @@ w32_update_end (struct frame *f)
 static void
 w32_frame_up_to_date (struct frame *f)
 {
-  if (FRAME_W32_P (f))
-    FRAME_MOUSE_UPDATE (f);
+  FRAME_MOUSE_UPDATE (f);
+
+  if (!buffer_flipping_blocked_p ()
+      && FRAME_OUTPUT_DATA (f)->paint_buffer_dirty)
+    w32_show_back_buffer (f);
 }
 
+static void
+w32_buffer_flipping_unblocked_hook (struct frame *f)
+{
+  if (FRAME_OUTPUT_DATA (f)->paint_buffer_dirty)
+    w32_show_back_buffer (f);
+}
 
 /* Draw truncation mark bitmaps, continuation mark bitmaps, overlay
    arrow bitmaps, or clear the fringes if no bitmaps are required
@@ -2872,8 +2932,7 @@ w32_scroll_run (struct window *w, struct run *run)
 {
   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;
+  HDC hdc;
 
   /* Get frame-relative bounding box of the text display area of W,
      without mode lines.  Include in this box the left and right
@@ -2892,7 +2951,6 @@ w32_scroll_run (struct window *w, struct run *run)
 	height = bottom_y - from_y;
       else
 	height = run->height;
-      expect_dirty = CreateRectRgn (x, y + height, x + width, bottom_y);
     }
   else
     {
@@ -2902,44 +2960,15 @@ w32_scroll_run (struct window *w, struct run *run)
 	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;
-    HRGN dirty = CreateRectRgn (0, 0, 0, 0);
-    HRGN combined = CreateRectRgn (0, 0, 0, 0);
-
-    from.left = to.left = x;
-    from.right = to.right = x + width;
-    from.top = from_y;
-    from.bottom = from_y + height;
-    to.top = y;
-    to.bottom = bottom_y;
-
-    ScrollWindowEx (hwnd, 0, to_y - from_y, &from, &to, dirty,
-		    NULL, SW_INVALIDATE);
-
-    /* Combine this with what we expect to be dirty. This covers the
-       case where not all of the region we expect is actually dirty.  */
-    CombineRgn (combined, dirty, expect_dirty, RGN_OR);
-
-    /* If the dirty region is not what we expected, redraw the entire frame.  */
-    if (!EqualRgn (combined, expect_dirty))
-      SET_FRAME_GARBAGED (f);
-
-    DeleteObject (dirty);
-    DeleteObject (combined);
-  }
-
+  hdc = get_frame_dc (f);
+  BitBlt (hdc, x, to_y, width, height, hdc, x, from_y, SRCCOPY);
+  release_frame_dc (f, hdc);
   unblock_input ();
-  DeleteObject (expect_dirty);
 }
 
 
@@ -4809,6 +4838,9 @@ w32_scroll_bar_clear (struct frame *f)
 {
   Lisp_Object bar;
 
+  if (FRAME_OUTPUT_DATA (f)->paint_buffer)
+    return;
+
   /* We can have scroll bars even if this is 0,
      if we just turned off scroll bar mode.
      But in that case we should not clear them.  */
@@ -4928,6 +4960,8 @@ w32_read_socket (struct terminal *terminal,
       /*            w32_name_of_message (msg.msg.message), */
       /*            msg.msg.time)); */
 
+      f = NULL;
+
       EVENT_INIT (inev);
       inev.kind = NO_EVENT;
       inev.arg = Qnil;
@@ -4969,24 +5003,33 @@ w32_read_socket (struct terminal *terminal,
 		}
 	      else
 		{
-		  /* Erase background again for safety.  But don't do
-		     that if the frame's 'garbaged' flag is set, since
-		     in that case expose_frame will do nothing, and if
-		     the various redisplay flags happen to be unset,
-		     we are left with a blank frame.  */
-		  if (!FRAME_GARBAGED_P (f) || FRAME_PARENT_FRAME (f))
+		  enter_crit ();
+		  if (!FRAME_OUTPUT_DATA (f)->paint_buffer)
 		    {
-		      HDC hdc = get_frame_dc (f);
-
-		      w32_clear_rect (f, hdc, &msg.rect);
-		      release_frame_dc (f, hdc);
+		      /* Erase background again for safety.  But don't do
+			 that if the frame's 'garbaged' flag is set, since
+			 in that case expose_frame will do nothing, and if
+			 the various redisplay flags happen to be unset,
+			 we are left with a blank frame.  */
+
+		      if (!FRAME_GARBAGED_P (f) || FRAME_PARENT_FRAME (f))
+			{
+			  HDC hdc = get_frame_dc (f);
+
+			  w32_clear_rect (f, hdc, &msg.rect);
+			  release_frame_dc (f, hdc);
+			}
+
+		      expose_frame (f,
+				    msg.rect.left,
+				    msg.rect.top,
+				    msg.rect.right - msg.rect.left,
+				    msg.rect.bottom - msg.rect.top);
+		      w32_clear_under_internal_border (f);
 		    }
-		  expose_frame (f,
-				msg.rect.left,
-				msg.rect.top,
-				msg.rect.right - msg.rect.left,
-				msg.rect.bottom - msg.rect.top);
-		  w32_clear_under_internal_border (f);
+		  else
+		    w32_show_back_buffer (f);
+		  leave_crit ();
 		}
 	    }
 	  break;
@@ -5659,6 +5702,8 @@ w32_read_socket (struct terminal *terminal,
 		  if (width != FRAME_PIXEL_WIDTH (f)
 		      || height != FRAME_PIXEL_HEIGHT (f))
 		    {
+		      w32_release_paint_buffer (f);
+
 		      change_frame_size
 			(f, width, height, false, true, false);
 		      SET_FRAME_GARBAGED (f);
@@ -5840,6 +5885,17 @@ w32_read_socket (struct terminal *terminal,
 	    }
 	  count++;
 	}
+
+      /* Event processing might have drawn to F outside redisplay.  If
+         that is the case, flush any changes that have been made to
+         the front buffer.  */
+
+      if (f && FRAME_OUTPUT_DATA (f)->paint_buffer_dirty
+	  /* WM_WINDOWPOSCHANGED makes the buffer dirty, but there's
+	     no reason to flush it here, and that also causes
+	     flicker.  */
+	  && !f->garbaged && msg.msg.message != WM_WINDOWPOSCHANGED)
+	w32_show_back_buffer (f);
     }
 
   /* If the focus was just given to an autoraising frame,
@@ -7054,6 +7110,9 @@ w32_free_frame_resources (struct frame *f)
      face.  */
   free_frame_faces (f);
 
+  /* Now release the back buffer if any exists.  */
+  w32_release_paint_buffer (f);
+
   if (FRAME_W32_WINDOW (f))
     my_destroy_window (f, FRAME_W32_WINDOW (f));
 
@@ -7350,6 +7409,7 @@ w32_create_terminal (struct w32_display_info *dpyinfo)
   terminal->update_end_hook = w32_update_end;
   terminal->read_socket_hook = w32_read_socket;
   terminal->frame_up_to_date_hook = w32_frame_up_to_date;
+  terminal->buffer_flipping_unblocked_hook = w32_buffer_flipping_unblocked_hook;
   terminal->defined_color_hook = w32_defined_color;
   terminal->query_frame_background_color = w32_query_frame_background_color;
   terminal->query_colors = w32_query_colors;
@@ -7505,6 +7565,7 @@ w32_delete_display (struct w32_display_info *dpyinfo)
     if (dpyinfo->palette)
       DeleteObject (dpyinfo->palette);
   }
+
   w32_reset_fringes ();
 }
 
diff --git a/src/w32term.h b/src/w32term.h
index 6c48323651..2dcc43fc59 100644
--- a/src/w32term.h
+++ b/src/w32term.h
@@ -412,6 +412,27 @@ #define PIX_TYPE COLORREF
      geometry when 'fullscreen' is reset to nil.  */
   WINDOWPLACEMENT normal_placement;
   int prev_fsmode;
+
+  /* The back buffer if there is an ongoing double-buffered drawing
+     operation.  */
+  HBITMAP paint_buffer;
+
+  /* The handle of the back buffer and a DC that ought to be released
+     alongside the back buffer.  */
+  HDC paint_dc, paint_buffer_handle;
+
+  /* The object previously selected into `paint_dc'.  */
+  HGDIOBJ paint_dc_object;
+
+  /* The width and height of `paint_buffer'.  */
+  int paint_buffer_width, paint_buffer_height;
+
+  /* Whether or not some painting was done to this window that has not
+     yet been drawn.  */
+  unsigned paint_buffer_dirty : 1;
+
+  /* Whether or not this frame should be double buffered.  */
+  unsigned want_paint_buffer : 1;
 };
 
 extern struct w32_output w32term_display;
@@ -876,6 +897,7 @@ #define GUI_SDATA(x) ((guichar_t*) SDATA (x))
 
 extern Lisp_Object w32_popup_dialog (struct frame *, Lisp_Object, Lisp_Object);
 extern void w32_arrow_cursor (void);
+extern void w32_release_paint_buffer (struct frame *);
 
 extern void syms_of_w32term (void);
 extern void syms_of_w32menu (void);
diff --git a/src/w32xfns.c b/src/w32xfns.c
index d5974b906e..139985c5bd 100644
--- a/src/w32xfns.c
+++ b/src/w32xfns.c
@@ -136,13 +136,13 @@ select_palette (struct frame *f, HDC hdc)
     f->output_data.w32->old_palette = NULL;
 
   if (RealizePalette (hdc) != GDI_ERROR)
-  {
-    Lisp_Object frame, framelist;
-    FOR_EACH_FRAME (framelist, frame)
     {
-      SET_FRAME_GARBAGED (XFRAME (frame));
+      Lisp_Object frame, framelist;
+      FOR_EACH_FRAME (framelist, frame)
+	{
+	  SET_FRAME_GARBAGED (XFRAME (frame));
+	}
     }
-  }
 }
 
 void
@@ -157,19 +157,68 @@ deselect_palette (struct frame *f, HDC hdc)
 HDC
 get_frame_dc (struct frame *f)
 {
-  HDC hdc;
+  HDC hdc, paint_dc;
+  HBITMAP back_buffer;
+  HGDIOBJ obj;
+  struct w32_output *output;
 
   if (f->output_method != output_w32)
     emacs_abort ();
 
   enter_crit ();
+  output = FRAME_OUTPUT_DATA (f);
+
+  if (output->paint_dc)
+    {
+      if (output->paint_buffer_width != FRAME_PIXEL_WIDTH (f)
+	  || output->paint_buffer_height != FRAME_PIXEL_HEIGHT (f))
+	w32_release_paint_buffer (f);
+      else
+	{
+	  output->paint_buffer_dirty = 1;
+	  return output->paint_dc;
+	}
+    }
 
-  hdc = GetDC (f->output_data.w32->window_desc);
+  hdc = GetDC (output->window_desc);
 
   /* If this gets called during startup before the frame is valid,
      there is a chance of corrupting random data or crashing. */
   if (hdc)
-    select_palette (f, hdc);
+    {
+      select_palette (f, hdc);
+
+      if (FRAME_OUTPUT_DATA (f)->want_paint_buffer)
+	{
+	  back_buffer
+	    = CreateCompatibleBitmap (hdc, FRAME_PIXEL_WIDTH (f),
+				      FRAME_PIXEL_HEIGHT (f));
+
+	  if (back_buffer)
+	    {
+	      paint_dc = CreateCompatibleDC (hdc);
+
+	      if (!paint_dc)
+		DeleteObject (back_buffer);
+	      else
+		{
+		  obj = SelectObject (paint_dc, back_buffer);
+
+		  output->paint_dc_object = obj;
+		  output->paint_dc = paint_dc;
+		  output->paint_buffer_handle = hdc;
+		  output->paint_buffer = back_buffer;
+		  output->paint_buffer_width = FRAME_PIXEL_WIDTH (f);
+		  output->paint_buffer_height = FRAME_PIXEL_HEIGHT (f);
+		  output->paint_buffer_dirty = 1;
+
+		  SET_FRAME_GARBAGED (f);
+
+		  return paint_dc;
+		}
+	    }
+	}
+    }
 
   return hdc;
 }
@@ -179,8 +228,15 @@ release_frame_dc (struct frame *f, HDC hdc)
 {
   int ret;
 
-  deselect_palette (f, hdc);
-  ret = ReleaseDC (f->output_data.w32->window_desc, hdc);
+  /* Avoid releasing the double-buffered DC here, since it'll be
+     released upon the next buffer flip instead.  */
+  if (hdc != FRAME_OUTPUT_DATA (f)->paint_dc)
+    {
+      deselect_palette (f, hdc);
+      ret = ReleaseDC (f->output_data.w32->window_desc, hdc);
+    }
+  else
+    ret = 0;
 
   leave_crit ();
 
-- 
2.34.1

Re: MS Windows double buffering

[Po Lu]

Po Lu <luangruo@yahoo.com> writes:

> From ee8cc91559c04a4f21a1ad14ff5461453638145e Mon Sep 17 00:00:00 2001
> From: Po Lu <luangruo@yahoo.com>
> Date: Fri, 29 Apr 2022 11:33:41 +0800
> Subject: [PATCH] Implement double buffering on MS Windows
>
> * etc/NEWS: Announce changes.
> * src/w32fns.c (w32_set_inhibit_double_buffering): New function.
> (w32_wnd_proc):
> (Fx_create_frame):
> (w32_create_tip_frame): Set `inhibit-double-buffering' parameter.
> (w32_frame_parm_handlers): Add new handler.
>
> * src/w32term.c (w32_show_back_buffer):
> (w32_release_paint_buffer): New functions.
> (w32_frame_up_to_date): Show back buffer if applicable.
> (w32_buffer_flipping_unblocked_hook): New hook.
> (w32_scroll_run): Use BitBlt to scroll instead of window
> scrolling functions.
> (w32_scroll_bar_clear): Don't clear scroll bars when double
> buffered.
> (w32_read_socket): Flip buffers after reading input events in
> some cases.
> (w32_free_frame_resources): Free back buffer.
> (w32_create_terminal): Add new hook.
>
> * src/w32term.h (struct w32_output): New fields for handling
> back buffers.
> * src/w32xfns.c (select_palette): Fix indentation.
> (get_frame_dc, release_frame_dc): Return back buffer when
> appropriate and set dirty flag.

I fixed a few minor problems with this, synchronized parts of the tool
bar code with X to fix a bug, and installed the resulting changes, since
they seems to be working fine for me and eliminates all of the dreadful
flickering that was previously present on MS Windows.

Re: MS Windows double buffering

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Sat, 30 Apr 2022 13:41:25 +0800
> 
> I fixed a few minor problems with this, synchronized parts of the tool
> bar code with X to fix a bug, and installed the resulting changes, since
> they seems to be working fine for me and eliminates all of the dreadful
> flickering that was previously present on MS Windows.

Thanks!

However, I don't understand why you went ahead without waiting for the
review of the patches you posted.  What's the rush?  The result can be
only one: the changeset is spread on a larger set of commits, and
makes it harder to understand what changed and why, when later these
questions are asked.

Please don't act as if we are under some time pressure when we really
aren't.

My review is below.

> +static void
> +w32_set_inhibit_double_buffering (struct frame *f,
> +				  Lisp_Object new_value,
> +				  Lisp_Object old_value)

The old_value argument is unused, which should at least be mentioned,
if not marked with an explicit "unused" attribute.

Also, this function is a frame redisplay-interface method, so it
should have a comment describing its functionality.

>  static void
> +w32_show_back_buffer (struct frame *f)
> +{
> +  struct w32_output *output;
> +  HDC raw_dc;
> +
> +  output = FRAME_OUTPUT_DATA (f);
> +
> +  enter_crit ();
> +
> +  if (output->paint_buffer)
> +    {
> +      raw_dc = GetDC (output->window_desc);
> +
> +      if (!raw_dc)
> +	emacs_abort ();
> +
> +      BitBlt (raw_dc, 0, 0, FRAME_PIXEL_WIDTH (f),
> +	      FRAME_PIXEL_HEIGHT (f),
> +	      output->paint_dc, 0, 0, SRCCOPY);
> +      ReleaseDC (output->window_desc, raw_dc);
> +
> +      output->paint_buffer_dirty = 0;
> +    }
> +
> +  leave_crit ();
> +}
> +
> +void
> +w32_release_paint_buffer (struct frame *f)
> +{
> +  /* Delete the back buffer so it gets created
> +     again the next time we ask for the DC.  */
> +
> +  enter_crit ();
> +  if (FRAME_OUTPUT_DATA (f)->paint_buffer)
> +    {
> +      SelectObject (FRAME_OUTPUT_DATA (f)->paint_dc,
> +		    FRAME_OUTPUT_DATA (f)->paint_dc_object);
> +      ReleaseDC (FRAME_OUTPUT_DATA (f)->window_desc,
> +		 FRAME_OUTPUT_DATA (f)->paint_buffer_handle);
> +      DeleteDC (FRAME_OUTPUT_DATA (f)->paint_dc);
> +      DeleteObject (FRAME_OUTPUT_DATA (f)->paint_buffer);
> +
> +      FRAME_OUTPUT_DATA (f)->paint_buffer = NULL;
> +      FRAME_OUTPUT_DATA (f)->paint_dc = NULL;
> +      FRAME_OUTPUT_DATA (f)->paint_buffer_handle = NULL;
> +    }
> +  leave_crit ();
> +}

These two functions should do nothing and return immediately if
double-buffering is disabled.  They currently will try to enter the
critical section before returning, which is an unnecessary slowdown.
At least w32_release_paint_buffer is called also when double-buffering
is disabled, AFAIU, so it should return immediately in that case.

Alternatively, make sure that these functions are called only if
double-buffering is in effect, but in that case the additional tests
inside the functions are redundant.

> @@ -4093,7 +4112,9 @@ w32_wnd_proc (HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
>      {
>      case WM_ERASEBKGND:
>        f = w32_window_to_frame (dpyinfo, hwnd);
> -      if (f)
> +
> +      enter_crit ();
> +      if (f && !FRAME_OUTPUT_DATA (f)->paint_buffer)
>  	{
>  	  HDC hdc = get_frame_dc (f);
>  	  GetUpdateRect (hwnd, &wmsg.rect, FALSE);
> @@ -4107,6 +4128,7 @@ w32_wnd_proc (HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
>  		     wmsg.rect.right, wmsg.rect.bottom));
>  #endif /* W32_DEBUG_DISPLAY */
>  	}
> +      leave_crit ();
>        return 1;
>      case WM_PALETTECHANGED:
>        /* ignore our own changes */

Here and elsewhere in the modified code, I'd like to have all the
changes that affect the no-double-buffering code to be conditioned on
some variable exposed to Lisp.  That way, if and when someone reports
a problem that could be related to this feature, we could easily get
back to exactly the old code as it was before the changeset, and see
if the problem is indeed due to this change.  For example, entering
the critical section above is one such change.

We could then remove that variable in some future Emacs version, when
we are sure the new code is reasonably bug-free.

>  /* Draw truncation mark bitmaps, continuation mark bitmaps, overlay
>     arrow bitmaps, or clear the fringes if no bitmaps are required
> @@ -2872,8 +2932,7 @@ w32_scroll_run (struct window *w, struct run *run)
>  {
>    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;
> +  HDC hdc;

This function, which is an important part of redisplay, was basically
rewritten from scratch.  As explained above, please add back the
deleted old code, conditioned under the variable I described there, so
that we could test whether the new code is responsible for some
problem that users may report.

> @@ -4809,6 +4838,9 @@ w32_scroll_bar_clear (struct frame *f)
>  {
>    Lisp_Object bar;
>  
> +  if (FRAME_OUTPUT_DATA (f)->paint_buffer)
> +    return;

There should be a comment here explaining why we return in that case.

> @@ -4969,24 +5003,33 @@ w32_read_socket (struct terminal *terminal,
>  		}
>  	      else
>  		{
> -		  /* Erase background again for safety.  But don't do
> -		     that if the frame's 'garbaged' flag is set, since
> -		     in that case expose_frame will do nothing, and if
> -		     the various redisplay flags happen to be unset,
> -		     we are left with a blank frame.  */
> -		  if (!FRAME_GARBAGED_P (f) || FRAME_PARENT_FRAME (f))
> +		  enter_crit ();
> +		  if (!FRAME_OUTPUT_DATA (f)->paint_buffer)
>  		    {
> -		      HDC hdc = get_frame_dc (f);
> -
> -		      w32_clear_rect (f, hdc, &msg.rect);
> -		      release_frame_dc (f, hdc);
> +		      /* Erase background again for safety.  But don't do
> +			 that if the frame's 'garbaged' flag is set, since
> +			 in that case expose_frame will do nothing, and if
> +			 the various redisplay flags happen to be unset,
> +			 we are left with a blank frame.  */
> +
> +		      if (!FRAME_GARBAGED_P (f) || FRAME_PARENT_FRAME (f))
> +			{
> +			  HDC hdc = get_frame_dc (f);
> +
> +			  w32_clear_rect (f, hdc, &msg.rect);
> +			  release_frame_dc (f, hdc);
> +			}
> +
> +		      expose_frame (f,
> +				    msg.rect.left,
> +				    msg.rect.top,
> +				    msg.rect.right - msg.rect.left,
> +				    msg.rect.bottom - msg.rect.top);
> +		      w32_clear_under_internal_border (f);
>  		    }
> -		  expose_frame (f,
> -				msg.rect.left,
> -				msg.rect.top,
> -				msg.rect.right - msg.rect.left,
> -				msg.rect.bottom - msg.rect.top);
> -		  w32_clear_under_internal_border (f);
> +		  else
> +		    w32_show_back_buffer (f);
> +		  leave_crit ();
>  		}
>  	    }
>  	  break;

I don't understand why you enter the critical section here:
w32_show_back_buffer does that internally, and the old code didn't
need that.

> @@ -5840,6 +5885,17 @@ w32_read_socket (struct terminal *terminal,
>  	    }
>  	  count++;
>  	}
> +
> +      /* Event processing might have drawn to F outside redisplay.  If
> +         that is the case, flush any changes that have been made to
> +         the front buffer.  */
> +
> +      if (f && FRAME_OUTPUT_DATA (f)->paint_buffer_dirty
> +	  /* WM_WINDOWPOSCHANGED makes the buffer dirty, but there's
> +	     no reason to flush it here, and that also causes
> +	     flicker.  */
> +	  && !f->garbaged && msg.msg.message != WM_WINDOWPOSCHANGED)
> +	w32_show_back_buffer (f);

This is not clean, since it references a WM_ message in a place other
than where its main processing happens.  Please change it so
processing of WM_WINDOWPOSCHANGED raises some flag (which is otherwise
always false), and have this code here test that flag.

> --- a/src/w32xfns.c
> +++ b/src/w32xfns.c
> @@ -136,13 +136,13 @@ select_palette (struct frame *f, HDC hdc)
>      f->output_data.w32->old_palette = NULL;
>  
>    if (RealizePalette (hdc) != GDI_ERROR)
> -  {
> -    Lisp_Object frame, framelist;
> -    FOR_EACH_FRAME (framelist, frame)
>      {
> -      SET_FRAME_GARBAGED (XFRAME (frame));
> +      Lisp_Object frame, framelist;
> +      FOR_EACH_FRAME (framelist, frame)
> +	{
> +	  SET_FRAME_GARBAGED (XFRAME (frame));
> +	}
>      }
> -  }
>  }

This looks like whitespace change?  Or did I miss something?

And two more general issues:

 . w32term.c is also used in the Cygwin w32 build, (which produces a
   Cygwin Emacs that uses the native MS-Windows GUI functions instead
   of X).  Will this code work in that build?  If not, the new code
   should be ifdef'ed away on Cygwin.  Ken, can you please chime in
   and help us DTRT here?

 . how does one test this feature?  I rarely see any flickering on
   MS-Windows, so what should I try to see the effect of double
   buffering in action?

Thanks again for working on this.

Re: MS Windows double buffering

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> However, I don't understand why you went ahead without waiting for the
> review of the patches you posted.  What's the rush?  The result can be
> only one: the changeset is spread on a larger set of commits, and
> makes it harder to understand what changed and why, when later these
> questions are asked.
>
> Please don't act as if we are under some time pressure when we really
> aren't.

I will keep that in mind in the future, sorry.

> The old_value argument is unused, which should at least be mentioned,
> if not marked with an explicit "unused" attribute.
>
> Also, this function is a frame redisplay-interface method, so it
> should have a comment describing its functionality.

I will write one, thanks.

> These two functions should do nothing and return immediately if
> double-buffering is disabled.  They currently will try to enter the
> critical section before returning, which is an unnecessary slowdown.
> At least w32_release_paint_buffer is called also when double-buffering
> is disabled, AFAIU, so it should return immediately in that case.

w32_release_paint_buffer releases the back buffer bitmap when a back
buffer exists.  It is only called if double buffering is enabled, or
when double buffering is being turned off, or during frame resize (to
resize the back buffer bitmap.)  But w32_show_back_buffer should indeed
work that way, yes.

> Here and elsewhere in the modified code, I'd like to have all the
> changes that affect the no-double-buffering code to be conditioned on
> some variable exposed to Lisp.  That way, if and when someone reports
> a problem that could be related to this feature, we could easily get
> back to exactly the old code as it was before the changeset, and see
> if the problem is indeed due to this change.  For example, entering
> the critical section above is one such change.

Thanks.  That's not something we do on X, but I will add such a
variable.

> This function, which is an important part of redisplay, was basically
> rewritten from scratch.  As explained above, please add back the
> deleted old code, conditioned under the variable I described there, so
> that we could test whether the new code is responsible for some
> problem that users may report.

[...]

> There should be a comment here explaining why we return in that case.

Likewise, I will do that.

> I don't understand why you enter the critical section here:
> w32_show_back_buffer does that internally, and the old code didn't
> need that.

`paint_buffer' can only be accessed safely inside the critical section,
as long as get_frame_dc can be called from the message pump thread, but
maybe it isn't called there.

> This is not clean, since it references a WM_ message in a place other
> than where its main processing happens.  Please change it so
> processing of WM_WINDOWPOSCHANGED raises some flag (which is otherwise
> always false), and have this code here test that flag.

Will do, thanks.

> This looks like whitespace change?  Or did I miss something?

Sorry for that, it snuck in somehow.

> And two more general issues:
>
>  . w32term.c is also used in the Cygwin w32 build, (which produces a
>    Cygwin Emacs that uses the native MS-Windows GUI functions instead
>    of X).  Will this code work in that build?  If not, the new code
>    should be ifdef'ed away on Cygwin.  Ken, can you please chime in
>    and help us DTRT here?

No idea, though I don't think double buffering uses anything that might
not work on Cygwin.

>  . how does one test this feature?  I rarely see any flickering on
>    MS-Windows, so what should I try to see the effect of double
>    buffering in action?

Emacs should flicker much less (or not at all) upon any of the test
cases in the last thread(s) on flicker, or when you run this:

  (while t (redraw-display) (redisplay))

Thanks.

Re: MS Windows double buffering

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Ken Brown <kbrown@cornell.edu>,  emacs-devel@gnu.org
> Date: Sat, 30 Apr 2022 15:46:43 +0800
> 
> > Here and elsewhere in the modified code, I'd like to have all the
> > changes that affect the no-double-buffering code to be conditioned on
> > some variable exposed to Lisp.  That way, if and when someone reports
> > a problem that could be related to this feature, we could easily get
> > back to exactly the old code as it was before the changeset, and see
> > if the problem is indeed due to this change.  For example, entering
> > the critical section above is one such change.
> 
> Thanks.  That's not something we do on X, but I will add such a
> variable.

Emacs is being tested on X much more than on MS-Windows, and people
who use it on X tend to be more knowledgeable about the technical
issues than a typical user of Emacs on Windows.  Moreover, the level
of expertise we have on our team about Windows-specific GUI aspects is
significantly lower than in the X case.

So having such a debugging device for significant changes in display
code is beneficial.

> > I don't understand why you enter the critical section here:
> > w32_show_back_buffer does that internally, and the old code didn't
> > need that.
> 
> `paint_buffer' can only be accessed safely inside the critical section,
> as long as get_frame_dc can be called from the message pump thread, but
> maybe it isn't called there.

So you are saying this code didn't work correctly before?  Or am I
misunderstanding something?

> >  . w32term.c is also used in the Cygwin w32 build, (which produces a
> >    Cygwin Emacs that uses the native MS-Windows GUI functions instead
> >    of X).  Will this code work in that build?  If not, the new code
> >    should be ifdef'ed away on Cygwin.  Ken, can you please chime in
> >    and help us DTRT here?
> 
> No idea, though I don't think double buffering uses anything that might
> not work on Cygwin.

Let's hope Ken will help us here.

> >  . how does one test this feature?  I rarely see any flickering on
> >    MS-Windows, so what should I try to see the effect of double
> >    buffering in action?
> 
> Emacs should flicker much less (or not at all) upon any of the test
> cases in the last thread(s) on flicker, or when you run this:
> 
>   (while t (redraw-display) (redisplay))

This still flickers considerably here, albeit with a significantly
lower frequency.

What other test cases in the discussions would you recommend to try?

Re: MS Windows double buffering

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> Emacs is being tested on X much more than on MS-Windows, and people
> who use it on X tend to be more knowledgeable about the technical
> issues than a typical user of Emacs on Windows.  Moreover, the level
> of expertise we have on our team about Windows-specific GUI aspects is
> significantly lower than in the X case.
>
> So having such a debugging device for significant changes in display
> code is beneficial.

Thanks for clarifying.

> So you are saying this code didn't work correctly before?  Or am I
> misunderstanding something?

No, it was a misunderstanding on my part.  Sorry for the noise.

> This still flickers considerably here, albeit with a significantly
> lower frequency.

What if you comment out these lines in w32term.c?

      if (f && !w32_disable_double_buffering
	  && FRAME_OUTPUT_DATA (f)->paint_buffer_dirty
	  && !f->garbaged && ignore_dirty_back_buffer)
	w32_show_back_buffer (f);

> What other test cases in the discussions would you recommend to try?

  (run-hooks 'xref-after-jump-hook)

for example, which causes considerable flickering without double
buffering, but none at all when double buffering is enabled.

Re: MS Windows double buffering

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: kbrown@cornell.edu,  emacs-devel@gnu.org
> Date: Sat, 30 Apr 2022 17:55:32 +0800
> 
> > This still flickers considerably here, albeit with a significantly
> > lower frequency.
> 
> What if you comment out these lines in w32term.c?
> 
>       if (f && !w32_disable_double_buffering
> 	  && FRAME_OUTPUT_DATA (f)->paint_buffer_dirty
> 	  && !f->garbaged && ignore_dirty_back_buffer)
> 	w32_show_back_buffer (f);

Then the flicker of the window parts of the frame (the tool bar, the
text area, the scroll bar, and the mode line) don't flicker at all,
but the menu bar and the title bar still do, albeit very
insignificantly.  Overall, the result is much better.

> > What other test cases in the discussions would you recommend to try?
> 
>   (run-hooks 'xref-after-jump-hook)
> 
> for example, which causes considerable flickering without double
> buffering, but none at all when double buffering is enabled.

Just evaluating it in "emacs -Q"?  I see no flickering at all, with or
without double-buffering.

Re: MS Windows double buffering

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> Then the flicker of the window parts of the frame (the tool bar, the
> text area, the scroll bar, and the mode line) don't flicker at all,
> but the menu bar and the title bar still do, albeit very
> insignificantly.  Overall, the result is much better.

Thanks.  Can you try to find out what type of message is being processed
when the call to `w32_show_back_buffer' in that piece of code is made?

> Just evaluating it in "emacs -Q"?  I see no flickering at all, with or
> without double-buffering.

Without double-buffering, the entire display flickers for me, while
there is no flicker at all with double buffering.

I think there is something very system-specific about exactly which
situations different people see flickering under, like on X, where some
people could not see flicker at all, while for others Emacs was
basically unusable without double buffering.

Re: MS Windows double buffering

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: kbrown@cornell.edu,  emacs-devel@gnu.org
> Date: Sat, 30 Apr 2022 18:33:31 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Then the flicker of the window parts of the frame (the tool bar, the
> > text area, the scroll bar, and the mode line) don't flicker at all,
> > but the menu bar and the title bar still do, albeit very
> > insignificantly.  Overall, the result is much better.
> 
> Thanks.  Can you try to find out what type of message is being processed
> when the call to `w32_show_back_buffer' in that piece of code is made?

It's WM_WINDOWPOSCHANGED.

Re: MS Windows double buffering

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> It's WM_WINDOWPOSCHANGED.

Thanks, it should be fixed soon.

Re: MS Windows double buffering

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: kbrown@cornell.edu,  emacs-devel@gnu.org
> Date: Sat, 30 Apr 2022 19:01:44 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > It's WM_WINDOWPOSCHANGED.
> 
> Thanks, it should be fixed soon.

Assuming that you installed a fix, the problem seems to be gone now --
but only until I move the mouse.  As long as the mouse is moved, the
text area, the tool bar, the fringes, and the mode line all flicker,
albeit with lower frequency than they do when double-buffering is
disabled.

Re: MS Windows double buffering

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> Assuming that you installed a fix, the problem seems to be gone now

Yes, I did.

> but only until I move the mouse.  As long as the mouse is moved, the
> text area, the tool bar, the fringes, and the mode line all flicker,
> albeit with lower frequency than they do when double-buffering is
> disabled.

I suppose something is marking the frame as "dirty" when the mouse is
moved, even if the display wasn't actually updated.

I will look into this tomorrow, thanks.

Re: MS Windows double buffering

[Ken Brown]

On 4/30/2022 2:50 AM, Eli Zaretskii wrote:
>   . w32term.c is also used in the Cygwin w32 build, (which produces a
>     Cygwin Emacs that uses the native MS-Windows GUI functions instead
>     of X).  Will this code work in that build?  If not, the new code
>     should be ifdef'ed away on Cygwin.  Ken, can you please chime in
>     and help us DTRT here?

I just built the Cygwin w32 build from git commit 0ea0aa255 and evaluated

   (while t (redraw-display) (redisplay))

I saw some flicker in the menu bar but nowhere else.  I tried the same thing in 
a Cygwin w32 build of emacs-28.1, and there was severe flickering throughout the 
frame.  So there's a noticeable improvement.

I'll keep testing over the next few days and let you know if I see any problems.

Ken

Re: MS Windows double buffering

[Eli Zaretskii]

> Date: Sat, 30 Apr 2022 13:34:32 -0400
> Cc: emacs-devel@gnu.org
> From: Ken Brown <kbrown@cornell.edu>
> 
> On 4/30/2022 2:50 AM, Eli Zaretskii wrote:
> >   . w32term.c is also used in the Cygwin w32 build, (which produces a
> >     Cygwin Emacs that uses the native MS-Windows GUI functions instead
> >     of X).  Will this code work in that build?  If not, the new code
> >     should be ifdef'ed away on Cygwin.  Ken, can you please chime in
> >     and help us DTRT here?
> 
> I just built the Cygwin w32 build from git commit 0ea0aa255 and evaluated
> 
>    (while t (redraw-display) (redisplay))
> 
> I saw some flicker in the menu bar but nowhere else.  I tried the same thing in 
> a Cygwin w32 build of emacs-28.1, and there was severe flickering throughout the 
> frame.  So there's a noticeable improvement.
> 
> I'll keep testing over the next few days and let you know if I see any problems.

Great, thanks.  It means most if not all of the new code works for the
Cygwin w32 build as-is.

Re: MS Windows double buffering

[Po Lu]

Ken Brown <kbrown@cornell.edu> writes:

> I just built the Cygwin w32 build from git commit 0ea0aa255 and evaluated
>
>   (while t (redraw-display) (redisplay))
>
> I saw some flicker in the menu bar but nowhere else.  I tried the same
> thing in a Cygwin w32 build of emacs-28.1, and there was severe
> flickering throughout the frame.  So there's a noticeable improvement.
>
> I'll keep testing over the next few days and let you know if I see any problems.

Thanks.

Re: MS Windows double buffering

[Arash Esbati]

Ken Brown <kbrown@cornell.edu> writes:

> I just built the Cygwin w32 build from git commit 0ea0aa255 and evaluated
>
>   (while t (redraw-display) (redisplay))
>
> I saw some flicker in the menu bar but nowhere else.

I can confirm this with Emacs 8734d60b053 built with Msys2.  Also the
flickering reported here[1] are gone.

Many thanks for implementing this, Po Lu.

Best, Arash

Footnotes:
[1]  https://lists.gnu.org/archive/html/bug-gnu-emacs/2022-04/msg00465.html

Re: MS Windows double buffering

[Eli Zaretskii]

> From: Arash Esbati <arash@gnu.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  Po Lu <luangruo@yahoo.com>,
>   emacs-devel@gnu.org
> Date: Sun, 01 May 2022 18:00:01 +0200
> 
> Ken Brown <kbrown@cornell.edu> writes:
> 
> > I just built the Cygwin w32 build from git commit 0ea0aa255 and evaluated
> >
> >   (while t (redraw-display) (redisplay))
> >
> > I saw some flicker in the menu bar but nowhere else.
> 
> I can confirm this with Emacs 8734d60b053 built with Msys2.

What do you mean by "built with Msys2"? is that different from MinGW64
in any way?

Re: MS Windows double buffering

[Arash Esbati]

Eli Zaretskii <eliz@gnu.org> writes:

> What do you mean by "built with Msys2"? is that different from MinGW64
> in any way?

Sorry, I meant Msys2/MinGW64, the bundle.

-> echo $MSYSTEM
MINGW64

is what I use to build Emacs.

Best, Arash

Re: MS Windows double buffering

[Robert Pluim]

>>>>> On Sat, 30 Apr 2022 15:46:43 +0800, Po Lu <luangruo@yahoo.com> said:
    >> Here and elsewhere in the modified code, I'd like to have all the
    >> changes that affect the no-double-buffering code to be conditioned on
    >> some variable exposed to Lisp.  That way, if and when someone reports
    >> a problem that could be related to this feature, we could easily get
    >> back to exactly the old code as it was before the changeset, and see
    >> if the problem is indeed due to this change.  For example, entering
    >> the critical section above is one such change.

    Po> Thanks.  That's not something we do on X, but I will add such a
    Po> variable.

For double-buffering itʼs a frame parameter called
`inhibit-double-buffering', so you could just reuse that.

Robert
-- 

Re: MS Windows double buffering

[Po Lu]

Robert Pluim <rpluim@gmail.com> writes:

> For double-buffering itʼs a frame parameter called
> `inhibit-double-buffering', so you could just reuse that.

That's also implemented on MS Windows, but it doesn't affect the
modifications to the event loop code and w32_scroll_run.