[PATCH weston 2/7] pixman-renderer: Move shadow buffer into renderer
Ander Conselvan de Oliveira
ander.conselvan.de.oliveira at intel.com
Wed Jan 23 02:34:02 PST 2013
On 01/23/2013 12:15 PM, Vasily Khoruzhick wrote:
> On Wed, Jan 23, 2013 at 12:02 PM, Ander Conselvan de Oliveira
> <conselvan2 at gmail.com> wrote:
>> On 01/23/2013 09:25 AM, Vasily Khoruzhick wrote:
>>>
>>> On Tue, Jan 22, 2013 at 7:07 PM, Ander Conselvan de Oliveira
>>> <ander.conselvan.de.oliveira at intel.com> wrote:
>>>>
>>>> The X11 backend uses a shadow buffer to be able to support transformed
>>>> outputs. However, this belongs in the renderer, since otherwise this
>>>> code would have to be copied into every backend that uses the pixman
>>>> renderer and supports transformed outputs.
>>>
>>>
>>> It's not a good idea, because with that change you won't be able to
>>> implement pageflips.
>>
>>
>> Can you elaborate on that? On the drm backend, I implemented page flips by
>> calling pixman_renderer_output_set_buffer() prior to rendering, alternating
>> between two dumb buffers. See drm_output_render_pixman() in the last patch
>> of this series.
>
> When doing pageflip you definitely don't need shadow buffer, do you?
> Why to do extra copy?
Reading from the dumb buffer is slow so compositing straight to it turns
out to be slower than using the shadow buffer. If I understand
correctly, pixman doesn't consider the transform of the destination
image when compositing, so we also need a shadow buffer to support
transformed outputs.
I see the transformed case code is about to be duplicated in the fbdev
backend, so with that, drm and x11, we would have three copies of the
code. I rather just have that on the renderer. If we end up with a case
where we really don't want a shadow buffer, we can make that an option
when creating the renderer or the renderer output.
Cheers,
Ander
>> Thanks,
>> Ander
>>
>>
>>
>>> Regards
>>> Vasily
>>>
>>>> ---
>>>> src/compositor-x11.c | 104
>>>> +------------------------------------------------
>>>> src/pixman-renderer.c | 99
>>>> ++++++++++++++++++++++++++++++++++++++++++++--
>>>> 2 files changed, 97 insertions(+), 106 deletions(-)
>>>>
>>>> diff --git a/src/compositor-x11.c b/src/compositor-x11.c
>>>> index 7ae2b75..b3c7fc5 100644
>>>> --- a/src/compositor-x11.c
>>>> +++ b/src/compositor-x11.c
>>>> @@ -116,10 +116,8 @@ struct x11_output {
>>>> xcb_gc_t gc;
>>>> xcb_shm_seg_t segment;
>>>> pixman_image_t *hw_surface;
>>>> - pixman_image_t *shadow_surface;
>>>> int shm_id;
>>>> void *buf;
>>>> - void *shadow_buf;
>>>> uint8_t depth;
>>>> };
>>>>
>>>> @@ -342,59 +340,12 @@ x11_output_repaint_shm(struct weston_output
>>>> *output_base,
>>>> struct x11_output *output = (struct x11_output *)output_base;
>>>> struct weston_compositor *ec = output->base.compositor;
>>>> struct x11_compositor *c = (struct x11_compositor *)ec;
>>>> - pixman_box32_t *rects;
>>>> - int nrects, i, src_x, src_y, x1, y1, x2, y2, width, height;
>>>> xcb_void_cookie_t cookie;
>>>> xcb_generic_error_t *err;
>>>>
>>>> - pixman_renderer_output_set_buffer(output_base,
>>>> output->shadow_surface);
>>>> + pixman_renderer_output_set_buffer(output_base,
>>>> output->hw_surface);
>>>> ec->renderer->repaint_output(output_base, damage);
>>>>
>>>> - width = pixman_image_get_width(output->shadow_surface);
>>>> - height = pixman_image_get_height(output->shadow_surface);
>>>> - rects = pixman_region32_rectangles(damage, &nrects);
>>>> - for (i = 0; i < nrects; i++) {
>>>> - switch (output_base->transform) {
>>>> - default:
>>>> - case WL_OUTPUT_TRANSFORM_NORMAL:
>>>> - x1 = rects[i].x1;
>>>> - x2 = rects[i].x2;
>>>> - y1 = rects[i].y1;
>>>> - y2 = rects[i].y2;
>>>> - break;
>>>> - case WL_OUTPUT_TRANSFORM_180:
>>>> - x1 = width - rects[i].x2;
>>>> - x2 = width - rects[i].x1;
>>>> - y1 = height - rects[i].y2;
>>>> - y2 = height - rects[i].y1;
>>>> - break;
>>>> - case WL_OUTPUT_TRANSFORM_90:
>>>> - x1 = height - rects[i].y2;
>>>> - x2 = height - rects[i].y1;
>>>> - y1 = rects[i].x1;
>>>> - y2 = rects[i].x2;
>>>> - break;
>>>> - case WL_OUTPUT_TRANSFORM_270:
>>>> - x1 = rects[i].y1;
>>>> - x2 = rects[i].y2;
>>>> - y1 = width - rects[i].x2;
>>>> - y2 = width - rects[i].x1;
>>>> - break;
>>>> - }
>>>> - src_x = x1;
>>>> - src_y = y1;
>>>> -
>>>> - pixman_image_composite32(PIXMAN_OP_SRC,
>>>> - output->shadow_surface, /* src */
>>>> - NULL /* mask */,
>>>> - output->hw_surface, /* dest */
>>>> - src_x, src_y, /* src_x, src_y */
>>>> - 0, 0, /* mask_x, mask_y */
>>>> - x1, y1, /* dest_x, dest_y */
>>>> - x2 - x1, /* width */
>>>> - y2 - y1 /* height */);
>>>> - }
>>>> -
>>>> pixman_region32_subtract(&ec->primary_plane.damage,
>>>> &ec->primary_plane.damage, damage);
>>>> cookie = xcb_shm_put_image_checked(c->conn, output->window,
>>>> output->gc,
>>>> @@ -444,10 +395,6 @@ x11_output_deinit_shm(struct x11_compositor *c,
>>>> struct x11_output *output)
>>>> free(err);
>>>> }
>>>> shmdt(output->buf);
>>>> -
>>>> - pixman_image_unref(output->shadow_surface);
>>>> - output->shadow_surface = NULL;
>>>> - free(output->shadow_buf);
>>>> }
>>>>
>>>> static void
>>>> @@ -622,8 +569,6 @@ x11_output_init_shm(struct x11_compositor *c, struct
>>>> x11_output *output,
>>>> xcb_format_iterator_t fmt;
>>>> xcb_void_cookie_t cookie;
>>>> xcb_generic_error_t *err;
>>>> - int shadow_width, shadow_height;
>>>> - pixman_transform_t transform;
>>>> const xcb_query_extension_reply_t *ext;
>>>> int bitsperpixel = 0;
>>>> pixman_format_code_t pixman_format;
>>>> @@ -701,53 +646,6 @@ x11_output_init_shm(struct x11_compositor *c, struct
>>>> x11_output *output,
>>>> /* Now create pixman image */
>>>> output->hw_surface = pixman_image_create_bits(pixman_format,
>>>> width, height, output->buf,
>>>> width * (bitsperpixel / 8));
>>>> - pixman_transform_init_identity(&transform);
>>>> - switch (output->base.transform) {
>>>> - default:
>>>> - case WL_OUTPUT_TRANSFORM_NORMAL:
>>>> - shadow_width = width;
>>>> - shadow_height = height;
>>>> - pixman_transform_rotate(&transform,
>>>> - NULL, 0, 0);
>>>> - pixman_transform_translate(&transform, NULL,
>>>> - 0, 0);
>>>> - break;
>>>> - case WL_OUTPUT_TRANSFORM_180:
>>>> - shadow_width = width;
>>>> - shadow_height = height;
>>>> - pixman_transform_rotate(&transform,
>>>> - NULL, -pixman_fixed_1, 0);
>>>> - pixman_transform_translate(NULL, &transform,
>>>> - pixman_int_to_fixed(shadow_width),
>>>> - pixman_int_to_fixed(shadow_height));
>>>> - break;
>>>> - case WL_OUTPUT_TRANSFORM_270:
>>>> - shadow_width = height;
>>>> - shadow_height = width;
>>>> - pixman_transform_rotate(&transform,
>>>> - NULL, 0, pixman_fixed_1);
>>>> - pixman_transform_translate(&transform,
>>>> - NULL,
>>>> - pixman_int_to_fixed(shadow_width),
>>>> - 0);
>>>> - break;
>>>> - case WL_OUTPUT_TRANSFORM_90:
>>>> - shadow_width = height;
>>>> - shadow_height = width;
>>>> - pixman_transform_rotate(&transform,
>>>> - NULL, 0, -pixman_fixed_1);
>>>> - pixman_transform_translate(&transform,
>>>> - NULL,
>>>> - 0,
>>>> - pixman_int_to_fixed(shadow_height));
>>>> - break;
>>>> - }
>>>> - output->shadow_buf = malloc(width * height * (bitsperpixel /
>>>> 8));
>>>> - output->shadow_surface = pixman_image_create_bits(pixman_format,
>>>> shadow_width, shadow_height,
>>>> - output->shadow_buf, shadow_width * (bitsperpixel / 8));
>>>> - /* No need in transform for normal output */
>>>> - if (output->base.transform != WL_OUTPUT_TRANSFORM_NORMAL)
>>>> - pixman_image_set_transform(output->shadow_surface,
>>>> &transform);
>>>>
>>>> output->gc = xcb_generate_id(c->conn);
>>>> xcb_create_gc(c->conn, output->gc, output->window, 0, NULL);
>>>> diff --git a/src/pixman-renderer.c b/src/pixman-renderer.c
>>>> index 8391cc6..77ae99e 100644
>>>> --- a/src/pixman-renderer.c
>>>> +++ b/src/pixman-renderer.c
>>>> @@ -31,6 +31,8 @@
>>>> #include <linux/input.h>
>>>>
>>>> struct pixman_output_state {
>>>> + void *shadow_buffer;
>>>> + pixman_image_t *shadow_image;
>>>> pixman_image_t *hw_buffer;
>>>> };
>>>>
>>>> @@ -140,7 +142,7 @@ repaint_region(struct weston_surface *es, struct
>>>> weston_output *output,
>>>> pixman_image_composite32(pixman_op,
>>>> ps->image, /* src */
>>>> NULL /* mask */,
>>>> - po->hw_buffer, /* dest */
>>>> + po->shadow_image, /* dest */
>>>> src_x, src_y, /* src_x, src_y */
>>>> 0, 0, /* mask_x, mask_y */
>>>> rects[i].x1, rects[i].y1, /* dest_x, dest_y */
>>>> @@ -153,7 +155,7 @@ repaint_region(struct weston_surface *es, struct
>>>> weston_output *output,
>>>> pixman_image_composite32(PIXMAN_OP_OVER,
>>>> pr->debug_color, /* src */
>>>> NULL /* mask */,
>>>> - po->hw_buffer, /* dest */
>>>> + po->shadow_image, /* dest */
>>>> src_x, src_y, /* src_x, src_y */
>>>> 0, 0, /* mask_x, mask_y */
>>>> rects[i].x1, rects[i].y1, /* dest_x, dest_y */
>>>> @@ -220,6 +222,35 @@ repaint_surfaces(struct weston_output *output,
>>>> pixman_region32_t *damage)
>>>> }
>>>>
>>>> static void
>>>> +copy_to_hw_buffer(struct weston_output *output, pixman_region32_t
>>>> *region)
>>>> +{
>>>> + struct pixman_output_state *po = get_output_state(output);
>>>> + int nrects, i, width, height;
>>>> + pixman_box32_t *rects;
>>>> + pixman_box32_t b;
>>>> +
>>>> + width = pixman_image_get_width(po->shadow_image);
>>>> + height = pixman_image_get_height(po->shadow_image);
>>>> +
>>>> + rects = pixman_region32_rectangles(region, &nrects);
>>>> + for (i = 0; i < nrects; i++) {
>>>> + b = weston_transformed_rect(width, height,
>>>> + output->transform, rects[i]);
>>>> +
>>>> + pixman_image_composite32(PIXMAN_OP_SRC,
>>>> + po->shadow_image, /* src */
>>>> + NULL /* mask */,
>>>> + po->hw_buffer, /* dest */
>>>> + b.x1, b.y1, /* src_x, src_y */
>>>> + 0, 0, /* mask_x, mask_y */
>>>> + b.x1, b.y1, /* dest_x, dest_y */
>>>> + b.x2 - b.x1, /* width */
>>>> + b.y2 - b.y1 /* height */);
>>>> + }
>>>> +
>>>> +}
>>>> +
>>>> +static void
>>>> pixman_renderer_repaint_output(struct weston_output *output,
>>>> pixman_region32_t *output_damage)
>>>> {
>>>> @@ -229,6 +260,7 @@ pixman_renderer_repaint_output(struct weston_output
>>>> *output,
>>>> return;
>>>>
>>>> repaint_surfaces(output, output_damage);
>>>> + copy_to_hw_buffer(output, output_damage);
>>>>
>>>> pixman_region32_copy(&output->previous_damage, output_damage);
>>>> wl_signal_emit(&output->frame_signal, output);
>>>> @@ -402,10 +434,68 @@ WL_EXPORT int
>>>> pixman_renderer_output_create(struct weston_output *output)
>>>> {
>>>> struct pixman_output_state *po = calloc(1, sizeof *po);
>>>> + pixman_transform_t transform;
>>>> + pixman_fixed_t fw, fh;
>>>> + int w, h;
>>>> + int rotated = 0;
>>>>
>>>> if (!po)
>>>> return -1;
>>>> -
>>>> +
>>>> + /* set shadow image transformation */
>>>> + w = output->current->width;
>>>> + h = output->current->height;
>>>> +
>>>> + fw = pixman_int_to_fixed(w);
>>>> + fh = pixman_int_to_fixed(h);
>>>> +
>>>> + pixman_transform_init_identity(&transform);
>>>> +
>>>> + switch (output->transform) {
>>>> + default:
>>>> + case WL_OUTPUT_TRANSFORM_NORMAL:
>>>> + break;
>>>> + case WL_OUTPUT_TRANSFORM_180:
>>>> + pixman_transform_rotate(&transform, NULL,
>>>> -pixman_fixed_1, 0);
>>>> + pixman_transform_translate(NULL, &transform, fw, fh);
>>>> + break;
>>>> + case WL_OUTPUT_TRANSFORM_270:
>>>> + rotated = 1;
>>>> + pixman_transform_rotate(&transform, NULL, 0,
>>>> pixman_fixed_1);
>>>> + pixman_transform_translate(&transform, NULL, fh, 0);
>>>> + break;
>>>> + case WL_OUTPUT_TRANSFORM_90:
>>>> + rotated = 1;
>>>> + pixman_transform_rotate(&transform, NULL, 0,
>>>> -pixman_fixed_1);
>>>> + pixman_transform_translate(&transform, NULL, 0, fw);
>>>> + break;
>>>> + }
>>>> +
>>>> + if (rotated) {
>>>> + int tmp = w;
>>>> + w = h;
>>>> + h = tmp;
>>>> + }
>>>> +
>>>> + po->shadow_buffer = malloc(w * h * 4);
>>>> +
>>>> + if (!po->shadow_buffer) {
>>>> + free(po);
>>>> + return -1;
>>>> + }
>>>> +
>>>> + po->shadow_image =
>>>> + pixman_image_create_bits(PIXMAN_x8r8g8b8, w, h,
>>>> + po->shadow_buffer, w * 4);
>>>> +
>>>> + if (!po->shadow_image) {
>>>> + free(po->shadow_buffer);
>>>> + free(po);
>>>> + return -1;
>>>> + }
>>>> +
>>>> + pixman_image_set_transform(po->shadow_image, &transform);
>>>> +
>>>> output->renderer_state = po;
>>>>
>>>> return 0;
>>>> @@ -416,7 +506,10 @@ pixman_renderer_output_destroy(struct weston_output
>>>> *output)
>>>> {
>>>> struct pixman_output_state *po = get_output_state(output);
>>>>
>>>> + pixman_image_unref(po->shadow_image);
>>>> pixman_image_unref(po->hw_buffer);
>>>> +
>>>> + po->shadow_image = NULL;
>>>> po->hw_buffer = NULL;
>>>>
>>>> free(po);
>>>> --
>>>> 1.7.10.4
>>>>
>>>> _______________________________________________
>>>> wayland-devel mailing list
>>>> wayland-devel at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>>
>>> _______________________________________________
>>> wayland-devel mailing list
>>> wayland-devel at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>>>
>>
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
More information about the wayland-devel
mailing list