<div dir="ltr">Ah, now that I looked at your other patch I see why you have image_height. </div><div class="gmail_extra"> <div class="gmail_quote">On Wed, Feb 25, 2015 at 5:33 PM, Laura Ekstrand <<a href="mailto:laura@jlekstrand.net" target="_blank">laura@jlekstrand.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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.) </div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"> <div class="gmail_quote">On Wed, Feb 25, 2015 at 8:19 AM, Neil Roberts <<a href="mailto:neil@linux.intel.com" target="_blank">neil@linux.intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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, <div><div>+ xoffset, yoffset, + xoffset + width, yoffset + height, + GL_COLOR_BUFFER_BIT, GL_NEAREST); } success = true; </div></div>-- 1.9.3 </blockquote></div> </div> </div></div></blockquote></div> </div>