[PATCH] Avoid unnecessarily re-allocating texture buffer when the size hasn't changed.

[Kristian Høgsberg]

On Thu, Jun 06, 2013 at 04:41:30PM -0700, Sinclair Yeh wrote:
> v4:
> Incorporated krh and anderco's comments.  Now adding newly allocated
> buffer's dimensions to texture_damage
> 
> v3:
> * Removed unnecessary parentheses
> * Added check for switching from EGL image to SHM buffer
> * Moved shader assignment out of IF condition
> 
> v2:
> Fixed the wrong comparison
> 
> v1:
> Depending on specific DRI driver implementation, glTexImage2D() with data
> set to NULL may or may not re-allocate the texture buffer each time it is
> called.  Unintended consequences happen if later glTexSubImage2D() is called
> to only update a sub-region of the texture buffer.
> 
> I've explored moving glTexImage2D() from gl_renderer_attach() and simply
> mark the texture dirty, but the current implemention seems cleaner because
> I won't have to worry about calling ensure_textures() and re-assigning
> gs->shader unnecessarily.

Thanks, I think we got it now.  The #1 issue I saw is still there, but
I've realized I only see it with gnome-terminal.  I think it's an
issue in how it resizes and restricts its size to a integer number of
character cells - either way, I don't see it with other gtk+ apps or
the weston sample apps.  Thanks for keeping this going - patch
committed and pushed.

Kristian

> ---
>  src/gl-renderer.c |   31 +++++++++++++++++++++++--------
>  1 files changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/src/gl-renderer.c b/src/gl-renderer.c
> index d783a0b..63c9c32 100644
> --- a/src/gl-renderer.c
> +++ b/src/gl-renderer.c
> @@ -1204,15 +1204,30 @@ gl_renderer_attach(struct weston_surface *es, struct wl_buffer *buffer)
>  	}
>  
>  	if (wl_buffer_is_shm(buffer)) {
> -		gs->pitch = wl_shm_buffer_get_stride(buffer) / 4;
> -		gs->height = wl_shm_buffer_get_height(buffer);
> -		gs->target = GL_TEXTURE_2D;
> +		/* Only allocate a texture if it doesn't match existing one.
> + 		 * If gs->num_images is not 0, then a switch from DRM allocated
> + 		 * buffer to a SHM buffer is happening, and we need to allocate
> + 		 * a new texture buffer.
> + 		 */
> +		if (wl_shm_buffer_get_stride(buffer) / 4 != gs->pitch ||
> +		    wl_shm_buffer_get_height(buffer) != gs->height ||
> +		    gs->num_images > 0) {
> +			gs->pitch = wl_shm_buffer_get_stride(buffer) / 4;
> +			gs->height = wl_shm_buffer_get_height(buffer);
> +			gs->target = GL_TEXTURE_2D;
> +
> +			ensure_textures(gs, 1);
> +			glBindTexture(GL_TEXTURE_2D, gs->textures[0]);
> +			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);
> +		}
>  
> -		ensure_textures(gs, 1);
> -		glBindTexture(GL_TEXTURE_2D, gs->textures[0]);
> -		glTexImage2D(GL_TEXTURE_2D, 0, GL_BGRA_EXT,
> -			     gs->pitch, buffer->height, 0,
> -			     GL_BGRA_EXT, GL_UNSIGNED_BYTE, NULL);
>  		if (wl_shm_buffer_get_format(buffer) == WL_SHM_FORMAT_XRGB8888)
>  			gs->shader = &gr->texture_shader_rgbx;
>  		else
> -- 
> 1.7.7.6
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel