On Mon, 7 Apr 2014 11:58:45 +0200 Manuel Bachmann <[email protected]> wrote:
> This fixes : > - leaking the mask used to simulate transparency ; > - code style (definitions moved up, use of brackets) ; > - applying an opaque region when transparency is > wanted (shound not happen). > > Signed-off-by: Manuel Bachmann <[email protected]> > --- > src/pixman-renderer.c | 19 +++++++++++++------ > 1 file changed, 13 insertions(+), 6 deletions(-) > > diff --git a/src/pixman-renderer.c b/src/pixman-renderer.c > index eff7201..4ac4693 100644 > --- a/src/pixman-renderer.c > +++ b/src/pixman-renderer.c > @@ -181,6 +181,8 @@ repaint_region(struct weston_view *ev, struct > weston_output *output, > float view_x, view_y; > pixman_transform_t transform; > pixman_fixed_t fw, fh; > + pixman_image_t *mask_image; > + pixman_color_t mask; > > /* The final region to be painted is the intersection of > * 'region' and 'surf_region'. However, 'region' is in the global > @@ -340,12 +342,12 @@ repaint_region(struct weston_view *ev, struct > weston_output *output, > if (ps->buffer_ref.buffer) > wl_shm_buffer_begin_access(ps->buffer_ref.buffer->shm_buffer); > > - pixman_image_t *mask_image; > if (ev->alpha < 1.0) { > - pixman_color_t mask = { 0x0000, 0x0000, 0x0000, > 0xffff*ev->alpha }; > + mask.alpha = 0xffff*ev->alpha; Hi, doesn't this now leave red, green and blue of 'mask' undefined? > mask_image = pixman_image_create_solid_fill(&mask); > - } else > + } else { > mask_image = NULL; > + } > > pixman_image_composite32(pixman_op, > ps->image, /* src */ > @@ -357,6 +359,9 @@ repaint_region(struct weston_view *ev, struct > weston_output *output, > pixman_image_get_width (po->shadow_image), /* > width */ > pixman_image_get_height (po->shadow_image) /* > height */); > > + if (mask_image) > + pixman_image_unref(mask_image); > + > if (ps->buffer_ref.buffer) > wl_shm_buffer_end_access(ps->buffer_ref.buffer->shm_buffer); > > @@ -408,12 +413,14 @@ draw_view(struct weston_view *ev, struct weston_output > *output, > ev->transform.matrix.type != WESTON_MATRIX_TRANSFORM_TRANSLATE) { > repaint_region(ev, output, &repaint, NULL, PIXMAN_OP_OVER); > } else { > - /* blended region is whole surface minus opaque region: */ > + /* blended region is whole surface minus opaque region, if not > transparent: */ > pixman_region32_init_rect(&surface_blend, 0, 0, > ev->surface->width, > ev->surface->height); > - pixman_region32_subtract(&surface_blend, &surface_blend, > &ev->surface->opaque); > + if (ev->alpha == 1.0) { > + pixman_region32_subtract(&surface_blend, > &surface_blend, &ev->surface->opaque); > + } > > - if (pixman_region32_not_empty(&ev->surface->opaque)) { > + if ((ev->alpha == 1.0) && > pixman_region32_not_empty(&ev->surface->opaque)) { > repaint_region(ev, output, &repaint, > &ev->surface->opaque, PIXMAN_OP_SRC); > } > You could merge the two if-statements. If the region is empty, the subtraction won't have an effect anyway. I wonder if the condition alpha == 1.0 is really the opposite of alpha < 1.0 in weston... maybe it should be written as alpha >= 1.0 or even !(alpha < 1.0) to exactly reflect the earlier test. This looks ok, but I suppose it will have the same problem with Xwayland that the gl-renderer is working around. Kristian, now on the new era of Xwayland DDX and possibly glamor, does Xwayland still produce garbage alpha channels sometimes, or could we drop that hack? Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
