On Mon, Apr 07, 2014 at 03:01:01PM +0100, Neil Roberts wrote:
> Jason Ekstrand wrote:
>
> > We may still have a problem if the client changes buffer formats
> > mid-stream. Say it starts out with RGB565 to save memory but latter
> > decides it wants alpha so it switches to ARGB8888. We should
> > probably detect this and make sure glTexImage2D gets called again to
> > reset the internal format.
>
> Yeah, you're right. I think it would be good to fix this especially
> now that we support multiple outputs with different formats. It's not
> entirely unlikely that a client might try to detect when it is moved
> from one output to another with a different bit depth and then try to
> switch buffer depths. The original patch has already landed in master
> so here is an extra patch.
Thanks, that's looks really good now.
Kristian
> Regards,
> - Neil
>
> ------- >8 --------------- (use git am --scissors to automatically chop here)
>
> If the client attaches a new SHM buffer with a different format from a
> previous one then we ought to trigger a full upload so that GL can
> allocate a new texture. Otherwise Weston would technically be doing
> invalid operations because it would call glTexSubImage2D with a
> different format from the one specified in glTexImage2D. Presumably it
> would also mean GL would have to convert the buffer as it copies the
> sub-region in which isn't ideal.
>
> This patch makes it decide the GL format when the buffer is attached
> instead of when processing the damage and it will set
> needs_full_upload if it is different from what it used before.
> ---
> src/gl-renderer.c | 45 ++++++++++++++++++++++-----------------------
> 1 file changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/src/gl-renderer.c b/src/gl-renderer.c
> index 1cebc79..f534f51 100644
> --- a/src/gl-renderer.c
> +++ b/src/gl-renderer.c
> @@ -90,6 +90,12 @@ struct gl_surface_state {
> int needs_full_upload;
> pixman_region32_t texture_damage;
>
> + /* These are only used by SHM surfaces to detect when we need
> + * to do a full upload to specify a new internal texture
> + * format */
> + GLenum gl_format;
> + GLenum gl_pixel_type;
> +
> EGLImageKHR images[3];
> GLenum target;
> int num_images;
> @@ -998,8 +1004,6 @@ gl_renderer_flush_damage(struct weston_surface *surface)
> struct weston_buffer *buffer = gs->buffer_ref.buffer;
> struct weston_view *view;
> int texture_used;
> - GLenum format;
> - int pixel_type;
>
> #ifdef GL_EXT_unpack_subimage
> pixman_box32_t *rectangles;
> @@ -1032,29 +1036,13 @@ gl_renderer_flush_damage(struct weston_surface
> *surface)
> !gs->needs_full_upload)
> goto done;
>
> - switch (wl_shm_buffer_get_format(buffer->shm_buffer)) {
> - case WL_SHM_FORMAT_XRGB8888:
> - case WL_SHM_FORMAT_ARGB8888:
> - format = GL_BGRA_EXT;
> - pixel_type = GL_UNSIGNED_BYTE;
> - break;
> - case WL_SHM_FORMAT_RGB565:
> - format = GL_RGB;
> - pixel_type = GL_UNSIGNED_SHORT_5_6_5;
> - break;
> - default:
> - weston_log("warning: unknown shm buffer format\n");
> - format = GL_BGRA_EXT;
> - pixel_type = GL_UNSIGNED_BYTE;
> - }
> -
> glBindTexture(GL_TEXTURE_2D, gs->textures[0]);
>
> if (!gr->has_unpack_subimage) {
> wl_shm_buffer_begin_access(buffer->shm_buffer);
> - glTexImage2D(GL_TEXTURE_2D, 0, format,
> + glTexImage2D(GL_TEXTURE_2D, 0, gs->gl_format,
> gs->pitch, buffer->height, 0,
> - format, pixel_type,
> + gs->gl_format, gs->gl_pixel_type,
> wl_shm_buffer_get_data(buffer->shm_buffer));
> wl_shm_buffer_end_access(buffer->shm_buffer);
>
> @@ -1069,9 +1057,9 @@ gl_renderer_flush_damage(struct weston_surface *surface)
> glPixelStorei(GL_UNPACK_SKIP_PIXELS_EXT, 0);
> glPixelStorei(GL_UNPACK_SKIP_ROWS_EXT, 0);
> wl_shm_buffer_begin_access(buffer->shm_buffer);
> - glTexImage2D(GL_TEXTURE_2D, 0, format,
> + glTexImage2D(GL_TEXTURE_2D, 0, gs->gl_format,
> gs->pitch, buffer->height, 0,
> - format, pixel_type, data);
> + gs->gl_format, gs->gl_pixel_type, data);
> wl_shm_buffer_end_access(buffer->shm_buffer);
> goto done;
> }
> @@ -1087,7 +1075,7 @@ gl_renderer_flush_damage(struct weston_surface *surface)
> glPixelStorei(GL_UNPACK_SKIP_ROWS_EXT, r.y1);
> glTexSubImage2D(GL_TEXTURE_2D, 0, r.x1, r.y1,
> r.x2 - r.x1, r.y2 - r.y1,
> - format, pixel_type, data);
> + gs->gl_format, gs->gl_pixel_type, data);
> }
> wl_shm_buffer_end_access(buffer->shm_buffer);
> #endif
> @@ -1127,6 +1115,7 @@ gl_renderer_attach_shm(struct weston_surface *es,
> struct weston_buffer *buffer,
> 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, gl_pixel_type;
> int pitch;
>
> buffer->shm_buffer = shm_buffer;
> @@ -1137,14 +1126,20 @@ gl_renderer_attach_shm(struct weston_surface *es,
> struct weston_buffer *buffer,
> case WL_SHM_FORMAT_XRGB8888:
> gs->shader = &gr->texture_shader_rgbx;
> pitch = wl_shm_buffer_get_stride(shm_buffer) / 4;
> + gl_format = GL_BGRA_EXT;
> + gl_pixel_type = GL_UNSIGNED_BYTE;
> break;
> case WL_SHM_FORMAT_ARGB8888:
> gs->shader = &gr->texture_shader_rgba;
> pitch = wl_shm_buffer_get_stride(shm_buffer) / 4;
> + gl_format = GL_BGRA_EXT;
> + gl_pixel_type = GL_UNSIGNED_BYTE;
> break;
> case WL_SHM_FORMAT_RGB565:
> gs->shader = &gr->texture_shader_rgbx;
> pitch = wl_shm_buffer_get_stride(shm_buffer) / 2;
> + gl_format = GL_RGB;
> + gl_pixel_type = GL_UNSIGNED_SHORT_5_6_5;
> break;
> default:
> weston_log("warning: unknown shm buffer format\n");
> @@ -1157,10 +1152,14 @@ gl_renderer_attach_shm(struct weston_surface *es,
> struct weston_buffer *buffer,
> * happening, we need to allocate a new texture buffer. */
> if (pitch != gs->pitch ||
> buffer->height != gs->height ||
> + gl_format != gs->gl_format ||
> + gl_pixel_type != gs->gl_pixel_type ||
> gs->buffer_type != BUFFER_TYPE_SHM) {
> gs->pitch = pitch;
> gs->height = buffer->height;
> gs->target = GL_TEXTURE_2D;
> + gs->gl_format = gl_format;
> + gs->gl_pixel_type = gl_pixel_type;
> gs->buffer_type = BUFFER_TYPE_SHM;
> gs->needs_full_upload = 1;
> gs->y_inverted = 1;
> --
> 1.8.5.3
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
