[PATCH weston 2/3] gl-renderer: Fix initial upload of SHM buffer as texture

[Kristian Høgsberg]

On Fri, Jun 07, 2013 at 04:52:45PM +0300, Ander Conselvan de Oliveira wrote:
> The fix to not call glTexImage2D() on every attach does not properly
> set the texture damage region appropriately when the surface has a
> buffer transform with 90 or 270 degrees rotation, since it would simply
> multiply the buffer dimensions by the buffer scale, but in this case
> width and height are inverted.
> 
> A possible fix for this would be to add the properly transformed region
> to the texture damage region. However, there is a conversion back to
> buffer coordinates when doing the actual upload and the entire buffer
> needs to be uploaded anyway. So we just set a flag signalling that and
> handle that special case in gl_renderer_flush_damage().

Thanks, all three applied.  Just one comment below

> ---
>  src/gl-renderer.c |   19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/src/gl-renderer.c b/src/gl-renderer.c
> index b869856..c5d683c 100644
> --- a/src/gl-renderer.c
> +++ b/src/gl-renderer.c
> @@ -66,6 +66,7 @@ struct gl_surface_state {
>  
>  	GLuint textures[3];
>  	int num_textures;
> +	int needs_full_upload;
>  	pixman_region32_t texture_damage;
>  
>  	EGLImageKHR images[3];
> @@ -1147,6 +1148,16 @@ gl_renderer_flush_damage(struct weston_surface *surface)
>  	/* Mesa does not define GL_EXT_unpack_subimage */
>  	glPixelStorei(GL_UNPACK_ROW_LENGTH, gs->pitch);
>  	data = wl_shm_buffer_get_data(buffer);
> +
> +	if (gs->needs_full_upload) {
> +		glPixelStorei(GL_UNPACK_SKIP_PIXELS, 0);
> +		glPixelStorei(GL_UNPACK_SKIP_ROWS, 0);
> +		glTexSubImage2D(GL_TEXTURE_2D, 0,
> +				0, 0, gs->pitch, buffer->height,
> +				GL_BGRA_EXT, GL_UNSIGNED_BYTE, data);
> +		goto done;
> +	}
> +
>  	rectangles = pixman_region32_rectangles(&gs->texture_damage, &n);
>  	for (i = 0; i < n; i++) {
>  		pixman_box32_t r;
> @@ -1164,6 +1175,7 @@ gl_renderer_flush_damage(struct weston_surface *surface)
>  done:
>  	pixman_region32_fini(&gs->texture_damage);
>  	pixman_region32_init(&gs->texture_damage);
> +	gs->needs_full_upload = 0;
>  
>  	weston_buffer_reference(&gs->buffer_ref, NULL);
>  }
> @@ -1227,11 +1239,8 @@ gl_renderer_attach(struct weston_surface *es, struct wl_buffer *buffer)
>  			glTexImage2D(GL_TEXTURE_2D, 0, GL_BGRA_EXT,
>  				     gs->pitch, buffer->height, 0,
>  				     GL_BGRA_EXT, GL_UNSIGNED_BYTE, NULL);
> -			pixman_region32_union_rect(&gs->texture_damage,
> -						   &gs->texture_damage,
> -						   0, 0,
> -						   gs->pitch / es->buffer_scale,
> -						   gs->height / es->buffer_scale);
> +
> +			gs->needs_full_upload = 1;

This is close to what I originally suggested, except that we should be
able to avoid the glTexImage2D and use glTexImage2D (not
glTexSubImage2d) in flush_damage above instead.  It shouldn't make a
difference - glTexImage2D with NULL here just reallocates the texture
and the glTexSubImage2D in flush_damage uploads the data.  It's just
nicer to be able to combine the two into one glTexImage2D call,
however, that doesn't work right in mesa (I tried).  I think we're
hitting a bug in mesa where glTexImage2D doesn't detach the texture
from the EGLImage and we end up overwriting the previously attached
EGLImage.

This works and fixes the issues, so lets stick with this for 1.2.

Kristian

>  		}
>  
>  		if (wl_shm_buffer_get_format(buffer) == WL_SHM_FORMAT_XRGB8888)
> -- 
> 1.7.10.4
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel