* [PATCH] Remove display member of glyph_string @ 2019-05-09 3:52 Alex Gramiak 2019-05-09 5:50 ` Eli Zaretskii 0 siblings, 1 reply; 9+ messages in thread From: Alex Gramiak @ 2019-05-09 3:52 UTC (permalink / raw) To: emacs-devel [-- Attachment #1: Type: text/plain, Size: 254 bytes --] The only other location that FRAME_X_DISPLAY appears in non-X code is in the argument to Free_Pixmap in image.c, which can hopefully be refactored out in a later patch; at that point the other terms can remove their trivial FRAME_X_DISPLAY definitions. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: Remove display --] [-- Type: text/x-patch, Size: 20843 bytes --] From d6f7b845573bcdd81552851f9d87cfd5179c7d70 Mon Sep 17 00:00:00 2001 From: Alexander Gramiak <agrambot@gmail.com> Date: Wed, 8 May 2019 21:40:24 -0600 Subject: [PATCH] Remove display member of glyph_string This member has little value even on X, and it leaks internal backend details to the glyph_string struct. * src/dispextern.h (glyph_string): Remove X display member. * src/xdisp.c (init_glyph_string): Remove initialization of display. * src/xfont.c (xfont_draw): * src/xterm.c: Use FRAME_X_DISPLAY (s->f) instead of display member. --- src/dispextern.h | 3 - src/xdisp.c | 1 - src/xfont.c | 19 ++++--- src/xterm.c | 141 +++++++++++++++++++++++++++-------------------- 4 files changed, 90 insertions(+), 74 deletions(-) diff --git a/src/dispextern.h b/src/dispextern.h index bb981f83fc..619f4c0767 100644 --- a/src/dispextern.h +++ b/src/dispextern.h @@ -1281,9 +1281,6 @@ struct glyph_string /* The window on which the glyph string is drawn. */ struct window *w; - /* X display and window for convenience. */ - Display *display; - /* The glyph row for which this string was built. It determines the y-origin and height of the string. */ struct glyph_row *row; diff --git a/src/xdisp.c b/src/xdisp.c index d380645c84..1aa677fcc7 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -25953,7 +25953,6 @@ init_glyph_string (struct glyph_string *s, #ifdef HAVE_NTGUI s->hdc = hdc; #endif - s->display = FRAME_X_DISPLAY (s->f); s->char2b = char2b; s->hl = hl; s->row = row; diff --git a/src/xfont.c b/src/xfont.c index 5ecbd6de33..ff80df407d 100644 --- a/src/xfont.c +++ b/src/xfont.c @@ -1000,6 +1000,7 @@ xfont_draw (struct glyph_string *s, int from, int to, int x, int y, bool with_background) { XFontStruct *xfont = ((struct xfont_info *) s->font)->xfont; + Display *display = FRAME_X_DISPLAY (s->f); int len = to - from; GC gc = s->gc; int i; @@ -1007,7 +1008,7 @@ xfont_draw (struct glyph_string *s, int from, int to, int x, int y, if (s->gc != s->face->gc) { block_input (); - XSetFont (s->display, gc, xfont->fid); + XSetFont (display, gc, xfont->fid); unblock_input (); } @@ -1022,20 +1023,20 @@ xfont_draw (struct glyph_string *s, int from, int to, int x, int y, { if (s->padding_p) for (i = 0; i < len; i++) - XDrawImageString (FRAME_X_DISPLAY (s->f), FRAME_X_DRAWABLE (s->f), + XDrawImageString (display, FRAME_X_DRAWABLE (s->f), gc, x + i, y, str + i, 1); else - XDrawImageString (FRAME_X_DISPLAY (s->f), FRAME_X_DRAWABLE (s->f), + XDrawImageString (display, FRAME_X_DRAWABLE (s->f), gc, x, y, str, len); } else { if (s->padding_p) for (i = 0; i < len; i++) - XDrawString (FRAME_X_DISPLAY (s->f), FRAME_X_DRAWABLE (s->f), + XDrawString (display, FRAME_X_DRAWABLE (s->f), gc, x + i, y, str + i, 1); else - XDrawString (FRAME_X_DISPLAY (s->f), FRAME_X_DRAWABLE (s->f), + XDrawString (display, FRAME_X_DRAWABLE (s->f), gc, x, y, str, len); } unblock_input (); @@ -1048,20 +1049,20 @@ xfont_draw (struct glyph_string *s, int from, int to, int x, int y, { if (s->padding_p) for (i = 0; i < len; i++) - XDrawImageString16 (FRAME_X_DISPLAY (s->f), FRAME_X_DRAWABLE (s->f), + XDrawImageString16 (display, FRAME_X_DRAWABLE (s->f), gc, x + i, y, s->char2b + from + i, 1); else - XDrawImageString16 (FRAME_X_DISPLAY (s->f), FRAME_X_DRAWABLE (s->f), + XDrawImageString16 (display, FRAME_X_DRAWABLE (s->f), gc, x, y, s->char2b + from, len); } else { if (s->padding_p) for (i = 0; i < len; i++) - XDrawString16 (FRAME_X_DISPLAY (s->f), FRAME_X_DRAWABLE (s->f), + XDrawString16 (display, FRAME_X_DRAWABLE (s->f), gc, x + i, y, s->char2b + from + i, 1); else - XDrawString16 (FRAME_X_DISPLAY (s->f), FRAME_X_DRAWABLE (s->f), + XDrawString16 (display, FRAME_X_DRAWABLE (s->f), gc, x, y, s->char2b + from, len); } unblock_input (); diff --git a/src/xterm.c b/src/xterm.c index 26f74cde91..7bedcabe98 100644 --- a/src/xterm.c +++ b/src/xterm.c @@ -1454,6 +1454,7 @@ x_set_cursor_gc (struct glyph_string *s) /* Cursor on non-default face: must merge. */ XGCValues xgcv; unsigned long mask; + Display *display = FRAME_X_DISPLAY (s->f); xgcv.background = s->f->output_data.x->cursor_pixel; xgcv.foreground = s->face->background; @@ -1479,11 +1480,11 @@ x_set_cursor_gc (struct glyph_string *s) mask = GCForeground | GCBackground | GCGraphicsExposures; if (FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc) - XChangeGC (s->display, FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc, + XChangeGC (display, FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc, mask, &xgcv); else FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc - = XCreateGC (s->display, FRAME_X_DRAWABLE (s->f), mask, &xgcv); + = XCreateGC (display, FRAME_X_DRAWABLE (s->f), mask, &xgcv); s->gc = FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc; } @@ -1519,6 +1520,7 @@ x_set_mouse_face_gc (struct glyph_string *s) except for FONT. */ XGCValues xgcv; unsigned long mask; + Display *display = FRAME_X_DISPLAY (s->f); xgcv.background = s->face->background; xgcv.foreground = s->face->foreground; @@ -1526,11 +1528,11 @@ x_set_mouse_face_gc (struct glyph_string *s) mask = GCForeground | GCBackground | GCGraphicsExposures; if (FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc) - XChangeGC (s->display, FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc, + XChangeGC (display, FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc, mask, &xgcv); else FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc - = XCreateGC (s->display, FRAME_X_DRAWABLE (s->f), mask, &xgcv); + = XCreateGC (display, FRAME_X_DRAWABLE (s->f), mask, &xgcv); s->gc = FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc; @@ -1672,11 +1674,12 @@ x_compute_glyph_string_overhangs (struct glyph_string *s) static void x_clear_glyph_string_rect (struct glyph_string *s, int x, int y, int w, int h) { + Display *display = FRAME_X_DISPLAY (s->f); XGCValues xgcv; - XGetGCValues (s->display, s->gc, GCForeground | GCBackground, &xgcv); - XSetForeground (s->display, s->gc, xgcv.background); + XGetGCValues (display, s->gc, GCForeground | GCBackground, &xgcv); + XSetForeground (display, s->gc, xgcv.background); x_fill_rectangle (s->f, s->gc, x, y, w, h); - XSetForeground (s->display, s->gc, xgcv.foreground); + XSetForeground (display, s->gc, xgcv.foreground); } @@ -1697,13 +1700,15 @@ x_draw_glyph_string_background (struct glyph_string *s, bool force_p) if (s->stippled_p) { + Display *display = FRAME_X_DISPLAY (s->f); + /* Fill background with a stipple pattern. */ - XSetFillStyle (s->display, s->gc, FillOpaqueStippled); + XSetFillStyle (display, s->gc, FillOpaqueStippled); x_fill_rectangle (s->f, s->gc, s->x, s->y + box_line_width, s->background_width, s->height - 2 * box_line_width); - XSetFillStyle (s->display, s->gc, FillSolid); + XSetFillStyle (display, s->gc, FillSolid); s->background_filled_p = true; } else if (FONT_HEIGHT (s->font) < s->height - 2 * box_line_width @@ -2591,7 +2596,7 @@ x_setup_relief_colors (struct glyph_string *s) XGCValues xgcv; /* Get the background color of the face. */ - XGetGCValues (s->display, s->gc, GCBackground, &xgcv); + XGetGCValues (FRAME_X_DISPLAY (s->f), s->gc, GCBackground, &xgcv); color = xgcv.background; } @@ -2801,10 +2806,11 @@ x_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, XRectangle *clip_rect) { + Display *display = FRAME_X_DISPLAY (s->f); XGCValues xgcv; - XGetGCValues (s->display, s->gc, GCForeground, &xgcv); - XSetForeground (s->display, s->gc, s->face->box_color); + XGetGCValues (display, s->gc, GCForeground, &xgcv); + XSetForeground (display, s->gc, s->face->box_color); x_set_clip_rectangles (s->f, s->gc, clip_rect, 1); /* Top. */ @@ -2825,7 +2831,7 @@ x_draw_box_rect (struct glyph_string *s, x_fill_rectangle (s->f, s->gc, right_x - width + 1, top_y, width, bottom_y - top_y + 1); - XSetForeground (s->display, s->gc, xgcv.foreground); + XSetForeground (display, s->gc, xgcv.foreground); x_reset_clip_rectangles (s->f, s->gc); } @@ -2888,6 +2894,7 @@ x_composite_image (struct glyph_string *s, Pixmap dest, int srcX, int srcY, int dstX, int dstY, int width, int height) { + Display *display = FRAME_X_DISPLAY (s->f); #ifdef HAVE_XRENDER if (s->img->picture) { @@ -2897,27 +2904,27 @@ x_composite_image (struct glyph_string *s, Pixmap dest, /* FIXME: Should we do this each time or would it make sense to store destination in the frame struct? */ - default_format = XRenderFindVisualFormat (s->display, - DefaultVisual (s->display, 0)); - destination = XRenderCreatePicture (s->display, dest, + default_format = XRenderFindVisualFormat (display, + DefaultVisual (display, 0)); + destination = XRenderCreatePicture (display, dest, default_format, 0, &attr); /* FIXME: It may make sense to use PictOpSrc instead of PictOpOver, as I don't know if we care about alpha values too much here. */ - XRenderComposite (s->display, PictOpOver, + XRenderComposite (display, PictOpOver, s->img->picture, s->img->mask_picture, destination, srcX, srcY, srcX, srcY, dstX, dstY, width, height); - XRenderFreePicture (s->display, destination); + XRenderFreePicture (display, destination); return; } #endif - XCopyArea (s->display, s->img->pixmap, + XCopyArea (display, s->img->pixmap, dest, s->gc, srcX, srcY, width, height, dstX, dstY); @@ -2992,7 +2999,7 @@ x_draw_image_foreground (struct glyph_string *s) xgcv.clip_x_origin = x; xgcv.clip_y_origin = y; xgcv.function = GXcopy; - XChangeGC (s->display, s->gc, mask, &xgcv); + XChangeGC (FRAME_X_DISPLAY (s->f), s->gc, mask, &xgcv); get_glyph_string_clip_rect (s, &clip_rect); image_rect.x = x; @@ -3141,6 +3148,8 @@ x_draw_image_foreground_1 (struct glyph_string *s, Pixmap pixmap) if (s->img->pixmap) { + Display *display = FRAME_X_DISPLAY (s->f); + if (s->img->mask) { /* We can't set both a clip mask and use XSetClipRectangles @@ -3156,16 +3165,16 @@ x_draw_image_foreground_1 (struct glyph_string *s, Pixmap pixmap) xgcv.clip_x_origin = x - s->slice.x; xgcv.clip_y_origin = y - s->slice.y; xgcv.function = GXcopy; - XChangeGC (s->display, s->gc, mask, &xgcv); + XChangeGC (display, s->gc, mask, &xgcv); - XCopyArea (s->display, s->img->pixmap, pixmap, s->gc, + XCopyArea (display, s->img->pixmap, pixmap, s->gc, s->slice.x, s->slice.y, s->slice.width, s->slice.height, x, y); - XSetClipMask (s->display, s->gc, None); + XSetClipMask (display, s->gc, None); } else { - XCopyArea (s->display, s->img->pixmap, pixmap, s->gc, + XCopyArea (display, s->img->pixmap, pixmap, s->gc, s->slice.x, s->slice.y, s->slice.width, s->slice.height, x, y); @@ -3200,10 +3209,12 @@ x_draw_glyph_string_bg_rect (struct glyph_string *s, int x, int y, int w, int h) { if (s->stippled_p) { + Display *display = FRAME_X_DISPLAY (s->f); + /* Fill background with a stipple pattern. */ - XSetFillStyle (s->display, s->gc, FillOpaqueStippled); + XSetFillStyle (display, s->gc, FillOpaqueStippled); x_fill_rectangle (s->f, s->gc, x, y, w, h); - XSetFillStyle (s->display, s->gc, FillSolid); + XSetFillStyle (display, s->gc, FillSolid); } else x_clear_glyph_string_rect (s, x, y, w, h); @@ -3230,6 +3241,7 @@ x_draw_image_glyph_string (struct glyph_string *s) int box_line_hwidth = eabs (s->face->box_line_width); int box_line_vwidth = max (s->face->box_line_width, 0); int height; + Display *display = FRAME_X_DISPLAY (s->f); #ifndef USE_CAIRO Pixmap pixmap = None; #endif @@ -3261,34 +3273,34 @@ x_draw_image_glyph_string (struct glyph_string *s) int depth = DefaultDepthOfScreen (screen); /* Create a pixmap as large as the glyph string. */ - pixmap = XCreatePixmap (s->display, FRAME_X_DRAWABLE (s->f), + pixmap = XCreatePixmap (display, FRAME_X_DRAWABLE (s->f), s->background_width, s->height, depth); /* Don't clip in the following because we're working on the pixmap. */ - XSetClipMask (s->display, s->gc, None); + XSetClipMask (display, s->gc, None); /* Fill the pixmap with the background color/stipple. */ if (s->stippled_p) { /* Fill background with a stipple pattern. */ - XSetFillStyle (s->display, s->gc, FillOpaqueStippled); - XSetTSOrigin (s->display, s->gc, - s->x, - s->y); - XFillRectangle (s->display, pixmap, s->gc, + XSetFillStyle (display, s->gc, FillOpaqueStippled); + XSetTSOrigin (display, s->gc, - s->x, - s->y); + XFillRectangle (display, pixmap, s->gc, 0, 0, s->background_width, s->height); - XSetFillStyle (s->display, s->gc, FillSolid); - XSetTSOrigin (s->display, s->gc, 0, 0); + XSetFillStyle (display, s->gc, FillSolid); + XSetTSOrigin (display, s->gc, 0, 0); } else { XGCValues xgcv; - XGetGCValues (s->display, s->gc, GCForeground | GCBackground, + XGetGCValues (display, s->gc, GCForeground | GCBackground, &xgcv); - XSetForeground (s->display, s->gc, xgcv.background); - XFillRectangle (s->display, pixmap, s->gc, + XSetForeground (display, s->gc, xgcv.background); + XFillRectangle (display, pixmap, s->gc, 0, 0, s->background_width, s->height); - XSetForeground (s->display, s->gc, xgcv.foreground); + XSetForeground (display, s->gc, xgcv.foreground); } } else @@ -3320,9 +3332,9 @@ x_draw_image_glyph_string (struct glyph_string *s) { x_draw_image_foreground_1 (s, pixmap); x_set_glyph_string_clipping (s); - XCopyArea (s->display, pixmap, FRAME_X_DRAWABLE (s->f), s->gc, + XCopyArea (display, pixmap, FRAME_X_DRAWABLE (s->f), s->gc, 0, 0, s->background_width, s->height, s->x, s->y); - XFreePixmap (s->display, pixmap); + XFreePixmap (display, pixmap); } else #endif /* ! USE_CAIRO */ @@ -3383,6 +3395,7 @@ x_draw_stretch_glyph_string (struct glyph_string *s) { int y = s->y; int w = background_width - width, h = s->height; + Display *display = FRAME_X_DISPLAY (s->f); XRectangle r; GC gc; @@ -3405,17 +3418,17 @@ x_draw_stretch_glyph_string (struct glyph_string *s) if (s->face->stipple) { /* Fill background with a stipple pattern. */ - XSetFillStyle (s->display, gc, FillOpaqueStippled); + XSetFillStyle (display, gc, FillOpaqueStippled); x_fill_rectangle (s->f, gc, x, y, w, h); - XSetFillStyle (s->display, gc, FillSolid); + XSetFillStyle (display, gc, FillSolid); } else { XGCValues xgcv; - XGetGCValues (s->display, gc, GCForeground | GCBackground, &xgcv); - XSetForeground (s->display, gc, xgcv.background); + XGetGCValues (display, gc, GCForeground | GCBackground, &xgcv); + XSetForeground (display, gc, xgcv.background); x_fill_rectangle (s->f, gc, x, y, w, h); - XSetForeground (s->display, gc, xgcv.foreground); + XSetForeground (display, gc, xgcv.foreground); } x_reset_clip_rectangles (s->f, gc); @@ -3470,10 +3483,12 @@ x_get_scale_factor(Display *disp, int *scale_x, int *scale_y) static void x_draw_underwave (struct glyph_string *s) { + Display *display = FRAME_X_DISPLAY (s->f); + /* Adjust for scale/HiDPI. */ int scale_x, scale_y; - x_get_scale_factor (s->display, &scale_x, &scale_y); + x_get_scale_factor (display, &scale_x, &scale_y); int wave_height = 3 * scale_y, wave_length = 2 * scale_x; @@ -3503,7 +3518,7 @@ x_draw_underwave (struct glyph_string *s) if (!gui_intersect_rectangles (&wave_clip, &string_clip, &final_clip)) return; - XSetClipRectangles (s->display, s->gc, 0, 0, &final_clip, 1, Unsorted); + XSetClipRectangles (display, s->gc, 0, 0, &final_clip, 1, Unsorted); /* Draw the waves */ @@ -3522,16 +3537,16 @@ x_draw_underwave (struct glyph_string *s) while (x1 <= xmax) { - XSetLineAttributes (s->display, s->gc, thickness, LineSolid, CapButt, + XSetLineAttributes (display, s->gc, thickness, LineSolid, CapButt, JoinRound); - XDrawLine (s->display, FRAME_X_DRAWABLE (s->f), s->gc, x1, y1, x2, y2); + XDrawLine (display, FRAME_X_DRAWABLE (s->f), s->gc, x1, y1, x2, y2); x1 = x2, y1 = y2; x2 += dx, y2 = y0 + odd*dy; odd = !odd; } /* Restore previous clipping rectangle(s) */ - XSetClipRectangles (s->display, s->gc, 0, 0, s->clip, s->num_clips, Unsorted); + XSetClipRectangles (display, s->gc, 0, 0, s->clip, s->num_clips, Unsorted); #endif /* not USE_CAIRO */ } @@ -3648,11 +3663,12 @@ x_draw_glyph_string (struct glyph_string *s) x_draw_underwave (s); else { + Display *display = FRAME_X_DISPLAY (s->f); XGCValues xgcv; - XGetGCValues (s->display, s->gc, GCForeground, &xgcv); - XSetForeground (s->display, s->gc, s->face->underline_color); + XGetGCValues (display, s->gc, GCForeground, &xgcv); + XSetForeground (display, s->gc, s->face->underline_color); x_draw_underwave (s); - XSetForeground (s->display, s->gc, xgcv.foreground); + XSetForeground (display, s->gc, xgcv.foreground); } } else if (s->face->underline_type == FACE_UNDER_LINE) @@ -3732,12 +3748,13 @@ x_draw_glyph_string (struct glyph_string *s) s->x, y, s->width, thickness); else { + Display *display = FRAME_X_DISPLAY (s->f); XGCValues xgcv; - XGetGCValues (s->display, s->gc, GCForeground, &xgcv); - XSetForeground (s->display, s->gc, s->face->underline_color); + XGetGCValues (display, s->gc, GCForeground, &xgcv); + XSetForeground (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); + XSetForeground (display, s->gc, xgcv.foreground); } } } @@ -3751,12 +3768,13 @@ x_draw_glyph_string (struct glyph_string *s) s->width, h); else { + Display *display = FRAME_X_DISPLAY (s->f); XGCValues xgcv; - XGetGCValues (s->display, s->gc, GCForeground, &xgcv); - XSetForeground (s->display, s->gc, s->face->overline_color); + XGetGCValues (display, s->gc, GCForeground, &xgcv); + XSetForeground (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); + XSetForeground (display, s->gc, xgcv.foreground); } } @@ -3780,12 +3798,13 @@ x_draw_glyph_string (struct glyph_string *s) s->width, h); else { + Display *display = FRAME_X_DISPLAY (s->f); XGCValues xgcv; - XGetGCValues (s->display, s->gc, GCForeground, &xgcv); - XSetForeground (s->display, s->gc, s->face->strike_through_color); + XGetGCValues (display, s->gc, GCForeground, &xgcv); + XSetForeground (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); + XSetForeground (display, s->gc, xgcv.foreground); } } -- 2.21.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Remove display member of glyph_string 2019-05-09 3:52 [PATCH] Remove display member of glyph_string Alex Gramiak @ 2019-05-09 5:50 ` Eli Zaretskii 2019-05-09 16:07 ` Alex Gramiak 0 siblings, 1 reply; 9+ messages in thread From: Eli Zaretskii @ 2019-05-09 5:50 UTC (permalink / raw) To: Alex Gramiak; +Cc: emacs-devel > From: Alex Gramiak <agrambot@gmail.com> > Date: Wed, 08 May 2019 21:52:56 -0600 Each FRAME_X_DISPLAY call expands into applying a struct offset 4 times. Do we care about the (small) additional inefficiency? AFAIU, that was the cause for maintaining the result inside the glyph_string structure in the first place. > The only other location that FRAME_X_DISPLAY appears in non-X code is in > the argument to Free_Pixmap in image.c, which can hopefully be > refactored out in a later patch; at that point the other terms can > remove their trivial FRAME_X_DISPLAY definitions. So should we do both in one go, perhaps? Thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Remove display member of glyph_string 2019-05-09 5:50 ` Eli Zaretskii @ 2019-05-09 16:07 ` Alex Gramiak 2019-05-09 17:02 ` Eli Zaretskii 0 siblings, 1 reply; 9+ messages in thread From: Alex Gramiak @ 2019-05-09 16:07 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel [-- Attachment #1: Type: text/plain, Size: 1244 bytes --] Eli Zaretskii <eliz@gnu.org> writes: >> From: Alex Gramiak <agrambot@gmail.com> >> Date: Wed, 08 May 2019 21:52:56 -0600 > > Each FRAME_X_DISPLAY call expands into applying a struct offset 4 > times. Do we care about the (small) additional inefficiency? AFAIU, > that was the cause for maintaining the result inside the glyph_string > structure in the first place. I doubt that it's much of a difference. However, it should be noted that in cases where the member is used several times in a single procedure, the FRAME_X_DISPLAY method is comparable or better: x_clear_glyph_string_rect, x_draw_stretch_glyph_string: Before: 3 offsets After: 4 offsets x_draw_image_foreground_1: Before: 4 offsets After: 4 offsets x_draw_image_glyph_string: Before: up to 9 offsets After: 4 offsets x_draw_underwave: Before 3 + 2 * ceiling ((xmax - x1)/dx) offsets After: 4 offsets >> The only other location that FRAME_X_DISPLAY appears in non-X code is in >> the argument to Free_Pixmap in image.c, which can hopefully be >> refactored out in a later patch; at that point the other terms can >> remove their trivial FRAME_X_DISPLAY definitions. > > So should we do both in one go, perhaps? Sure, here's a patch that does it: [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: Free_Pixmap --] [-- Type: text/x-patch, Size: 8906 bytes --] From 749a61a0bda6a3023e6f4b19528c6a6be940c929 Mon Sep 17 00:00:00 2001 From: Alexander Gramiak <agrambot@gmail.com> Date: Thu, 9 May 2019 09:37:50 -0600 Subject: [PATCH] Convert Free_Pixmap macro into terminal hook * src/termhooks.h (terminal): New terminal hook free_pixmap. * src/image.c: Replace Free_Pixmap with free_pixmap. * src/msdos.h: * src/nsgui.h: * src/nsterm.h: * src/w32term.h: Remove unused X-compatibility macros and typedefs. * src/nsterm.m: * src/w32term.c: * src/xterm.c: Implement and set free_pixmap hook. --- src/image.c | 24 ++---------------------- src/msdos.h | 1 - src/nsgui.h | 1 - src/nsterm.h | 6 ------ src/nsterm.m | 16 ++++++++++++++-- src/termhooks.h | 8 ++++++++ src/w32term.c | 14 ++++++++++++++ src/w32term.h | 3 --- src/xterm.c | 12 ++++++++++++ 9 files changed, 50 insertions(+), 35 deletions(-) diff --git a/src/image.c b/src/image.c index e8cb434177..0779594989 100644 --- a/src/image.c +++ b/src/image.c @@ -1221,26 +1221,6 @@ four_corners_best (XImagePtr_or_DC ximg, int *corners, return best; } -/* Portability macros */ - -#ifdef HAVE_NTGUI - -#define Free_Pixmap(display, pixmap) \ - DeleteObject (pixmap) - -#elif defined (HAVE_NS) - -#define Free_Pixmap(display, pixmap) \ - ns_release_object (pixmap) - -#else - -#define Free_Pixmap(display, pixmap) \ - XFreePixmap (display, pixmap) - -#endif /* !HAVE_NTGUI && !HAVE_NS */ - - /* Return the `background' field of IMG. If IMG doesn't have one yet, it is guessed heuristically. If non-zero, XIMG is an existing XImage object (or device context with the image selected on W32) to @@ -1328,7 +1308,7 @@ image_clear_image_1 (struct frame *f, struct image *img, int flags) { if (img->pixmap) { - Free_Pixmap (FRAME_X_DISPLAY (f), img->pixmap); + FRAME_TERMINAL (f)->free_pixmap (f, img->pixmap); img->pixmap = NO_PIXMAP; /* NOTE (HAVE_NS): background color is NOT an indexed color! */ img->background_valid = 0; @@ -1347,7 +1327,7 @@ image_clear_image_1 (struct frame *f, struct image *img, int flags) { if (img->mask) { - Free_Pixmap (FRAME_X_DISPLAY (f), img->mask); + FRAME_TERMINAL (f)->free_pixmap (f, img->mask); img->mask = NO_PIXMAP; img->background_transparent_valid = 0; } diff --git a/src/msdos.h b/src/msdos.h index 0d15df7a33..90ceea8e3d 100644 --- a/src/msdos.h +++ b/src/msdos.h @@ -95,7 +95,6 @@ typedef struct tty_display_info Display_Info; extern struct tty_display_info the_only_display_info; extern struct tty_output the_only_tty_output; -#define FRAME_X_DISPLAY(f) ((Display *) 0) #define FRAME_FONT(f) ((f)->output_data.tty->font) #define FRAME_DISPLAY_INFO(f) (&the_only_display_info) diff --git a/src/nsgui.h b/src/nsgui.h index c147f4dec4..ab6cdff1e5 100644 --- a/src/nsgui.h +++ b/src/nsgui.h @@ -115,7 +115,6 @@ typedef NSColor * Color; typedef void * Color; #endif typedef int Window; -typedef int Display; /* Some sort of attempt to normalize rectangle handling. Seems a bit diff --git a/src/nsterm.h b/src/nsterm.h index 683f2dd934..ffaf809785 100644 --- a/src/nsterm.h +++ b/src/nsterm.h @@ -997,12 +997,6 @@ struct x_output #define FRAME_NS_WINDOW(f) ((f)->output_data.ns->window_desc) #define FRAME_NATIVE_WINDOW(f) FRAME_NS_WINDOW (f) -/* This is the `Display *' which frame F is on. */ -#define FRAME_NS_DISPLAY(f) (0) -#define FRAME_X_DISPLAY(f) (0) -#define FRAME_X_SCREEN(f) (0) -#define FRAME_X_VISUAL(f) FRAME_DISPLAY_INFO(f)->visual - #define FRAME_FOREGROUND_COLOR(f) ((f)->output_data.ns->foreground_color) #define FRAME_BACKGROUND_COLOR(f) ((f)->output_data.ns->background_color) diff --git a/src/nsterm.m b/src/nsterm.m index ffb7b7692b..d688aceca5 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -2515,8 +2515,7 @@ so some key presses (TAB) are swallowed by the system. */ /* Clear the mouse-moved flag for every frame on this display. */ FOR_EACH_FRAME (tail, frame) - if (FRAME_NS_P (XFRAME (frame)) - && FRAME_NS_DISPLAY (XFRAME (frame)) == FRAME_NS_DISPLAY (*fp)) + if (FRAME_NS_P (XFRAME (frame))) XFRAME (frame)->mouse_moved = 0; dpyinfo->last_mouse_scroll_bar = nil; @@ -4966,6 +4965,18 @@ in certain situations (rapid incoming events). [eview updateFrameSize: NO]; } +/* ========================================================================== + + Image Hooks + + ========================================================================== */ + +static void +ns_free_pixmap (struct frame *_f, Pixmap pixmap) +{ + ns_release_object (pixmap); +} + /* ========================================================================== Initialization @@ -5196,6 +5207,7 @@ static Lisp_Object ns_new_font (struct frame *f, Lisp_Object font_object, terminal->redeem_scroll_bar_hook = ns_redeem_scroll_bar; terminal->judge_scroll_bars_hook = ns_judge_scroll_bars; terminal->get_string_resource_hook = ns_get_string_resource; + terminal->free_pixmap = ns_free_pixmap; terminal->delete_frame_hook = ns_destroy_window; terminal->delete_terminal_hook = ns_delete_terminal; /* Other hooks are NULL by default. */ diff --git a/src/termhooks.h b/src/termhooks.h index 54f09e0303..617df86c5c 100644 --- a/src/termhooks.h +++ b/src/termhooks.h @@ -741,6 +741,14 @@ struct terminal const char *name, const char *class); \f + /* Image hooks */ + + /* Free the pixmap PIXMAP on F. */ + void (*free_pixmap) (struct frame *f, Pixmap pixmap); + +\f + /* Deletion hooks */ + /* Called to delete the device-specific portions of a frame that is on this terminal device. */ void (*delete_frame_hook) (struct frame *); diff --git a/src/w32term.c b/src/w32term.c index 0abec3d92a..435455e1a6 100644 --- a/src/w32term.c +++ b/src/w32term.c @@ -6869,6 +6869,7 @@ w32_wm_set_size_hint (struct frame *f, long flags, bool user_position) leave_crit (); } +\f /*********************************************************************** Fonts ***********************************************************************/ @@ -6940,6 +6941,18 @@ w32_toggle_invisible_pointer (struct frame *f, bool invisible) unblock_input (); } +\f +/*********************************************************************** + Image Hooks + ***********************************************************************/ + +static void +w32_free_pixmap (struct frame *_f, Pixmap pixmap) +{ + DeleteObject (pixmap); +} + +\f /*********************************************************************** Initialization ***********************************************************************/ @@ -7119,6 +7132,7 @@ w32_create_terminal (struct w32_display_info *dpyinfo) terminal->redeem_scroll_bar_hook = w32_redeem_scroll_bar; terminal->judge_scroll_bars_hook = w32_judge_scroll_bars; terminal->get_string_resource_hook = w32_get_string_resource; + terminal->free_pixmap = w32_free_pixmap; terminal->delete_frame_hook = w32_destroy_window; terminal->delete_terminal_hook = w32_delete_terminal; /* Other hooks are NULL by default. */ diff --git a/src/w32term.h b/src/w32term.h index de372d7e5d..a03b9fd331 100644 --- a/src/w32term.h +++ b/src/w32term.h @@ -420,9 +420,6 @@ extern struct w32_output w32term_display; /* This gives the w32_display_info structure for the display F is on. */ #define FRAME_DISPLAY_INFO(f) ((void) (f), (&one_w32_display_info)) -/* This is the `Display *' which frame F is on. */ -#define FRAME_X_DISPLAY(f) (0) - #define FRAME_NORMAL_PLACEMENT(F) ((F)->output_data.w32->normal_placement) #define FRAME_PREV_FSMODE(F) ((F)->output_data.w32->prev_fsmode) diff --git a/src/xterm.c b/src/xterm.c index 7bedcabe98..a7b84f46cf 100644 --- a/src/xterm.c +++ b/src/xterm.c @@ -12181,6 +12181,17 @@ x_check_font (struct frame *f, struct font *font) #endif /* GLYPH_DEBUG */ \f +/*********************************************************************** + Image Hooks + ***********************************************************************/ + +static void +x_free_pixmap (struct frame *f, Pixmap pixmap) +{ + XFreePixmap (FRAME_X_DISPLAY (f), pixmap); +} + +\f /*********************************************************************** Initialization ***********************************************************************/ @@ -13257,6 +13268,7 @@ x_create_terminal (struct x_display_info *dpyinfo) terminal->redeem_scroll_bar_hook = XTredeem_scroll_bar; terminal->judge_scroll_bars_hook = XTjudge_scroll_bars; terminal->get_string_resource_hook = x_get_string_resource; + terminal->free_pixmap = x_free_pixmap; terminal->delete_frame_hook = x_destroy_window; terminal->delete_terminal_hook = x_delete_terminal; /* Other hooks are NULL by default. */ -- 2.21.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Remove display member of glyph_string 2019-05-09 16:07 ` Alex Gramiak @ 2019-05-09 17:02 ` Eli Zaretskii 2019-05-09 17:16 ` Alex Gramiak 0 siblings, 1 reply; 9+ messages in thread From: Eli Zaretskii @ 2019-05-09 17:02 UTC (permalink / raw) To: Alex Gramiak; +Cc: emacs-devel > From: Alex Gramiak <agrambot@gmail.com> > Cc: emacs-devel@gnu.org > Date: Thu, 09 May 2019 10:07:25 -0600 > > >> The only other location that FRAME_X_DISPLAY appears in non-X code is in > >> the argument to Free_Pixmap in image.c, which can hopefully be > >> refactored out in a later patch; at that point the other terms can > >> remove their trivial FRAME_X_DISPLAY definitions. > > > > So should we do both in one go, perhaps? > > Sure, here's a patch that does it: Thanks. > * src/msdos.h: > * src/nsgui.h: > * src/nsterm.h: > * src/w32term.h: Remove unused X-compatibility macros and typedefs. Please mention the macros and typedefs being removed explicitly, it is important for later forensics. > src/image.c | 24 ++---------------------- > src/msdos.h | 1 - > src/nsgui.h | 1 - > src/nsterm.h | 6 ------ > src/nsterm.m | 16 ++++++++++++++-- > src/termhooks.h | 8 ++++++++ > src/w32term.c | 14 ++++++++++++++ > src/w32term.h | 3 --- > src/xterm.c | 12 ++++++++++++ > 9 files changed, 50 insertions(+), 35 deletions(-) There's one more instance of FRAME_X_DISPLAY in xdisp.c which was left, and it will fail compilation on non-X platforms. Otherwise, this LGTM, thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Remove display member of glyph_string 2019-05-09 17:02 ` Eli Zaretskii @ 2019-05-09 17:16 ` Alex Gramiak 2019-05-09 17:59 ` Eli Zaretskii 0 siblings, 1 reply; 9+ messages in thread From: Alex Gramiak @ 2019-05-09 17:16 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel [-- Attachment #1: Type: text/plain, Size: 968 bytes --] Eli Zaretskii <eliz@gnu.org> writes: >> * src/msdos.h: >> * src/nsgui.h: >> * src/nsterm.h: >> * src/w32term.h: Remove unused X-compatibility macros and typedefs. > > Please mention the macros and typedefs being removed explicitly, it is > important for later forensics. Okay, I've attached a reworded patch. >> src/image.c | 24 ++---------------------- >> src/msdos.h | 1 - >> src/nsgui.h | 1 - >> src/nsterm.h | 6 ------ >> src/nsterm.m | 16 ++++++++++++++-- >> src/termhooks.h | 8 ++++++++ >> src/w32term.c | 14 ++++++++++++++ >> src/w32term.h | 3 --- >> src/xterm.c | 12 ++++++++++++ >> 9 files changed, 50 insertions(+), 35 deletions(-) > > There's one more instance of FRAME_X_DISPLAY in xdisp.c which was > left, and it will fail compilation on non-X platforms. > > Otherwise, this LGTM, thanks. I only see one such instance, and it's surrounded by: #if false && defined HAVE_X_WINDOWS So it should be good, no? [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: reworded --] [-- Type: text/x-patch, Size: 9021 bytes --] From c05ab038c280ba7c6041f81ceecf818ddb5bbf03 Mon Sep 17 00:00:00 2001 From: Alexander Gramiak <agrambot@gmail.com> Date: Thu, 9 May 2019 09:37:50 -0600 Subject: [PATCH] Convert Free_Pixmap macro into terminal hook * src/termhooks.h (terminal): New terminal hook free_pixmap. * src/image.c: Replace Free_Pixmap with free_pixmap. * src/msdos.h (FRAME_X_DISPLAY): * src/nsgui.h (Display): * src/nsterm.h (FRAME_NS_DISPLAY, FRAME_X_DISPLAY, FRAME_X_SCREEN) (FRAME_X_VISUAL): * src/w32term.h (FRAME_X_DISPLAY): Remove unused X-compatibility macros and typedefs. * src/nsterm.m: * src/w32term.c: * src/xterm.c: Implement and set free_pixmap hook. --- src/image.c | 24 ++---------------------- src/msdos.h | 1 - src/nsgui.h | 1 - src/nsterm.h | 6 ------ src/nsterm.m | 16 ++++++++++++++-- src/termhooks.h | 8 ++++++++ src/w32term.c | 14 ++++++++++++++ src/w32term.h | 3 --- src/xterm.c | 12 ++++++++++++ 9 files changed, 50 insertions(+), 35 deletions(-) diff --git a/src/image.c b/src/image.c index e8cb434177..0779594989 100644 --- a/src/image.c +++ b/src/image.c @@ -1221,26 +1221,6 @@ four_corners_best (XImagePtr_or_DC ximg, int *corners, return best; } -/* Portability macros */ - -#ifdef HAVE_NTGUI - -#define Free_Pixmap(display, pixmap) \ - DeleteObject (pixmap) - -#elif defined (HAVE_NS) - -#define Free_Pixmap(display, pixmap) \ - ns_release_object (pixmap) - -#else - -#define Free_Pixmap(display, pixmap) \ - XFreePixmap (display, pixmap) - -#endif /* !HAVE_NTGUI && !HAVE_NS */ - - /* Return the `background' field of IMG. If IMG doesn't have one yet, it is guessed heuristically. If non-zero, XIMG is an existing XImage object (or device context with the image selected on W32) to @@ -1328,7 +1308,7 @@ image_clear_image_1 (struct frame *f, struct image *img, int flags) { if (img->pixmap) { - Free_Pixmap (FRAME_X_DISPLAY (f), img->pixmap); + FRAME_TERMINAL (f)->free_pixmap (f, img->pixmap); img->pixmap = NO_PIXMAP; /* NOTE (HAVE_NS): background color is NOT an indexed color! */ img->background_valid = 0; @@ -1347,7 +1327,7 @@ image_clear_image_1 (struct frame *f, struct image *img, int flags) { if (img->mask) { - Free_Pixmap (FRAME_X_DISPLAY (f), img->mask); + FRAME_TERMINAL (f)->free_pixmap (f, img->mask); img->mask = NO_PIXMAP; img->background_transparent_valid = 0; } diff --git a/src/msdos.h b/src/msdos.h index 0d15df7a33..90ceea8e3d 100644 --- a/src/msdos.h +++ b/src/msdos.h @@ -95,7 +95,6 @@ typedef struct tty_display_info Display_Info; extern struct tty_display_info the_only_display_info; extern struct tty_output the_only_tty_output; -#define FRAME_X_DISPLAY(f) ((Display *) 0) #define FRAME_FONT(f) ((f)->output_data.tty->font) #define FRAME_DISPLAY_INFO(f) (&the_only_display_info) diff --git a/src/nsgui.h b/src/nsgui.h index c147f4dec4..ab6cdff1e5 100644 --- a/src/nsgui.h +++ b/src/nsgui.h @@ -115,7 +115,6 @@ typedef NSColor * Color; typedef void * Color; #endif typedef int Window; -typedef int Display; /* Some sort of attempt to normalize rectangle handling. Seems a bit diff --git a/src/nsterm.h b/src/nsterm.h index 683f2dd934..ffaf809785 100644 --- a/src/nsterm.h +++ b/src/nsterm.h @@ -997,12 +997,6 @@ struct x_output #define FRAME_NS_WINDOW(f) ((f)->output_data.ns->window_desc) #define FRAME_NATIVE_WINDOW(f) FRAME_NS_WINDOW (f) -/* This is the `Display *' which frame F is on. */ -#define FRAME_NS_DISPLAY(f) (0) -#define FRAME_X_DISPLAY(f) (0) -#define FRAME_X_SCREEN(f) (0) -#define FRAME_X_VISUAL(f) FRAME_DISPLAY_INFO(f)->visual - #define FRAME_FOREGROUND_COLOR(f) ((f)->output_data.ns->foreground_color) #define FRAME_BACKGROUND_COLOR(f) ((f)->output_data.ns->background_color) diff --git a/src/nsterm.m b/src/nsterm.m index ffb7b7692b..d688aceca5 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -2515,8 +2515,7 @@ so some key presses (TAB) are swallowed by the system. */ /* Clear the mouse-moved flag for every frame on this display. */ FOR_EACH_FRAME (tail, frame) - if (FRAME_NS_P (XFRAME (frame)) - && FRAME_NS_DISPLAY (XFRAME (frame)) == FRAME_NS_DISPLAY (*fp)) + if (FRAME_NS_P (XFRAME (frame))) XFRAME (frame)->mouse_moved = 0; dpyinfo->last_mouse_scroll_bar = nil; @@ -4966,6 +4965,18 @@ in certain situations (rapid incoming events). [eview updateFrameSize: NO]; } +/* ========================================================================== + + Image Hooks + + ========================================================================== */ + +static void +ns_free_pixmap (struct frame *_f, Pixmap pixmap) +{ + ns_release_object (pixmap); +} + /* ========================================================================== Initialization @@ -5196,6 +5207,7 @@ static Lisp_Object ns_new_font (struct frame *f, Lisp_Object font_object, terminal->redeem_scroll_bar_hook = ns_redeem_scroll_bar; terminal->judge_scroll_bars_hook = ns_judge_scroll_bars; terminal->get_string_resource_hook = ns_get_string_resource; + terminal->free_pixmap = ns_free_pixmap; terminal->delete_frame_hook = ns_destroy_window; terminal->delete_terminal_hook = ns_delete_terminal; /* Other hooks are NULL by default. */ diff --git a/src/termhooks.h b/src/termhooks.h index 54f09e0303..617df86c5c 100644 --- a/src/termhooks.h +++ b/src/termhooks.h @@ -741,6 +741,14 @@ struct terminal const char *name, const char *class); \f + /* Image hooks */ + + /* Free the pixmap PIXMAP on F. */ + void (*free_pixmap) (struct frame *f, Pixmap pixmap); + +\f + /* Deletion hooks */ + /* Called to delete the device-specific portions of a frame that is on this terminal device. */ void (*delete_frame_hook) (struct frame *); diff --git a/src/w32term.c b/src/w32term.c index 0abec3d92a..435455e1a6 100644 --- a/src/w32term.c +++ b/src/w32term.c @@ -6869,6 +6869,7 @@ w32_wm_set_size_hint (struct frame *f, long flags, bool user_position) leave_crit (); } +\f /*********************************************************************** Fonts ***********************************************************************/ @@ -6940,6 +6941,18 @@ w32_toggle_invisible_pointer (struct frame *f, bool invisible) unblock_input (); } +\f +/*********************************************************************** + Image Hooks + ***********************************************************************/ + +static void +w32_free_pixmap (struct frame *_f, Pixmap pixmap) +{ + DeleteObject (pixmap); +} + +\f /*********************************************************************** Initialization ***********************************************************************/ @@ -7119,6 +7132,7 @@ w32_create_terminal (struct w32_display_info *dpyinfo) terminal->redeem_scroll_bar_hook = w32_redeem_scroll_bar; terminal->judge_scroll_bars_hook = w32_judge_scroll_bars; terminal->get_string_resource_hook = w32_get_string_resource; + terminal->free_pixmap = w32_free_pixmap; terminal->delete_frame_hook = w32_destroy_window; terminal->delete_terminal_hook = w32_delete_terminal; /* Other hooks are NULL by default. */ diff --git a/src/w32term.h b/src/w32term.h index de372d7e5d..a03b9fd331 100644 --- a/src/w32term.h +++ b/src/w32term.h @@ -420,9 +420,6 @@ extern struct w32_output w32term_display; /* This gives the w32_display_info structure for the display F is on. */ #define FRAME_DISPLAY_INFO(f) ((void) (f), (&one_w32_display_info)) -/* This is the `Display *' which frame F is on. */ -#define FRAME_X_DISPLAY(f) (0) - #define FRAME_NORMAL_PLACEMENT(F) ((F)->output_data.w32->normal_placement) #define FRAME_PREV_FSMODE(F) ((F)->output_data.w32->prev_fsmode) diff --git a/src/xterm.c b/src/xterm.c index 7bedcabe98..a7b84f46cf 100644 --- a/src/xterm.c +++ b/src/xterm.c @@ -12181,6 +12181,17 @@ x_check_font (struct frame *f, struct font *font) #endif /* GLYPH_DEBUG */ \f +/*********************************************************************** + Image Hooks + ***********************************************************************/ + +static void +x_free_pixmap (struct frame *f, Pixmap pixmap) +{ + XFreePixmap (FRAME_X_DISPLAY (f), pixmap); +} + +\f /*********************************************************************** Initialization ***********************************************************************/ @@ -13257,6 +13268,7 @@ x_create_terminal (struct x_display_info *dpyinfo) terminal->redeem_scroll_bar_hook = XTredeem_scroll_bar; terminal->judge_scroll_bars_hook = XTjudge_scroll_bars; terminal->get_string_resource_hook = x_get_string_resource; + terminal->free_pixmap = x_free_pixmap; terminal->delete_frame_hook = x_destroy_window; terminal->delete_terminal_hook = x_delete_terminal; /* Other hooks are NULL by default. */ -- 2.21.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Remove display member of glyph_string 2019-05-09 17:16 ` Alex Gramiak @ 2019-05-09 17:59 ` Eli Zaretskii 2019-05-09 18:21 ` Alex Gramiak 0 siblings, 1 reply; 9+ messages in thread From: Eli Zaretskii @ 2019-05-09 17:59 UTC (permalink / raw) To: Alex Gramiak; +Cc: emacs-devel > From: Alex Gramiak <agrambot@gmail.com> > Cc: emacs-devel@gnu.org > Date: Thu, 09 May 2019 11:16:06 -0600 > > > There's one more instance of FRAME_X_DISPLAY in xdisp.c which was > > left, and it will fail compilation on non-X platforms. > > > > Otherwise, this LGTM, thanks. > > I only see one such instance, and it's surrounded by: > > #if false && defined HAVE_X_WINDOWS > > So it should be good, no? That one, yes. But there's one more, and it isn't conditioned by HAVE_X_WINDOWS. Are you maybe looking on a wrong branch? I'm on master. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Remove display member of glyph_string 2019-05-09 17:59 ` Eli Zaretskii @ 2019-05-09 18:21 ` Alex Gramiak 2019-05-10 5:30 ` Eli Zaretskii 0 siblings, 1 reply; 9+ messages in thread From: Alex Gramiak @ 2019-05-09 18:21 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel [-- Attachment #1: Type: text/plain, Size: 727 bytes --] Eli Zaretskii <eliz@gnu.org> writes: >> From: Alex Gramiak <agrambot@gmail.com> >> Cc: emacs-devel@gnu.org >> Date: Thu, 09 May 2019 11:16:06 -0600 >> >> > There's one more instance of FRAME_X_DISPLAY in xdisp.c which was >> > left, and it will fail compilation on non-X platforms. >> > >> > Otherwise, this LGTM, thanks. >> >> I only see one such instance, and it's surrounded by: >> >> #if false && defined HAVE_X_WINDOWS >> >> So it should be good, no? > > That one, yes. But there's one more, and it isn't conditioned by > HAVE_X_WINDOWS. Are you maybe looking on a wrong branch? I'm on > master. You mean the one in init_glyph_string, right? That was removed in my first patch, which I've attached again below: [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Remove-display-member-of-glyph_string.patch --] [-- Type: text/x-patch, Size: 20843 bytes --] From d6f7b845573bcdd81552851f9d87cfd5179c7d70 Mon Sep 17 00:00:00 2001 From: Alexander Gramiak <agrambot@gmail.com> Date: Wed, 8 May 2019 21:40:24 -0600 Subject: [PATCH] Remove display member of glyph_string This member has little value even on X, and it leaks internal backend details to the glyph_string struct. * src/dispextern.h (glyph_string): Remove X display member. * src/xdisp.c (init_glyph_string): Remove initialization of display. * src/xfont.c (xfont_draw): * src/xterm.c: Use FRAME_X_DISPLAY (s->f) instead of display member. --- src/dispextern.h | 3 - src/xdisp.c | 1 - src/xfont.c | 19 ++++--- src/xterm.c | 141 +++++++++++++++++++++++++++-------------------- 4 files changed, 90 insertions(+), 74 deletions(-) diff --git a/src/dispextern.h b/src/dispextern.h index bb981f83fc..619f4c0767 100644 --- a/src/dispextern.h +++ b/src/dispextern.h @@ -1281,9 +1281,6 @@ struct glyph_string /* The window on which the glyph string is drawn. */ struct window *w; - /* X display and window for convenience. */ - Display *display; - /* The glyph row for which this string was built. It determines the y-origin and height of the string. */ struct glyph_row *row; diff --git a/src/xdisp.c b/src/xdisp.c index d380645c84..1aa677fcc7 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -25953,7 +25953,6 @@ init_glyph_string (struct glyph_string *s, #ifdef HAVE_NTGUI s->hdc = hdc; #endif - s->display = FRAME_X_DISPLAY (s->f); s->char2b = char2b; s->hl = hl; s->row = row; diff --git a/src/xfont.c b/src/xfont.c index 5ecbd6de33..ff80df407d 100644 --- a/src/xfont.c +++ b/src/xfont.c @@ -1000,6 +1000,7 @@ xfont_draw (struct glyph_string *s, int from, int to, int x, int y, bool with_background) { XFontStruct *xfont = ((struct xfont_info *) s->font)->xfont; + Display *display = FRAME_X_DISPLAY (s->f); int len = to - from; GC gc = s->gc; int i; @@ -1007,7 +1008,7 @@ xfont_draw (struct glyph_string *s, int from, int to, int x, int y, if (s->gc != s->face->gc) { block_input (); - XSetFont (s->display, gc, xfont->fid); + XSetFont (display, gc, xfont->fid); unblock_input (); } @@ -1022,20 +1023,20 @@ xfont_draw (struct glyph_string *s, int from, int to, int x, int y, { if (s->padding_p) for (i = 0; i < len; i++) - XDrawImageString (FRAME_X_DISPLAY (s->f), FRAME_X_DRAWABLE (s->f), + XDrawImageString (display, FRAME_X_DRAWABLE (s->f), gc, x + i, y, str + i, 1); else - XDrawImageString (FRAME_X_DISPLAY (s->f), FRAME_X_DRAWABLE (s->f), + XDrawImageString (display, FRAME_X_DRAWABLE (s->f), gc, x, y, str, len); } else { if (s->padding_p) for (i = 0; i < len; i++) - XDrawString (FRAME_X_DISPLAY (s->f), FRAME_X_DRAWABLE (s->f), + XDrawString (display, FRAME_X_DRAWABLE (s->f), gc, x + i, y, str + i, 1); else - XDrawString (FRAME_X_DISPLAY (s->f), FRAME_X_DRAWABLE (s->f), + XDrawString (display, FRAME_X_DRAWABLE (s->f), gc, x, y, str, len); } unblock_input (); @@ -1048,20 +1049,20 @@ xfont_draw (struct glyph_string *s, int from, int to, int x, int y, { if (s->padding_p) for (i = 0; i < len; i++) - XDrawImageString16 (FRAME_X_DISPLAY (s->f), FRAME_X_DRAWABLE (s->f), + XDrawImageString16 (display, FRAME_X_DRAWABLE (s->f), gc, x + i, y, s->char2b + from + i, 1); else - XDrawImageString16 (FRAME_X_DISPLAY (s->f), FRAME_X_DRAWABLE (s->f), + XDrawImageString16 (display, FRAME_X_DRAWABLE (s->f), gc, x, y, s->char2b + from, len); } else { if (s->padding_p) for (i = 0; i < len; i++) - XDrawString16 (FRAME_X_DISPLAY (s->f), FRAME_X_DRAWABLE (s->f), + XDrawString16 (display, FRAME_X_DRAWABLE (s->f), gc, x + i, y, s->char2b + from + i, 1); else - XDrawString16 (FRAME_X_DISPLAY (s->f), FRAME_X_DRAWABLE (s->f), + XDrawString16 (display, FRAME_X_DRAWABLE (s->f), gc, x, y, s->char2b + from, len); } unblock_input (); diff --git a/src/xterm.c b/src/xterm.c index 26f74cde91..7bedcabe98 100644 --- a/src/xterm.c +++ b/src/xterm.c @@ -1454,6 +1454,7 @@ x_set_cursor_gc (struct glyph_string *s) /* Cursor on non-default face: must merge. */ XGCValues xgcv; unsigned long mask; + Display *display = FRAME_X_DISPLAY (s->f); xgcv.background = s->f->output_data.x->cursor_pixel; xgcv.foreground = s->face->background; @@ -1479,11 +1480,11 @@ x_set_cursor_gc (struct glyph_string *s) mask = GCForeground | GCBackground | GCGraphicsExposures; if (FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc) - XChangeGC (s->display, FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc, + XChangeGC (display, FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc, mask, &xgcv); else FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc - = XCreateGC (s->display, FRAME_X_DRAWABLE (s->f), mask, &xgcv); + = XCreateGC (display, FRAME_X_DRAWABLE (s->f), mask, &xgcv); s->gc = FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc; } @@ -1519,6 +1520,7 @@ x_set_mouse_face_gc (struct glyph_string *s) except for FONT. */ XGCValues xgcv; unsigned long mask; + Display *display = FRAME_X_DISPLAY (s->f); xgcv.background = s->face->background; xgcv.foreground = s->face->foreground; @@ -1526,11 +1528,11 @@ x_set_mouse_face_gc (struct glyph_string *s) mask = GCForeground | GCBackground | GCGraphicsExposures; if (FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc) - XChangeGC (s->display, FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc, + XChangeGC (display, FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc, mask, &xgcv); else FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc - = XCreateGC (s->display, FRAME_X_DRAWABLE (s->f), mask, &xgcv); + = XCreateGC (display, FRAME_X_DRAWABLE (s->f), mask, &xgcv); s->gc = FRAME_DISPLAY_INFO (s->f)->scratch_cursor_gc; @@ -1672,11 +1674,12 @@ x_compute_glyph_string_overhangs (struct glyph_string *s) static void x_clear_glyph_string_rect (struct glyph_string *s, int x, int y, int w, int h) { + Display *display = FRAME_X_DISPLAY (s->f); XGCValues xgcv; - XGetGCValues (s->display, s->gc, GCForeground | GCBackground, &xgcv); - XSetForeground (s->display, s->gc, xgcv.background); + XGetGCValues (display, s->gc, GCForeground | GCBackground, &xgcv); + XSetForeground (display, s->gc, xgcv.background); x_fill_rectangle (s->f, s->gc, x, y, w, h); - XSetForeground (s->display, s->gc, xgcv.foreground); + XSetForeground (display, s->gc, xgcv.foreground); } @@ -1697,13 +1700,15 @@ x_draw_glyph_string_background (struct glyph_string *s, bool force_p) if (s->stippled_p) { + Display *display = FRAME_X_DISPLAY (s->f); + /* Fill background with a stipple pattern. */ - XSetFillStyle (s->display, s->gc, FillOpaqueStippled); + XSetFillStyle (display, s->gc, FillOpaqueStippled); x_fill_rectangle (s->f, s->gc, s->x, s->y + box_line_width, s->background_width, s->height - 2 * box_line_width); - XSetFillStyle (s->display, s->gc, FillSolid); + XSetFillStyle (display, s->gc, FillSolid); s->background_filled_p = true; } else if (FONT_HEIGHT (s->font) < s->height - 2 * box_line_width @@ -2591,7 +2596,7 @@ x_setup_relief_colors (struct glyph_string *s) XGCValues xgcv; /* Get the background color of the face. */ - XGetGCValues (s->display, s->gc, GCBackground, &xgcv); + XGetGCValues (FRAME_X_DISPLAY (s->f), s->gc, GCBackground, &xgcv); color = xgcv.background; } @@ -2801,10 +2806,11 @@ x_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, XRectangle *clip_rect) { + Display *display = FRAME_X_DISPLAY (s->f); XGCValues xgcv; - XGetGCValues (s->display, s->gc, GCForeground, &xgcv); - XSetForeground (s->display, s->gc, s->face->box_color); + XGetGCValues (display, s->gc, GCForeground, &xgcv); + XSetForeground (display, s->gc, s->face->box_color); x_set_clip_rectangles (s->f, s->gc, clip_rect, 1); /* Top. */ @@ -2825,7 +2831,7 @@ x_draw_box_rect (struct glyph_string *s, x_fill_rectangle (s->f, s->gc, right_x - width + 1, top_y, width, bottom_y - top_y + 1); - XSetForeground (s->display, s->gc, xgcv.foreground); + XSetForeground (display, s->gc, xgcv.foreground); x_reset_clip_rectangles (s->f, s->gc); } @@ -2888,6 +2894,7 @@ x_composite_image (struct glyph_string *s, Pixmap dest, int srcX, int srcY, int dstX, int dstY, int width, int height) { + Display *display = FRAME_X_DISPLAY (s->f); #ifdef HAVE_XRENDER if (s->img->picture) { @@ -2897,27 +2904,27 @@ x_composite_image (struct glyph_string *s, Pixmap dest, /* FIXME: Should we do this each time or would it make sense to store destination in the frame struct? */ - default_format = XRenderFindVisualFormat (s->display, - DefaultVisual (s->display, 0)); - destination = XRenderCreatePicture (s->display, dest, + default_format = XRenderFindVisualFormat (display, + DefaultVisual (display, 0)); + destination = XRenderCreatePicture (display, dest, default_format, 0, &attr); /* FIXME: It may make sense to use PictOpSrc instead of PictOpOver, as I don't know if we care about alpha values too much here. */ - XRenderComposite (s->display, PictOpOver, + XRenderComposite (display, PictOpOver, s->img->picture, s->img->mask_picture, destination, srcX, srcY, srcX, srcY, dstX, dstY, width, height); - XRenderFreePicture (s->display, destination); + XRenderFreePicture (display, destination); return; } #endif - XCopyArea (s->display, s->img->pixmap, + XCopyArea (display, s->img->pixmap, dest, s->gc, srcX, srcY, width, height, dstX, dstY); @@ -2992,7 +2999,7 @@ x_draw_image_foreground (struct glyph_string *s) xgcv.clip_x_origin = x; xgcv.clip_y_origin = y; xgcv.function = GXcopy; - XChangeGC (s->display, s->gc, mask, &xgcv); + XChangeGC (FRAME_X_DISPLAY (s->f), s->gc, mask, &xgcv); get_glyph_string_clip_rect (s, &clip_rect); image_rect.x = x; @@ -3141,6 +3148,8 @@ x_draw_image_foreground_1 (struct glyph_string *s, Pixmap pixmap) if (s->img->pixmap) { + Display *display = FRAME_X_DISPLAY (s->f); + if (s->img->mask) { /* We can't set both a clip mask and use XSetClipRectangles @@ -3156,16 +3165,16 @@ x_draw_image_foreground_1 (struct glyph_string *s, Pixmap pixmap) xgcv.clip_x_origin = x - s->slice.x; xgcv.clip_y_origin = y - s->slice.y; xgcv.function = GXcopy; - XChangeGC (s->display, s->gc, mask, &xgcv); + XChangeGC (display, s->gc, mask, &xgcv); - XCopyArea (s->display, s->img->pixmap, pixmap, s->gc, + XCopyArea (display, s->img->pixmap, pixmap, s->gc, s->slice.x, s->slice.y, s->slice.width, s->slice.height, x, y); - XSetClipMask (s->display, s->gc, None); + XSetClipMask (display, s->gc, None); } else { - XCopyArea (s->display, s->img->pixmap, pixmap, s->gc, + XCopyArea (display, s->img->pixmap, pixmap, s->gc, s->slice.x, s->slice.y, s->slice.width, s->slice.height, x, y); @@ -3200,10 +3209,12 @@ x_draw_glyph_string_bg_rect (struct glyph_string *s, int x, int y, int w, int h) { if (s->stippled_p) { + Display *display = FRAME_X_DISPLAY (s->f); + /* Fill background with a stipple pattern. */ - XSetFillStyle (s->display, s->gc, FillOpaqueStippled); + XSetFillStyle (display, s->gc, FillOpaqueStippled); x_fill_rectangle (s->f, s->gc, x, y, w, h); - XSetFillStyle (s->display, s->gc, FillSolid); + XSetFillStyle (display, s->gc, FillSolid); } else x_clear_glyph_string_rect (s, x, y, w, h); @@ -3230,6 +3241,7 @@ x_draw_image_glyph_string (struct glyph_string *s) int box_line_hwidth = eabs (s->face->box_line_width); int box_line_vwidth = max (s->face->box_line_width, 0); int height; + Display *display = FRAME_X_DISPLAY (s->f); #ifndef USE_CAIRO Pixmap pixmap = None; #endif @@ -3261,34 +3273,34 @@ x_draw_image_glyph_string (struct glyph_string *s) int depth = DefaultDepthOfScreen (screen); /* Create a pixmap as large as the glyph string. */ - pixmap = XCreatePixmap (s->display, FRAME_X_DRAWABLE (s->f), + pixmap = XCreatePixmap (display, FRAME_X_DRAWABLE (s->f), s->background_width, s->height, depth); /* Don't clip in the following because we're working on the pixmap. */ - XSetClipMask (s->display, s->gc, None); + XSetClipMask (display, s->gc, None); /* Fill the pixmap with the background color/stipple. */ if (s->stippled_p) { /* Fill background with a stipple pattern. */ - XSetFillStyle (s->display, s->gc, FillOpaqueStippled); - XSetTSOrigin (s->display, s->gc, - s->x, - s->y); - XFillRectangle (s->display, pixmap, s->gc, + XSetFillStyle (display, s->gc, FillOpaqueStippled); + XSetTSOrigin (display, s->gc, - s->x, - s->y); + XFillRectangle (display, pixmap, s->gc, 0, 0, s->background_width, s->height); - XSetFillStyle (s->display, s->gc, FillSolid); - XSetTSOrigin (s->display, s->gc, 0, 0); + XSetFillStyle (display, s->gc, FillSolid); + XSetTSOrigin (display, s->gc, 0, 0); } else { XGCValues xgcv; - XGetGCValues (s->display, s->gc, GCForeground | GCBackground, + XGetGCValues (display, s->gc, GCForeground | GCBackground, &xgcv); - XSetForeground (s->display, s->gc, xgcv.background); - XFillRectangle (s->display, pixmap, s->gc, + XSetForeground (display, s->gc, xgcv.background); + XFillRectangle (display, pixmap, s->gc, 0, 0, s->background_width, s->height); - XSetForeground (s->display, s->gc, xgcv.foreground); + XSetForeground (display, s->gc, xgcv.foreground); } } else @@ -3320,9 +3332,9 @@ x_draw_image_glyph_string (struct glyph_string *s) { x_draw_image_foreground_1 (s, pixmap); x_set_glyph_string_clipping (s); - XCopyArea (s->display, pixmap, FRAME_X_DRAWABLE (s->f), s->gc, + XCopyArea (display, pixmap, FRAME_X_DRAWABLE (s->f), s->gc, 0, 0, s->background_width, s->height, s->x, s->y); - XFreePixmap (s->display, pixmap); + XFreePixmap (display, pixmap); } else #endif /* ! USE_CAIRO */ @@ -3383,6 +3395,7 @@ x_draw_stretch_glyph_string (struct glyph_string *s) { int y = s->y; int w = background_width - width, h = s->height; + Display *display = FRAME_X_DISPLAY (s->f); XRectangle r; GC gc; @@ -3405,17 +3418,17 @@ x_draw_stretch_glyph_string (struct glyph_string *s) if (s->face->stipple) { /* Fill background with a stipple pattern. */ - XSetFillStyle (s->display, gc, FillOpaqueStippled); + XSetFillStyle (display, gc, FillOpaqueStippled); x_fill_rectangle (s->f, gc, x, y, w, h); - XSetFillStyle (s->display, gc, FillSolid); + XSetFillStyle (display, gc, FillSolid); } else { XGCValues xgcv; - XGetGCValues (s->display, gc, GCForeground | GCBackground, &xgcv); - XSetForeground (s->display, gc, xgcv.background); + XGetGCValues (display, gc, GCForeground | GCBackground, &xgcv); + XSetForeground (display, gc, xgcv.background); x_fill_rectangle (s->f, gc, x, y, w, h); - XSetForeground (s->display, gc, xgcv.foreground); + XSetForeground (display, gc, xgcv.foreground); } x_reset_clip_rectangles (s->f, gc); @@ -3470,10 +3483,12 @@ x_get_scale_factor(Display *disp, int *scale_x, int *scale_y) static void x_draw_underwave (struct glyph_string *s) { + Display *display = FRAME_X_DISPLAY (s->f); + /* Adjust for scale/HiDPI. */ int scale_x, scale_y; - x_get_scale_factor (s->display, &scale_x, &scale_y); + x_get_scale_factor (display, &scale_x, &scale_y); int wave_height = 3 * scale_y, wave_length = 2 * scale_x; @@ -3503,7 +3518,7 @@ x_draw_underwave (struct glyph_string *s) if (!gui_intersect_rectangles (&wave_clip, &string_clip, &final_clip)) return; - XSetClipRectangles (s->display, s->gc, 0, 0, &final_clip, 1, Unsorted); + XSetClipRectangles (display, s->gc, 0, 0, &final_clip, 1, Unsorted); /* Draw the waves */ @@ -3522,16 +3537,16 @@ x_draw_underwave (struct glyph_string *s) while (x1 <= xmax) { - XSetLineAttributes (s->display, s->gc, thickness, LineSolid, CapButt, + XSetLineAttributes (display, s->gc, thickness, LineSolid, CapButt, JoinRound); - XDrawLine (s->display, FRAME_X_DRAWABLE (s->f), s->gc, x1, y1, x2, y2); + XDrawLine (display, FRAME_X_DRAWABLE (s->f), s->gc, x1, y1, x2, y2); x1 = x2, y1 = y2; x2 += dx, y2 = y0 + odd*dy; odd = !odd; } /* Restore previous clipping rectangle(s) */ - XSetClipRectangles (s->display, s->gc, 0, 0, s->clip, s->num_clips, Unsorted); + XSetClipRectangles (display, s->gc, 0, 0, s->clip, s->num_clips, Unsorted); #endif /* not USE_CAIRO */ } @@ -3648,11 +3663,12 @@ x_draw_glyph_string (struct glyph_string *s) x_draw_underwave (s); else { + Display *display = FRAME_X_DISPLAY (s->f); XGCValues xgcv; - XGetGCValues (s->display, s->gc, GCForeground, &xgcv); - XSetForeground (s->display, s->gc, s->face->underline_color); + XGetGCValues (display, s->gc, GCForeground, &xgcv); + XSetForeground (display, s->gc, s->face->underline_color); x_draw_underwave (s); - XSetForeground (s->display, s->gc, xgcv.foreground); + XSetForeground (display, s->gc, xgcv.foreground); } } else if (s->face->underline_type == FACE_UNDER_LINE) @@ -3732,12 +3748,13 @@ x_draw_glyph_string (struct glyph_string *s) s->x, y, s->width, thickness); else { + Display *display = FRAME_X_DISPLAY (s->f); XGCValues xgcv; - XGetGCValues (s->display, s->gc, GCForeground, &xgcv); - XSetForeground (s->display, s->gc, s->face->underline_color); + XGetGCValues (display, s->gc, GCForeground, &xgcv); + XSetForeground (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); + XSetForeground (display, s->gc, xgcv.foreground); } } } @@ -3751,12 +3768,13 @@ x_draw_glyph_string (struct glyph_string *s) s->width, h); else { + Display *display = FRAME_X_DISPLAY (s->f); XGCValues xgcv; - XGetGCValues (s->display, s->gc, GCForeground, &xgcv); - XSetForeground (s->display, s->gc, s->face->overline_color); + XGetGCValues (display, s->gc, GCForeground, &xgcv); + XSetForeground (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); + XSetForeground (display, s->gc, xgcv.foreground); } } @@ -3780,12 +3798,13 @@ x_draw_glyph_string (struct glyph_string *s) s->width, h); else { + Display *display = FRAME_X_DISPLAY (s->f); XGCValues xgcv; - XGetGCValues (s->display, s->gc, GCForeground, &xgcv); - XSetForeground (s->display, s->gc, s->face->strike_through_color); + XGetGCValues (display, s->gc, GCForeground, &xgcv); + XSetForeground (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); + XSetForeground (display, s->gc, xgcv.foreground); } } -- 2.21.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Remove display member of glyph_string 2019-05-09 18:21 ` Alex Gramiak @ 2019-05-10 5:30 ` Eli Zaretskii 2019-05-11 4:20 ` Alex Gramiak 0 siblings, 1 reply; 9+ messages in thread From: Eli Zaretskii @ 2019-05-10 5:30 UTC (permalink / raw) To: Alex Gramiak; +Cc: emacs-devel > From: Alex Gramiak <agrambot@gmail.com> > Cc: emacs-devel@gnu.org > Date: Thu, 09 May 2019 12:21:07 -0600 > > > That one, yes. But there's one more, and it isn't conditioned by > > HAVE_X_WINDOWS. Are you maybe looking on a wrong branch? I'm on > > master. > > You mean the one in init_glyph_string, right? That was removed in my > first patch, which I've attached again below: Thanks, you never said that you were sending only part of a patch. (I never liked breaking patches into several parts, as they make the review harder in many cases.) ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Remove display member of glyph_string 2019-05-10 5:30 ` Eli Zaretskii @ 2019-05-11 4:20 ` Alex Gramiak 0 siblings, 0 replies; 9+ messages in thread From: Alex Gramiak @ 2019-05-11 4:20 UTC (permalink / raw) To: Eli Zaretskii; +Cc: emacs-devel Eli Zaretskii <eliz@gnu.org> writes: > Thanks, you never said that you were sending only part of a patch. > (I never liked breaking patches into several parts, as they make the > review harder in many cases.) I figured that the two patches were distinct enough to be split up, but perhaps I should have attached the first patch again at the beginning. I pushed the patches as 6bfc5fc6c4..616ce44ac5. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-05-11 4:20 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-05-09 3:52 [PATCH] Remove display member of glyph_string Alex Gramiak 2019-05-09 5:50 ` Eli Zaretskii 2019-05-09 16:07 ` Alex Gramiak 2019-05-09 17:02 ` Eli Zaretskii 2019-05-09 17:16 ` Alex Gramiak 2019-05-09 17:59 ` Eli Zaretskii 2019-05-09 18:21 ` Alex Gramiak 2019-05-10 5:30 ` Eli Zaretskii 2019-05-11 4:20 ` Alex Gramiak
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).