[Mesa-dev] [PATCH 09/16] mesa: When storing texture data for a 1D array, map each slice separately.

[Brian Paul]

On 09/29/2011 04:40 PM, Eric Anholt wrote:
> ---
>   src/mesa/main/texstore.c |   68 +++++++++++++++++++++++++++++++++++-----------
>   1 files changed, 52 insertions(+), 16 deletions(-)
>
> diff --git a/src/mesa/main/texstore.c b/src/mesa/main/texstore.c
> index cbed26c..2aaff0d 100644
> --- a/src/mesa/main/texstore.c
> +++ b/src/mesa/main/texstore.c
> @@ -4626,22 +4626,53 @@ _mesa_store_teximage2d(struct gl_context *ctx, GLenum target, GLint level,
>         return;
>      }
>
> -   /* Map dest texture buffer (write to whole region) */
> -   ctx->Driver.MapTextureImage(ctx, texImage, 0,
> -                               0, 0, width, height,
> -                               rwMode,
> -&dstMap,&dstRowStride);
> -   assert(dstMap);
> -   success = _mesa_texstore(ctx, 2, texImage->_BaseFormat,
> -                            texImage->TexFormat,
> -                            dstMap,
> -                            0, 0, 0,  /* dstX/Y/Zoffset */
> -                            dstRowStride,
> -&zeroImageOffset,
> -                            width, height, 1,
> -                            format, type, pixels, packing);
> -
> -   ctx->Driver.UnmapTextureImage(ctx, texImage, 0);
> +   if (target != GL_TEXTURE_1D_ARRAY) {
> +      /* Map dest texture buffer (write to whole region) */
> +      ctx->Driver.MapTextureImage(ctx, texImage, 0,
> +                                  0, 0, width, height,
> +                                  rwMode,
> +&dstMap,&dstRowStride);
> +      assert(dstMap);
> +      success = _mesa_texstore(ctx, 2, texImage->_BaseFormat,
> +                               texImage->TexFormat,
> +                               dstMap,
> +                               0, 0, 0,  /* dstX/Y/Zoffset */
> +                               dstRowStride,
> +&zeroImageOffset,
> +                               width, height, 1,
> +                               format, type, pixels, packing);
> +
> +      ctx->Driver.UnmapTextureImage(ctx, texImage, 0);
> +   } else {
> +      const GLint srcStride =
> +         _mesa_image_row_stride(packing, width, format, type);
> +      int y;
> +
> +      success = GL_TRUE;
> +
> +      for (y = 0; y<  height; y++) {
> +         /* Map dest texture buffer (write to whole region) */
> +         ctx->Driver.MapTextureImage(ctx, texImage, y,
> +                                     0, 0, width, 1,
> +                                     rwMode,
> +&dstMap,&dstRowStride);
> +         assert(dstMap);
> +         success = _mesa_texstore(ctx, 2, texImage->_BaseFormat,
> +                                  texImage->TexFormat,
> +                                  dstMap,
> +                                  0, 0, 0,  /* dstX/Y/Zoffset */
> +                                  dstRowStride,
> +&zeroImageOffset,
> +                                  width, 1, 1,
> +                                  format, type, pixels, packing);
> +         ctx->Driver.UnmapTextureImage(ctx, texImage, y);
> +
> +         if (!success)
> +            break;
> +
> +         pixels += srcStride;
> +      }
> +   }

Just a minor stylistic comment on the above code.  I'd prefer it to be:

  if (target == GL_TEXTURE_1D_ARRAY) {
     /* 1D array code here */
  }
  else {
     /* other target code here */
  }

Maybe it's just me, but I find code easier to read when conditionals 
are structured like that.

In any case, the mesa and swrast changes look good to me.  I only 
glanced at the intel driver changes.

Reviewed-by: Brian Paul <brianp at vmware.com>