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

[Anuj Phogat]

On Fri, Feb 10, 2012 at 3:00 PM, Brian Paul <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>
>> ---
>>  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.


> 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.

Thanks
Anuj
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120210/853eb149/attachment-0001.html>