[Cogl] [PATCH 2/5] Use GL_PACK_ALIGNMENT of 1 whenever possible
Robert Bragg
robert at sixbynine.org
Tue Apr 3 08:40:16 PDT 2012
Reviewed-by: Robert Bragg <robert at linux.intel.com>
On Tue, Mar 27, 2012 at 5:56 PM, Neil Roberts <neil at linux.intel.com> wrote:
> 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
>
> _______________________________________________
> Cogl mailing list
> Cogl at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/cogl
More information about the Cogl
mailing list