[PATCH weston-ivi-shell v2 01/10] This vfunc lets us read out a rectangle of pixels from the currently attached surface buffer.
Nobuhiko Tanibata
nobuhiko_tanibata at xddp.denso.co.jp
Fri Mar 14 05:29:36 PDT 2014
2014-03-13 17:47 に Pekka Paalanen さんは書きました:
> On Wed, 12 Mar 2014 23:46:35 +0900
> Nobuhiko Tanibata <NOBUHIKO_TANIBATA at xddp.denso.co.jp> wrote:
>
>> Signed-off-by: Nobuhiko Tanibata <NOBUHIKO_TANIBATA at xddp.denso.co.jp>
>> ---
>>
>> Changes for v2:
>> - gl_renderer_read_shmbuffer_pixels to support different types of
>> format of
>> shmbuffer.
>>
>> src/compositor.h | 3 ++
>> src/gl-renderer.c | 114
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 117 insertions(+)
>>
>> diff --git a/src/compositor.h b/src/compositor.h
>> index 22a485f..ace75da 100644
>> --- a/src/compositor.h
>> +++ b/src/compositor.h
>> @@ -540,6 +540,9 @@ struct weston_renderer {
>> pixman_format_code_t format, void *pixels,
>> uint32_t x, uint32_t y,
>> uint32_t width, uint32_t height);
>> + int (*read_surface_pixels)(struct weston_surface *es,
>> + pixman_format_code_t format, void *pixels,
>> + int x, int y, int width, int height);
>> void (*repaint_output)(struct weston_output *output,
>> pixman_region32_t *output_damage);
>> void (*flush_damage)(struct weston_surface *surface);
>> diff --git a/src/gl-renderer.c b/src/gl-renderer.c
>> index 0e5afbe..7881fd8 100644
>> --- a/src/gl-renderer.c
>> +++ b/src/gl-renderer.c
>> @@ -106,6 +106,8 @@ struct gl_renderer {
>> EGLContext egl_context;
>> EGLConfig egl_config;
>>
>> + GLuint fbo;
>> +
>> struct wl_array vertices;
>> struct wl_array vtxcnt;
>>
>> @@ -585,6 +587,114 @@ out:
>> pixman_region32_fini(&repaint);
>> }
>>
>> +static int
>> +gl_renderer_read_shmbuffer_pixels(struct wl_shm_buffer *shm_buffer,
>> + void *pixels, int x, int y, int width, int height)
>
> How is 'pixels' allocated, what should we assume?
>
> Is the size of the 'pixels' buffer as width*height*4 bytes, with
> stride=width*4 bytes?
>
> What do we assume about the 'pixels' color format?
>
> What if the shm_buffer has color format RGB565?
>
> What if shm_buffer stride is more than width*4 bytes?
>
> But, see below.
>
>> +{
>> + int i;
>> + int pixel_size = 0;
>> + uint8_t *ptr = NULL;
>> + int32_t src_width, src_height, src_stride;
>> + int32_t dst_stride;
>> + int32_t offset;
>> +
>> + /* Get some parameters of wl_shm_buffer. */
>> + src_width = wl_shm_buffer_get_width(shm_buffer);
>> + src_height = wl_shm_buffer_get_height(shm_buffer);
>> + src_stride = wl_shm_buffer_get_stride(shm_buffer);
>> +
>> + assert((src_width > 0) && (src_height > 0));
>> + if ((src_width <= 0) || (src_height <= 0))
>> + return -1;
>> +
>> + /* the start of reading position has to be changed. */
>> + ptr = wl_shm_buffer_get_data(shm_buffer);
>> + ptr += y * src_stride;
>> + if ((x == 0) && (width == src_width) && (height <= (src_height -
>> y))) {
>> + /* If x is 0 and widths are the same,
>> + * whole pixel can be copied in only one time.
>> + */
>> + memcpy(pixels, ptr, src_stride * height);
>> + } else {
>> + /* If x is not 0,
>> + * every line have to be copied one by one.
>> + */
>> + pixel_size = src_stride / src_width;
>> + assert(pixel_size < 5);
>> + if (pixel_size > 4) return -1;
>> +
>> + dst_stride = width * pixel_size;
>> + offset = x * pixel_size;
>> +
>> + for (i = 0; i < height; i++, ptr += src_stride) {
>> + memcpy(pixels, ptr + offset, dst_stride);
>> + pixels = (uint8_t*)pixels + dst_stride;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int
>> +gl_renderer_read_glsurface_pixels(struct weston_surface *es,
>> + pixman_format_code_t format, void *pixels,
>> + int x, int y, int width, int height)
>> +{
>> + struct weston_compositor *ec = es->compositor;
>> + struct gl_renderer *gr = get_renderer(ec);
>> + struct gl_surface_state *gs = get_surface_state(es);
>> + GLenum gl_format;
>> +
>> + switch (format) {
>> + case PIXMAN_a8r8g8b8:
>> + gl_format = GL_BGRA_EXT;
>> + break;
>> + case PIXMAN_a8b8g8r8:
>> + gl_format = GL_RGBA;
>> + break;
>> + default:
>> + return -1;
>> + }
>> +
>> + if (gr->fbo == 0)
>> + glGenFramebuffers(1, &gr->fbo);
>> + glBindFramebuffer(GL_FRAMEBUFFER, gr->fbo);
>> + glFramebufferTexture2D(GL_FRAMEBUFFER,
>> + GL_COLOR_ATTACHMENT0,
>> + GL_TEXTURE_2D,
>> + gs->textures[0], 0);
>> +
>> + glReadPixels(x, y, width, height,
>> + gl_format, GL_UNSIGNED_BYTE, pixels);
>> +
>> + glBindFramebuffer(GL_FRAMEBUFFER, 0);
>> +
>> + return 0;
>> +}
>> +
>> +static int
>> +gl_renderer_read_surface_pixels(struct weston_surface *es,
>> + pixman_format_code_t format, void *pixels,
>> + int x, int y, int width, int height)
>> +{
>> + struct weston_buffer *buffer = es->buffer_ref.buffer;
>> + struct wl_shm_buffer *shm_buffer = NULL;
>> + int ret = 0;
>> +
>> + if (buffer) {
>> + shm_buffer = wl_shm_buffer_get(buffer->resource);
>> + }
>> + if (shm_buffer) {
>> + ret = gl_renderer_read_shmbuffer_pixels(shm_buffer,
>> + pixels, x, y, width, height);
>> + } else {
>> + ret = gl_renderer_read_glsurface_pixels(es,
>> + format, pixels, x, y, width, height);
>> + }
>
> I just realized that we are dealing with the GL-renderer only here.
> That means that the surface will always have a GL texture, which you
> can read back, regardless of if the original wl_buffer was wl_shm or
> EGL based.
>
Hi pq,
Thanks, it is very useful comment.
Yes, you are right. I thought I try to remove wl_shm patch to release
this patch at once
according to your comment
However, as you mention below, I also think how to handle YUV format.
This
format, YUV, is a important in Automotive for handling capture of e.g.
Back
Guide monitor as well. So I need time to discuss more internally. So I
drop this patch
to rethink more.
And I will remove parts calling gl_renderer_read_surface_pixels in
westion_layout
to support capture snapshot per a surface. This is not mandatory to
support for the time being.
BR,
Nobuhiko
> Therefore I think you could remove the whole shm path. I suppose
> optimizing the shm read-back path is not too useful, right?
>
> That is also the reason why you would probably never hit the shm case
> to begin with. GL-renderer releases the wl_buffer as soon as it has
> copied its content into a texture, so the buffer pointer is likely
> always NULL.
>
> Sorry for not realizing that earlier.
>
> So just one more question: what about YUV color formats? That's why
> gl_surface_state has an array of textures instead of just one. YUV data
> can come in via EGL based buffers. You could at least return failure
> for unhandled color formats, maybe?
>
>
> Thanks,
> pq
>
>> +
>> + return ret;
>> +}
>> +
>> static void
>> repaint_views(struct weston_output *output, pixman_region32_t
>> *damage)
>> {
>> @@ -1602,6 +1712,9 @@ gl_renderer_destroy(struct weston_compositor
>> *ec)
>>
>> wl_signal_emit(&gr->destroy_signal, gr);
>>
>> + if (gr->fbo)
>> + glDeleteFramebuffers(1, &gr->fbo);
>> +
>> if (gr->has_bind_display)
>> gr->unbind_display(gr->egl_display, ec->wl_display);
>>
>> @@ -1699,6 +1812,7 @@ gl_renderer_create(struct weston_compositor *ec,
>> EGLNativeDisplayType display,
>> return -1;
>>
>> gr->base.read_pixels = gl_renderer_read_pixels;
>> + gr->base.read_surface_pixels = gl_renderer_read_surface_pixels;
>> gr->base.repaint_output = gl_renderer_repaint_output;
>> gr->base.flush_damage = gl_renderer_flush_damage;
>> gr->base.attach = gl_renderer_attach;
More information about the wayland-devel
mailing list