On 06/04/2013 08:06 AM, Kristian Høgsberg wrote:
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...

That happens when reducing the size. The new buffer dimensions are not propagated to the surface until the call to surface->configure() that comes after the attach call. In that case, you add a rectangle to texture_damage that is bigger than the texture itself. When Weston tries to upload this rectangle with glTexSubImage2D(), the check for the rectangle being inside of the texture fails and nothing is uploaded.

I guess it would make sense to just make texture damage equal to the size of the texture. But there's a problem that texture_damage is in surface coordinates, so we either convert the buffer dimensions to that at this point, or change it to be in actual buffer coordinates.

Cheers,
Ander

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


_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to