From ec5009fa23b11f9df9ffeead7bdf0de3fa12cd07 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Thu, 22 Mar 2012 11:40:16 +0000 Subject: [PATCH] Use GL_PACK_ALIGNMENT of 1 whenever possible 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. Reviewed-by: Robert Bragg --- 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 dff08fff9..b55b016a5 100644 --- a/cogl/cogl-framebuffer.c +++ b/cogl/cogl-framebuffer.c @@ -2065,7 +2065,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, @@ -2113,7 +2116,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 7328423f0..5e988bd6f 100644 --- a/cogl/cogl-texture-2d.c +++ b/cogl/cogl-texture-2d.c @@ -832,7 +832,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 bd48b769e..56ec80567 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 1fd6bb167..8cec89f0c 100644 --- a/cogl/cogl-texture-private.h +++ b/cogl/cogl-texture-private.h @@ -246,7 +246,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 3f8594f79..192e0dc05 100644 --- a/cogl/cogl-texture-rectangle.c +++ b/cogl/cogl-texture-rectangle.c @@ -561,7 +561,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 261734516..294898386 100644 --- a/cogl/cogl-texture.c +++ b/cogl/cogl-texture.c @@ -274,12 +274,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 31f6f836c..bb8c67d25 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 e6a67d66e..bce772ef2 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 *