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

2015-03-11 Thread Emil Velikov 
On 9 March 2015 at 11:36, Neil Roberts  wrote:
> Hi Emil,
>
> The resolve looks good, however I think it would also make sense to
> cherry pick a44606 to the stable branch. It doesn't do any harm either
> way but it should be slightly faster and cleaner with that patch as
> well.
>
Thanks Neil. The commit message of a44606 does mention "A layered PBO
image is now interpreted a..." and considering I'm not an expert in
the area, it wasn't clear if the "now" point is prior to the 10.5
branch point or not.

Based on your feedback I've picked both commits and will push then out
after a piglit run :-)

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-03-09 Thread Neil Roberts 
Hi Emil,

The resolve looks good, however I think it would also make sense to
cherry pick a44606 to the stable branch. It doesn't do any harm either
way but it should be slightly faster and cleaner with that patch as
well.

Regards,
- Neil

Emil Velikov  writes:

> On 4 March 2015 at 23:15, Emil Velikov  wrote:
>> On 4 March 2015 at 17:22, Neil Roberts  wrote:
>>> This reverts commit 546aba143d13ba3f993ead4cc30b2404abfc0202.
>>>
>>> I think the changes to the calls to glBlitFramebuffer from this patch
>>> are no different to what it was doing previously because it used to
>>> set height to 1 before doing the blits. However it was introducing
>>> some problems with the blit for layer 0 because this was no longer
>>> special cased. It didn't fix problems with the yoffset which needs to
>>> be interpreted as a slice offset. I think a better solution would be
>>> to modify the original if statement to cope with the yoffset.
>>>
>> Neil, if others agree with this revert can you cc stable. Seems that
>> the offending commit already has the tag, plus I've already picked it
>> up :-\
>>
> Hi Neil,
>
> There was a conflict while picking this for 10.5 due to commit
> a44606eb816(meta: In pbo_{Get,}TexSubImage don't repeatedly rebind the
> source tex). I've resolved it as follows, and I'm planning to commit
> it around Tuesday lunchtime. Do let me know if you have any comments.
>
> Thanks
> Emil
>
> diff --cc src/mesa/drivers/common/meta_tex_subimage.c
> index 9f0c115,1fef79d..000
> --- a/src/mesa/drivers/common/meta_tex_subimage.c
> +++ b/src/mesa/drivers/common/meta_tex_subimage.c
> @@@ -213,12 -229,7 +219,9 @@@ _mesa_meta_pbo_TexSubImage(struct gl_co
> 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_READ_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
>  +pbo_tex_image, z);
> _mesa_meta_bind_fbo_image(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
>   tex_image, zoffset + z);
>
> @@@ -342,16 -352,9 +344,11 @@@ _mesa_meta_pbo_GetTexSubImage(struct gl
> GL_COLOR_BUFFER_BIT, GL_NEAREST))
> goto fail;
>
> -if (tex_image && tex_image->TexObject->Target == GL_TEXTURE_1D_ARRAY)
> -   iters = height;
> -else
> -   iters = depth;
> -
> -for (z = 1; z < iters; z++) {
> +for (z = 1; z < depth; z++) {
> _mesa_meta_bind_fbo_image(GL_READ_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
>   tex_image, zoffset + z);
>  +  _mesa_meta_bind_fbo_image(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
>  +pbo_tex_image, z);
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-03-07 Thread Emil Velikov 
On 4 March 2015 at 23:15, Emil Velikov  wrote:
> On 4 March 2015 at 17:22, Neil Roberts  wrote:
>> This reverts commit 546aba143d13ba3f993ead4cc30b2404abfc0202.
>>
>> I think the changes to the calls to glBlitFramebuffer from this patch
>> are no different to what it was doing previously because it used to
>> set height to 1 before doing the blits. However it was introducing
>> some problems with the blit for layer 0 because this was no longer
>> special cased. It didn't fix problems with the yoffset which needs to
>> be interpreted as a slice offset. I think a better solution would be
>> to modify the original if statement to cope with the yoffset.
>>
> Neil, if others agree with this revert can you cc stable. Seems that
> the offending commit already has the tag, plus I've already picked it
> up :-\
>
Hi Neil,

There was a conflict while picking this for 10.5 due to commit
a44606eb816(meta: In pbo_{Get,}TexSubImage don't repeatedly rebind the
source tex). I've resolved it as follows, and I'm planning to commit
it around Tuesday lunchtime. Do let me know if you have any comments.

Thanks
Emil

diff --cc src/mesa/drivers/common/meta_tex_subimage.c
index 9f0c115,1fef79d..000
--- a/src/mesa/drivers/common/meta_tex_subimage.c
+++ b/src/mesa/drivers/common/meta_tex_subimage.c
@@@ -213,12 -229,7 +219,9 @@@ _mesa_meta_pbo_TexSubImage(struct gl_co
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_READ_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
 +pbo_tex_image, z);
_mesa_meta_bind_fbo_image(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
  tex_image, zoffset + z);

@@@ -342,16 -352,9 +344,11 @@@ _mesa_meta_pbo_GetTexSubImage(struct gl
GL_COLOR_BUFFER_BIT, GL_NEAREST))
goto fail;

-if (tex_image && tex_image->TexObject->Target == GL_TEXTURE_1D_ARRAY)
-   iters = height;
-else
-   iters = depth;
-
-for (z = 1; z < iters; z++) {
+for (z = 1; z < depth; z++) {
_mesa_meta_bind_fbo_image(GL_READ_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
  tex_image, zoffset + z);
 +  _mesa_meta_bind_fbo_image(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
 +pbo_tex_image, z);
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-03-04 Thread Jason Ekstrand 
Series is
Reviewed-by: Jason Ekstrand 

On Wed, Mar 4, 2015 at 3:15 PM, Emil Velikov 
wrote:

> On 4 March 2015 at 17:22, Neil Roberts  wrote:
> > This reverts commit 546aba143d13ba3f993ead4cc30b2404abfc0202.
> >
> > I think the changes to the calls to glBlitFramebuffer from this patch
> > are no different to what it was doing previously because it used to
> > set height to 1 before doing the blits. However it was introducing
> > some problems with the blit for layer 0 because this was no longer
> > special cased. It didn't fix problems with the yoffset which needs to
> > be interpreted as a slice offset. I think a better solution would be
> > to modify the original if statement to cope with the yoffset.
> >
> Neil, if others agree with this revert can you cc stable. Seems that
> the offending commit already has the tag, plus I've already picked it
> up :-\
>

That's ok.  Just add this series to stable once it gets pushed too.
--Jason


>
> Cc: "10.5" 
>
> Thanks
> Emil
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-03-04 Thread Emil Velikov 
On 4 March 2015 at 17:22, Neil Roberts  wrote:
> This reverts commit 546aba143d13ba3f993ead4cc30b2404abfc0202.
>
> I think the changes to the calls to glBlitFramebuffer from this patch
> are no different to what it was doing previously because it used to
> set height to 1 before doing the blits. However it was introducing
> some problems with the blit for layer 0 because this was no longer
> special cased. It didn't fix problems with the yoffset which needs to
> be interpreted as a slice offset. I think a better solution would be
> to modify the original if statement to cope with the yoffset.
>
Neil, if others agree with this revert can you cc stable. Seems that
the offending commit already has the tag, plus I've already picked it
up :-\

Cc: "10.5" 

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2015-03-04 Thread Neil Roberts 
This reverts commit 546aba143d13ba3f993ead4cc30b2404abfc0202.

I think the changes to the calls to glBlitFramebuffer from this patch
are no different to what it was doing previously because it used to
set height to 1 before doing the blits. However it was introducing
some problems with the blit for layer 0 because this was no longer
special cased. It didn't fix problems with the yoffset which needs to
be interpreted as a slice offset. I think a better solution would be
to modify the original if statement to cope with the yoffset.

Conflicts:
src/mesa/drivers/common/meta_tex_subimage.c
---
 src/mesa/drivers/common/meta_tex_subimage.c | 62 -
 1 file changed, 26 insertions(+), 36 deletions(-)

diff --git a/src/mesa/drivers/common/meta_tex_subimage.c 
b/src/mesa/drivers/common/meta_tex_subimage.c
index 971ed59..1fef79d 100644
--- a/src/mesa/drivers/common/meta_tex_subimage.c
+++ b/src/mesa/drivers/common/meta_tex_subimage.c
@@ -147,7 +147,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";
@@ -200,6 +200,12 @@ _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);
+  depth = height;
+  height = 1;
+   }
+
_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 */
@@ -223,27 +229,17 @@ _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 * height, width, (z + 1) * height,
-xoffset, yoffset,
-xoffset + width, yoffset + height,
-GL_COLOR_BUFFER_BIT, GL_NEAREST);
+  _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);
}
 
success = true;
@@ -270,7 +266,7 @@ _mesa_meta_pbo_GetTexSubImage(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_GetTexSubImage";
@@ -317,6 +313,12 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx, 
GLuint dims,
 
_mesa_GenFramebuffers(2, fbos);
 
+   if (tex_image && tex_image->TexObject->Target == GL_TEXTURE_1D_ARRAY) {
+  assert(depth == 1);
+  depth = height;
+  height = 1;
+   }
+
/* If we were given a texture, bind it to the read framebuffer.  If not,
 * we're doing a ReadPixels and we should just use whatever framebuffer
 * the client has bound.
@@ -350,29 +352,17 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx, 
GLuint dims,
   GL_COLOR_BUFFER_BIT, GL_NEAREST))
   goto fail;
 
-   if (tex_image && tex_image->TexObject->Target == GL_TEXTURE_1D_ARRAY)
-  iters = height;
-   else
-  iters = depth;
-
-   for (z = 1; z < iters; z++) {
+   for (z = 1; z < depth; z++) {
   _mesa_meta_bind_fbo_image(GL_READ_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,
-xoffset, yoffset,
-