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

[Laura Ekstrand]

This was an unfortunate artifact of rebasing; I fixed the 1D array bug
before figuring out a robust fix for the 2D array bug (7a49f2e).  I think
that you are right, although I'm confused what you are doing with the
variable image_height that you added.  It seems like you should be able to
get rid of image_height here. (But that's just a quick gut feeling.)

On Wed, Feb 25, 2015 at 8:19 AM, Neil Roberts <neil at linux.intel.com> wrote:

> 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
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150225/a055b1cf/attachment-0001.html>