[Mesa-dev] [PATCH 3/4] common: Fix PBOs for 1D_ARRAY.

[Neil Roberts]

Sorry for the late review.

Can you explain what this patch does? The previous code was doing a blit
like this:

      _mesa_meta_BlitFramebuffer(ctx, ctx->ReadBuffer, ctx->DrawBuffer,
                                 0, z * height, width, (z + 1) * height,
                                 xoffset, yoffset,
                                 xoffset + width, yoffset + height,
                                 GL_COLOR_BUFFER_BIT, GL_NEAREST);

However, it was first setting the height to 1 so that would end up like
this:

      _mesa_meta_BlitFramebuffer(ctx, ctx->ReadBuffer, ctx->DrawBuffer,
                                 0, z * 1, width, (z + 1) * 1,
                                 xoffset, yoffset,
                                 xoffset + width, yoffset + 1,
                                 GL_COLOR_BUFFER_BIT, GL_NEAREST);

The patch makes it do this:

         _mesa_meta_BlitFramebuffer(ctx, ctx->ReadBuffer, ctx->DrawBuffer,
                                    0, z, width, z + 1,
                                    xoffset, yoffset,
                                    xoffset + width, yoffset + 1,
                                    GL_COLOR_BUFFER_BIT, GL_NEAREST);

This looks like it would have exactly the same result.

The patch doesn't modify the blit call for slice 0 which looks wrong to
me.

Neither the version with or without the patch appears to handle the
yoffset correctly because for the 1D array case this needs to be
interpreted as a slice offset.

I'm attaching a patch which I think fixes it, although I haven't managed
to test it with the arb_direct_state_access/gettextureimage-targets test
so I don't know if I'm misunderstanding something. It is also not
complete because it doesn't touch GetTexSubImage. I have tested it with
the texsubimage test which does use the yoffset, but in order to use
that test the code path first needs to be able to accept the
IMAGE_HEIGHT packing option which I will also post a patch for.

Regards,
- Neil

------- >8 --------------- (use git am --scissors to automatically chop here)
Subject: meta: Fix the y offset for 1D_ARRAY in _mesa_meta_pbo_TexSubImage

This partially reverts 546aba143d13ba3f99 and brings back the if
statement to move the height over to the depth. However it
additionally moves the yoffset to the zoffset. This fixes texsubimage
array pbo for 1D_ARRAY textures.

The previous fix in 546aba143 wasn't taking into account the yoffset
correctly because it needs to be used to alter the selected layer.
---
 src/mesa/drivers/common/meta_tex_subimage.c | 36 ++++++++++++++---------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/mesa/drivers/common/meta_tex_subimage.c b/src/mesa/drivers/common/meta_tex_subimage.c
index 3965d31..be89102 100644
--- a/src/mesa/drivers/common/meta_tex_subimage.c
+++ b/src/mesa/drivers/common/meta_tex_subimage.c
@@ -138,7 +138,7 @@ _mesa_meta_pbo_TexSubImage(struct gl_context *ctx, GLuint dims,
    struct gl_texture_image *pbo_tex_image;
    GLenum status;
    bool success = false;
-   int z, iters;
+   int z;
 
    /* XXX: This should probably be passed in from somewhere */
    const char *where = "_mesa_meta_pbo_TexSubImage";
@@ -193,6 +193,16 @@ _mesa_meta_pbo_TexSubImage(struct gl_context *ctx, GLuint dims,
    _mesa_BindFramebuffer(GL_READ_FRAMEBUFFER, fbos[0]);
    _mesa_BindFramebuffer(GL_DRAW_FRAMEBUFFER, fbos[1]);
 
+   if (tex_image->TexObject->Target == GL_TEXTURE_1D_ARRAY) {
+      assert(depth == 1);
+      assert(zoffset == 0);
+      depth = height;
+      height = 1;
+      image_height = 1;
+      zoffset = yoffset;
+      yoffset = 0;
+   }
+
    _mesa_meta_bind_fbo_image(GL_READ_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
                              pbo_tex_image, 0);
    /* If this passes on the first layer it should pass on the others */
@@ -216,28 +226,18 @@ _mesa_meta_pbo_TexSubImage(struct gl_context *ctx, GLuint dims,
                                   GL_COLOR_BUFFER_BIT, GL_NEAREST))
       goto fail;
 
-   iters = tex_image->TexObject->Target == GL_TEXTURE_1D_ARRAY ?
-           height : depth;
-
-   for (z = 1; z < iters; z++) {
+   for (z = 1; z < depth; z++) {
       _mesa_meta_bind_fbo_image(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
                                 tex_image, zoffset + z);
 
       _mesa_update_state(ctx);
 
-      if (tex_image->TexObject->Target == GL_TEXTURE_1D_ARRAY)
-         _mesa_meta_BlitFramebuffer(ctx, ctx->ReadBuffer, ctx->DrawBuffer,
-                                    0, z, width, z + 1,
-                                    xoffset, yoffset,
-                                    xoffset + width, yoffset + 1,
-                                    GL_COLOR_BUFFER_BIT, GL_NEAREST);
-      else
-         _mesa_meta_BlitFramebuffer(ctx, ctx->ReadBuffer, ctx->DrawBuffer,
-                                    0, z * image_height,
-                                    width, z * image_height + height,
-                                    xoffset, yoffset,
-                                    xoffset + width, yoffset + height,
-                                    GL_COLOR_BUFFER_BIT, GL_NEAREST);
+      _mesa_meta_BlitFramebuffer(ctx, ctx->ReadBuffer, ctx->DrawBuffer,
+                                 0, z * image_height,
+                                 width, z * image_height + height,
+                                 xoffset, yoffset,
+                                 xoffset + width, yoffset + height,
+                                 GL_COLOR_BUFFER_BIT, GL_NEAREST);
    }
 
    success = true;
-- 
1.9.3