From: Carlos Pita <carlosjosepita@gmail.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 37755@debbugs.gnu.org
Subject: bug#37755: Logic in init_fringe_bitmap should be moved to backends (maybe rif->define_fringe_bitmap)
Date: Tue, 15 Oct 2019 16:45:20 -0300 [thread overview]
Message-ID: <CAELgYheHR-1hymjVHwBLOORo8wQfenJvKTQdy-HLnsBDvHV9sQ@mail.gmail.com> (raw)
In-Reply-To: <CAELgYhcwut8=oDGpYS7b1X7T0pkHAgmORopWmkJWUh1mW4BGZg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2251 bytes --]
This is the modified initialization logic implemented by the attached patch:
# When a frame is created, actually call rif->define_fringe_bitmap
# for each standard bitmap and also for lisp defined
# bitmaps that were created in a daemon with no frame / rif
gui_init_fringe (I assume this is called once per frame)
for each standard bitmap
rif->define_fringe_bitmap
for each additional bitmap (recently introduced by [1])
rif->define_fringe_bitmap
# When defined from lisp call rif->define_fringe_bitmap
# except that we're in daemon with no frame / rif
define-fringe-bitmap (lisp)
when rif: rif->define_fringe_bitmap
Much simpler, don't you think? Also removed ~30 LOCs.
Except that I'm overlooking something obvious it seems to do exactly
the same, only that the bit shuffling part is postponed until the
frame is actually created (that is, moved into
rif->define_fringe_bitmap).
Some remarks:
1. There was this comment on top of init_fringe_bitmap: "On MAC with
big-endian CPU, we need to byte-swap each short". But all the code
there were either specific to NTGUI, specific to XWINDOWS or related
to bitmap destruction. So nothing remained that could be actually
moved into the NS backend.
2. I left the HAVE_NTGUI guard even after moving the relevant code
into w32term.c because I'm not sure whether all w32 code share the
NTGUI path or not.
3. The previous implementation modified bits as a way of connecting
init with define, this was the state passed from one stage to the
other. Now, since both parts are done together, there is no need to
modify the passed bits array, the array is only required in order to
initialize backend-specific structures. Nevertheless, I decided to
keep that as it was in order to prevent regressions, but I believe an
implementation that preserves the original value of bits would be
preferable now that it is indeed possible. In particular, for X the
only "backend-specific structure" is an utterly modified bits array
(in some cases shorts are converted into chars, that's why I said in
#37689 that it's difficult for the upper layer to "split rows" after
this kind of manipulations had already taken place).
Any reviews would be much appreciated.
Best regards
--
Carlos
[-- Attachment #2: 0001-Fringe-refactor-move-platform-specific-code-into-red.patch --]
[-- Type: text/x-patch, Size: 8538 bytes --]
From c4792b1d7ad7957bb07107d6e461173ab42d87a7 Mon Sep 17 00:00:00 2001
From: memeplex <carlosjosepita@gmail.com>
Date: Tue, 15 Oct 2019 16:37:24 -0300
Subject: [PATCH] Fringe refactor: move platform-specific code into redisplay
interface
* src/fringe.c: remove init_fringe_bitmap
* src/w32term.c: add NTGUI parts to w32_define_fringe_bitmap
* src/xterm.c:
- New interface function for X11: x_define_fringe_bitmap
- add XWINDOWS parts to x_define_fringe_bitmap
- add CAIRO parts to x_cr_define_fringe_bitmap
Detailed discussion: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=37755
---
src/fringe.c | 128 +++++---------------------------------------------
src/w32term.c | 12 +++++
src/xterm.c | 58 ++++++++++++++++++++++-
3 files changed, 80 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..a8c2944 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 - 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 - wd));
+#endif
+ }
+
if (which >= max_fringe_bmp)
{
i = max_fringe_bmp;
@@ -1393,6 +1412,43 @@ x_after_update_window_line (struct window *w, struct glyph_row *desired_row)
#endif
}
+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;
+ }
+ }
+}
+
static void
x_draw_fringe_bitmap (struct window *w, struct glyph_row *row, struct draw_fringe_bitmap_params *p)
{
@@ -13361,7 +13417,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
next prev parent reply other threads:[~2019-10-15 19:45 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-15 2:30 bug#37755: Logic in init_fringe_bitmap should be moved to backends (maybe rif->define_fringe_bitmap) Carlos Pita
2019-10-15 9:33 ` Eli Zaretskii
2019-10-15 19:04 ` Carlos Pita
2019-10-15 19:45 ` Carlos Pita [this message]
2019-10-16 20:18 ` Carlos Pita
2019-10-16 22:07 ` Carlos Pita
2019-10-20 12:21 ` Eli Zaretskii
2019-10-20 15:47 ` Carlos Pita
2019-10-20 16:07 ` Eli Zaretskii
2019-10-20 16:32 ` Carlos Pita
2019-10-26 10:39 ` Eli Zaretskii
2019-10-26 15:46 ` Carlos Pita
2019-10-26 16:03 ` Eli Zaretskii
2019-10-26 16:11 ` Carlos Pita
2019-10-27 14:47 ` Carlos Pita
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAELgYheHR-1hymjVHwBLOORo8wQfenJvKTQdy-HLnsBDvHV9sQ@mail.gmail.com \
--to=carlosjosepita@gmail.com \
--cc=37755@debbugs.gnu.org \
--cc=eliz@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.