Sv: Support for background transparency

["Håkon Flatval" <hkon20@hotmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3985 bytes --]

Hi again, and thanks for reviewing.

I have addressed most of your comments in this new patch.

However, I have not changed the way the input argument is handled in gui_set_alpha_background​. You proposed using XFLOATINT​, but as far as I understand, it will return the exact value of the input argument. I want the input to be interpreted as a ratio (0 - 1) if the value is a float and a percentage (0 - 100) if the value is an integer. This is how the arguments are intepreted in gui_set_alpha​ in frame.c​, which I have tried to mimick when creating this function. Please do tell if you disagree with this decision.

In addition, I have corrected the compiler warnings reported by Lars, as well as a couple of errors which would prevent compilation / create strange behavior for builds not using Cairo.

Best,
Håkon

________________________________
Fra: Eli Zaretskii <eliz@gnu.org>
Sendt: søndag 28. november 2021 08:41
Til: Håkon Flatval <hkon20@hotmail.com>
Kopi: larsi@gnus.org <larsi@gnus.org>; emacs-devel@gnu.org <emacs-devel@gnu.org>
Emne: Re: Support for background transparency

> From: Håkon Flatval <hkon20@hotmail.com>
> Date: Sat, 27 Nov 2021 23:01:39 +0000
> Cc: "emacs-devel@gnu.org" <emacs-devel@gnu.org>
>
> I believe the gui_set_alpha_background​ function in frame.c​ should return some error message
> when the frame does not support 32 bits of depth, but am not sure how to do this best.

Why do we want an error message?  Why not simply do nothing and return
in that case?  Silently doing nothing when the display doesn't support
some feature is perfectly fine, IMO.

> Newbie question: I can't guarantee that this patch works perfectly with all build variations using GDK+Cairo,
> nor in all (desktop) environments where Emacs might be used. How, in general, is Emacs tested before
> release to ensure (most) such edge cases are caught?

We rely on people who track the development code to report any
problems they see.

> I'm currently working on implementing this feature for other XLib-based build configurations too, offline,
> planning to submit another patch for those in the not-so-distant future.

Thanks.

> +  else if (FLOATP (arg))
> +    {
> +      alpha = XFLOAT_DATA(arg);
> +      if (! (0 <= alpha && alpha <= 1.0))
> +     args_out_of_range (make_float(0.0), make_float(1.0));
> +    }
> +  else if (FIXNUMP (arg))
> +    {
> +      EMACS_INT ialpha = XFIXNUM (arg);
> +      if (! (0 <= ialpha && ialpha <= 100))
> +     args_out_of_range (make_fixnum (0), make_fixnum (100));
> +      alpha = ialpha / 100.0;
> +    }

We have XFLOATINT to do both of the above, so you could avoid
repetitions.

Also, instead of signaling an error, how about using clip_to_bounds to
limit the value to the correct range?  Once again, it is perfectly OK
to do that in display-related code, instead of signaling an error.
Moreover, signaling an error could potentially hang Emacs if this
function is called in some context where recovery is not possible.

> +  else
> +    wrong_type_argument (Qnumberp, arg);

IMO, the CHECK_NUMBER test should be up front.

> +  if (alpha != 1.0)
> +    {
> +      // Abort if frame does not support 32-bit color

I don't see any need to abort, see above.

Also, please use C-style comments, /* Like this.  */

> +      gtk_widget_set_visual(wtop, visual);
> +      gtk_widget_set_visual(wfixed, visual);
                             ^^
Our style is to leave one space between the function's name and the
opening parenthesis.

> --- a/src/termhooks.h
> +++ b/src/termhooks.h
> @@ -748,11 +748,13 @@ #define EVENT_INIT(event) memset (&(event), 0, sizeof (struct input_event))
>                                              const char *name,
>                                              const char *class);
>
> -  /* Image hooks */
> +  /* Window hooks */

I'm not sure this comment replacement is for the better.

[-- Attachment #1.2: Type: text/html, Size: 7312 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: gdk-cairo-background-transparency-002.patch --]
[-- Type: text/x-patch; name="gdk-cairo-background-transparency-002.patch", Size: 9304 bytes --]

diff --git a/src/frame.c b/src/frame.c
index 33e9606e41..e9a4c1edc3 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -3898,6 +3898,7 @@ DEFUN ("frame-scale-factor", Fframe_scale_factor, Sframe_scale_factor,
   {"z-group",			SYMBOL_INDEX (Qz_group)},
   {"override-redirect",		SYMBOL_INDEX (Qoverride_redirect)},
   {"no-special-glyphs",		SYMBOL_INDEX (Qno_special_glyphs)},
+  {"alpha-background",          SYMBOL_INDEX (Qalpha_background)},
 #ifdef NS_IMPL_COCOA
   {"ns-appearance",		SYMBOL_INDEX (Qns_appearance)},
   {"ns-transparent-titlebar",	SYMBOL_INDEX (Qns_transparent_titlebar)},
@@ -5015,6 +5016,33 @@ gui_set_alpha (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
     }
 }
 
+void
+gui_set_alpha_background (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
+{
+  double alpha = 1.0;
+
+  if (NILP (arg))
+    alpha = 1.0;
+  else if (FLOATP (arg))
+    {
+      alpha = XFLOAT_DATA (arg);
+      if (! (0 <= alpha && alpha <= 1.0))
+	args_out_of_range (make_float (0.0), make_float (1.0));
+    }
+  else if (FIXNUMP (arg))
+    {
+      EMACS_INT ialpha = XFIXNUM (arg);
+      if (! (0 <= ialpha && ialpha <= 100))
+	args_out_of_range (make_fixnum (0), make_fixnum (100));
+      alpha = ialpha / 100.0;
+    }
+  else
+    wrong_type_argument (Qnumberp, arg);
+
+  f->alpha_background = alpha;
+
+  SET_FRAME_GARBAGED (f);
+}
 
 /**
  * gui_set_no_special_glyphs:
@@ -6083,6 +6111,7 @@ syms_of_frame (void)
 #endif
 
   DEFSYM (Qalpha, "alpha");
+  DEFSYM (Qalpha_background, "alpha-background");
   DEFSYM (Qauto_lower, "auto-lower");
   DEFSYM (Qauto_raise, "auto-raise");
   DEFSYM (Qborder_color, "border-color");
diff --git a/src/frame.h b/src/frame.h
index cb2bad71c5..e45e0714a1 100644
--- a/src/frame.h
+++ b/src/frame.h
@@ -636,6 +636,9 @@ #define EMACS_FRAME_H
      Negative values mean not to change alpha.  */
   double alpha[2];
 
+  /* Background opacity */
+  double alpha_background;
+
   /* Exponent for gamma correction of colors.  1/(VIEWING_GAMMA *
      SCREEN_GAMMA) where viewing_gamma is 0.4545 and SCREEN_GAMMA is a
      frame parameter.  0 means don't do gamma correction.  */
@@ -1658,6 +1661,7 @@ #define EMACS_CLASS "Emacs"
 extern long gui_figure_window_size (struct frame *, Lisp_Object, bool, bool);
 
 extern void gui_set_alpha (struct frame *, Lisp_Object, Lisp_Object);
+extern void gui_set_alpha_background (struct frame *, Lisp_Object, Lisp_Object);
 extern void gui_set_no_special_glyphs (struct frame *, Lisp_Object, Lisp_Object);
 
 extern void validate_x_resource_name (void);
diff --git a/src/gtkutil.c b/src/gtkutil.c
index 9e676cd025..ab99392596 100644
--- a/src/gtkutil.c
+++ b/src/gtkutil.c
@@ -195,6 +195,27 @@ xg_get_scale (struct frame *f)
   return xg_get_gdk_scale ();
 }
 
+static unsigned int
+x_get_bit_depth (struct frame *f)
+{
+  Visual *visual = FRAME_X_VISUAL (f);
+  VisualID visual_id = XVisualIDFromVisual (visual);
+
+  XVisualInfo vinfo_template;
+  vinfo_template.visualid = visual_id;
+
+  int num_items;
+
+  XVisualInfo *visual_infos = XGetVisualInfo (FRAME_X_DISPLAY (f),
+					      VisualIDMask,
+					      &vinfo_template,
+					      &num_items);
+
+  eassert (num_items > 0);
+
+  return visual_infos[0].depth;
+}
+
 /* Close display DPY.  */
 
 void
@@ -1232,6 +1253,10 @@ xg_create_frame_widgets (struct frame *f)
   else
     wtop = gtk_window_new (GTK_WINDOW_TOPLEVEL);
 
+  /* This prevents GTK from painting the window's background, which
+     would interfere with transparent background in some environments */
+  gtk_widget_set_app_paintable (wtop, TRUE);
+
   /* gtk_window_set_has_resize_grip is a Gtk+ 3.0 function but Ubuntu
      has backported it to Gtk+ 2.0 and they add the resize grip for
      Gtk+ 2.0 applications also.  But it has a bug that makes Emacs loop
@@ -1339,6 +1364,15 @@ xg_create_frame_widgets (struct frame *f)
                          | GDK_STRUCTURE_MASK
                          | GDK_VISIBILITY_NOTIFY_MASK);
 
+  GdkScreen *screen = gtk_widget_get_screen (wtop);
+
+  if (x_get_bit_depth (f) == 32)
+    {
+      GdkVisual *visual = gdk_screen_get_rgba_visual (screen);
+      gtk_widget_set_visual (wtop, visual);
+      gtk_widget_set_visual (wfixed, visual);
+    }
+
   /* Must realize the windows so the X window gets created.  It is used
      by callers of this function.  */
   gtk_widget_realize (wfixed);
@@ -1379,7 +1413,6 @@ xg_create_frame_widgets (struct frame *f)
   g_signal_connect (wtop, "query-tooltip", G_CALLBACK (qttip_cb), f);
 
   {
-    GdkScreen *screen = gtk_widget_get_screen (wtop);
     GtkSettings *gs = gtk_settings_get_for_screen (screen);
     /* Only connect this signal once per screen.  */
     if (! g_signal_handler_find (G_OBJECT (gs),
diff --git a/src/xfns.c b/src/xfns.c
index 5eff9f5b0f..6660d725a6 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -4118,6 +4118,8 @@ DEFUN ("x-create-frame", Fx_create_frame, Sx_create_frame,
                          RES_TYPE_NUMBER);
   gui_default_parameter (f, parms, Qalpha, Qnil,
                          "alpha", "Alpha", RES_TYPE_NUMBER);
+  gui_default_parameter (f, parms, Qalpha_background, Qnil,
+			 "alpha_background", "AlphaBackground", RES_TYPE_NUMBER);
 
   if (!NILP (parent_frame))
     {
@@ -5734,14 +5736,35 @@ select_visual (struct x_display_info *dpyinfo)
       int n_visuals;
       XVisualInfo *vinfo, vinfo_template;
 
-      dpyinfo->visual = DefaultVisualOfScreen (screen);
+      vinfo_template.screen = XScreenNumberOfScreen (screen);
+
+#if defined (USE_GTK) && defined (USE_CAIRO)
+      /* First attempt to use 32-bit visual if available */
+
+      vinfo_template.depth = 32;
+
+      vinfo = XGetVisualInfo (dpy, VisualScreenMask | VisualDepthMask,
+			      &vinfo_template, &n_visuals);
+
+      if (n_visuals > 0)
+	{
+	  dpyinfo->n_planes = vinfo->depth;
+	  dpyinfo->visual = vinfo->visual;
+	  XFree (vinfo);
+	  return;
+	}
 
+#endif // defined (USE_GTK) && defined (USE_CAIRO)
+
+      /* 32-bit visual not available, fallback to default visual */
+      dpyinfo->visual = DefaultVisualOfScreen (screen);
       vinfo_template.visualid = XVisualIDFromVisual (dpyinfo->visual);
-      vinfo_template.screen = XScreenNumberOfScreen (screen);
+
       vinfo = XGetVisualInfo (dpy, VisualIDMask | VisualScreenMask,
 			      &vinfo_template, &n_visuals);
+
       if (n_visuals <= 0)
-	fatal ("Can't get proper X visual info");
+	  fatal ("Can't get proper X visual info");
 
       dpyinfo->n_planes = vinfo->depth;
       XFree (vinfo);
@@ -6549,6 +6572,8 @@ x_create_tip_frame (struct x_display_info *dpyinfo, Lisp_Object parms)
                          "cursorType", "CursorType", RES_TYPE_SYMBOL);
   gui_default_parameter (f, parms, Qalpha, Qnil,
                          "alpha", "Alpha", RES_TYPE_NUMBER);
+  gui_default_parameter (f, parms, Qalpha_background, Qnil,
+			 "alpha_background", "AlphaBackground", RES_TYPE_NUMBER);
 
   /* Add `tooltip' frame parameter's default value. */
   if (NILP (Fframe_parameter (frame, Qtooltip)))
@@ -7878,6 +7903,7 @@ DEFUN ("x-gtk-debug", Fx_gtk_debug, Sx_gtk_debug, 1, 1, 0,
   x_set_z_group,
   x_set_override_redirect,
   gui_set_no_special_glyphs,
+  gui_set_alpha_background,
 };
 
 void
diff --git a/src/xterm.c b/src/xterm.c
index 8045470bdd..2a4556a8fd 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -684,9 +684,12 @@ x_set_cr_source_with_gc_background (struct frame *f, GC gc)
 
   XGetGCValues (FRAME_X_DISPLAY (f), gc, GCBackground, &xgcv);
   color.pixel = xgcv.background;
+
   x_query_colors (f, &color, 1);
-  cairo_set_source_rgb (FRAME_CR_CONTEXT (f), color.red / 65535.0,
-			color.green / 65535.0, color.blue / 65535.0);
+  cairo_set_source_rgba (FRAME_CR_CONTEXT (f), color.red / 65535.0,
+                         color.green / 65535.0, color.blue / 65535.0, f->alpha_background);
+
+  cairo_set_operator (FRAME_CR_CONTEXT (f), CAIRO_OPERATOR_SOURCE);
 }
 
 static const cairo_user_data_key_t xlib_surface_key, saved_drawable_key;
@@ -1096,6 +1099,29 @@ x_fill_rectangle (struct frame *f, GC gc, int x, int y, int width, int height)
 #endif
 }
 
+
+static void
+x_clear_rectangle (struct frame *f, GC gc, int x, int y, int width, int height)
+{
+#ifdef USE_CAIRO
+  cairo_t *cr;
+
+  cr = x_begin_cr_clip (f, gc);
+  x_set_cr_source_with_gc_background (f, gc);
+  cairo_rectangle (cr, x, y, width, height);
+  cairo_fill (cr);
+  x_end_cr_clip (f);
+#else
+  XGCValues xgcv;
+  Display *dpy = FRAME_X_DISPLAY (f);
+  XGetGCValues (dpy, gc, GCBackground | GCForeground, &xgcv);
+  XSetForeground (dpy, gc, xgcv.background);
+  XFillRectangle (dpy, FRAME_X_DRAWABLE (f),
+		  gc, x, y, width, height);
+  XSetForeground (dpy, gc, xgcv.foreground);
+#endif
+}
+
 static void
 x_draw_rectangle (struct frame *f, GC gc, int x, int y, int width, int height)
 {
@@ -1975,12 +2001,7 @@ 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 (display, s->gc, GCForeground | GCBackground, &xgcv);
-  XSetForeground (display, s->gc, xgcv.background);
-  x_fill_rectangle (s->f, s->gc, x, y, w, h);
-  XSetForeground (display, s->gc, xgcv.foreground);
+  x_clear_rectangle (s->f, s->gc, x, y, w, h);
 }