Subject: Re: bug#37755: Logic in init_fringe_bitmap should be moved to backends (maybe rif->define_fringe_bitmap)

From: Carlos Pita <[email-masked]>
Date: Wed, 16 Oct 2019 17:18:16 -0300

* Improved commit message according to CONTRIBUTE rules.

* Made some minor changes to avoid conditional compilation warnings.

* I'm aware of some issues when rendering fringe bitmaps (nothing
fatal, though) and I'm still debugging these changes, but it would be
great if you could review them meanwhile.
From 7613e7908f9918726aa4aefce13d10f0e2c5fb90 Mon Sep 17 00:00:00 2001
From: memeplex <[email-masked]>
Date: Tue, 15 Oct 2019 16:37:24 -0300
Subject: [PATCH] Fringe refactor: move platform-specific code into rif
 (Bug#37755)

* src/fringe.c (init_fringe_bitmap): Remove it.
* src/w32term.c (w32_define_fringe_bitmap): Add NTGUI part.
* src/xterm.c (x_define_fringe_bitmap): New interface function for X11,
add XWINDOWS part.
(x_cr_define_fringe_bitmap): Add CAIRO part.
---
 src/fringe.c  | 128 +++++---------------------------------------------
 src/w32term.c |  12 +++++
 src/xterm.c   |  60 ++++++++++++++++++++++-
 3 files changed, 82 insertions(+), 118 deletions(-)

diff --git a/src/fringe.c b/src/fringe.c
index 22f3bdc..a17b684 100644
--- a/src/fringe.c
+++ b/src/fringe.c
@@ -1388,112 +1388,6 @@ DEFUN ("destroy-fringe-bitmap", Fdestroy_fringe_bitmap, Sdestroy_fringe_bitmap,
   return Qnil;
 }
 
-
-/* Initialize bitmap bit.
-
-   On X, we bit-swap the built-in bitmaps and reduce bitmap
-   from short to char array if width is <= 8 bits.
-
-   On MAC with big-endian CPU, we need to byte-swap each short.
-
-   On W32 and MAC (little endian), there's no need to do this.
-*/
-
-#if defined (HAVE_X_WINDOWS)
-static const unsigned char swap_nibble[16] = {
-  0x0, 0x8, 0x4, 0xc,           /* 0000 1000 0100 1100 */
-  0x2, 0xa, 0x6, 0xe,           /* 0010 1010 0110 1110 */
-  0x1, 0x9, 0x5, 0xd,           /* 0001 1001 0101 1101 */
-  0x3, 0xb, 0x7, 0xf};          /* 0011 1011 0111 1111 */
-#endif                          /* HAVE_X_WINDOWS */
-
-static void
-init_fringe_bitmap (int which, struct fringe_bitmap *fb, int once_p)
-{
-  if (once_p || fb->dynamic)
-    {
-#if defined (HAVE_X_WINDOWS)
-      unsigned short *bits = fb->bits;
-      int j;
-
-#ifdef USE_CAIRO
-      for (j = 0; j < fb->height; j++)
-	{
-	  unsigned short b = *bits;
-#ifdef WORDS_BIGENDIAN
-	  *bits++ = (b << (16 - fb->width));
-#else
-	  b = (unsigned short)((swap_nibble[b & 0xf] << 12)
-			       | (swap_nibble[(b>>4) & 0xf] << 8)
-			       | (swap_nibble[(b>>8) & 0xf] << 4)
-			       | (swap_nibble[(b>>12) & 0xf]));
-	  *bits++ = (b >> (16 - fb->width));
-#endif
-	}
-#else  /* not USE_CAIRO */
-      if (fb->width <= 8)
-	{
-	  unsigned char *cbits = (unsigned char *)fb->bits;
-	  for (j = 0; j < fb->height; j++)
-	    {
-	      unsigned short b = *bits++;
-	      unsigned char c;
-	      c = (unsigned char)((swap_nibble[b & 0xf] << 4)
-				  | (swap_nibble[(b>>4) & 0xf]));
-	      *cbits++ = (c >> (8 - fb->width));
-	    }
-	}
-      else
-	{
-	  for (j = 0; j < fb->height; j++)
-	    {
-	      unsigned short b = *bits;
-	      b = (unsigned short)((swap_nibble[b & 0xf] << 12)
-				   | (swap_nibble[(b>>4) & 0xf] << 8)
-				   | (swap_nibble[(b>>8) & 0xf] << 4)
-				   | (swap_nibble[(b>>12) & 0xf]));
-	      b >>= (16 - fb->width);
-#ifdef WORDS_BIGENDIAN
-	      b = bswap_16 (b);
-#endif
-	      *bits++ = b;
-	    }
-	}
-#endif /* not USE_CAIRO */
-#endif /* HAVE_X_WINDOWS */
-
-#ifdef HAVE_NTGUI
-      unsigned short *bits = fb->bits;
-      int j;
-      for (j = 0; j < fb->height; j++)
-	{
-	  unsigned short b = *bits;
-	  b <<= (16 - fb->width);
-	  /* Windows is little-endian, so the next line is always
-	     needed.  */
-	  b = ((b >> 8) | (b << 8));
-	  *bits++ = b;
-	}
-#endif
-    }
-
-  if (!once_p)
-    {
-      /* XXX Is SELECTED_FRAME OK here? */
-      struct redisplay_interface *rif = FRAME_RIF (SELECTED_FRAME ());
-
-      destroy_fringe_bitmap (which);
-
-      if (rif && rif->define_fringe_bitmap)
-	rif->define_fringe_bitmap (which, fb->bits, fb->height, fb->width);
-
-      fringe_bitmaps[which] = fb;
-      if (which >= max_used_fringe_bitmap)
-	max_used_fringe_bitmap = which + 1;
-    }
-}
-
-
 DEFUN ("define-fringe-bitmap", Fdefine_fringe_bitmap, Sdefine_fringe_bitmap,
        2, 5, 0,
        doc: /* Define fringe bitmap BITMAP from BITS of size HEIGHT x WIDTH.
@@ -1625,7 +1519,17 @@ list (ALIGN PERIODIC) where PERIODIC non-nil specifies that the bitmap
 
   *xfb = fb;
 
-  init_fringe_bitmap (n, xfb, 0);
+  /* XXX Is SELECTED_FRAME OK here? */
+  struct redisplay_interface *rif = FRAME_RIF (SELECTED_FRAME ());
+
+  destroy_fringe_bitmap (n);
+
+  if (rif && rif->define_fringe_bitmap)
+    rif->define_fringe_bitmap (n, xfb->bits, xfb->height, xfb->width);
+
+  fringe_bitmaps[n] = xfb;
+  if (n >= max_used_fringe_bitmap)
+    max_used_fringe_bitmap = n + 1;
 
   return bitmap;
 }
@@ -1743,19 +1647,9 @@ mark_fringe_data (void)
 
 /* Initialize this module when Emacs starts.  */
 
-static void init_fringe_once_for_pdumper (void);
-
 void
 init_fringe_once (void)
 {
-  pdumper_do_now_and_after_load (init_fringe_once_for_pdumper);
-}
-
-static void
-init_fringe_once_for_pdumper (void)
-{
-  for (int bt = NO_FRINGE_BITMAP + 1; bt < MAX_STANDARD_FRINGE_BITMAPS; bt++)
-    init_fringe_bitmap (bt, &standard_bitmaps[bt], 1);
 }
 
 void
diff --git a/src/w32term.c b/src/w32term.c
index 9da0845..c43ad81 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -835,6 +835,18 @@ w32_draw_fringe_bitmap (struct window *w, struct glyph_row *row,
 static void
 w32_define_fringe_bitmap (int which, unsigned short *bits, int h, int wd)
 {
+#ifdef HAVE_NTGUI
+  int j;
+  for (j = 0; j < h; j++)
+    {
+      unsigned short b = *bits;
+      b <<= (16 - wd);
+      /* Windows is little-endian, so the next line is always
+         needed.  */
+      b = ((b >> 8) | (b << 8));
+      *bits++ = b;
+    }
+#endif
   if (which >= max_fringe_bmp)
     {
       int i = max_fringe_bmp;
diff --git a/src/xterm.c b/src/xterm.c
index 5d8b148..95fb25e 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -548,6 +548,11 @@ x_end_cr_xlib_drawable (struct frame *f, GC gc)
 
 static int max_fringe_bmp = 0;
 static cairo_pattern_t **fringe_bmp = 0;
+static const unsigned char swap_nibble[16] = {
+  0x0, 0x8, 0x4, 0xc,           /* 0000 1000 0100 1100 */
+  0x2, 0xa, 0x6, 0xe,           /* 0010 1010 0110 1110 */
+  0x1, 0x9, 0x5, 0xd,           /* 0001 1001 0101 1101 */
+  0x3, 0xb, 0x7, 0xf};          /* 0011 1011 0111 1111 */
 
 static void
 x_cr_define_fringe_bitmap (int which, unsigned short *bits, int h, int wd)
@@ -557,6 +562,20 @@ x_cr_define_fringe_bitmap (int which, unsigned short *bits, int h, int wd)
   cairo_pattern_t *pattern;
   unsigned char *data;
 
+  for (i = 0; i < h; i++)
+    {
+      unsigned short b = *bits;
+#ifdef WORDS_BIGENDIAN
+      *bits++ = (b << (16 - wd));
+#else
+      b = (unsigned short)((swap_nibble[b & 0xf] << 12)
+                           | (swap_nibble[(b>>4) & 0xf] << 8)
+                           | (swap_nibble[(b>>8) & 0xf] << 4)
+                           | (swap_nibble[(b>>12) & 0xf]));
+      *bits++ = (b >> (16 - wd));
+#endif
+    }
+
   if (which >= max_fringe_bmp)
     {
       i = max_fringe_bmp;
@@ -1393,6 +1412,45 @@ x_after_update_window_line (struct window *w, struct glyph_row *desired_row)
 #endif
 }
 
+#ifndef USE_CAIRO
+static void
+x_define_fringe_bitmap (int which, unsigned short *bits, int h, int wd)
+{
+  /* On X, we bit-swap the built-in bitmaps and reduce bitmap */
+  /* from short to char array if width is <= 8 bits. */
+  int j;
+
+  if (wd <= 8)
+    {
+      unsigned char *cbits = (unsigned char *)bits;
+      for (j = 0; j < h; j++)
+        {
+          unsigned short b = *bits++;
+          unsigned char c;
+          c = (unsigned char)((swap_nibble[b & 0xf] << 4)
+                              | (swap_nibble[(b>>4) & 0xf]));
+          *cbits++ = (c >> (8 - wd));
+        }
+    }
+  else
+    {
+      for (j = 0; j < h; j++)
+        {
+          unsigned short b = *bits;
+          b = (unsigned short)((swap_nibble[b & 0xf] << 12)
+                               | (swap_nibble[(b>>4) & 0xf] << 8)
+                               | (swap_nibble[(b>>8) & 0xf] << 4)
+                               | (swap_nibble[(b>>12) & 0xf]));
+          b >>= (16 - wd);
+#ifdef WORDS_BIGENDIAN
+          b = bswap_16 (b);
+#endif
+          *bits++ = b;
+        }
+    }
+}
+#endif /* USE_CAIRO */
+
 static void
 x_draw_fringe_bitmap (struct window *w, struct glyph_row *row, struct draw_fringe_bitmap_params *p)
 {
@@ -13361,7 +13419,7 @@ x_activate_timeout_atimer (void)
     x_cr_define_fringe_bitmap,
     x_cr_destroy_fringe_bitmap,
 #else
-    0, /* define_fringe_bitmap */
+    x_define_fringe_bitmap,
     0, /* destroy_fringe_bitmap */
 #endif
     x_compute_glyph_string_overhangs,
-- 
2.20.1