unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [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).