[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: new frame-parameter "alpha"
From: |
Stefan Monnier |
Subject: |
Re: new frame-parameter "alpha" |
Date: |
Fri, 14 Mar 2008 14:28:22 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.0.60 (gnu/linux) |
> The attached patch adds the window (frame) opacity feature to GNU Emacs.
> The user can control the frame opacity via a frame parameter 'alpha'.
> If you like the code, I'll start to contact major contributors.
I do not personally care for transparency, and I'm wondering if it's not
better handled by the (compositing) window-manager, but if some people
like this feature, I'm not opposed to it.
A few comments on the code:
> + (defun set-alpha (alpha &optional frame)
> + "Set the opacity of FRAME to ALPHA. First argument ALPHA
> +should range from 0 (invisible) to 100 (completely opaque).
> +You can also use an floating point number 0.0 to 1.0.
> +When called interactively, prompt for the value of the opacity to set.
> +FRAME defaults to the selected frame. To get the frame's current
> +alpha value state, use `frame-parameters'."
> + (interactive "XWindow Opacity (0-100 or 0.0-1.0) : ")
> + (modify-frame-parameters frame
> + (list (cons 'alpha alpha))))
Is this necessary/important?
> +#ifdef HAVE_X_WINDOWS
> +/* Lower limit value of frame transparency. */
> +
> +Lisp_Object Vframe_alpha_lower_limit;
> +#endif
> +
> #endif
Only use #ifdef conditional compilation if either it's necessary to get
the code to compile, or for code which is inherently only meaningful for
a particular configuration. Since Vframe_alpha_lower_limit makes sense
with other GUIs than X11, it should not be protected with #ifdef
HAVE_X_WINDOWS.
> +#ifdef HAVE_X_WINDOWS
> +Lisp_Object Qalpha;
> +#endif
Same here and various other places.
> +#ifdef HAVE_X_WINDOWS
> +void
> +x_set_alpha (f, arg, oldval)
> + struct frame *f;
> + Lisp_Object arg, oldval;
> +{
> + double alpha = 1.0;
> + double newval[2];
> + int i, ialpha;
> + Lisp_Object item;
> +
> + for( i=0; i<2; i++ )
> + newval[i] = 1.0;
> +
> + for( i=0; i<2; i++ )
> + {
> + if( CONSP (arg) )
> + {
> + item = CAR (arg);
> + arg = CDR (arg);
> + }
> + else
> + item=arg;
> +
> + if( !NILP (item) )
> + {
> + if( FLOATP(item) )
> + {
> + alpha = XFLOAT_DATA (item);
> + if ( alpha < 0.0 || 1.0 < alpha )
> + args_out_of_range (make_float (0.0), make_float (1.0));
> + }
> + else if( INTEGERP(item) )
> + {
> + ialpha = XINT (item);
> + if ( ialpha < 0 || 100 < ialpha )
> + args_out_of_range (make_number (0), make_number (100));
> + else
> + alpha = ialpha / 100.0;
> + }
> + else
> + wrong_type_argument (Qnumberp, item);
> + }
> + newval[i]=alpha;
> + }
> +
> + for ( i=0; i<2; i++ )
> + f->alpha[i] = newval[i];
> +
> + BLOCK_INPUT;
> + x_set_frame_alpha (f);
> + UNBLOCK_INPUT;
> +
> + return;
> +}
> +#endif
Explain why you do things twice.
Also, note the coding conventions we use and try to reproduce them.
E.g. we place a space before open parentheses but no space after (and
no space before the close parenthesis either).
> +#ifdef HAVE_X_WINDOWS
> + /* Opacity of the Frame */
> + double alpha[2];
> +#endif
The comment should describe what the field holds, so in this case it
should describe *both* entries of the table.
Also in the docstring, don't just use "alpha" but "alpha transparency"
or something like that, for people who don't know what you mean by "alpha".
Stefan
- new frame-parameter "alpha", Seiji Zenitani, 2008/03/14
- Re: new frame-parameter "alpha",
Stefan Monnier <=
- Re: new frame-parameter "alpha", Seiji Zenitani, 2008/03/15
- Re: new frame-parameter "alpha", Stefan Monnier, 2008/03/15
- Re: new frame-parameter "alpha", Brian Cully, 2008/03/18
- Re: new frame-parameter "alpha", Seiji Zenitani, 2008/03/19
- Re: new frame-parameter "alpha", David De La Harpe Golden, 2008/03/19
- Re: new frame-parameter "alpha", Seiji Zenitani, 2008/03/25
- Re: new frame-parameter "alpha", Seiji Zenitani, 2008/03/19