Sv: Support for background transparency

[Håkon Flatval]

I eventually decided that it makes sense for the fringe to be considered background, i.e. be rendered transparent as well. I've included it in this patch.

Håkon

---

Fra: Håkon Flatval <hkon20@hotmail.com>
Sendt: onsdag 1. desember 2021 19:54
Til: Eli Zaretskii <eliz@gnu.org>
Kopi: emacs-devel@gnu.org <emacs-devel@gnu.org>; Lars Ingebrigtsen <larsi@gnus.org>
Emne: Sv: Support for background transparency

 

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.

gdk-cairo-background-transparency-003.patch
Description: gdk-cairo-background-transparency-003.patch