[Cogl] [PATCH 2/5] Use GL_PACK_ALIGNMENT of 1 whenever possible

Neil Roberts neil at linux.intel.com
Tue Mar 27 09:56:22 PDT 2012


The Intel driver currently has an optimisation when calling
glReadPixels into a PBO so that it will use a blit instead of the Mesa
fallback path. However this only works if the GL_PACK_ALIGNMENT is
exactly 1, even if this would be equivalent to a higher alignment
value because the bpp*width is already aligned. To make it more likely
to hit this fast path, we now detect this situation and explicitly use
an alignment of 1. To make this work the texture driver needs to be
passed down the bpp*width as well as the rowstride when configuring
the alignment.
---
 cogl/cogl-framebuffer.c                     |   10 ++++++++--
 cogl/cogl-texture-2d.c                      |    5 ++++-
 cogl/cogl-texture-driver.h                  |    1 +
 cogl/cogl-texture-private.h                 |    4 +++-
 cogl/cogl-texture-rectangle.c               |    5 ++++-
 cogl/cogl-texture.c                         |   23 ++++++++++++++++++++---
 cogl/driver/gl/cogl-texture-driver-gl.c     |   13 +++++++++++--
 cogl/driver/gles/cogl-texture-driver-gles.c |    5 ++++-
 8 files changed, 55 insertions(+), 11 deletions(-)

diff --git a/cogl/cogl-framebuffer.c b/cogl/cogl-framebuffer.c
index af95cac..ee11f7c 100644
--- a/cogl/cogl-framebuffer.c
+++ b/cogl/cogl-framebuffer.c
@@ -2062,7 +2062,10 @@ cogl_framebuffer_read_pixels_into_bitmap (CoglFramebuffer *framebuffer,
       bpp = _cogl_pixel_format_get_bytes_per_pixel (read_format);
       rowstride = cogl_bitmap_get_rowstride (tmp_bmp);
 
-      ctx->texture_driver->prep_gl_for_pixels_download (ctx, rowstride, bpp);
+      ctx->texture_driver->prep_gl_for_pixels_download (ctx,
+                                                        rowstride,
+                                                        width,
+                                                        bpp);
 
       tmp_data = _cogl_bitmap_bind (tmp_bmp,
                                     COGL_BUFFER_ACCESS_WRITE,
@@ -2110,7 +2113,10 @@ cogl_framebuffer_read_pixels_into_bitmap (CoglFramebuffer *framebuffer,
 
       bpp = _cogl_pixel_format_get_bytes_per_pixel (bmp_format);
 
-      ctx->texture_driver->prep_gl_for_pixels_download (ctx, rowstride, bpp);
+      ctx->texture_driver->prep_gl_for_pixels_download (ctx,
+                                                        rowstride,
+                                                        width,
+                                                        bpp);
 
       pixels = _cogl_bitmap_bind (shared_bmp,
                                   COGL_BUFFER_ACCESS_WRITE,
diff --git a/cogl/cogl-texture-2d.c b/cogl/cogl-texture-2d.c
index a3f0426..d3f12f1 100644
--- a/cogl/cogl-texture-2d.c
+++ b/cogl/cogl-texture-2d.c
@@ -825,7 +825,10 @@ _cogl_texture_2d_get_data (CoglTexture     *tex,
                                           &gl_format,
                                           &gl_type);
 
-  ctx->texture_driver->prep_gl_for_pixels_download (ctx, rowstride, bpp);
+  ctx->texture_driver->prep_gl_for_pixels_download (ctx,
+                                                    rowstride,
+                                                    tex_2d->width,
+                                                    bpp);
 
   _cogl_bind_gl_texture_transient (GL_TEXTURE_2D,
                                    tex_2d->gl_texture,
diff --git a/cogl/cogl-texture-driver.h b/cogl/cogl-texture-driver.h
index bd48b76..56ec805 100644
--- a/cogl/cogl-texture-driver.h
+++ b/cogl/cogl-texture-driver.h
@@ -123,6 +123,7 @@ struct _CoglTextureDriver
    * this function that it uses internally. */
   void
   (* prep_gl_for_pixels_download) (CoglContext *ctx,
+                                   int image_width,
                                    int pixels_rowstride,
                                    int pixels_bpp);
 
diff --git a/cogl/cogl-texture-private.h b/cogl/cogl-texture-private.h
index 9695179..f949a2e 100644
--- a/cogl/cogl-texture-private.h
+++ b/cogl/cogl-texture-private.h
@@ -240,7 +240,9 @@ void
 _cogl_texture_prep_gl_alignment_for_pixels_upload (int pixels_rowstride);
 
 void
-_cogl_texture_prep_gl_alignment_for_pixels_download (int pixels_rowstride);
+_cogl_texture_prep_gl_alignment_for_pixels_download (int bpp,
+                                                     int width,
+                                                     int rowstride);
 
 /* Utility function to use as a fallback for getting the data of any
    texture via the framebuffer */
diff --git a/cogl/cogl-texture-rectangle.c b/cogl/cogl-texture-rectangle.c
index e4db54d..f49dd39 100644
--- a/cogl/cogl-texture-rectangle.c
+++ b/cogl/cogl-texture-rectangle.c
@@ -552,7 +552,10 @@ _cogl_texture_rectangle_get_data (CoglTexture     *tex,
                                           &gl_format,
                                           &gl_type);
 
-  ctx->texture_driver->prep_gl_for_pixels_download (ctx, rowstride, bpp);
+  ctx->texture_driver->prep_gl_for_pixels_download (ctx,
+                                                    rowstride,
+                                                    tex_rect->width,
+                                                    bpp);
 
   _cogl_bind_gl_texture_transient (GL_TEXTURE_RECTANGLE_ARB,
                                    tex_rect->gl_texture,
diff --git a/cogl/cogl-texture.c b/cogl/cogl-texture.c
index df4fdcc..0cb38c8 100644
--- a/cogl/cogl-texture.c
+++ b/cogl/cogl-texture.c
@@ -273,12 +273,29 @@ _cogl_texture_prep_gl_alignment_for_pixels_upload (int pixels_rowstride)
 }
 
 void
-_cogl_texture_prep_gl_alignment_for_pixels_download (int pixels_rowstride)
+_cogl_texture_prep_gl_alignment_for_pixels_download (int bpp,
+                                                     int width,
+                                                     int rowstride)
 {
+  int alignment;
+
   _COGL_GET_CONTEXT (ctx, NO_RETVAL);
 
-  GE( ctx, glPixelStorei (GL_PACK_ALIGNMENT,
-                          calculate_alignment (pixels_rowstride)) );
+  /* If no padding is needed then we can always use an alignment of 1.
+   * We want to do this even though it is equivalent to the alignment
+   * of the rowstride because the Intel driver in Mesa currently has
+   * an optimisation when reading data into a PBO that only works if
+   * the alignment is exactly 1.
+   *
+   * https://bugs.freedesktop.org/show_bug.cgi?id=46632
+   */
+
+  if (rowstride == bpp * width)
+    alignment = 1;
+  else
+    alignment = calculate_alignment (rowstride);
+
+  GE( ctx, glPixelStorei (GL_PACK_ALIGNMENT, alignment) );
 }
 
 /* FIXME: wrap modes should be set on pipelines not textures */
diff --git a/cogl/driver/gl/cogl-texture-driver-gl.c b/cogl/driver/gl/cogl-texture-driver-gl.c
index 31f6f83..bb8c67d 100644
--- a/cogl/driver/gl/cogl-texture-driver-gl.c
+++ b/cogl/driver/gl/cogl-texture-driver-gl.c
@@ -116,6 +116,7 @@ _cogl_texture_driver_prep_gl_for_pixels_upload (CoglContext *ctx,
  * a larger destination buffer */
 static void
 prep_gl_for_pixels_download_full (CoglContext *ctx,
+                                  int image_width,
                                   int pixels_rowstride,
                                   int image_height,
                                   int pixels_src_x,
@@ -130,15 +131,23 @@ prep_gl_for_pixels_download_full (CoglContext *ctx,
   if (cogl_has_feature (ctx, COGL_FEATURE_ID_TEXTURE_3D))
     GE( ctx, glPixelStorei (GL_PACK_IMAGE_HEIGHT, image_height) );
 
-  _cogl_texture_prep_gl_alignment_for_pixels_download (pixels_rowstride);
+  _cogl_texture_prep_gl_alignment_for_pixels_download (pixels_bpp,
+                                                       image_width,
+                                                       pixels_rowstride);
 }
 
 static void
 _cogl_texture_driver_prep_gl_for_pixels_download (CoglContext *ctx,
+                                                      int image_width,
                                                   int pixels_rowstride,
                                                   int pixels_bpp)
 {
-  prep_gl_for_pixels_download_full (ctx, pixels_rowstride, 0, 0, 0, pixels_bpp);
+  prep_gl_for_pixels_download_full (ctx,
+                                    pixels_rowstride,
+                                    image_width,
+                                    0 /* image height */,
+                                    0, 0, /* pixels_src_x/y */
+                                    pixels_bpp);
 }
 
 static void
diff --git a/cogl/driver/gles/cogl-texture-driver-gles.c b/cogl/driver/gles/cogl-texture-driver-gles.c
index e6a67d6..bce772e 100644
--- a/cogl/driver/gles/cogl-texture-driver-gles.c
+++ b/cogl/driver/gles/cogl-texture-driver-gles.c
@@ -132,9 +132,12 @@ _cogl_texture_driver_prep_gl_for_pixels_upload (CoglContext *ctx,
 static void
 _cogl_texture_driver_prep_gl_for_pixels_download (CoglContext *ctx,
                                                   int pixels_rowstride,
+                                                  int image_width,
                                                   int pixels_bpp)
 {
-  _cogl_texture_prep_gl_alignment_for_pixels_download (pixels_rowstride);
+  _cogl_texture_prep_gl_alignment_for_pixels_download (pixels_bpp,
+                                                       image_width,
+                                                       pixels_rowstride);
 }
 
 static CoglBitmap *
-- 
1.7.3.16.g9464b



More information about the Cogl mailing list