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 <n...@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
>
>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
