On Tue, May 28, 2013 at 05:28:49PM -0700, Sinclair Yeh wrote:
> 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.
> ---
>  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 be74eba..aa8f581 100644
> --- a/src/gl-renderer.c
> +++ b/src/gl-renderer.c
> @@ -68,6 +68,7 @@ struct gl_surface_state {
>  
>       struct weston_buffer_reference buffer_ref;
>       int pitch; /* in pixels */
> +     int height;
>  };
>  
>  struct gl_renderer {
> @@ -1213,18 +1214,31 @@ 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->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 ||
> +                 buffer->height != gs->height ||
> +                 gs->num_images > 0) {
> +                     gs->pitch = wl_shm_buffer_get_stride(buffer) / 4;
> +                     gs->height = buffer->height;
> +                     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);

Looking better now, but we still need to make sure we do a full upload
before we repaint.  If a client is clever about it's resizing and only
posts a partial damage request after attaching the new buffer, we end
up reallocating the texture storage but not uploading the full
contents.

However, weird behavior #1: This shouldn't happen with any of the
clients I have (they all post full damage on resize), yet, I still see
flicker when resizing.

Weird behavior #2: The fix should be something like this:

+                       pixman_region32_union_rect(&gs->texture_damage,
+                                                  &gs->texture_damage,
+                                                  0, 0,
+                                                  es->geometry.width,
+                                                  es->geometry.height);

after the glTexImage2D call, to make sure we upload all of the
texture.  Instead of always uploading the contents (which would upload
twice: immediately and then when we flush damage), we add the new
surface rectangle to texture_damage.  However, when I add that, the
window contents just sometimes completely disappears...

This is what's holding up the patch - I don't have a lot of time to
look into it right now, but it is something I want to get in this week
so any help is apprecaited.

Kristian

> +             }
>  
> -             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)
> +             if (wl_shm_buffer_get_format(buffer) ==
> +                 WL_SHM_FORMAT_XRGB8888)
>                       gs->shader = &gr->texture_shader_rgbx;
>               else
>                       gs->shader = &gr->texture_shader_rgba;
> +
>       } else if (gr->query_buffer(gr->egl_display, buffer,
>                                   EGL_TEXTURE_FORMAT, &format)) {
>               for (i = 0; i < gs->num_images; i++)
> @@ -1279,6 +1293,7 @@ gl_renderer_attach(struct weston_surface *es, struct 
> wl_buffer *buffer)
>               }
>  
>               gs->pitch = buffer->width;
> +             gs->height = buffer->height;
>       } else {
>               weston_log("unhandled buffer type!\n");
>               weston_buffer_reference(&gs->buffer_ref, NULL);
> -- 
> 1.7.7.6
> 
> _______________________________________________
> wayland-devel mailing list
> [email protected]
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to