<div dir="ltr"><div><div>Hi Kristian,<br><br></div>Thanks a lot for your review !<br></div>You're totally right, I was leaking the mask (shame on me :-( ) and wasn't totally consistent on the code style part.<br>I just submitted another patch, which should address most of our concerns :<br>
<br><a href="http://lists.freedesktop.org/archives/wayland-devel/2014-April/014080.html">http://lists.freedesktop.org/archives/wayland-devel/2014-April/014080.html</a><br><div><div><br></div><div>This fixes :<br></div><div>
- the leak ;<br></div><div>- code style (added brackets, put definitions on top) ;<br><br></div><div>Additionally, this fixes the case of clients using an opaque surface (weston-terminal e.g.) ; the surface was still computed when using transparency, which would make them render an ugly black rectangle. The patch now totally disables the opaque surface in this case.<br>
<br></div><div>For the window fade out animation, you're right, it doesn't happen ; I think I saw the shell fade out and considered it should work everywhere. Will address that.<br></div><div><br>Regards,<br>Manuel<br>
</div></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">2014-04-07 7:59 GMT+02:00 Kristian Høgsberg <span dir="ltr"><<a href="mailto:hoegsberg@gmail.com" target="_blank">hoegsberg@gmail.com</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">On Sat, Apr 05, 2014 at 05:31:37AM +0200, Manuel Bachmann wrote:<br>
> When the alpha channel of a surface is changed and the surface<br>
> refreshed, pixman renderer will now apply a mask corresponding<br>
> to the alpha channel value.<br>
><br>
> This allows visual effects like shell fade in, shell fade out,<br>
> window switching, to work when using pixman renderer.<br>
><br>
> Signed-off-by: Manuel Bachmann <<a href="mailto:manuel.bachmann@open.eurogiciel.org">manuel.bachmann@open.eurogiciel.org</a>><br>
> ---<br>
>  src/pixman-renderer.c |    9 ++++++++-<br>
>  1 file changed, 8 insertions(+), 1 deletion(-)<br>
><br>
> diff --git a/src/pixman-renderer.c b/src/pixman-renderer.c<br>
> index b999343..eff7201 100644<br>
> --- a/src/pixman-renderer.c<br>
> +++ b/src/pixman-renderer.c<br>
> @@ -340,9 +340,16 @@ repaint_region(struct weston_view *ev, struct weston_output *output,<br>
>       if (ps->buffer_ref.buffer)<br>
>               wl_shm_buffer_begin_access(ps->buffer_ref.buffer->shm_buffer);<br>
><br>
> +     pixman_image_t *mask_image;<br>
> +     if (ev->alpha < 1.0) {<br>
> +             pixman_color_t mask = { 0x0000, 0x0000, 0x0000, 0xffff*ev->alpha };<br>
> +             mask_image = pixman_image_create_solid_fill(&mask);<br>
> +     } else<br>
> +             mask_image = NULL;<br>
<br>
</div>I like this but it doesn't quite look right, it looks like the surface also fades<br>
from black.  I also doesn't see it trigger on the close window fade anmation.<br>
And stylistically, please don't mix code and declarations (move mask_image<br>
declartion up) and use braces for both branches (the rule is both or none).<br>
<div class=""><br>
>       pixman_image_composite32(pixman_op,<br>
>                                ps->image, /* src */<br>
> -                              NULL /* mask */,<br>
> +                              mask_image, /* mask */<br>
>                                po->shadow_image, /* dest */<br>
>                                0, 0, /* src_x, src_y */<br>
>                                0, 0, /* mask_x, mask_y */<br>
<br>
</div>We're leaking mask_image.<br>
<span class="HOEnZb"><font color="#888888"><br>
Kristian<br>
<br>
> --<br>
> 1.7.10.4<br>
><br>
> _______________________________________________<br>
> wayland-devel mailing list<br>
> <a href="mailto:wayland-devel@lists.freedesktop.org">wayland-devel@lists.freedesktop.org</a><br>
> <a href="http://lists.freedesktop.org/mailman/listinfo/wayland-devel" target="_blank">http://lists.freedesktop.org/mailman/listinfo/wayland-devel</a><br>
</font></span></blockquote></div><br><br clear="all"><br>-- <br><div dir="ltr"><font>Regards,<br>
<br>
<i><b>Manuel BACHMANN</b><br>
Tizen Project<br>
VANNES-FR</i><br>
</font></div>
</div>