[Mesa-dev] [PATCH] meta: add pixel pack operations in glGetTexImage

Brian Paul brianp at vmware.com
Fri Feb 10 16:54:25 PST 2012


On 02/10/2012 05:25 PM, Anuj Phogat wrote:
> On Fri, Feb 10, 2012 at 3:00 PM, Brian Paul <brianp at vmware.com
> <mailto:brianp at vmware.com>> wrote:
>
>     On 02/10/2012 02:39 PM, Anuj Phogat wrote:
>
>         This patch adds the pixel pack operations in glGetTexImage for
>         compressed textures with unsigned, normalized values. It also
>         fixes the failures in intel oglconform pxstore-gettex due to
>         following sub test cases:
>
>           - Test all mipmaps with byte swapping enabled
>           - Test all small mipmaps with all allowable alignment values
>           - Test subimage packing for all mipmap levels
>
>         Bugzilla: https://bugs.freedesktop.org/__show_bug.cgi?id=40864
>         <https://bugs.freedesktop.org/show_bug.cgi?id=40864>
>
>         Note: This is a candidate for stable branches
>
>         Signed-off-by: Anuj Phogat<anuj.phogat at gmail.com
>         <mailto:anuj.phogat at gmail.com>>
>         ---
>           src/mesa/drivers/common/meta.c |   55
>         ++++++++++++++++++++++++++++++__+++++++--
>           1 files changed, 52 insertions(+), 3 deletions(-)
>
>         diff --git a/src/mesa/drivers/common/__meta.c
>         b/src/mesa/drivers/common/__meta.c
>         index aa5fef8..97c1912 100644
>         --- a/src/mesa/drivers/common/__meta.c
>         +++ b/src/mesa/drivers/common/__meta.c
>         @@ -53,6 +53,7 @@
>           #include "main/pixel.h"
>           #include "main/pbo.h"
>           #include "main/polygon.h"
>         +#include "main/pack.h"
>           #include "main/readpix.h"
>           #include "main/scissor.h"
>           #include "main/shaderapi.h"
>         @@ -3438,6 +3439,14 @@ _mesa_meta_GetTexImage(struct
>         gl_context *ctx,
>                                  GLenum format, GLenum type, GLvoid
>         *pixels,
>                                  struct gl_texture_image *texImage)
>           {
>         +   GLuint row;
>         +   GLfloat *srcRow;
>         +   void *tempImage, *dest;
>         +   const GLuint width = texImage->Width;
>         +   const GLuint height = texImage->Height;
>         +   const GLint tempRowLength = 0;
>         +   GLbitfield transferOps = 0x0;
>         +
>              /* We can only use the decompress-with-blit method here
>         if the texels are
>               * unsigned, normalized values.  We could handle signed
>         and unnormalized
>               * with floating point renderbuffers...
>         @@ -3445,14 +3454,54 @@ _mesa_meta_GetTexImage(struct
>         gl_context *ctx,
>              if (_mesa_is_format_compressed(__texImage->TexFormat)&&
>                  _mesa_get_format_datatype(__texImage->TexFormat)
>                  == GL_UNSIGNED_NORMALIZED) {
>         +
>                 struct gl_texture_object *texObj = texImage->TexObject;
>         -      const GLuint slice = 0; /* only 2D compressed textures
>         for now */
>         +      /* only 2D compressed textures for now */
>         +      const GLuint slice = 0;
>         +      const int dimensions = 2;
>         +
>         +      if (format == GL_LUMINANCE ||
>         +          format == GL_LUMINANCE_ALPHA) {
>         +         transferOps |= IMAGE_CLAMP_BIT;
>         +      }
>         +
>         +      /* Allocate a temporary 2D RGBA float buffer */
>         +      tempImage = malloc(width * height * 4 * sizeof(GLfloat));
>         +      if (tempImage == NULL) {
>         +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "glGetTexImage");
>         +         return;
>         +      }
>         +
>                 /* Need to unlock the texture here to prevent
>         deadlock... */
>                 _mesa_unlock_texture(ctx, texObj);
>         -      decompress_texture_image(ctx, texImage, slice, format,
>         type, pixels,
>         -                               ctx->Pack.RowLength);
>         +
>         +      /* Decompress in to temporary buffer with format = GL_RGBA,
>         +       * type = GL_FLOAT and RowLength = 0 then pack in to user
>         +       * supplied buffer
>         +       */
>         +      decompress_texture_image(ctx, texImage, slice, GL_RGBA,
>         GL_FLOAT,
>         +                               tempImage, tempRowLength);
>                 /* ... and relock it */
>                 _mesa_lock_texture(ctx, texObj);
>         +
>         +      srcRow = (GLfloat*) tempImage;
>         +      for (row = 0; row<  height; row++) {
>         +         dest = _mesa_image_address(__dimensions,&ctx->Pack,
>         pixels,
>
>         +                                    width, height, format, type,
>         +                                    0, row, 0);
>         +         if (_mesa_is_integer_format(__format)) {
>
>
>     I don't think the format can be integer-valued at this point if we
>     tested for datatype = GL_UNSIGNED_NORMALIZED above.
>
>
>         +            _mesa_pack_rgba_span_int(ctx, width, (GLuint
>         (*)[4]) srcRow,
>         +                                     format, type, dest);
>         +         }
>         +         else {
>         +            _mesa_pack_rgba_span_float(__ctx, width, (GLfloat
>         (*)[4]) srcRow,
>         +                                       format, type,
>         dest,&ctx->Pack,
>
>         +                                       transferOps);
>         +         }
>         +         srcRow += width * 4;
>         +      }
>         +
>         +      free(tempImage);
>              }
>              else {
>                 _mesa_get_teximage(ctx, format, type, pixels, texImage);
>
>
>
>     If the main problem here is the pixel store/pack operations, I
>     think that another fix is much simpler:
>
>     First, let's remove the destRowLength parameter to
>     decompress_texture_image().  We're just passing
>     ctx->Pack.RowLength but then we assign that value to
>     ctx->Pack.RowLength at line 3412. That's pointless.
>
>     Then, instead of calling _mesa_meta_begin(ctx, MESA_META_ALL) at
>     line 3277, use (MESA_META_ALL & ~MESA_META_PIXEL_STORE).  The
>     current pixel store params will stay in effect and
>     _mesa_ReadPixels() call at line 3413 will apply them for us.
>
>
> Yes, this fix works for me and is much simpler. I'll send out a patch
> for review.

Great.  Thanks.


>     BTW, I think we could avoid some FBO resizing/reallocating if we
>     change line 3295 to read:
>
>       if (width > decompress->Width || height > decompress->Height) {
>
> I agree.

Let's do that in a separate patch though.

-Brian


More information about the mesa-dev mailing list