[Mesa-dev] [PATCH 04/11] mesa: plumb offset/size parameters through GetTexImage code

[Brian Paul]

On 12/16/2014 03:22 PM, Laura Ekstrand wrote:
>
>
> On Sat, Dec 13, 2014 at 6:42 AM, Brian Paul <brianp at vmware.com
> <mailto:brianp at vmware.com>> wrote:
>
>     Needed for GL_ARB_get_texture_sub_image.  But at this point, the
>     offsets are always zero and the sizes match the whole texture image.
>     ---
>       src/mesa/main/texgetimage.c | 105
>     ++++++++++++++++++++++++--------------------
>       1 file changed, 58 insertions(+), 47 deletions(-)
>
>     diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
>     index 03ad5f4..71c25bb 100644
>     --- a/src/mesa/main/texgetimage.c
>     +++ b/src/mesa/main/texgetimage.c
>     @@ -74,12 +74,11 @@ type_needs_clamping(GLenum type)
>        */
>       static void
>       get_tex_depth(struct gl_context *ctx, GLuint dimensions,
>     +              GLint xoffset, GLint yoffset, GLint zoffset,
>     +              GLsizei width, GLsizei height, GLint depth,
>                     GLenum format, GLenum type, GLvoid *pixels,
>                     struct gl_texture_image *texImage)
>       {
>     -   const GLint width = texImage->Width;
>     -   GLint height = texImage->Height;
>     -   GLint depth = texImage->Depth;
>          GLint img, row;
>          GLfloat *depthRow = malloc(width * sizeof(GLfloat));
>
>     @@ -98,9 +97,9 @@ get_tex_depth(struct gl_context *ctx, GLuint
>     dimensions,
>             GLint srcRowStride;
>
> You are not checking here to make sure that zoffset + img < depth.  I
> looked in the next patch (implement _mesa_GetTextureSubImage) and found
> that the dimensions_error_check does this.  It might be good to make a
> note of that here.

I've added a comment and assertion there and before similar loops.


>
>             /* map src texture buffer */
>     -      ctx->Driver.MapTextureImage(ctx, texImage, img,
>     -                                  0, 0, width, height, GL_MAP_READ_BIT,
>     -                                &srcMap, &srcRowStride);
>     +      ctx->Driver.MapTextureImage(ctx, texImage, zoffset + img,
>     +                                  xoffset, yoffset, width, height,
>     +                                  GL_MAP_READ_BIT, &srcMap,
>     &srcRowStride);
>
>             if (srcMap) {
>                for (row = 0; row < height; row++) {
>
> Why didn't you update ctx->Driver.UnmapTextureImage(ctx, texImage, img)
> to take zoffset + img here?  You did that for
> get_tex_rgba_uncompressed.  Is there some reason that it has to be
> UnmapTextureImage(.. img only) for all of the other functions?

Those were oversights.  Fixed now.  I'll post a v2 patch.

Thanks.

-Brian