* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
@ 2019-04-28 1:29 Alex Gramiak
2019-04-28 17:11 ` Eli Zaretskii
2021-05-12 14:43 ` Lars Ingebrigtsen
0 siblings, 2 replies; 26+ messages in thread
From: Alex Gramiak @ 2019-04-28 1:29 UTC (permalink / raw)
To: 35468
[-- Attachment #1: Type: text/plain, Size: 192 bytes --]
This merges x_draw_glyph_string and w32_draw_glyph_string together to
remove duplicated code.
I wanted to do it for NS as well, but it seems just a bit too different
to make it easily work.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Refactor-draw_glyph_string-on-X-and-w32.patch --]
[-- Type: text/x-patch, Size: 48961 bytes --]
From 1cd69c3bcfb79f8df61d1cacf75effb1947d869f Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Sat, 27 Apr 2019 18:45:59 -0600
Subject: [PATCH] Refactor draw_glyph_string on X and w32
* src/dispextern.h (draw_glyph_string_interface): New interface for
system-dependent functions for draw_glyph_string.
(redisplay_interface): Add draw_glyph_string_interface.
* src/xdisp.c (gui_draw_glyph_string): New function implementing the
common behaviour of x_draw_glyph_string and w32_draw_glyph_string.
* src/w32term.c (w32_draw_glyph_string): Remove in favour of
gui_draw_glyph_string.
(w32_draw_underwave): Rename to w32_draw_underwave_1.
(w32_draw_underwave, w32_draw_underline, w32_draw_overline)
(w32_draw_strike_through, w32_reset_glyph_string_clipping): Define
extracted functions out of w32_draw_glyph_string.
* src/xterm.c (x_draw_glyph_string): Remove in favour of
gui_draw_glyph_string.
(x_draw_underwave): Rename to x_draw_underwave_1.
(x_draw_underwave, x_draw_underline, x_draw_overline)
(x_draw_strike_through, x_reset_glyph_string_clipping): Define
extracted functions out of x_draw_glyph_string.
---
src/dispextern.h | 98 +++++++++++
src/nsterm.m | 14 +-
src/w32term.c | 414 +++++++++++------------------------------------
src/xdisp.c | 292 +++++++++++++++++++++++++++++++++
src/xterm.c | 405 ++++++++++------------------------------------
5 files changed, 568 insertions(+), 655 deletions(-)
diff --git a/src/dispextern.h b/src/dispextern.h
index 827eed86f1..20124be817 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1389,6 +1389,98 @@ struct glyph_string
struct glyph_string *next, *prev;
};
+/* Structure holding system-dependent interface functions needed
+ for drawing glyph strings as used by `gui_draw_glyph_string'.
+
+ * If gui_draw_glyph_string is used as the RIF draw_glyph_string,
+ then all of the following must be defined with the possible
+ exception of draw_xwidget. */
+
+struct draw_glyph_string_interface
+{
+ /* Set S->gc of glyph string S for drawing that glyph string. */
+
+ void (*set_gc) (struct glyph_string *s);
+
+ /* Set clipping for output of glyph string S. */
+
+ void (*set_clipping) (struct glyph_string *s);
+
+ /* Set SRC's clipping for output of glyph string DST. This is
+ called when we are drawing DST's left_overhang or right_overhang
+ only in the area of SRC. */
+
+ void (*set_clipping_exactly) (struct glyph_string *src,
+ struct glyph_string *dst);
+
+ /* Reset clipping for output of glyph string S. */
+
+ void (*reset_clipping) (struct glyph_string *s);
+
+ /* Draw the background of glyph_string S. If S->background_filled_p
+ is non-zero don't draw it. FORCE_P non-zero means draw the
+ background even if it wouldn't be drawn normally. This is used
+ when a string preceding S draws into the background of S, or S
+ contains the first component of a composition. */
+
+ void (*draw_background) (struct glyph_string *s, bool force_p);
+
+ /* Draw the foreground of glyph string S. */
+
+ void (*draw_foreground) (struct glyph_string *s);
+
+ /* Draw a box around glyph string S. */
+
+ void (*draw_box) (struct glyph_string *s);
+
+ /* Draw image glyph string S.
+
+ s->y
+ s->x +-------------------------
+ | s->face->box
+ |
+ | +-------------------------
+ | | s->img->margin
+ | |
+ | | +-------------------
+ | | | the image
+
+ */
+
+ void (*draw_image) (struct glyph_string *s);
+
+ /* Draw stretch glyph string S. */
+
+ void (*draw_stretch) (struct glyph_string *s);
+
+ /* Draw xwidget stretch glyph string S. */
+
+ void (*draw_xwidget) (struct glyph_string *s);
+
+ /* Draw the foreground of composite glyph string S. */
+
+ void (*draw_composite_foreground) (struct glyph_string *s);
+
+ /* Draw the foreground of glyph string S for glyphless characters. */
+
+ void (*draw_glyphless_foreground) (struct glyph_string *s);
+
+ /* Draw underwave glyph string S. */
+
+ void (*draw_underwave) (struct glyph_string *s);
+
+ /* Draw underline glyph string S. */
+
+ void (*draw_underline) (struct glyph_string *s);
+
+ /* Draw overline glyph string S. */
+
+ void (*draw_overline) (struct glyph_string *s);
+
+ /* Draw strike-through glyph string S. */
+
+ void (*draw_strike_through) (struct glyph_string *s, int y, int height);
+};
#endif /* HAVE_WINDOW_SYSTEM */
\f
@@ -2890,6 +2982,10 @@ struct redisplay_interface
/* Draw a glyph string S. */
void (*draw_glyph_string) (struct glyph_string *s);
+ /* Interface for drawing glyph strings used by
+ gui_draw_glyph_string. */
+ struct draw_glyph_string_interface *gsif;
+
/* Define cursor CURSOR on frame F. */
void (*define_frame_cursor) (struct frame *f, Cursor cursor);
@@ -3288,6 +3384,8 @@ extern void gui_write_glyphs (struct window *, struct glyph_row *,
struct glyph *, enum glyph_row_area, int);
extern void gui_insert_glyphs (struct window *, struct glyph_row *,
struct glyph *, enum glyph_row_area, int);
+extern void gui_draw_glyph_string (struct glyph_string *);
+
extern void gui_clear_end_of_line (struct window *, struct glyph_row *,
enum glyph_row_area, int);
extern void gui_fix_overlapping_area (struct window *, struct glyph_row *,
diff --git a/src/nsterm.m b/src/nsterm.m
index 2a2b8cbaba..e7b3ab4ebf 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -5107,6 +5107,7 @@ static Lisp_Object ns_string_to_lispmod (const char *s)
0, /* destroy_fringe_bitmap */
ns_compute_glyph_string_overhangs,
ns_draw_glyph_string,
+ NULL, /* draw_glyph_string_interface */
ns_define_frame_cursor,
ns_clear_frame_area,
0, /* clear_under_internal_border */
@@ -9507,19 +9508,6 @@ Nil means use fullscreen the old (< 10.7) way. The old way works better with
doc: /* SKIP: real doc in xterm.c. */);
Vx_toolkit_scroll_bars = Qt;
- DEFVAR_BOOL ("x-use-underline-position-properties",
- x_use_underline_position_properties,
- doc: /* SKIP: real doc in xterm.c. */);
- x_use_underline_position_properties = 0;
- DEFSYM (Qx_use_underline_position_properties,
- "x-use-underline-position-properties");
-
- DEFVAR_BOOL ("x-underline-at-descent-line",
- x_underline_at_descent_line,
- doc: /* SKIP: real doc in xterm.c. */);
- x_underline_at_descent_line = 0;
- DEFSYM (Qx_underline_at_descent_line, "x-underline-at-descent-line");
-
/* Tell Emacs about this window system. */
Fprovide (Qns, Qnil);
diff --git a/src/w32term.c b/src/w32term.c
index 8d5f57836c..947c67abb3 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -328,7 +328,7 @@ w32_get_scale_factor(struct w32_display_info *dpyinfo, int *scale_x, int *scale_
*/
static void
-w32_draw_underwave (struct glyph_string *s, COLORREF color)
+w32_draw_underwave_1 (struct glyph_string *s, COLORREF color)
{
struct w32_display_info *dpyinfo = FRAME_DISPLAY_INFO (s->f);
@@ -394,6 +394,71 @@ w32_draw_underwave (struct glyph_string *s, COLORREF color)
DeleteObject (hp);
}
+static void
+w32_draw_underwave (struct glyph_string *s)
+{
+ COLORREF color;
+
+ if (s->face->underline_defaulted_p)
+ color = s->gc->foreground;
+ else
+ color = s->face->underline_color;
+
+ w32_draw_underwave_1 (s, color);
+}
+
+static void
+w32_draw_underline (struct glyph_string *s)
+{
+ const int y = s->ybase + s->underline_position;
+
+ if (s->face->underline_defaulted_p)
+ {
+ w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
+ y, s->width, 1);
+ }
+ else
+ {
+ w32_fill_area (s->f, s->hdc, s->face->underline_color, s->x,
+ y, s->width, 1);
+ }
+}
+
+static void
+w32_draw_overline (struct glyph_string *s)
+{
+ unsigned long dy = 0, h = 1;
+
+ if (s->face->overline_color_defaulted_p)
+ {
+ w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
+ s->y + dy, s->width, h);
+ }
+ else
+ {
+ w32_fill_area (s->f, s->hdc, s->face->overline_color, s->x,
+ s->y + dy, s->width, h);
+ }
+}
+
+static void
+w32_draw_strike_through (struct glyph_string *s, int y, int height)
+{
+ if (!FONT_TEXTMETRIC (s->font).tmStruckOut)
+ {
+ if (s->face->strike_through_color_defaulted_p)
+ {
+ w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
+ y, s->width, height);
+ }
+ else
+ {
+ w32_fill_area (s->f, s->hdc, s->face->strike_through_color, s->x,
+ y, s->width, height);
+ }
+ }
+}
+
/* Draw a hollow rectangle at the specified position. */
static void
w32_draw_rectangle (HDC hdc, XGCValues *gc, int x, int y,
@@ -1132,6 +1197,13 @@ w32_set_glyph_string_clipping_exactly (struct glyph_string *src,
w32_set_clip_rectangle (dst->hdc, &r);
}
+static void
+w32_reset_glyph_string_clipping (struct glyph_string *s)
+{
+ w32_set_clip_rectangle (s->hdc, NULL);
+ s->num_clips = 0;
+}
+
/* RIF:
Compute left and right overhang of glyph string S. */
@@ -2384,308 +2456,6 @@ w32_draw_stretch_glyph_string (struct glyph_string *s)
}
-/* Draw glyph string S. */
-
-static void
-w32_draw_glyph_string (struct glyph_string *s)
-{
- bool relief_drawn_p = 0;
-
- /* If S draws into the background of its successor, draw the
- background of the successor first so that S can draw into it.
- This makes S->next use XDrawString instead of XDrawImageString. */
- if (s->next && s->right_overhang && !s->for_overlaps)
- {
- int width;
- struct glyph_string *next;
- for (width = 0, next = s->next;
- next && width < s->right_overhang;
- width += next->width, next = next->next)
- if (next->first_glyph->type != IMAGE_GLYPH)
- {
- w32_set_glyph_string_gc (next);
- w32_set_glyph_string_clipping (next);
- if (next->first_glyph->type == STRETCH_GLYPH)
- w32_draw_stretch_glyph_string (next);
- else
- w32_draw_glyph_string_background (next, true);
- next->num_clips = 0;
- }
- }
-
- /* Set up S->gc, set clipping and draw S. */
- w32_set_glyph_string_gc (s);
-
- /* Draw relief (if any) in advance for char/composition so that the
- glyph string can be drawn over it. */
- if (!s->for_overlaps
- && s->face->box != FACE_NO_BOX
- && (s->first_glyph->type == CHAR_GLYPH
- || s->first_glyph->type == COMPOSITE_GLYPH))
-
- {
- w32_set_glyph_string_clipping (s);
- w32_draw_glyph_string_background (s, true);
- w32_draw_glyph_string_box (s);
- w32_set_glyph_string_clipping (s);
- relief_drawn_p = 1;
- }
- else if (!s->clip_head /* draw_glyphs didn't specify a clip mask. */
- && !s->clip_tail
- && ((s->prev && s->prev->hl != s->hl && s->left_overhang)
- || (s->next && s->next->hl != s->hl && s->right_overhang)))
- /* We must clip just this glyph. left_overhang part has already
- drawn when s->prev was drawn, and right_overhang part will be
- drawn later when s->next is drawn. */
- w32_set_glyph_string_clipping_exactly (s, s);
- else
- w32_set_glyph_string_clipping (s);
-
- switch (s->first_glyph->type)
- {
- case IMAGE_GLYPH:
- w32_draw_image_glyph_string (s);
- break;
-
- case STRETCH_GLYPH:
- w32_draw_stretch_glyph_string (s);
- break;
-
- case CHAR_GLYPH:
- if (s->for_overlaps)
- s->background_filled_p = true;
- else
- w32_draw_glyph_string_background (s, false);
- w32_draw_glyph_string_foreground (s);
- break;
-
- case COMPOSITE_GLYPH:
- if (s->for_overlaps || (s->cmp_from > 0
- && ! s->first_glyph->u.cmp.automatic))
- s->background_filled_p = true;
- else
- w32_draw_glyph_string_background (s, true);
- w32_draw_composite_glyph_string_foreground (s);
- break;
-
- case GLYPHLESS_GLYPH:
- if (s->for_overlaps)
- s->background_filled_p = true;
- else
- w32_draw_glyph_string_background (s, false);
- w32_draw_glyphless_glyph_string_foreground (s);
- break;
-
- default:
- emacs_abort ();
- }
-
- if (!s->for_overlaps)
- {
- /* Draw underline. */
- if (s->face->underline_p)
- {
- if (s->face->underline_type == FACE_UNDER_WAVE)
- {
- COLORREF color;
-
- if (s->face->underline_defaulted_p)
- color = s->gc->foreground;
- else
- color = s->face->underline_color;
-
- w32_draw_underwave (s, color);
- }
- else if (s->face->underline_type == FACE_UNDER_LINE)
- {
- unsigned long thickness, position;
- int y;
-
- if (s->prev && s->prev->face->underline_p
- && s->prev->face->underline_type == FACE_UNDER_LINE)
- {
- /* We use the same underline style as the previous one. */
- thickness = s->prev->underline_thickness;
- position = s->prev->underline_position;
- }
- else
- {
- struct font *font = font_for_underline_metrics (s);
- unsigned long minimum_offset;
- BOOL underline_at_descent_line;
- BOOL use_underline_position_properties;
- Lisp_Object val
- = buffer_local_value (Qunderline_minimum_offset,
- s->w->contents);
- if (FIXNUMP (val))
- minimum_offset = XFIXNAT (val);
- else
- minimum_offset = 1;
- val = buffer_local_value (Qx_underline_at_descent_line,
- s->w->contents);
- underline_at_descent_line
- = !(NILP (val) || EQ (val, Qunbound));
- val
- = buffer_local_value (Qx_use_underline_position_properties,
- s->w->contents);
- use_underline_position_properties
- = !(NILP (val) || EQ (val, Qunbound));
-
- /* Get the underline thickness. Default is 1 pixel. */
- if (font && font->underline_thickness > 0)
- thickness = font->underline_thickness;
- else
- thickness = 1;
- if (underline_at_descent_line
- || !font)
- position = (s->height - thickness) - (s->ybase - s->y);
- else
- {
- /* Get the underline position. This is the
- recommended vertical offset in pixels from
- the baseline to the top of the underline.
- This is a signed value according to the
- specs, and its default is
-
- ROUND ((maximum_descent) / 2), with
- ROUND (x) = floor (x + 0.5) */
-
- if (use_underline_position_properties
- && font->underline_position >= 0)
- position = font->underline_position;
- else
- position = (font->descent + 1) / 2;
- }
- position = max (position, minimum_offset);
- }
- /* Check the sanity of thickness and position. We should
- avoid drawing underline out of the current line area. */
- if (s->y + s->height <= s->ybase + position)
- position = (s->height - 1) - (s->ybase - s->y);
- if (s->y + s->height < s->ybase + position + thickness)
- thickness = (s->y + s->height) - (s->ybase + position);
- s->underline_thickness = thickness;
- s->underline_position = position;
- y = s->ybase + position;
- if (s->face->underline_defaulted_p)
- {
- w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
- y, s->width, 1);
- }
- else
- {
- w32_fill_area (s->f, s->hdc, s->face->underline_color, s->x,
- y, s->width, 1);
- }
- }
- }
- /* Draw overline. */
- if (s->face->overline_p)
- {
- unsigned long dy = 0, h = 1;
-
- if (s->face->overline_color_defaulted_p)
- {
- w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
- s->y + dy, s->width, h);
- }
- else
- {
- w32_fill_area (s->f, s->hdc, s->face->overline_color, s->x,
- s->y + dy, s->width, h);
- }
- }
-
- /* Draw strike-through. */
- if (s->face->strike_through_p
- && !FONT_TEXTMETRIC (s->font).tmStruckOut)
- {
- /* Y-coordinate and height of the glyph string's first
- glyph. We cannot use s->y and s->height because those
- could be larger if there are taller display elements
- (e.g., characters displayed with a larger font) in the
- same glyph row. */
- int glyph_y = s->ybase - s->first_glyph->ascent;
- int glyph_height = s->first_glyph->ascent + s->first_glyph->descent;
- /* Strike-through width and offset from the glyph string's
- top edge. */
- unsigned long h = 1;
- unsigned long dy = (glyph_height - h) / 2;
-
- if (s->face->strike_through_color_defaulted_p)
- {
- w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
- glyph_y + dy, s->width, h);
- }
- else
- {
- w32_fill_area (s->f, s->hdc, s->face->strike_through_color, s->x,
- glyph_y + dy, s->width, h);
- }
- }
-
- /* Draw relief if not yet drawn. */
- if (!relief_drawn_p && s->face->box != FACE_NO_BOX)
- w32_draw_glyph_string_box (s);
-
- if (s->prev)
- {
- struct glyph_string *prev;
-
- for (prev = s->prev; prev; prev = prev->prev)
- if (prev->hl != s->hl
- && prev->x + prev->width + prev->right_overhang > s->x)
- {
- /* As prev was drawn while clipped to its own area, we
- must draw the right_overhang part using s->hl now. */
- enum draw_glyphs_face save = prev->hl;
-
- prev->hl = s->hl;
- w32_set_glyph_string_gc (prev);
- w32_set_glyph_string_clipping_exactly (s, prev);
- if (prev->first_glyph->type == CHAR_GLYPH)
- w32_draw_glyph_string_foreground (prev);
- else
- w32_draw_composite_glyph_string_foreground (prev);
- w32_set_clip_rectangle (prev->hdc, NULL);
- prev->hl = save;
- prev->num_clips = 0;
- }
- }
-
- if (s->next)
- {
- struct glyph_string *next;
-
- for (next = s->next; next; next = next->next)
- if (next->hl != s->hl
- && next->x - next->left_overhang < s->x + s->width)
- {
- /* As next will be drawn while clipped to its own area,
- we must draw the left_overhang part using s->hl now. */
- enum draw_glyphs_face save = next->hl;
-
- next->hl = s->hl;
- w32_set_glyph_string_gc (next);
- w32_set_glyph_string_clipping_exactly (s, next);
- if (next->first_glyph->type == CHAR_GLYPH)
- w32_draw_glyph_string_foreground (next);
- else
- w32_draw_composite_glyph_string_foreground (next);
- w32_set_clip_rectangle (next->hdc, NULL);
- next->hl = save;
- next->num_clips = 0;
- next->clip_head = s->next;
- }
- }
- }
-
- /* Reset clipping. */
- w32_set_clip_rectangle (s->hdc, NULL);
- s->num_clips = 0;
-}
-
-
/* Shift display to make room for inserted glyphs. */
static void
@@ -7077,6 +6847,26 @@ w32_make_rdb (char *xrm_option)
extern frame_parm_handler w32_frame_parm_handlers[];
+static struct draw_glyph_string_interface w32_draw_glyph_string_interface =
+{
+ w32_set_glyph_string_gc,
+ w32_set_glyph_string_clipping,
+ w32_set_glyph_string_clipping_exactly,
+ w32_reset_glyph_string_clipping,
+ w32_draw_glyph_string_background,
+ w32_draw_glyph_string_foreground,
+ w32_draw_glyph_string_box,
+ w32_draw_image_glyph_string,
+ w32_draw_stretch_glyph_string,
+ NULL, /* draw_xwidget_glyph_string */
+ w32_draw_composite_glyph_string_foreground,
+ w32_draw_glyphless_glyph_string_foreground,
+ w32_draw_underwave,
+ w32_draw_underline,
+ w32_draw_overline,
+ w32_draw_strike_through
+};
+
static struct redisplay_interface w32_redisplay_interface =
{
w32_frame_parm_handlers,
@@ -7096,7 +6886,8 @@ static struct redisplay_interface w32_redisplay_interface =
w32_define_fringe_bitmap,
w32_destroy_fringe_bitmap,
w32_compute_glyph_string_overhangs,
- w32_draw_glyph_string,
+ gui_draw_glyph_string,
+ &w32_draw_glyph_string_interface,
w32_define_frame_cursor,
w32_clear_frame_area,
w32_clear_under_internal_border,
@@ -7471,21 +7262,6 @@ the cursor have no effect. */);
w32_use_visible_system_caret = 0;
- /* We don't yet support this, but defining this here avoids whining
- from cus-start.el and other places, like "M-x set-variable". */
- DEFVAR_BOOL ("x-use-underline-position-properties",
- x_use_underline_position_properties,
- doc: /* SKIP: real doc in xterm.c. */);
- x_use_underline_position_properties = 0;
- DEFSYM (Qx_use_underline_position_properties,
- "x-use-underline-position-properties");
-
- DEFVAR_BOOL ("x-underline-at-descent-line",
- x_underline_at_descent_line,
- doc: /* SKIP: real doc in xterm.c. */);
- x_underline_at_descent_line = 0;
- DEFSYM (Qx_underline_at_descent_line, "x-underline-at-descent-line");
-
DEFVAR_LISP ("x-toolkit-scroll-bars", Vx_toolkit_scroll_bars,
doc: /* SKIP: real doc in xterm.c. */);
Vx_toolkit_scroll_bars = Qt;
diff --git a/src/xdisp.c b/src/xdisp.c
index a3dddfa8a5..2f5aaf6aff 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -29099,6 +29099,269 @@ gui_insert_glyphs (struct window *w, struct glyph_row *updated_row,
unblock_input ();
}
+/* EXPORT for RIF:
+ Draw glyph string S. */
+
+void
+gui_draw_glyph_string (struct glyph_string *s)
+{
+ bool relief_drawn_p = false;
+ struct draw_glyph_string_interface *gsif = FRAME_RIF (s->f)->gsif;
+
+ if (!gsif)
+ emacs_abort ();
+
+ /* If S draws into the background of its successors, draw the
+ background of the successors first so that S can draw into it. */
+ if (s->next && s->right_overhang && !s->for_overlaps)
+ {
+ int width;
+ struct glyph_string *next;
+
+ for (width = 0, next = s->next;
+ next && width < s->right_overhang;
+ width += next->width, next = next->next)
+ if (next->first_glyph->type != IMAGE_GLYPH)
+ {
+ gsif->set_gc (next);
+ gsif->set_clipping (next);
+ if (next->first_glyph->type == STRETCH_GLYPH)
+ gsif->draw_stretch (next);
+ else
+ gsif->draw_background (next, true);
+ next->num_clips = 0;
+ }
+ }
+
+ /* Set up S->gc, set clipping and draw S. */
+ gsif->set_gc (s);
+
+ /* Draw relief (if any) in advance for char/composition so that the
+ glyph string can be drawn over it. */
+ if (!s->for_overlaps
+ && s->face->box != FACE_NO_BOX
+ && (s->first_glyph->type == CHAR_GLYPH
+ || s->first_glyph->type == COMPOSITE_GLYPH))
+
+ {
+ gsif->set_clipping (s);
+ gsif->draw_background (s, true);
+ gsif->draw_box (s);
+ gsif->set_clipping (s);
+ relief_drawn_p = true;
+ }
+ else if (!s->clip_head /* draw_glyphs didn't specify a clip mask. */
+ && !s->clip_tail
+ && ((s->prev && s->prev->hl != s->hl && s->left_overhang)
+ || (s->next && s->next->hl != s->hl && s->right_overhang)))
+ /* We must clip just this glyph. left_overhang part has already
+ drawn when s->prev was drawn, and right_overhang part will be
+ drawn later when s->next is drawn. */
+ gsif->set_clipping_exactly (s, s);
+ else
+ gsif->set_clipping (s);
+
+ switch (s->first_glyph->type)
+ {
+ case IMAGE_GLYPH:
+ gsif->draw_image (s);
+ break;
+
+ case STRETCH_GLYPH:
+ gsif->draw_stretch (s);
+ break;
+
+ case CHAR_GLYPH:
+ if (s->for_overlaps)
+ s->background_filled_p = true;
+ else
+ gsif->draw_background (s, false);
+ gsif->draw_foreground (s);
+ break;
+
+ case COMPOSITE_GLYPH:
+ if (s->for_overlaps || (s->cmp_from > 0
+ && ! s->first_glyph->u.cmp.automatic))
+ s->background_filled_p = true;
+ else
+ gsif->draw_background (s, true);
+ gsif->draw_composite_foreground (s);
+ break;
+
+ case XWIDGET_GLYPH:
+ if (gsif->draw_xwidget)
+ gsif->draw_xwidget (s);
+ break;
+
+ case GLYPHLESS_GLYPH:
+ if (s->for_overlaps)
+ s->background_filled_p = true;
+ else
+ gsif->draw_background (s, true);
+ gsif->draw_glyphless_foreground (s);
+ break;
+
+ default:
+ emacs_abort ();
+ }
+
+ if (!s->for_overlaps)
+ {
+ /* Draw underline. */
+ if (s->face->underline_p)
+ {
+ if (s->face->underline_type == FACE_UNDER_WAVE)
+ gsif->draw_underwave (s);
+ else if (s->face->underline_type == FACE_UNDER_LINE)
+ {
+ unsigned long thickness, position;
+
+ if (s->prev && s->prev->face->underline_p
+ && s->prev->face->underline_type == FACE_UNDER_LINE)
+ {
+ /* We use the same underline style as the previous one. */
+ thickness = s->prev->underline_thickness;
+ position = s->prev->underline_position;
+ }
+ else
+ {
+ struct font *font = font_for_underline_metrics (s);
+ unsigned long minimum_offset;
+ bool underline_at_descent_line;
+ bool use_underline_position_properties;
+ Lisp_Object val
+ = buffer_local_value (Qunderline_minimum_offset,
+ s->w->contents);
+ if (FIXNUMP (val))
+ minimum_offset = XFIXNAT (val);
+ else
+ minimum_offset = 1;
+ val = buffer_local_value (Qx_underline_at_descent_line,
+ s->w->contents);
+ underline_at_descent_line
+ = !(NILP (val) || EQ (val, Qunbound));
+ val
+ = buffer_local_value (Qx_use_underline_position_properties,
+ s->w->contents);
+ use_underline_position_properties
+ = !(NILP (val) || EQ (val, Qunbound));
+
+ /* Get the underline thickness. Default is 1 pixel. */
+ if (font && font->underline_thickness > 0)
+ thickness = font->underline_thickness;
+ else
+ thickness = 1;
+ if (underline_at_descent_line)
+ position = (s->height - thickness) - (s->ybase - s->y);
+ else
+ {
+ /* Get the underline position. This is the
+ recommended vertical offset in pixels from
+ the baseline to the top of the underline.
+ This is a signed value according to the
+ specs, and its default is
+
+ ROUND ((maximum descent) / 2), with
+ ROUND(x) = floor (x + 0.5) */
+
+ if (use_underline_position_properties
+ && font && font->underline_position >= 0)
+ position = font->underline_position;
+ else if (font)
+ position = (font->descent + 1) / 2;
+ else
+ position = minimum_offset;
+ }
+ position = max (position, minimum_offset);
+ }
+ /* Check the sanity of thickness and position. We should
+ avoid drawing underline out of the current line area. */
+ if (s->y + s->height <= s->ybase + position)
+ position = (s->height - 1) - (s->ybase - s->y);
+ if (s->y + s->height < s->ybase + position + thickness)
+ thickness = (s->y + s->height) - (s->ybase + position);
+ s->underline_thickness = thickness;
+ s->underline_position = position;
+ gsif->draw_underline (s);
+ }
+ }
+ /* Draw overline. */
+ if (s->face->overline_p)
+ gsif->draw_overline (s);
+
+ /* Draw strike-through. */
+ if (s->face->strike_through_p)
+ {
+ /* Y-coordinate and height of the glyph string's first
+ glyph. We cannot use s->y and s->height because those
+ could be larger if there are taller display elements
+ (e.g., characters displayed with a larger font) in the
+ same glyph row. */
+ int glyph_y = s->ybase - s->first_glyph->ascent;
+ int glyph_height = s->first_glyph->ascent + s->first_glyph->descent;
+ /* Strike-through width and offset from the glyph string's
+ top edge. */
+ unsigned long dy = (glyph_height - 1) / 2;
+ gsif->draw_strike_through (s, glyph_y + dy, 1);
+ }
+
+ /* Draw relief if not yet drawn. */
+ if (!relief_drawn_p && s->face->box != FACE_NO_BOX)
+ gsif->draw_box (s);
+
+ if (s->prev)
+ {
+ struct glyph_string *prev;
+
+ for (prev = s->prev; prev; prev = prev->prev)
+ if (prev->hl != s->hl
+ && prev->x + prev->width + prev->right_overhang > s->x)
+ {
+ /* As prev was drawn while clipped to its own area, we
+ must draw the right_overhang part using s->hl now. */
+ enum draw_glyphs_face save = prev->hl;
+
+ prev->hl = s->hl;
+ gsif->set_gc (prev);
+ gsif->set_clipping_exactly (s, prev);
+ if (prev->first_glyph->type == CHAR_GLYPH)
+ gsif->draw_foreground (prev);
+ else
+ gsif->draw_composite_foreground (prev);
+ gsif->reset_clipping (prev);
+ prev->hl = save;
+ }
+ }
+
+ if (s->next)
+ {
+ struct glyph_string *next;
+
+ for (next = s->next; next; next = next->next)
+ if (next->hl != s->hl
+ && next->x - next->left_overhang < s->x + s->width)
+ {
+ /* As next will be drawn while clipped to its own area,
+ we must draw the left_overhang part using s->hl now. */
+ enum draw_glyphs_face save = next->hl;
+
+ next->hl = s->hl;
+ gsif->set_gc (next);
+ gsif->set_clipping_exactly (s, next);
+ if (next->first_glyph->type == CHAR_GLYPH)
+ gsif->draw_foreground (next);
+ else
+ gsif->draw_composite_foreground (next);
+ gsif->reset_clipping (next);
+ next->hl = save;
+ next->clip_head = s->next;
+ }
+ }
+ }
+
+ /* Reset clipping. */
+ gsif->reset_clipping (s);
+}
/* EXPORT for RIF:
Erase the current text line from the nominal cursor position
@@ -33344,6 +33607,35 @@ baseline. The default value is 1. */);
underline_minimum_offset = 1;
DEFSYM (Qunderline_minimum_offset, "underline-minimum-offset");
+#ifdef HAVE_WINDOW_SYSTEM
+ DEFVAR_BOOL ("x-use-underline-position-properties",
+ x_use_underline_position_properties,
+ doc: /* Non-nil means make use of UNDERLINE_POSITION font properties.
+A value of nil means ignore them. If you encounter fonts with bogus
+UNDERLINE_POSITION font properties, set this to nil. You can also use
+`underline-minimum-offset' to override the font's UNDERLINE_POSITION for
+small font display sizes. */);
+# ifdef HAVE_X_WINDOWS
+ x_use_underline_position_properties = true;
+# else
+ /* Other terms don't support this (yet?). */
+ x_use_underline_position_properties = false;
+# endif
+ DEFSYM (Qx_use_underline_position_properties,
+ "x-use-underline-position-properties");
+
+ DEFVAR_BOOL ("x-underline-at-descent-line",
+ x_underline_at_descent_line,
+ doc: /* Non-nil means to draw the underline at the same place as the descent line.
+(If `line-spacing' is in effect, that moves the underline lower by
+that many pixels.)
+A value of nil means to draw the underline according to the value of the
+variable `x-use-underline-position-properties', which is usually at the
+baseline level. The default value is nil. */);
+ x_underline_at_descent_line = false;
+ DEFSYM (Qx_underline_at_descent_line, "x-underline-at-descent-line");
+#endif
+
DEFVAR_BOOL ("display-hourglass", display_hourglass_p,
doc: /* Non-nil means show an hourglass pointer, when Emacs is busy.
This feature only works when on a window system that can change
diff --git a/src/xterm.c b/src/xterm.c
index b08f5f6a62..5fe4491f7e 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -1626,6 +1626,12 @@ x_set_glyph_string_clipping_exactly (struct glyph_string *src, struct glyph_stri
x_set_clip_rectangles (dst->f, dst->gc, &r, 1);
}
+static void
+x_reset_glyph_string_clipping (struct glyph_string *s)
+{
+ x_reset_clip_rectangles (s->f, s->gc);
+ s->num_clips = 0;
+}
/* RIF:
Compute left and right overhang of glyph string S. */
@@ -3468,7 +3474,7 @@ x_get_scale_factor(Display *disp, int *scale_x, int *scale_y)
*/
static void
-x_draw_underwave (struct glyph_string *s)
+x_draw_underwave_1 (struct glyph_string *s)
{
/* Adjust for scale/HiDPI. */
int scale_x, scale_y;
@@ -3535,319 +3541,73 @@ x_draw_underwave (struct glyph_string *s)
#endif /* not USE_CAIRO */
}
-
-/* Draw glyph string S. */
-
static void
-x_draw_glyph_string (struct glyph_string *s)
+x_draw_underwave (struct glyph_string *s)
{
- bool relief_drawn_p = false;
-
- /* If S draws into the background of its successors, draw the
- background of the successors first so that S can draw into it.
- This makes S->next use XDrawString instead of XDrawImageString. */
- if (s->next && s->right_overhang && !s->for_overlaps)
+ if (s->face->underline_defaulted_p)
+ x_draw_underwave_1 (s);
+ else
{
- int width;
- struct glyph_string *next;
-
- for (width = 0, next = s->next;
- next && width < s->right_overhang;
- width += next->width, next = next->next)
- if (next->first_glyph->type != IMAGE_GLYPH)
- {
- x_set_glyph_string_gc (next);
- x_set_glyph_string_clipping (next);
- if (next->first_glyph->type == STRETCH_GLYPH)
- x_draw_stretch_glyph_string (next);
- else
- x_draw_glyph_string_background (next, true);
- next->num_clips = 0;
- }
+ XGCValues xgcv;
+ XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
+ XSetForeground (s->display, s->gc, s->face->underline_color);
+ x_draw_underwave_1 (s);
+ XSetForeground (s->display, s->gc, xgcv.foreground);
}
+}
- /* Set up S->gc, set clipping and draw S. */
- x_set_glyph_string_gc (s);
-
- /* Draw relief (if any) in advance for char/composition so that the
- glyph string can be drawn over it. */
- if (!s->for_overlaps
- && s->face->box != FACE_NO_BOX
- && (s->first_glyph->type == CHAR_GLYPH
- || s->first_glyph->type == COMPOSITE_GLYPH))
+static void
+x_draw_underline (struct glyph_string *s)
+{
+ const int y = s->ybase + s->underline_position;
+ const int thickness = s->underline_thickness;
- {
- x_set_glyph_string_clipping (s);
- x_draw_glyph_string_background (s, true);
- x_draw_glyph_string_box (s);
- x_set_glyph_string_clipping (s);
- relief_drawn_p = true;
- }
- else if (!s->clip_head /* draw_glyphs didn't specify a clip mask. */
- && !s->clip_tail
- && ((s->prev && s->prev->hl != s->hl && s->left_overhang)
- || (s->next && s->next->hl != s->hl && s->right_overhang)))
- /* We must clip just this glyph. left_overhang part has already
- drawn when s->prev was drawn, and right_overhang part will be
- drawn later when s->next is drawn. */
- x_set_glyph_string_clipping_exactly (s, s);
+ if (s->face->underline_defaulted_p)
+ x_fill_rectangle (s->f, s->gc,
+ s->x, y, s->width, thickness);
else
- x_set_glyph_string_clipping (s);
-
- switch (s->first_glyph->type)
{
- case IMAGE_GLYPH:
- x_draw_image_glyph_string (s);
- break;
-
- case XWIDGET_GLYPH:
- x_draw_xwidget_glyph_string (s);
- break;
-
- case STRETCH_GLYPH:
- x_draw_stretch_glyph_string (s);
- break;
-
- case CHAR_GLYPH:
- if (s->for_overlaps)
- s->background_filled_p = true;
- else
- x_draw_glyph_string_background (s, false);
- x_draw_glyph_string_foreground (s);
- break;
-
- case COMPOSITE_GLYPH:
- if (s->for_overlaps || (s->cmp_from > 0
- && ! s->first_glyph->u.cmp.automatic))
- s->background_filled_p = true;
- else
- x_draw_glyph_string_background (s, true);
- x_draw_composite_glyph_string_foreground (s);
- break;
+ XGCValues xgcv;
+ XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
+ XSetForeground (s->display, s->gc, s->face->underline_color);
+ x_fill_rectangle (s->f, s->gc,
+ s->x, y, s->width, thickness);
+ XSetForeground (s->display, s->gc, xgcv.foreground);
+ }
+}
- case GLYPHLESS_GLYPH:
- if (s->for_overlaps)
- s->background_filled_p = true;
- else
- x_draw_glyph_string_background (s, true);
- x_draw_glyphless_glyph_string_foreground (s);
- break;
+static void
+x_draw_overline (struct glyph_string *s)
+{
+ unsigned long dy = 0, h = 1;
- default:
- emacs_abort ();
+ if (s->face->overline_color_defaulted_p)
+ x_fill_rectangle (s->f, s->gc, s->x, s->y + dy,
+ s->width, h);
+ else
+ {
+ XGCValues xgcv;
+ XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
+ XSetForeground (s->display, s->gc, s->face->overline_color);
+ x_fill_rectangle (s->f, s->gc, s->x, s->y + dy,
+ s->width, h);
+ XSetForeground (s->display, s->gc, xgcv.foreground);
}
+}
- if (!s->for_overlaps)
+static void
+x_draw_strike_through (struct glyph_string *s, int y, int height)
+{
+ if (s->face->strike_through_color_defaulted_p)
+ x_fill_rectangle (s->f, s->gc, s->x, y, s->width, height);
+ else
{
- /* Draw underline. */
- if (s->face->underline_p)
- {
- if (s->face->underline_type == FACE_UNDER_WAVE)
- {
- if (s->face->underline_defaulted_p)
- x_draw_underwave (s);
- else
- {
- XGCValues xgcv;
- XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
- XSetForeground (s->display, s->gc, s->face->underline_color);
- x_draw_underwave (s);
- XSetForeground (s->display, s->gc, xgcv.foreground);
- }
- }
- else if (s->face->underline_type == FACE_UNDER_LINE)
- {
- unsigned long thickness, position;
- int y;
-
- if (s->prev && s->prev->face->underline_p
- && s->prev->face->underline_type == FACE_UNDER_LINE)
- {
- /* We use the same underline style as the previous one. */
- thickness = s->prev->underline_thickness;
- position = s->prev->underline_position;
- }
- else
- {
- struct font *font = font_for_underline_metrics (s);
- unsigned long minimum_offset;
- bool underline_at_descent_line;
- bool use_underline_position_properties;
- Lisp_Object val
- = buffer_local_value (Qunderline_minimum_offset,
- s->w->contents);
- if (FIXNUMP (val))
- minimum_offset = XFIXNAT (val);
- else
- minimum_offset = 1;
- val = buffer_local_value (Qx_underline_at_descent_line,
- s->w->contents);
- underline_at_descent_line
- = !(NILP (val) || EQ (val, Qunbound));
- val
- = buffer_local_value (Qx_use_underline_position_properties,
- s->w->contents);
- use_underline_position_properties
- = !(NILP (val) || EQ (val, Qunbound));
-
- /* Get the underline thickness. Default is 1 pixel. */
- if (font && font->underline_thickness > 0)
- thickness = font->underline_thickness;
- else
- thickness = 1;
- if (underline_at_descent_line)
- position = (s->height - thickness) - (s->ybase - s->y);
- else
- {
- /* Get the underline position. This is the
- recommended vertical offset in pixels from
- the baseline to the top of the underline.
- This is a signed value according to the
- specs, and its default is
-
- ROUND ((maximum descent) / 2), with
- ROUND(x) = floor (x + 0.5) */
-
- if (use_underline_position_properties
- && font && font->underline_position >= 0)
- position = font->underline_position;
- else if (font)
- position = (font->descent + 1) / 2;
- else
- position = minimum_offset;
- }
- position = max (position, minimum_offset);
- }
- /* Check the sanity of thickness and position. We should
- avoid drawing underline out of the current line area. */
- if (s->y + s->height <= s->ybase + position)
- position = (s->height - 1) - (s->ybase - s->y);
- if (s->y + s->height < s->ybase + position + thickness)
- thickness = (s->y + s->height) - (s->ybase + position);
- s->underline_thickness = thickness;
- s->underline_position = position;
- y = s->ybase + position;
- if (s->face->underline_defaulted_p)
- x_fill_rectangle (s->f, s->gc,
- s->x, y, s->width, thickness);
- else
- {
- XGCValues xgcv;
- XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
- XSetForeground (s->display, s->gc, s->face->underline_color);
- x_fill_rectangle (s->f, s->gc,
- s->x, y, s->width, thickness);
- XSetForeground (s->display, s->gc, xgcv.foreground);
- }
- }
- }
- /* Draw overline. */
- if (s->face->overline_p)
- {
- unsigned long dy = 0, h = 1;
-
- if (s->face->overline_color_defaulted_p)
- x_fill_rectangle (s->f, s->gc, s->x, s->y + dy,
- s->width, h);
- else
- {
- XGCValues xgcv;
- XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
- XSetForeground (s->display, s->gc, s->face->overline_color);
- x_fill_rectangle (s->f, s->gc, s->x, s->y + dy,
- s->width, h);
- XSetForeground (s->display, s->gc, xgcv.foreground);
- }
- }
-
- /* Draw strike-through. */
- if (s->face->strike_through_p)
- {
- /* Y-coordinate and height of the glyph string's first
- glyph. We cannot use s->y and s->height because those
- could be larger if there are taller display elements
- (e.g., characters displayed with a larger font) in the
- same glyph row. */
- int glyph_y = s->ybase - s->first_glyph->ascent;
- int glyph_height = s->first_glyph->ascent + s->first_glyph->descent;
- /* Strike-through width and offset from the glyph string's
- top edge. */
- unsigned long h = 1;
- unsigned long dy = (glyph_height - h) / 2;
-
- if (s->face->strike_through_color_defaulted_p)
- x_fill_rectangle (s->f, s->gc, s->x, glyph_y + dy,
- s->width, h);
- else
- {
- XGCValues xgcv;
- XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
- XSetForeground (s->display, s->gc, s->face->strike_through_color);
- x_fill_rectangle (s->f, s->gc, s->x, glyph_y + dy,
- s->width, h);
- XSetForeground (s->display, s->gc, xgcv.foreground);
- }
- }
-
- /* Draw relief if not yet drawn. */
- if (!relief_drawn_p && s->face->box != FACE_NO_BOX)
- x_draw_glyph_string_box (s);
-
- if (s->prev)
- {
- struct glyph_string *prev;
-
- for (prev = s->prev; prev; prev = prev->prev)
- if (prev->hl != s->hl
- && prev->x + prev->width + prev->right_overhang > s->x)
- {
- /* As prev was drawn while clipped to its own area, we
- must draw the right_overhang part using s->hl now. */
- enum draw_glyphs_face save = prev->hl;
-
- prev->hl = s->hl;
- x_set_glyph_string_gc (prev);
- x_set_glyph_string_clipping_exactly (s, prev);
- if (prev->first_glyph->type == CHAR_GLYPH)
- x_draw_glyph_string_foreground (prev);
- else
- x_draw_composite_glyph_string_foreground (prev);
- x_reset_clip_rectangles (prev->f, prev->gc);
- prev->hl = save;
- prev->num_clips = 0;
- }
- }
-
- if (s->next)
- {
- struct glyph_string *next;
-
- for (next = s->next; next; next = next->next)
- if (next->hl != s->hl
- && next->x - next->left_overhang < s->x + s->width)
- {
- /* As next will be drawn while clipped to its own area,
- we must draw the left_overhang part using s->hl now. */
- enum draw_glyphs_face save = next->hl;
-
- next->hl = s->hl;
- x_set_glyph_string_gc (next);
- x_set_glyph_string_clipping_exactly (s, next);
- if (next->first_glyph->type == CHAR_GLYPH)
- x_draw_glyph_string_foreground (next);
- else
- x_draw_composite_glyph_string_foreground (next);
- x_reset_clip_rectangles (next->f, next->gc);
- next->hl = save;
- next->num_clips = 0;
- next->clip_head = s->next;
- }
- }
+ XGCValues xgcv;
+ XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
+ XSetForeground (s->display, s->gc, s->face->strike_through_color);
+ x_fill_rectangle (s->f, s->gc, s->x, y, s->width, height);
+ XSetForeground (s->display, s->gc, xgcv.foreground);
}
-
- /* Reset clipping. */
- x_reset_clip_rectangles (s->f, s->gc);
- s->num_clips = 0;
}
/* Shift display to make room for inserted glyphs. */
@@ -13060,6 +12820,26 @@ x_activate_timeout_atimer (void)
extern frame_parm_handler x_frame_parm_handlers[];
+static struct draw_glyph_string_interface x_draw_glyph_string_interface =
+ {
+ x_set_glyph_string_gc,
+ x_set_glyph_string_clipping,
+ x_set_glyph_string_clipping_exactly,
+ x_reset_glyph_string_clipping,
+ x_draw_glyph_string_background,
+ x_draw_glyph_string_foreground,
+ x_draw_glyph_string_box,
+ x_draw_image_glyph_string,
+ x_draw_stretch_glyph_string,
+ x_draw_xwidget_glyph_string,
+ x_draw_composite_glyph_string_foreground,
+ x_draw_glyphless_glyph_string_foreground,
+ x_draw_underwave,
+ x_draw_underline,
+ x_draw_overline,
+ x_draw_strike_through
+ };
+
static struct redisplay_interface x_redisplay_interface =
{
x_frame_parm_handlers,
@@ -13084,7 +12864,8 @@ static struct redisplay_interface x_redisplay_interface =
0, /* destroy_fringe_bitmap */
#endif
x_compute_glyph_string_overhangs,
- x_draw_glyph_string,
+ gui_draw_glyph_string,
+ &x_draw_glyph_string_interface,
x_define_frame_cursor,
x_clear_frame_area,
x_clear_under_internal_border,
@@ -13327,28 +13108,6 @@ syms_of_xterm (void)
DEFSYM (Qx_gtk_map_stock, "x-gtk-map-stock");
#endif
- DEFVAR_BOOL ("x-use-underline-position-properties",
- x_use_underline_position_properties,
- doc: /* Non-nil means make use of UNDERLINE_POSITION font properties.
-A value of nil means ignore them. If you encounter fonts with bogus
-UNDERLINE_POSITION font properties, set this to nil. You can also use
-`underline-minimum-offset' to override the font's UNDERLINE_POSITION for
-small font display sizes. */);
- x_use_underline_position_properties = true;
- DEFSYM (Qx_use_underline_position_properties,
- "x-use-underline-position-properties");
-
- DEFVAR_BOOL ("x-underline-at-descent-line",
- x_underline_at_descent_line,
- doc: /* Non-nil means to draw the underline at the same place as the descent line.
-(If `line-spacing' is in effect, that moves the underline lower by
-that many pixels.)
-A value of nil means to draw the underline according to the value of the
-variable `x-use-underline-position-properties', which is usually at the
-baseline level. The default value is nil. */);
- x_underline_at_descent_line = false;
- DEFSYM (Qx_underline_at_descent_line, "x-underline-at-descent-line");
-
DEFVAR_BOOL ("x-mouse-click-focus-ignore-position",
x_mouse_click_focus_ignore_position,
doc: /* Non-nil means that a mouse click to focus a frame does not move point.
--
2.21.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-04-28 1:29 bug#35468: [PATCH] Refactor draw_glyph_string on X and w32 Alex Gramiak
@ 2019-04-28 17:11 ` Eli Zaretskii
2019-04-28 19:46 ` Alex Gramiak
2021-05-12 14:43 ` Lars Ingebrigtsen
1 sibling, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2019-04-28 17:11 UTC (permalink / raw)
To: Alex Gramiak; +Cc: 35468
> From: Alex Gramiak <agrambot@gmail.com>
> Date: Sat, 27 Apr 2019 19:29:30 -0600
>
> This merges x_draw_glyph_string and w32_draw_glyph_string together to
> remove duplicated code.
>
> I wanted to do it for NS as well, but it seems just a bit too different
> to make it easily work.
If we don't include NS in this, we won't actually gain enough to
justify the reshuffle. What prevented you from including NS, it looks
to me the code is very similar? But maybe wait with the answer until
you read the rest of the comments below.
> +struct draw_glyph_string_interface
I'd prefer draw_glyphs_interface, it's shorter. And given the
comments below, maybe the name should be gui_interface.
> -x_draw_glyph_string (struct glyph_string *s)
> +x_draw_underwave (struct glyph_string *s)
> {
> - bool relief_drawn_p = false;
> -
> - /* If S draws into the background of its successors, draw the
> - background of the successors first so that S can draw into it.
> - This makes S->next use XDrawString instead of XDrawImageString. */
> - if (s->next && s->right_overhang && !s->for_overlaps)
> + if (s->face->underline_defaulted_p)
> + x_draw_underwave_1 (s);
> + else
> {
> - int width;
> - struct glyph_string *next;
> -
> - for (width = 0, next = s->next;
> - next && width < s->right_overhang;
> - width += next->width, next = next->next)
> - if (next->first_glyph->type != IMAGE_GLYPH)
> - {
> - x_set_glyph_string_gc (next);
> - x_set_glyph_string_clipping (next);
> - if (next->first_glyph->type == STRETCH_GLYPH)
> - x_draw_stretch_glyph_string (next);
> - else
> - x_draw_glyph_string_background (next, true);
> - next->num_clips = 0;
> - }
> + XGCValues xgcv;
> + XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
> + XSetForeground (s->display, s->gc, s->face->underline_color);
> + x_draw_underwave_1 (s);
> + XSetForeground (s->display, s->gc, xgcv.foreground);
> }
> +}
I think this stops short of the goal. The goal is to refactor the
code into terminal-independent code and terminal-dependent code, and
then arrange the latter in meaningful interfaces that will be useful
henceforth for writing display code. By contrast, what you did leaves
in terminal-specific parts code that is completely independent of the
window-system, and OTOH doesn't try to identify terminal-specific
parts which are different implementations of the same general idea.
For example, all GUI terminals have code that sets foreground and
background colors, code that clears a rectangle, code that fills a
rectangle with a given color, code that draws a line between 2 points,
etc. We should identify such pieces of code, give them names, and
make them the methods called via the redisplay interface pointers. As
a rule, no terminal-independent code should ever appear in the
window-system specific part, not even logic that looks at
terminal-independent data structures, like this, for example:
+ if (s->face->strike_through_color_defaulted_p)
+ {
+ w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
+ y, s->width, height);
+ }
+ else
+ {
+ w32_fill_area (s->f, s->hdc, s->face->strike_through_color, s->x,
+ y, s->width, height);
+ }
The meaning of s->face->strike_through_color_defaulted_p has no place
in w32term.c, it should be in the terminal-independent part, and
w32_fill_area should be a w32-specific implementation of a GUI drawing
primitive. Likewise with stuff like XGetGCValues on X and DC on w32.
IOW, if we are refactoring this stuff, let's do it right, not just
ad-hoc because the current code by some chance lends itself to a
particular division into parts, not just because it's easy. If we go
the easy way, chances are that tomorrow someone who'd like to add a
new GUI feature will again have to duplicate code because of lack of
necessary primitive building blocks, which were left lumped together
with terminal-independent code instead of being separated.
The result of this refactoring should be more low-level and more
primitive interfaces, and they should each one make sense, not be
ad-hoc. It means the job becomes more complex, and you will probably
need to ask questions regarding the GUI systems you are less familiar
with. But the result will IMO much better and future-proof.
And once again, please put all the new gui_* functions extracted from
*term.c on a new file gui_term.c, not in xdisp.c.
Thanks.
^ permalink raw reply [flat|nested] 26+ messages in thread* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-04-28 17:11 ` Eli Zaretskii
@ 2019-04-28 19:46 ` Alex Gramiak
2019-04-29 17:43 ` Alex Gramiak
0 siblings, 1 reply; 26+ messages in thread
From: Alex Gramiak @ 2019-04-28 19:46 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 35468
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Alex Gramiak <agrambot@gmail.com>
>> Date: Sat, 27 Apr 2019 19:29:30 -0600
>>
>> This merges x_draw_glyph_string and w32_draw_glyph_string together to
>> remove duplicated code.
>>
>> I wanted to do it for NS as well, but it seems just a bit too different
>> to make it easily work.
>
> If we don't include NS in this, we won't actually gain enough to
> justify the reshuffle.
I still think that removing duplication between 2 out of 3 is worth it
considering the size of the removal, but I started to do this because
I'm working on a new backend (GTK without depending on X) that would
benefit from this refactoring.
> What prevented you from including NS, it looks to me the code is very
> similar?
Mostly the differences in clipping behaviour that seemed just slightly
incompatible with the way gui_draw_glyphs_string does it. I'll look at
it a bit harder.
It also doesn't have the two sections (the s->prev and s->next
conditionals) at the end, which I'm not sure is a bug or unnecessary
there. Those could just be surrounded by a (!FRAME_NS_P (s->f))
conditional though.
>> +struct draw_glyph_string_interface
>
> I'd prefer draw_glyphs_interface, it's shorter. And given the
> comments below, maybe the name should be gui_interface.
draw_glyphs_interface doesn't seem bad, as long as glyph strings are the
only types of glyphs that are drawn (otherwise draw_glyphs would be too
broad).
I think gui_interface is too broad of a description, since many of the
terminal hooks and frame parameter handlers (setting GUI window
properties, accessing color structures, setting WM hints, etc.) could
also be considered as belonging to a GUI interface. Perhaps
gui_drawing_interface or graphical_drawing_interface?
> The result of this refactoring should be more low-level and more
> primitive interfaces, and they should each one make sense, not be
> ad-hoc. It means the job becomes more complex, and you will probably
> need to ask questions regarding the GUI systems you are less familiar
> with. But the result will IMO much better and future-proof.
I'll see what I can do.
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-04-28 19:46 ` Alex Gramiak
@ 2019-04-29 17:43 ` Alex Gramiak
2019-04-30 4:59 ` Eli Zaretskii
0 siblings, 1 reply; 26+ messages in thread
From: Alex Gramiak @ 2019-04-29 17:43 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 35468
>> The result of this refactoring should be more low-level and more
>> primitive interfaces, and they should each one make sense, not be
>> ad-hoc. It means the job becomes more complex, and you will probably
>> need to ask questions regarding the GUI systems you are less familiar
>> with. But the result will IMO much better and future-proof.
>
> I'll see what I can do.
Alright, for the first question may I ask if you're okay with the
following abstraction (which may be the start of a pattern)?
I'm in {x,w32}_draw_box_rect right now, trying to generalize both
versions. The issue is that the fill command in each accepts different
arguments; specifically the w32 version takes in the color explicitly
and uses s->hdc instead of s->gc. So I think there will have to be 2
different fill_rectangle interface procedures: one for glyph strings (so
that the w32 version can access s->hdc), and another for other
procedures like *_draw_bar_cursor.
void
gui_draw_glyph_string_box (struct glyph_string *s)
{
int width, left_x, right_x, top_y, bottom_y, last_x;
bool raised_p, left_p, right_p;
struct glyph *last_glyph;
last_x = ((s->row->full_width_p && !s->w->pseudo_window_p)
? WINDOW_RIGHT_EDGE_X (s->w)
: window_box_right (s->w, s->area));
/* The glyph that may have a right box line. */
last_glyph = (s->cmp || s->img
? s->first_glyph
: s->first_glyph + s->nchars - 1);
width = eabs (s->face->box_line_width);
raised_p = s->face->box == FACE_RAISED_BOX;
left_x = s->x;
right_x = (s->row->full_width_p && s->extends_to_end_of_line_p
? last_x - 1
: min (last_x, s->x + s->background_width) - 1);
top_y = s->y;
bottom_y = top_y + s->height - 1;
left_p = (s->first_glyph->left_box_line_p
|| (s->hl == DRAW_MOUSE_FACE
&& (s->prev == NULL
|| s->prev->hl != s->hl)));
right_p = (last_glyph->right_box_line_p
|| (s->hl == DRAW_MOUSE_FACE
&& (s->next == NULL
|| s->next->hl != s->hl)));
if (s->face->box == FACE_SIMPLE_BOX)
FRAME_GDIF (s->f)->draw_box_rect (s,
left_x,
top_y,
right_x,
bottom_y,
width,
left_p,
right_p);
else
{
FRAME_GDIF (s->f)->setup_relief_colors (s);
FRAME_GDIF (s->f)->draw_relief_rect (s->f,
left_x,
top_y,
right_x,
bottom_y,
width,
raised_p,
true,
true,
left_p,
right_p);
}
}
void
gui_draw_box_rect (struct glyph_string *s,
int left_x, int top_y, int right_x, int bottom_y, int width,
bool left_p, bool right_p)
{
struct graphical_drawing_interface *gdif = FRAME_GDIF (s->f);
gdif->save_context (s);
gdif->set_context_clip (s);
gdif->set_context_foreground (s, s->face->box_color);
/* Top. */
gdif->fill_rectangle (s, s->gc, left_x, top_y, right_x - left_x + 1, width);
/* Left. */
if (left_p)
{
gdif->fill_rectangle (s, s->gc,
left_x, top_y,
width, bottom_y - top_y + 1);
}
/* Bottom. */
gdif->fill_rectangle (s, s->gc,
left_x, bottom_y - width + 1,
right_x - left_x + 1, width);
/* Right. */
if (right_p)
{
gdif->fill_rectangle (s, s->gc,
right_x - width + 1, top_y,
width, bottom_y - top_y + 1);
}
gdif->reset_context (s);
}
It doesn't work for NS partially because of the following section only
present in the NS equivalent of gui_draw_glyph_string_box. Would you be
okay with putting this in the generic version inside a FRAME_NS_P (s->f)
check? I don't know why only NS has this, though...
if (s->hl == DRAW_MOUSE_FACE)
{
face = FACE_FROM_ID_OR_NULL (s->f,
MOUSE_HL_INFO (s->f)->mouse_face_face_id);
if (!face)
face = FACE_FROM_ID (s->f, MOUSE_FACE_ID);
}
else
face = s->face;
^ permalink raw reply [flat|nested] 26+ messages in thread* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-04-29 17:43 ` Alex Gramiak
@ 2019-04-30 4:59 ` Eli Zaretskii
2019-04-30 18:00 ` Alex Gramiak
2019-04-30 20:11 ` Alan Third
0 siblings, 2 replies; 26+ messages in thread
From: Eli Zaretskii @ 2019-04-30 4:59 UTC (permalink / raw)
To: Alex Gramiak; +Cc: 35468
> From: Alex Gramiak <agrambot@gmail.com>
> Cc: 35468@debbugs.gnu.org
> Date: Mon, 29 Apr 2019 11:43:23 -0600
>
> I'm in {x,w32}_draw_box_rect right now, trying to generalize both
> versions. The issue is that the fill command in each accepts different
> arguments; specifically the w32 version takes in the color explicitly
> and uses s->hdc instead of s->gc. So I think there will have to be 2
> different fill_rectangle interface procedures: one for glyph strings (so
> that the w32 version can access s->hdc), and another for other
> procedures like *_draw_bar_cursor.
Would it be possible to generalize this into a single interface? The
GC vs HDC part can be generalized by passing both (HDC will be NULL in
the X and NS cases), and the color will be passed as in the w32
version, with the X version doing its thing with GCForeground
internally. Alternatively, we could have a set_foreground interface
that will do nothing for w32 and for X call XGetGCValues and
XSetForeground, to set up s->gc. The second alternative will probably
be more efficient.
If this doesn't work, can you tell why?
> It doesn't work for NS partially because of the following section only
> present in the NS equivalent of gui_draw_glyph_string_box. Would you be
> okay with putting this in the generic version inside a FRAME_NS_P (s->f)
> check? I don't know why only NS has this, though...
>
> if (s->hl == DRAW_MOUSE_FACE)
> {
> face = FACE_FROM_ID_OR_NULL (s->f,
> MOUSE_HL_INFO (s->f)->mouse_face_face_id);
> if (!face)
> face = FACE_FROM_ID (s->f, MOUSE_FACE_ID);
> }
> else
> face = s->face;
I don't know why this is TRT, either. We could ask Alan to look into
this, or we could simply remove that, as other terminals don't use
box_line_width from the mouse face, they use s->face instead.
Thanks.
^ permalink raw reply [flat|nested] 26+ messages in thread* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-04-30 4:59 ` Eli Zaretskii
@ 2019-04-30 18:00 ` Alex Gramiak
2019-05-01 0:14 ` mituharu
2019-05-01 18:19 ` Eli Zaretskii
2019-04-30 20:11 ` Alan Third
1 sibling, 2 replies; 26+ messages in thread
From: Alex Gramiak @ 2019-04-30 18:00 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 35468
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Alex Gramiak <agrambot@gmail.com>
>> Cc: 35468@debbugs.gnu.org
>> Date: Mon, 29 Apr 2019 11:43:23 -0600
>>
>> I'm in {x,w32}_draw_box_rect right now, trying to generalize both
>> versions. The issue is that the fill command in each accepts different
>> arguments; specifically the w32 version takes in the color explicitly
>> and uses s->hdc instead of s->gc. So I think there will have to be 2
>> different fill_rectangle interface procedures: one for glyph strings (so
>> that the w32 version can access s->hdc), and another for other
>> procedures like *_draw_bar_cursor.
>
> Would it be possible to generalize this into a single interface? The
> GC vs HDC part can be generalized by passing both (HDC will be NULL in
> the X and NS cases), and the color will be passed as in the w32
> version, with the X version doing its thing with GCForeground
> internally. Alternatively, we could have a set_foreground interface
> that will do nothing for w32 and for X call XGetGCValues and
> XSetForeground, to set up s->gc. The second alternative will probably
> be more efficient.
>
> If this doesn't work, can you tell why?
I think that there can be a single interface for both glyph_string
filling (e.g., in *_clear_glyph_string_rect), and for non-glyph_string
filling (e.g., in *_draw_bar_cursor), but I'm not sure it would be worth
it. I would think that having two separate interfaces, one taking in a
glyph string and the other taking in a frame, would be cleaner:
fill_rectangle_glyph (struct glyph_string *, GC, int, int, int, int);
fill_rectangle (struct frame *, GC, int, int, int, int);
There needs to be a glyph_string argument in one version so that the w32
version can use s->HDC. If there was only one interface, then one would
have to supply NULL to the glyph_string argument whenever not dealing
with glyph strings, which IMO is unclean. So is having to have two
versions, but I think it's the best option unless you know of a way to
embed HDC in w32's version of GC.
As for the color manipulation, I went low-level as you said before:
unsigned long foreground, background;
gdif->get_context_foreground (gc, &foreground);
gdif->get_context_background (gc, &background);
gdif->set_context_foreground (gc, background);
gdif->fill_rectangle (s, gc, x, y, w, h);
gdif->set_context_foreground (gc, foreground);
which replaces the XGetGCValues section in x_draw_stretch_glyph_string.
This unfortunately is more work in the w32 case as it manipulates s->gc
instead of just using the calculated gc->background. I'm not sure how
one would make a no-op version of setting the context foreground work in
all fill calls.
If that is unsatisfactory), then instead I could do something like:
gdif->fill_rectangle_with_color (s, gc->background, gc, x, y, w, h);
Which wouldn't touch s->gc for the w32 version and would do the whole
XGetGCValues dance for the X version.
Alternatively, if you don't want another version, there could be a
mandatory color argument that one could supply with a pre-defined
invalid color to instead use the GC color.
I have some more questions regarding w32 incompatibilities that I ran
into:
1) Why does w32_set_mouse_face_gc use GCFont in its mask when the X
version doesn't?
2) Why does w32_draw_glyphless_glyph_string_foreground have the boolean
`with_background' instead of using false unconditionally like the X
version does? Should the X version be updated?
3) Why does w32_draw_glyph_string for FACE_UNDER_LINE use a thickness of
1 for w32_fill_area instead of using s->underline_thickness like X/NS
do? This seems like a bug.
4) The w32 versions of some procedures need to save the font around the
calls to font->driver->draw; is this necessary? Specifically, see
w32_draw_glyph_string_foreground. Assuming it's necessary, I generalized
it as:
static void
gui_draw_glyph_string_foreground (struct glyph_string *s)
{
int i, x;
struct graphical_drawing_interface *gdif = FRAME_GDIF (s->f);
/* If first glyph of S has a left box line, start drawing the text
of S to the right of that box line. */
if (s->face->box != FACE_NO_BOX
&& s->first_glyph->left_box_line_p)
x = s->x + eabs (s->face->box_line_width);
else
x = s->x;
/* Currently just used by the w32 port to update S->hdc. */
if (gsif->update_secondary_context)
gsif->update_secondary_context (s, CON_BACK | CON_FORE | CON_ALIGN);
/* Draw characters of S as rectangles if S's font could not be
loaded. */
if (s->font_not_found_p)
{
for (i = 0; i < s->nchars; ++i)
{
struct glyph *g = s->first_glyph + i;
gdif->draw_rectangle (s, x, s->y, g->pixel_width - 1, s->height - 1);
x += g->pixel_width;
}
}
else
{
struct font *font = s->font;
int boff = font->baseline_offset;
int y;
void *old_font;
if (gdif->save_secondary_context_font)
old_font = gdif->save_secondary_context_font (s);
if (font->vertical_centering)
boff = VCENTER_BASELINE_OFFSET (font, s->f) - boff;
y = s->ybase - boff;
if (s->for_overlaps
|| (s->background_filled_p && s->hl != DRAW_CURSOR))
font->driver->draw (s, 0, s->nchars, x, y, false);
else
font->driver->draw (s, 0, s->nchars, x, y, true);
if (s->face->overstrike)
font->driver->draw (s, 0, s->nchars, x + 1, y, false);
if (gdif->set_secondary_context_font)
gdif->set_secondary_context_font (s, old_font);
}
}
Currently this requires save_secondary_context_font to allocate memory,
which is unideal. One could avoid this by introducing a new union type
(backend_font or somesuch) and using that instead of a void*. WDYT?
^ permalink raw reply [flat|nested] 26+ messages in thread* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-04-30 18:00 ` Alex Gramiak
@ 2019-05-01 0:14 ` mituharu
2019-05-03 19:01 ` Alex Gramiak
2019-05-01 18:19 ` Eli Zaretskii
1 sibling, 1 reply; 26+ messages in thread
From: mituharu @ 2019-05-01 0:14 UTC (permalink / raw)
To: Alex Gramiak; +Cc: 35468
> As for the color manipulation, I went low-level as you said before:
>
> unsigned long foreground, background;
> gdif->get_context_foreground (gc, &foreground);
> gdif->get_context_background (gc, &background);
> gdif->set_context_foreground (gc, background);
> gdif->fill_rectangle (s, gc, x, y, w, h);
> gdif->set_context_foreground (gc, foreground);
>
> which replaces the XGetGCValues section in x_draw_stretch_glyph_string.
> This unfortunately is more work in the w32 case as it manipulates s->gc
> instead of just using the calculated gc->background. I'm not sure how
> one would make a no-op version of setting the context foreground work in
> all fill calls.
>
> If that is unsatisfactory), then instead I could do something like:
>
> gdif->fill_rectangle_with_color (s, gc->background, gc, x, y, w, h);
>
> Which wouldn't touch s->gc for the w32 version and would do the whole
> XGetGCValues dance for the X version.
Unlike the NS port, the terminal code in the Mac port is much like the X11
version.
But there is one notable difference in the above respect.
It defines mac_erase_rectangle, which is like mac_fill_rectangle (the
XFillRectangle counterpart)
but uses the background color of GC and also handles stipples:
/* Mac replacement for XFillRectangle. */
static void
mac_fill_rectangle (struct frame *f, GC gc, int x, int y, int width, int
height)
{
MAC_BEGIN_DRAW_TO_FRAME (f, gc, context);
CGContextSetFillColorWithColor (context, gc->cg_fore_color);
{
CGRect rect = mac_rect_make (f, x, y, width, height);
CGContextFillRects (context, &rect, 1);
}
MAC_END_DRAW_TO_FRAME (f);
}
static void
mac_erase_rectangle (struct frame *f, GC gc, int x, int y,
int width, int height)
{
MAC_BEGIN_DRAW_TO_FRAME (f, gc, context);
{
CGRect rect = mac_rect_make (f, x, y, width, height);
CG_CONTEXT_FILL_RECT_WITH_GC_BACKGROUND (f, context, rect, gc);
if (gc->xgcv.fill_style == FillOpaqueStippled && gc->xgcv.stipple)
{
CGContextClipToRects (context, &rect, 1);
CGContextSetFillColorWithColor (context, gc->cg_fore_color);
int scale = CFArrayGetCount (gc->xgcv.stipple);
if (FRAME_BACKING_SCALE_FACTOR (f) < scale)
scale = FRAME_BACKING_SCALE_FACTOR (f);
CGImageRef image_mask =
(CGImageRef) CFArrayGetValueAtIndex (gc->xgcv.stipple, scale - 1);
rect = CGRectMake (0, 0, CGImageGetWidth (image_mask) / (CGFloat) scale,
CGImageGetHeight (image_mask) / (CGFloat) scale);
CGContextScaleCTM (context, 1, -1);
CGContextSetInterpolationQuality (context, kCGInterpolationNone);
CGContextDrawTiledImage (context, rect, image_mask);
}
}
MAC_END_DRAW_TO_FRAME (f);
}
BTW, the code between MAC_BEGIN_DRAW_TO_FRAME and MAC_END_DRAW_TO_FRAME
is executed in the dedicated drawing thread with the help of Grand Central
Dispatch.
See https://bitbucket.org/mituharu/emacs-mac/src/master/src/macterm.c for
the full source code.
I guess introducing the erase_rectagle handler makes things simpler and
efficient.
YAMAMOTO Mitsuharu
mituharu@math.s.chiba-u.ac.jp
^ permalink raw reply [flat|nested] 26+ messages in thread* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-05-01 0:14 ` mituharu
@ 2019-05-03 19:01 ` Alex Gramiak
2019-05-03 21:33 ` mituharu
0 siblings, 1 reply; 26+ messages in thread
From: Alex Gramiak @ 2019-05-03 19:01 UTC (permalink / raw)
To: mituharu; +Cc: 35468
mituharu@math.s.chiba-u.ac.jp writes:
> Unlike the NS port, the terminal code in the Mac port is much like the X11
> version.
I looked at your code and fortunately it indeed looks like the same
abstractions for glyph drawing that would work for the X11 version would
work for the Mac port.
> But there is one notable difference in the above respect.
> It defines mac_erase_rectangle, which is like mac_fill_rectangle (the
> XFillRectangle counterpart)
> but uses the background color of GC and also handles stipples:
>
> /* Mac replacement for XFillRectangle. */
>
> static void
> mac_fill_rectangle (struct frame *f, GC gc, int x, int y, int width, int
> height)
> {
> MAC_BEGIN_DRAW_TO_FRAME (f, gc, context);
> CGContextSetFillColorWithColor (context, gc->cg_fore_color);
> {
> CGRect rect = mac_rect_make (f, x, y, width, height);
>
> CGContextFillRects (context, &rect, 1);
> }
> MAC_END_DRAW_TO_FRAME (f);
> }
>
> static void
> mac_erase_rectangle (struct frame *f, GC gc, int x, int y,
> int width, int height)
> {
> MAC_BEGIN_DRAW_TO_FRAME (f, gc, context);
> {
> CGRect rect = mac_rect_make (f, x, y, width, height);
>
> CG_CONTEXT_FILL_RECT_WITH_GC_BACKGROUND (f, context, rect, gc);
> if (gc->xgcv.fill_style == FillOpaqueStippled && gc->xgcv.stipple)
> {
> CGContextClipToRects (context, &rect, 1);
> CGContextSetFillColorWithColor (context, gc->cg_fore_color);
> int scale = CFArrayGetCount (gc->xgcv.stipple);
> if (FRAME_BACKING_SCALE_FACTOR (f) < scale)
> scale = FRAME_BACKING_SCALE_FACTOR (f);
> CGImageRef image_mask =
> (CGImageRef) CFArrayGetValueAtIndex (gc->xgcv.stipple, scale - 1);
> rect = CGRectMake (0, 0, CGImageGetWidth (image_mask) / (CGFloat) scale,
> CGImageGetHeight (image_mask) / (CGFloat) scale);
> CGContextScaleCTM (context, 1, -1);
> CGContextSetInterpolationQuality (context, kCGInterpolationNone);
> CGContextDrawTiledImage (context, rect, image_mask);
> }
> }
> MAC_END_DRAW_TO_FRAME (f);
> }
Why does it use the background color of GC? It appears that the
Cairo version uses the foreground color of GC; is that because the Cairo
version doesn't handle stippling?
> I guess introducing the erase_rectagle handler makes things simpler and
> efficient.
Looking over the code, and considering that stippling is quite uncommon
nowadays (GTK 3 removed it, AFAIK), I think the best approach would be
to define 3 separate interface procedures:
1) fill_rectangle: handles x_fill_rectangle/mac_fill_rectangle
2) fill_rectangle_with_color: handles (1), but also with the temporary
overriding of the GC that's done frequently for mac_erase_rectangle.
3) fill_rectangle_with_stipple: handles the temporary XFillStyle
overriding of the GC.
A third procedure wouldn't be inelegant in comparison to the
alternatives, since otherwise I would need a set_context_stipple
procedure to indicate stippling.
Then mac_fill_rectangle can be assigned to fill_rectangle, and
mac_erase_rectangle can be assigned to fill_rectangle_with_stipple.
^ permalink raw reply [flat|nested] 26+ messages in thread* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-05-03 19:01 ` Alex Gramiak
@ 2019-05-03 21:33 ` mituharu
2019-05-04 4:00 ` mituharu
0 siblings, 1 reply; 26+ messages in thread
From: mituharu @ 2019-05-03 21:33 UTC (permalink / raw)
To: Alex Gramiak; +Cc: 35468
> mituharu@math.s.chiba-u.ac.jp writes:
>
>> Unlike the NS port, the terminal code in the Mac port is much like the
>> X11
>> version.
>
> I looked at your code and fortunately it indeed looks like the same
> abstractions for glyph drawing that would work for the X11 version would
> work for the Mac port.
I deliberately made the Mac port code aligned to the X11 version
for the consistency in behavior as well as the ease of catching
up upstream changes.
> Why does it use the background color of GC? It appears that the
> Cairo version uses the foreground color of GC; is that because the Cairo
> version doesn't handle stippling?
I wrote both Mac and cairo versions, and I used the foreground
color for cairo (although I already used the background color on
the Mac port) to minimize the changes from the Xlib version
because it was incorporated into xterm.c.
Actually the stipple bitmap support for the Mac port was added
very recently. It had been missing in the Mac port for more than
a decade because I also thought it is an outdated thing and not
worth implemented. But I changed my mind by looking at an
unexpected use of stipple bitmaps:
https://lists.gnu.org/r/emacs-devel/2019-03/msg00502.html
So the use of the background color of GC on the Mac port is not
for stipple bitmaps (at least historically) but mainly for the
following reasons:
1. It can avoid XGetGCValues dance you mentioned without the
drastic changes in the code that may make catching up
mainstream difficult.
2. There are multiple cases that "filling with the background
color" is useful.
3. The Mac port supports "vibrancy" (a frosted-glass-like
translucency effect) only for the background color. So
filling with the background color is implemented differently
from doing that with the foreground color on the Mac
port (see the macro CG_CONTEXT_FILL_RECT_WITH_GC_BACKGROUND
in macterm.h). It is regarded as "special stipples" and
actually controlled by the stipple attibute of faces:
https://bitbucket.org/mituharu/emacs-mac/src/df827786d7a7fb0a0e2f27577af67e32d9a888a9/doc/emacs/macport.texi#lines-519
In retrospect, it is natural to associate "stipple"
operations with "erasing" (filling with the background
color) in this sense.
>> I guess introducing the erase_rectagle handler makes things simpler and
>> efficient.
>
> Looking over the code, and considering that stippling is quite uncommon
> nowadays (GTK 3 removed it, AFAIK), I think the best approach would be
> to define 3 separate interface procedures:
>
> 1) fill_rectangle: handles x_fill_rectangle/mac_fill_rectangle
>
> 2) fill_rectangle_with_color: handles (1), but also with the temporary
> overriding of the GC that's done frequently for mac_erase_rectangle.
>
> 3) fill_rectangle_with_stipple: handles the temporary XFillStyle
> overriding of the GC.
>
> A third procedure wouldn't be inelegant in comparison to the
> alternatives, since otherwise I would need a set_context_stipple
> procedure to indicate stippling.
>
> Then mac_fill_rectangle can be assigned to fill_rectangle, and
> mac_erase_rectangle can be assigned to fill_rectangle_with_stipple.
Because of the reasons I mentioned above, I'd prefer
erase_rectangle to fill_rectangle_with_color/stipple. Also, many
use cases in the Mac port involve erasure of multiple rectangles,
so erase_rectangles (multi-rectangle version like
XFillRectangles) would eliminate some overhead of setting up
colors and the clipping area.
YAMAMOTO Mitsuharu
mituharu@math.s.chiba-u.ac.jp
^ permalink raw reply [flat|nested] 26+ messages in thread* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-05-03 21:33 ` mituharu
@ 2019-05-04 4:00 ` mituharu
0 siblings, 0 replies; 26+ messages in thread
From: mituharu @ 2019-05-04 4:00 UTC (permalink / raw)
To: Alex Gramiak, 35468
> Because of the reasons I mentioned above, I'd prefer
> erase_rectangle to fill_rectangle_with_color/stipple. Also, many
> use cases in the Mac port involve erasure of multiple rectangles,
> so erase_rectangles (multi-rectangle version like
> XFillRectangles) would eliminate some overhead of setting up
> colors and the clipping area.
Sorry, there weren't so many occurrence of multi-rectangle
filling/erasing. Maybe I counted them wrong previously.
YAMAMOTO Mitsuharu
mituharu@math.s.chiba-u.ac.jp
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-04-30 18:00 ` Alex Gramiak
2019-05-01 0:14 ` mituharu
@ 2019-05-01 18:19 ` Eli Zaretskii
2019-05-02 19:41 ` Alex Gramiak
1 sibling, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2019-05-01 18:19 UTC (permalink / raw)
To: Alex Gramiak; +Cc: 35468
> From: Alex Gramiak <agrambot@gmail.com>
> Cc: 35468@debbugs.gnu.org
> Date: Tue, 30 Apr 2019 12:00:52 -0600
>
> I think that there can be a single interface for both glyph_string
> filling (e.g., in *_clear_glyph_string_rect), and for non-glyph_string
> filling (e.g., in *_draw_bar_cursor), but I'm not sure it would be worth
> it. I would think that having two separate interfaces, one taking in a
> glyph string and the other taking in a frame, would be cleaner:
>
> fill_rectangle_glyph (struct glyph_string *, GC, int, int, int, int);
> fill_rectangle (struct frame *, GC, int, int, int, int);
>
> There needs to be a glyph_string argument in one version so that the w32
> version can use s->HDC. If there was only one interface, then one would
> have to supply NULL to the glyph_string argument whenever not dealing
> with glyph strings, which IMO is unclean.
It is slightly inelegant, but it certainly isn't a catastrophe.
Unused arguments that are there for the sake of a function signature
compatibility is not an uncommon technique in C.
What is needed in fill_rectangle_glyph from the glyph_string except
the frame pointer? (The name is also sub-optimal: what is a
"rectangle glyph"?)
> So is having to have two versions, but I think it's the best option
> unless you know of a way to embed HDC in w32's version of GC.
I see no reason to add HDC to the w32 GC, they can remain separate at
this point. But please note that gc->foreground and background used
in some cases are exactly the color numbers used directly in other
cases, so I think you could always pass colors or always pass a GC.
> As for the color manipulation, I went low-level as you said before:
>
> unsigned long foreground, background;
> gdif->get_context_foreground (gc, &foreground);
> gdif->get_context_background (gc, &background);
> gdif->set_context_foreground (gc, background);
> gdif->fill_rectangle (s, gc, x, y, w, h);
> gdif->set_context_foreground (gc, foreground);
>
> which replaces the XGetGCValues section in x_draw_stretch_glyph_string.
> This unfortunately is more work in the w32 case as it manipulates s->gc
> instead of just using the calculated gc->background.
I don't think I understand what you mean by "manipulates s->gc". Can
you show the code which does that?
> If that is unsatisfactory), then instead I could do something like:
>
> gdif->fill_rectangle_with_color (s, gc->background, gc, x, y, w, h);
>
> Which wouldn't touch s->gc for the w32 version and would do the whole
> XGetGCValues dance for the X version.
So fill_rectangle will fill with the current background and
fill_rectangle_with_color will fill with the specified color? It's
possible, yes.
> 1) Why does w32_set_mouse_face_gc use GCFont in its mask when the X
> version doesn't?
It's a remnant of old code which was deleted from the X version. We
should remove that.
> 2) Why does w32_draw_glyphless_glyph_string_foreground have the boolean
> `with_background' instead of using false unconditionally like the X
> version does? Should the X version be updated?
I don't know, but the w32 code was added in a dedicated commit.
Unfortunately, I cannot find any trace of discussing this change or
any bug reports that led to it. We could leave this code under the
FRAME_W32_P condition.
> 3) Why does w32_draw_glyph_string for FACE_UNDER_LINE use a thickness of
> 1 for w32_fill_area instead of using s->underline_thickness like X/NS
> do? This seems like a bug.
It's a bug of sorts, but you will never see any buggy behavior as
result of it, because there are no Windows fonts that specify
thickness other than zero or 1. I'm okay with using the X code for
w32 as well, it cannot do any harm.
> 4) The w32 versions of some procedures need to save the font around the
> calls to font->driver->draw; is this necessary?
Yes. The X 'draw' methods accept the font as an argument, but the w32
implementation relies on setting the font outside of the 'draw' method
because the 'draw' method draws using the "currently selected font".
Then we need to restore the original font.
You could define a set_font interface, with only a w32 implementation.
> if (gdif->save_secondary_context_font)
This name is misleading: this is not a "secondary" font. We are
selecting the font to be used for drawing by the font driver's 'draw'
method.
> Currently this requires save_secondary_context_font to allocate memory,
> which is unideal. One could avoid this by introducing a new union type
> (backend_font or somesuch) and using that instead of a void*. WDYT?
There's no need: the HFONT data type is just a pointer, so you can
store it in a 'void *' without allocating anything. If you want to be
"more Catholic than the Pope", make that variable a unitptr_t.
Thanks.
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-05-01 18:19 ` Eli Zaretskii
@ 2019-05-02 19:41 ` Alex Gramiak
2019-05-02 20:14 ` Eli Zaretskii
2019-05-04 8:17 ` Eli Zaretskii
0 siblings, 2 replies; 26+ messages in thread
From: Alex Gramiak @ 2019-05-02 19:41 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 35468
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Alex Gramiak <agrambot@gmail.com>
>> Cc: 35468@debbugs.gnu.org
>> Date: Tue, 30 Apr 2019 12:00:52 -0600
>>
>> I think that there can be a single interface for both glyph_string
>> filling (e.g., in *_clear_glyph_string_rect), and for non-glyph_string
>> filling (e.g., in *_draw_bar_cursor), but I'm not sure it would be worth
>> it. I would think that having two separate interfaces, one taking in a
>> glyph string and the other taking in a frame, would be cleaner:
>>
>> fill_rectangle_glyph (struct glyph_string *, GC, int, int, int, int);
>> fill_rectangle (struct frame *, GC, int, int, int, int);
>>
>> There needs to be a glyph_string argument in one version so that the w32
>> version can use s->HDC. If there was only one interface, then one would
>> have to supply NULL to the glyph_string argument whenever not dealing
>> with glyph strings, which IMO is unclean.
>
> It is slightly inelegant, but it certainly isn't a catastrophe.
> Unused arguments that are there for the sake of a function signature
> compatibility is not an uncommon technique in C.
I suppose. It's only because of the w32 version that uses s->HDC that
`s' needs to be passed in at all for the glyph string version.
> What is needed in fill_rectangle_glyph from the glyph_string except
> the frame pointer?
It depends, sometimes it's just the frame and GC (and HDC), and
sometimes it's other attributes like x/y/width/height. AFAICT, all
arguments except the frame (it's always s->f) are needed for the glyph
string filling.
> (The name is also sub-optimal: what is a "rectangle glyph"?)
I meant it as "fill rectangle from glyph", but I guess I was too terse.
>> So is having to have two versions, but I think it's the best option
>> unless you know of a way to embed HDC in w32's version of GC.
>
> I see no reason to add HDC to the w32 GC, they can remain separate at
> this point.
I'm not sure how it would affect the w32 side, but I just mentioned
combining the two since it would make "more sense" from the generic POV.
> But please note that gc->foreground and background used in some cases
> are exactly the color numbers used directly in other cases, so I think
> you could always pass colors or always pass a GC.
Do you mean that I could leave out either the color or the GC argument?
The X/Cairo side at least need the GC (since it's not always s->gc), and
outside of the {get,set}_context_* way I posted, I don't see how I could
leave out the color argument.
>> As for the color manipulation, I went low-level as you said before:
>>
>> unsigned long foreground, background;
>> gdif->get_context_foreground (gc, &foreground);
>> gdif->get_context_background (gc, &background);
>> gdif->set_context_foreground (gc, background);
>> gdif->fill_rectangle (s, gc, x, y, w, h);
>> gdif->set_context_foreground (gc, foreground);
>>
>> which replaces the XGetGCValues section in x_draw_stretch_glyph_string.
>> This unfortunately is more work in the w32 case as it manipulates s->gc
>> instead of just using the calculated gc->background.
>
> I don't think I understand what you mean by "manipulates s->gc". Can
> you show the code which does that?
I just meant that it does something like:
unsigned long foreground = s->gc->foreground;
unsigned long background = s->gc->background;
s->gc->foreground = background;
fill_rectangle (...)
s->gc->foreground = foreground;
Where previously the foreground didn't need to be saved/restored in the
w32 implementation.
>> If that is unsatisfactory), then instead I could do something like:
>>
>> gdif->fill_rectangle_with_color (s, gc->background, gc, x, y, w, h);
>>
>> Which wouldn't touch s->gc for the w32 version and would do the whole
>> XGetGCValues dance for the X version.
>
> So fill_rectangle will fill with the current background and
> fill_rectangle_with_color will fill with the specified color? It's
> possible, yes.
Possible, yes, but would you prefer that over the above?
>> 1) Why does w32_set_mouse_face_gc use GCFont in its mask when the X
>> version doesn't?
>
> It's a remnant of old code which was deleted from the X version. We
> should remove that.
Okay, I did that and removed all usages of GCFont in the W32 version
(and an unused #define GCFont in nsgui.h).
>> 4) The w32 versions of some procedures need to save the font around the
>> calls to font->driver->draw; is this necessary?
>
> Yes. The X 'draw' methods accept the font as an argument, but the w32
> implementation relies on setting the font outside of the 'draw' method
> because the 'draw' method draws using the "currently selected font".
> Then we need to restore the original font.
Where do the X 'draw' methods accept the font as an argument? Looking
at, e.g., *_draw_glyph_string_foreground, font->driver->draw takes the
same arguments.
Since font->driver->draw takes in the glyph string, why can't the 'draw'
methods use SelectObject (s->hdc, FONT_HANDLER (s->font)) and
SelectObject (s->hdc, old_font)?
If they can't, is there any other way to do it inside the draw methods?
It seems like it would simplify the code on the w32 side and make it
more in line with the other backends.
> You could define a set_font interface, with only a w32 implementation.
Did you have something else in mind besides the
save/restore_secondary_context_font I posted?
>> if (gdif->save_secondary_context_font)
>
> This name is misleading: this is not a "secondary" font. We are
> selecting the font to be used for drawing by the font driver's 'draw'
> method.
The "secondary" here applies to "context", not "context font". I used
the name since the code changes s->hdc's (what I dubbed to be the
"secondary context") font. Would you prefer just
{save,restore}_context_font?
>> Currently this requires save_secondary_context_font to allocate memory,
>> which is unideal. One could avoid this by introducing a new union type
>> (backend_font or somesuch) and using that instead of a void*. WDYT?
>
> There's no need: the HFONT data type is just a pointer, so you can
> store it in a 'void *' without allocating anything. If you want to be
> "more Catholic than the Pope", make that variable a unitptr_t.
Okay, I was thinking about other backends, but as long as the only
backends using that interface just pass pointers void * works just fine.
I'm having trouble with *_draw_image_foreground -- they just seem too
different to nicely abstract. Would it be okay if some generic
constructs leak into it (namely: s->img->mask)? Otherwise the common
setup that w32 does would be problematic.
Does the RestoreDC/SaveDC call need to be around both toplevel branches
(i.e., also around the w32_draw_rectangle), or just the s->img->pixmap
branch?
For reference, this is what I have right now for
gui_draw_image_foreground:
static void
gui_draw_image_foreground (struct glyph_string *s)
{
struct graphical_drawing_interface *gdif = FRAME_GDIF (s->f);
int x = s->x;
int y = s->ybase - image_ascent (s->img, s->face, &s->slice);
/* If first glyph of S has a left box line, start drawing it to the
right of that line. */
if (s->face->box != FACE_NO_BOX
&& s->first_glyph->left_box_line_p
&& s->slice.x == 0)
x += eabs (s->face->box_line_width);
/* If there is a margin around the image, adjust x- and y-position
by that margin. */
if (s->slice.x == 0)
x += s->img->hmargin;
if (s->slice.y == 0)
y += s->img->vmargin;
if (gdif->save_secondary_context)
gdif->save_secondary_context (s, CON_ALL); // SaveDC (s->hdc);
if (gdif->glyph_has_image (s))
{
gdif->draw_image (s, s->img->width, s->img->height,
s->slice.x, s->slice.y, s->slice.width, s->slice.height,
x, y, true);
if (!gdif->glyph_image_uses_mask (s))
{
/* When the image has a mask, we can expect that at
least part of a mouse highlight or a block cursor will
be visible. If the image doesn't have a mask, make
a block cursor visible by drawing a rectangle around
the image. I believe it's looking better if we do
nothing here for mouse-face. */
if (s->hl == DRAW_CURSOR)
{
const int relief = eabs (s->img->relief);
gdif->draw_rectangle (s,
x - relief,
y - relief,
s->slice.width + relief*2 - 1,
s->slice.height + relief*2 - 1);
}
}
}
else
/* Draw a rectangle if image could not be loaded. */
gdif->draw_rectangle (s, x, y, s->slice.width - 1, s->slice.height - 1);
if (gdif->restore_secondary_context)
gdif->restore_secondary_context (s); // RestoreDC (s->hdc, -1);
}
^ permalink raw reply [flat|nested] 26+ messages in thread* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-05-02 19:41 ` Alex Gramiak
@ 2019-05-02 20:14 ` Eli Zaretskii
2019-05-03 15:26 ` Basil L. Contovounesios
2019-05-04 8:17 ` Eli Zaretskii
1 sibling, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2019-05-02 20:14 UTC (permalink / raw)
To: Alex Gramiak; +Cc: 35468
> From: Alex Gramiak <agrambot@gmail.com>
> Cc: 35468@debbugs.gnu.org
> Date: Thu, 02 May 2019 13:41:56 -0600
It's late down here, and I had a bad week and an even worse day. So
just a few quick answers for now, and the rest will follow in a few
hours.
> > It is slightly inelegant, but it certainly isn't a catastrophe.
> > Unused arguments that are there for the sake of a function signature
> > compatibility is not an uncommon technique in C.
>
> I suppose. It's only because of the w32 version that uses s->HDC that
> `s' needs to be passed in at all for the glyph string version.
Why can't you pass HDC itself? It's just a pointer.
> > But please note that gc->foreground and background used in some cases
> > are exactly the color numbers used directly in other cases, so I think
> > you could always pass colors or always pass a GC.
>
> Do you mean that I could leave out either the color or the GC argument?
Yes, that was my impression from looking at the code. We will just
need to pack the color into a GC in some cases.
> >> 4) The w32 versions of some procedures need to save the font around the
> >> calls to font->driver->draw; is this necessary?
> >
> > Yes. The X 'draw' methods accept the font as an argument, but the w32
> > implementation relies on setting the font outside of the 'draw' method
> > because the 'draw' method draws using the "currently selected font".
> > Then we need to restore the original font.
>
> Where do the X 'draw' methods accept the font as an argument? Looking
> at, e.g., *_draw_glyph_string_foreground, font->driver->draw takes the
> same arguments.
Look at the driver's 'draw' method itself, e.g. xftfont_draw.
> Since font->driver->draw takes in the glyph string, why can't the 'draw'
> methods use SelectObject (s->hdc, FONT_HANDLER (s->font)) and
> SelectObject (s->hdc, old_font)?
It could, but that would be less efficient, since we many times call
the draw method several times in a row.
> If they can't, is there any other way to do it inside the draw methods?
> It seems like it would simplify the code on the w32 side and make it
> more in line with the other backends.
I would leave this for another rainy day, and for now just have an
interface which just w32 will implement.
> > You could define a set_font interface, with only a w32 implementation.
>
> Did you have something else in mind besides the
> save/restore_secondary_context_font I posted?
Not besides, instead.
> >> if (gdif->save_secondary_context_font)
> >
> > This name is misleading: this is not a "secondary" font. We are
> > selecting the font to be used for drawing by the font driver's 'draw'
> > method.
>
> The "secondary" here applies to "context", not "context font". I used
> the name since the code changes s->hdc's (what I dubbed to be the
> "secondary context") font. Would you prefer just
> {save,restore}_context_font?
There's no context here, either. That call selects a different font,
and returns a handle of the previous font so that it could be restored
later.
Stay tuned for the rest.
Thanks.
^ permalink raw reply [flat|nested] 26+ messages in thread* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-05-02 20:14 ` Eli Zaretskii
@ 2019-05-03 15:26 ` Basil L. Contovounesios
0 siblings, 0 replies; 26+ messages in thread
From: Basil L. Contovounesios @ 2019-05-03 15:26 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 35468, Alex Gramiak
Eli Zaretskii <eliz@gnu.org> writes:
> It's late down here, and I had a bad week and an even worse day. So
> just a few quick answers for now, and the rest will follow in a few
> hours.
I hope things look up sooner than later, and thank you for your efforts
regardless.
--
Basil
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-05-02 19:41 ` Alex Gramiak
2019-05-02 20:14 ` Eli Zaretskii
@ 2019-05-04 8:17 ` Eli Zaretskii
2019-05-04 19:29 ` Alex Gramiak
1 sibling, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2019-05-04 8:17 UTC (permalink / raw)
To: Alex Gramiak; +Cc: 35468
> From: Alex Gramiak <agrambot@gmail.com>
> Cc: 35468@debbugs.gnu.org
> Date: Thu, 02 May 2019 13:41:56 -0600
Sorry, it took slightly more than "a few hours".
> >> As for the color manipulation, I went low-level as you said before:
> >>
> >> unsigned long foreground, background;
> >> gdif->get_context_foreground (gc, &foreground);
> >> gdif->get_context_background (gc, &background);
> >> gdif->set_context_foreground (gc, background);
> >> gdif->fill_rectangle (s, gc, x, y, w, h);
> >> gdif->set_context_foreground (gc, foreground);
> >>
> >> which replaces the XGetGCValues section in x_draw_stretch_glyph_string.
> >> This unfortunately is more work in the w32 case as it manipulates s->gc
> >> instead of just using the calculated gc->background.
> >
> > I don't think I understand what you mean by "manipulates s->gc". Can
> > you show the code which does that?
>
> I just meant that it does something like:
>
> unsigned long foreground = s->gc->foreground;
> unsigned long background = s->gc->background;
> s->gc->foreground = background;
> fill_rectangle (...)
> s->gc->foreground = foreground;
>
> Where previously the foreground didn't need to be saved/restored in the
> w32 implementation.
First, it should be possible to have just 1 interface for getting both
colors, with the semantics that if one of the pointers is NULL, the
corresponding color is not required. I'd call this interface
select_colors. If w32 doesn't need that, we could either (1) omit
implementing such an interface for w32 (if it's _never_ needed there),
or (2) not call it when the frame is a w32 GUI frame. So I see no
problems in this part.
> >> If that is unsatisfactory), then instead I could do something like:
> >>
> >> gdif->fill_rectangle_with_color (s, gc->background, gc, x, y, w, h);
> >>
> >> Which wouldn't touch s->gc for the w32 version and would do the whole
> >> XGetGCValues dance for the X version.
> >
> > So fill_rectangle will fill with the current background and
> > fill_rectangle_with_color will fill with the specified color? It's
> > possible, yes.
>
> Possible, yes, but would you prefer that over the above?
I think so, yes. And it sounds like Mituharu-san prefers something
like that as well.
> >> 4) The w32 versions of some procedures need to save the font around the
> >> calls to font->driver->draw; is this necessary?
> >
> > Yes. The X 'draw' methods accept the font as an argument, but the w32
> > implementation relies on setting the font outside of the 'draw' method
> > because the 'draw' method draws using the "currently selected font".
> > Then we need to restore the original font.
>
> Where do the X 'draw' methods accept the font as an argument? Looking
> at, e.g., *_draw_glyph_string_foreground, font->driver->draw takes the
> same arguments.
The way I wrote it was confusing: by the 'draw' method I actually
meant the external APIs called by the 'draw' method, like
XftDrawGlyphs. Compare that with w32's ExtTextOutW in w32font_draw.
> Since font->driver->draw takes in the glyph string, why can't the 'draw'
> methods use SelectObject (s->hdc, FONT_HANDLER (s->font)) and
> SelectObject (s->hdc, old_font)?
>
> If they can't, is there any other way to do it inside the draw methods?
The draw method doesn't know if it's the last call with a given font.
Only the caller knows that. So the caller knows when it needs to
restore the previous font.
> I'm having trouble with *_draw_image_foreground -- they just seem too
> different to nicely abstract. Would it be okay if some generic
> constructs leak into it (namely: s->img->mask)? Otherwise the common
> setup that w32 does would be problematic.
I don't think I understand the difficulties, sorry. Why is
s->img->mask a problem?
In any case, it's not 100% "verboten" for platform-specific code to
look at the internals of 'struct glyph_string', if the interface needs
many different members of that struct. Avoiding this is just a
general rule, which makes it easier to implement generic interfaces
that will fit future uses. However draw_image (which, btw, I'd call
draw_image_foreground) looks specialized enough to be exempt of that
rule.
> Does the RestoreDC/SaveDC call need to be around both toplevel branches
> (i.e., also around the w32_draw_rectangle), or just the s->img->pixmap
> branch?
It doesn't look like the call to w32_draw_rectangle needs that, as the
code doesn't do that elsewhere.
> For reference, this is what I have right now for
> gui_draw_image_foreground:
>
> static void
> gui_draw_image_foreground (struct glyph_string *s)
> {
> struct graphical_drawing_interface *gdif = FRAME_GDIF (s->f);
> int x = s->x;
> int y = s->ybase - image_ascent (s->img, s->face, &s->slice);
>
> /* If first glyph of S has a left box line, start drawing it to the
> right of that line. */
> if (s->face->box != FACE_NO_BOX
> && s->first_glyph->left_box_line_p
> && s->slice.x == 0)
> x += eabs (s->face->box_line_width);
>
> /* If there is a margin around the image, adjust x- and y-position
> by that margin. */
> if (s->slice.x == 0)
> x += s->img->hmargin;
> if (s->slice.y == 0)
> y += s->img->vmargin;
>
> if (gdif->save_secondary_context)
> gdif->save_secondary_context (s, CON_ALL); // SaveDC (s->hdc);
>
> if (gdif->glyph_has_image (s))
What details does glyph_has_image hide? Is that just to test
s->img->pixmap?
> {
> gdif->draw_image (s, s->img->width, s->img->height,
> s->slice.x, s->slice.y, s->slice.width, s->slice.height,
> x, y, true);
> if (!gdif->glyph_image_uses_mask (s))
And what does glyph_image_uses_mask hide? AFAIU, the current code
simply looks at s->img->mask, and if so, why do we need an interface
for that?
Thanks.
^ permalink raw reply [flat|nested] 26+ messages in thread* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-05-04 8:17 ` Eli Zaretskii
@ 2019-05-04 19:29 ` Alex Gramiak
2019-05-05 0:10 ` mituharu
2019-05-05 2:34 ` Eli Zaretskii
0 siblings, 2 replies; 26+ messages in thread
From: Alex Gramiak @ 2019-05-04 19:29 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 35468
Eli Zaretskii <eliz@gnu.org> writes:
>> Where do the X 'draw' methods accept the font as an argument? Looking
>> at, e.g., *_draw_glyph_string_foreground, font->driver->draw takes the
>> same arguments.
>
> The way I wrote it was confusing: by the 'draw' method I actually
> meant the external APIs called by the 'draw' method, like
> XftDrawGlyphs. Compare that with w32's ExtTextOutW in w32font_draw.
Ah, I see. I'll keep the setter and rename it to, say,
set_device_context_font.
>> I'm having trouble with *_draw_image_foreground -- they just seem too
>> different to nicely abstract. Would it be okay if some generic
>> constructs leak into it (namely: s->img->mask)? Otherwise the common
>> setup that w32 does would be problematic.
>
> I don't think I understand the difficulties, sorry. Why is
> s->img->mask a problem?
I meant problem as in that it's "leaking" the internals a bit.
> In any case, it's not 100% "verboten" for platform-specific code to
> look at the internals of 'struct glyph_string', if the interface needs
> many different members of that struct. Avoiding this is just a
> general rule, which makes it easier to implement generic interfaces
> that will fit future uses. However draw_image (which, btw, I'd call
> draw_image_foreground) looks specialized enough to be exempt of that
> rule.
That's good; it will be easier to work with the image procedures in that
case.
>> For reference, this is what I have right now for
>> gui_draw_image_foreground:
>>
>> static void
>> gui_draw_image_foreground (struct glyph_string *s)
>> {
>> struct graphical_drawing_interface *gdif = FRAME_GDIF (s->f);
>> int x = s->x;
>> int y = s->ybase - image_ascent (s->img, s->face, &s->slice);
>>
>> /* If first glyph of S has a left box line, start drawing it to the
>> right of that line. */
>> if (s->face->box != FACE_NO_BOX
>> && s->first_glyph->left_box_line_p
>> && s->slice.x == 0)
>> x += eabs (s->face->box_line_width);
>>
>> /* If there is a margin around the image, adjust x- and y-position
>> by that margin. */
>> if (s->slice.x == 0)
>> x += s->img->hmargin;
>> if (s->slice.y == 0)
>> y += s->img->vmargin;
>>
>> if (gdif->save_secondary_context)
>> gdif->save_secondary_context (s, CON_ALL); // SaveDC (s->hdc);
>>
>> if (gdif->glyph_has_image (s))
>
> What details does glyph_has_image hide? Is that just to test
> s->img->pixmap?
On most platforms, yes, but the Cairo drawing uses s->img->cr_data
instead.
>> {
>> gdif->draw_image (s, s->img->width, s->img->height,
>> s->slice.x, s->slice.y, s->slice.width, s->slice.height,
>> x, y, true);
>> if (!gdif->glyph_image_uses_mask (s))
>
> And what does glyph_image_uses_mask hide? AFAIU, the current code
> simply looks at s->img->mask, and if so, why do we need an interface
> for that?
I was thinking that since AFAIU the Cairo drawing doesn't set
s->img->mask it wouldn't make sense, from an interface POV, to check it
directly. I suppose it doesn't really matter in that case, and it would
be faster to just check s->img->mask even if the backend doesn't use it.
^ permalink raw reply [flat|nested] 26+ messages in thread* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-05-04 19:29 ` Alex Gramiak
@ 2019-05-05 0:10 ` mituharu
2019-05-05 16:00 ` Eli Zaretskii
2019-05-05 2:34 ` Eli Zaretskii
1 sibling, 1 reply; 26+ messages in thread
From: mituharu @ 2019-05-05 0:10 UTC (permalink / raw)
To: Alex Gramiak; +Cc: 35468
>> And what does glyph_image_uses_mask hide? AFAIU, the current code
>> simply looks at s->img->mask, and if so, why do we need an interface
>> for that?
>
> I was thinking that since AFAIU the Cairo drawing doesn't set
> s->img->mask it wouldn't make sense, from an interface POV, to check it
> directly. I suppose it doesn't really matter in that case, and it would
> be faster to just check s->img->mask even if the backend doesn't use it.
IMO, image support for cairo is still premature and needs some
restructuring. It does not support postprocessing (:conversion
ALGORITHM), mask removal (:mask nil), or image-mask-p. Bitmaps
for some image format are not stored in the premultiplied alpha
format that cairo expects.
All of them are supported in the Mac port and I set dummy
s->img->mask there (not containing the actual mask bitmap data)
if the image has an alpha channel. Probably setting a bitfield
for the existence of mask (alpha channel) when creating image
data would work as an alternative way.
YAMAMOTO Mitsuharu
mituharu@math.s.chiba-u.ac.jp
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-05-05 0:10 ` mituharu
@ 2019-05-05 16:00 ` Eli Zaretskii
0 siblings, 0 replies; 26+ messages in thread
From: Eli Zaretskii @ 2019-05-05 16:00 UTC (permalink / raw)
To: mituharu; +Cc: 35468, agrambot
> Date: Sun, 5 May 2019 09:10:38 +0900
> From: mituharu@math.s.chiba-u.ac.jp
> Cc: "Eli Zaretskii" <eliz@gnu.org>,
> 35468@debbugs.gnu.org
>
> >> And what does glyph_image_uses_mask hide? AFAIU, the current code
> >> simply looks at s->img->mask, and if so, why do we need an interface
> >> for that?
> >
> > I was thinking that since AFAIU the Cairo drawing doesn't set
> > s->img->mask it wouldn't make sense, from an interface POV, to check it
> > directly. I suppose it doesn't really matter in that case, and it would
> > be faster to just check s->img->mask even if the backend doesn't use it.
>
> IMO, image support for cairo is still premature and needs some
> restructuring. It does not support postprocessing (:conversion
> ALGORITHM), mask removal (:mask nil), or image-mask-p. Bitmaps
> for some image format are not stored in the premultiplied alpha
> format that cairo expects.
>
> All of them are supported in the Mac port and I set dummy
> s->img->mask there (not containing the actual mask bitmap data)
> if the image has an alpha channel. Probably setting a bitfield
> for the existence of mask (alpha channel) when creating image
> data would work as an alternative way.
Would you like to propose the abstractions for this that are supposed
to be future-proof (if what Alex shows isn't already sufficiently so)?
TIA
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-05-04 19:29 ` Alex Gramiak
2019-05-05 0:10 ` mituharu
@ 2019-05-05 2:34 ` Eli Zaretskii
1 sibling, 0 replies; 26+ messages in thread
From: Eli Zaretskii @ 2019-05-05 2:34 UTC (permalink / raw)
To: Alex Gramiak; +Cc: 35468
> From: Alex Gramiak <agrambot@gmail.com>
> Cc: 35468@debbugs.gnu.org, mituharu@math.s.chiba-u.ac.jp
> Date: Sat, 04 May 2019 13:29:34 -0600
>
> > The way I wrote it was confusing: by the 'draw' method I actually
> > meant the external APIs called by the 'draw' method, like
> > XftDrawGlyphs. Compare that with w32's ExtTextOutW in w32font_draw.
>
> Ah, I see. I'll keep the setter and rename it to, say,
> set_device_context_font.
"Device context" is a w32-specific concept, I wouldn't propagate it
into an abstraction. set_font sounds much better to me.
> > I don't think I understand the difficulties, sorry. Why is
> > s->img->mask a problem?
>
> I meant problem as in that it's "leaking" the internals a bit.
Yes, but where and why do you need to leak it?
> > What details does glyph_has_image hide? Is that just to test
> > s->img->pixmap?
>
> On most platforms, yes, but the Cairo drawing uses s->img->cr_data
> instead.
A macro should do here, there's no need for an interface, IMO.
> >> if (!gdif->glyph_image_uses_mask (s))
> >
> > And what does glyph_image_uses_mask hide? AFAIU, the current code
> > simply looks at s->img->mask, and if so, why do we need an interface
> > for that?
>
> I was thinking that since AFAIU the Cairo drawing doesn't set
> s->img->mask it wouldn't make sense, from an interface POV, to check it
> directly. I suppose it doesn't really matter in that case, and it would
> be faster to just check s->img->mask even if the backend doesn't use it.
Yes, I think so.
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-04-30 4:59 ` Eli Zaretskii
2019-04-30 18:00 ` Alex Gramiak
@ 2019-04-30 20:11 ` Alan Third
2019-05-01 17:38 ` Eli Zaretskii
1 sibling, 1 reply; 26+ messages in thread
From: Alan Third @ 2019-04-30 20:11 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 35468, Alex Gramiak
On Tue, Apr 30, 2019 at 07:59:37AM +0300, Eli Zaretskii wrote:
> > From: Alex Gramiak <agrambot@gmail.com>
> > Cc: 35468@debbugs.gnu.org
> > Date: Mon, 29 Apr 2019 11:43:23 -0600
> >
> > It doesn't work for NS partially because of the following section only
> > present in the NS equivalent of gui_draw_glyph_string_box. Would you be
> > okay with putting this in the generic version inside a FRAME_NS_P (s->f)
> > check? I don't know why only NS has this, though...
> >
> > if (s->hl == DRAW_MOUSE_FACE)
> > {
> > face = FACE_FROM_ID_OR_NULL (s->f,
> > MOUSE_HL_INFO (s->f)->mouse_face_face_id);
> > if (!face)
> > face = FACE_FROM_ID (s->f, MOUSE_FACE_ID);
> > }
> > else
> > face = s->face;
>
> I don't know why this is TRT, either. We could ask Alan to look into
> this, or we could simply remove that, as other terminals don't use
> box_line_width from the mouse face, they use s->face instead.
A quick test doesn’t turn up any immediate issues with removing this,
but I’m unsure in what circumstances DRAW_MOUSE_FACE would be in use.
--
Alan Third
^ permalink raw reply [flat|nested] 26+ messages in thread* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-04-30 20:11 ` Alan Third
@ 2019-05-01 17:38 ` Eli Zaretskii
2019-05-01 21:08 ` Alan Third
0 siblings, 1 reply; 26+ messages in thread
From: Eli Zaretskii @ 2019-05-01 17:38 UTC (permalink / raw)
To: Alan Third; +Cc: 35468, agrambot
> Date: Tue, 30 Apr 2019 21:11:29 +0100
> From: Alan Third <alan@idiocy.org>
> Cc: Alex Gramiak <agrambot@gmail.com>, 35468@debbugs.gnu.org
>
> > > if (s->hl == DRAW_MOUSE_FACE)
> > > {
> > > face = FACE_FROM_ID_OR_NULL (s->f,
> > > MOUSE_HL_INFO (s->f)->mouse_face_face_id);
> > > if (!face)
> > > face = FACE_FROM_ID (s->f, MOUSE_FACE_ID);
> > > }
> > > else
> > > face = s->face;
> >
> > I don't know why this is TRT, either. We could ask Alan to look into
> > this, or we could simply remove that, as other terminals don't use
> > box_line_width from the mouse face, they use s->face instead.
>
> A quick test doesn’t turn up any immediate issues with removing this,
> but I’m unsure in what circumstances DRAW_MOUSE_FACE would be in use.
E.g., when you hover the mouse pointer above the mouse-sensitive
portions of the mode line.
^ permalink raw reply [flat|nested] 26+ messages in thread* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-05-01 17:38 ` Eli Zaretskii
@ 2019-05-01 21:08 ` Alan Third
2019-05-02 18:14 ` Alex Gramiak
0 siblings, 1 reply; 26+ messages in thread
From: Alan Third @ 2019-05-01 21:08 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 35468, agrambot
On Wed, May 01, 2019 at 08:38:17PM +0300, Eli Zaretskii wrote:
> > Date: Tue, 30 Apr 2019 21:11:29 +0100
> > From: Alan Third <alan@idiocy.org>
> > Cc: Alex Gramiak <agrambot@gmail.com>, 35468@debbugs.gnu.org
> >
> > > > if (s->hl == DRAW_MOUSE_FACE)
> > > > {
> > > > face = FACE_FROM_ID_OR_NULL (s->f,
> > > > MOUSE_HL_INFO (s->f)->mouse_face_face_id);
> > > > if (!face)
> > > > face = FACE_FROM_ID (s->f, MOUSE_FACE_ID);
> > > > }
> > > > else
> > > > face = s->face;
> > >
> > > I don't know why this is TRT, either. We could ask Alan to look into
> > > this, or we could simply remove that, as other terminals don't use
> > > box_line_width from the mouse face, they use s->face instead.
> >
> > A quick test doesn’t turn up any immediate issues with removing this,
> > but I’m unsure in what circumstances DRAW_MOUSE_FACE would be in use.
>
> E.g., when you hover the mouse pointer above the mouse-sensitive
> portions of the mode line.
It looks exactly the same to me with or without the code. Perhaps I’d
see something different if I was using some theme which defined mouse
face differently.
I really don’t see any reason why NS should behave differently from
the other terms here.
--
Alan Third
^ permalink raw reply [flat|nested] 26+ messages in thread* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-05-01 21:08 ` Alan Third
@ 2019-05-02 18:14 ` Alex Gramiak
2019-05-03 21:12 ` Alan Third
0 siblings, 1 reply; 26+ messages in thread
From: Alex Gramiak @ 2019-05-02 18:14 UTC (permalink / raw)
To: Alan Third; +Cc: 35468
Alan Third <alan@idiocy.org> writes:
> It looks exactly the same to me with or without the code. Perhaps I’d
> see something different if I was using some theme which defined mouse
> face differently.
>
> I really don’t see any reason why NS should behave differently from
> the other terms here.
There's a similar block in ns_maybe_dumpglyphs_background. Could you
test with that block removed as well? Also, there's a conditional check
(s->hl != DRAW_CURSOR) there that's not present in the other versions;
could you see if that's necessary?
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-05-02 18:14 ` Alex Gramiak
@ 2019-05-03 21:12 ` Alan Third
0 siblings, 0 replies; 26+ messages in thread
From: Alan Third @ 2019-05-03 21:12 UTC (permalink / raw)
To: Alex Gramiak; +Cc: 35468
On Thu, May 02, 2019 at 12:14:10PM -0600, Alex Gramiak wrote:
> Alan Third <alan@idiocy.org> writes:
>
> > It looks exactly the same to me with or without the code. Perhaps I’d
> > see something different if I was using some theme which defined mouse
> > face differently.
> >
> > I really don’t see any reason why NS should behave differently from
> > the other terms here.
>
> There's a similar block in ns_maybe_dumpglyphs_background. Could you
> test with that block removed as well? Also, there's a conditional check
> (s->hl != DRAW_CURSOR) there that's not present in the other versions;
> could you see if that's necessary?
Removing the block in ns_maybe_dumpglyphs_background seems to make no
difference here. Removing the DRAW_CURSOR check results in the cursor
being drawn as a blank (background colour) rectangle on occasion.
I’m not sure what the difference is, but there are a couple of
comments in nsterm.m which reference NS drawing the cursor differently
to other terms, i.e.
/* We draw the cursor (with NSRectFill), then draw the glyph on top
(other terminals do it the other way round). We must set
w->phys_cursor_width to the cursor width. For bar cursors, that
is CURSOR_WIDTH; for box cursors, it is the glyph width. */
and
/* NOTE: under NS this is NOT used to draw cursors, but we must avoid
overwriting cursor (usually when cursor on a tab). */
and I suspect this check is related.
--
Alan Third
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2019-04-28 1:29 bug#35468: [PATCH] Refactor draw_glyph_string on X and w32 Alex Gramiak
2019-04-28 17:11 ` Eli Zaretskii
@ 2021-05-12 14:43 ` Lars Ingebrigtsen
2021-07-22 12:55 ` Lars Ingebrigtsen
1 sibling, 1 reply; 26+ messages in thread
From: Lars Ingebrigtsen @ 2021-05-12 14:43 UTC (permalink / raw)
To: Alex Gramiak; +Cc: 35468
Alex Gramiak <agrambot@gmail.com> writes:
> This merges x_draw_glyph_string and w32_draw_glyph_string together to
> remove duplicated code.
>
> I wanted to do it for NS as well, but it seems just a bit too different
> to make it easily work.
Skimming this thread, it seems like the consensus was that this patch is
an improvement, but that it'd be nice to cover NS as well. But it seems
like the discussion mostly dissipated without reaching any conclusion.
Alex, have you done any further work on this, or is this abandoned?
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 26+ messages in thread
* bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
2021-05-12 14:43 ` Lars Ingebrigtsen
@ 2021-07-22 12:55 ` Lars Ingebrigtsen
0 siblings, 0 replies; 26+ messages in thread
From: Lars Ingebrigtsen @ 2021-07-22 12:55 UTC (permalink / raw)
To: Alex Gramiak; +Cc: 35468
Lars Ingebrigtsen <larsi@gnus.org> writes:
> Skimming this thread, it seems like the consensus was that this patch is
> an improvement, but that it'd be nice to cover NS as well. But it seems
> like the discussion mostly dissipated without reaching any conclusion.
>
> Alex, have you done any further work on this, or is this abandoned?
This was two months ago, so I guess there's not going to be any further
progress here, and I'm closing this bug report. If further progress can
be made, please respond to the debbugs address and we'll reopen.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2021-07-22 12:55 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-04-28 1:29 bug#35468: [PATCH] Refactor draw_glyph_string on X and w32 Alex Gramiak
2019-04-28 17:11 ` Eli Zaretskii
2019-04-28 19:46 ` Alex Gramiak
2019-04-29 17:43 ` Alex Gramiak
2019-04-30 4:59 ` Eli Zaretskii
2019-04-30 18:00 ` Alex Gramiak
2019-05-01 0:14 ` mituharu
2019-05-03 19:01 ` Alex Gramiak
2019-05-03 21:33 ` mituharu
2019-05-04 4:00 ` mituharu
2019-05-01 18:19 ` Eli Zaretskii
2019-05-02 19:41 ` Alex Gramiak
2019-05-02 20:14 ` Eli Zaretskii
2019-05-03 15:26 ` Basil L. Contovounesios
2019-05-04 8:17 ` Eli Zaretskii
2019-05-04 19:29 ` Alex Gramiak
2019-05-05 0:10 ` mituharu
2019-05-05 16:00 ` Eli Zaretskii
2019-05-05 2:34 ` Eli Zaretskii
2019-04-30 20:11 ` Alan Third
2019-05-01 17:38 ` Eli Zaretskii
2019-05-01 21:08 ` Alan Third
2019-05-02 18:14 ` Alex Gramiak
2019-05-03 21:12 ` Alan Third
2021-05-12 14:43 ` Lars Ingebrigtsen
2021-07-22 12:55 ` Lars Ingebrigtsen
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).