Ah, now that I looked at your other patch I see why you have image_height.
On Wed, Feb 25, 2015 at 5:33 PM, Laura Ekstrand <la...@jlekstrand.net>
wrote:
> 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
