[Mesa-dev] mesa/st: Fix pipe_framebuffer_state::height for PIPE_TEXTURE_1D_ARRAY.

Submitted by Jose Fonseca on April 3, 2014, 2:57 p.m.

Details

Message ID 1396537051-6137-1-git-send-email-jfonseca@vmware.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Jose Fonseca April 3, 2014, 2:57 p.m.
From: José Fonseca <jfonseca@vmware.com>

This prevents buffer overflow w/ llvmpipe when running piglit

  bin/gl-3.2-layered-rendering-clear-color-all-types 1d_array single_level -fbo -auto
---
 src/mesa/state_tracker/st_atom_framebuffer.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/mesa/state_tracker/st_atom_framebuffer.c b/src/mesa/state_tracker/st_atom_framebuffer.c
index 4c4f839..f8eb1f0 100644
--- a/src/mesa/state_tracker/st_atom_framebuffer.c
+++ b/src/mesa/state_tracker/st_atom_framebuffer.c
@@ -52,13 +52,13 @@  update_framebuffer_state( struct st_context *st )
    struct pipe_framebuffer_state *framebuffer = &st->state.framebuffer;
    struct gl_framebuffer *fb = st->ctx->DrawBuffer;
    struct st_renderbuffer *strb;
+   unsigned width = 0;
+   unsigned height = 0;
    GLuint i;
 
    st_flush_bitmap_cache(st);
 
    st->state.fb_orientation = st_fb_orientation(fb);
-   framebuffer->width = fb->Width;
-   framebuffer->height = fb->Height;
 
    /*printf("------ fb size %d x %d\n", fb->Width, fb->Height);*/
 
@@ -81,6 +81,8 @@  update_framebuffer_state( struct st_context *st )
 
          if (strb->surface) {
             pipe_surface_reference(&framebuffer->cbufs[i], strb->surface);
+            width  = MAX2(width,  strb->surface->width);
+            height = MAX2(height, strb->surface->height);
          }
          strb->defined = GL_TRUE; /* we'll be drawing something */
       }
@@ -100,12 +102,18 @@  update_framebuffer_state( struct st_context *st )
          st_update_renderbuffer_surface(st, strb);
       }
       pipe_surface_reference(&framebuffer->zsbuf, strb->surface);
+      if (strb->surface) {
+         width  = MAX2(width,  strb->surface->width);
+         height = MAX2(height, strb->surface->height);
+      }
    }
    else {
       strb = st_renderbuffer(fb->Attachment[BUFFER_STENCIL].Renderbuffer);
       if (strb) {
          assert(strb->surface);
          pipe_surface_reference(&framebuffer->zsbuf, strb->surface);
+         width  = MAX2(width,  strb->surface->width);
+         height = MAX2(height, strb->surface->height);
       }
       else
          pipe_surface_reference(&framebuffer->zsbuf, NULL);
@@ -122,6 +130,13 @@  update_framebuffer_state( struct st_context *st )
    }
 #endif
 
+   /*
+    * framebuffer->width should match fb->Weight, but for PIPE_TEXTURE_1D_ARRAY
+    * fb->Height has the number of layers as opposed to height.
+    */
+   framebuffer->width  = width;
+   framebuffer->height = height;
+
    cso_set_framebuffer(st->cso_context, framebuffer);
 }
 

Comments

On 04/03/2014 08:57 AM, jfonseca@vmware.com wrote:
> From: José Fonseca <jfonseca@vmware.com>
>
> This prevents buffer overflow w/ llvmpipe when running piglit
>
>    bin/gl-3.2-layered-rendering-clear-color-all-types 1d_array single_level -fbo -auto
> ---
>   src/mesa/state_tracker/st_atom_framebuffer.c | 19 +++++++++++++++++--
>   1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_atom_framebuffer.c b/src/mesa/state_tracker/st_atom_framebuffer.c
> index 4c4f839..f8eb1f0 100644
> --- a/src/mesa/state_tracker/st_atom_framebuffer.c
> +++ b/src/mesa/state_tracker/st_atom_framebuffer.c
> @@ -52,13 +52,13 @@ update_framebuffer_state( struct st_context *st )
>      struct pipe_framebuffer_state *framebuffer = &st->state.framebuffer;
>      struct gl_framebuffer *fb = st->ctx->DrawBuffer;
>      struct st_renderbuffer *strb;
> +   unsigned width = 0;
> +   unsigned height = 0;
>      GLuint i;
>
>      st_flush_bitmap_cache(st);
>
>      st->state.fb_orientation = st_fb_orientation(fb);
> -   framebuffer->width = fb->Width;
> -   framebuffer->height = fb->Height;
>
>      /*printf("------ fb size %d x %d\n", fb->Width, fb->Height);*/
>
> @@ -81,6 +81,8 @@ update_framebuffer_state( struct st_context *st )
>
>            if (strb->surface) {
>               pipe_surface_reference(&framebuffer->cbufs[i], strb->surface);
> +            width  = MAX2(width,  strb->surface->width);
> +            height = MAX2(height, strb->surface->height);
>            }
>            strb->defined = GL_TRUE; /* we'll be drawing something */
>         }
> @@ -100,12 +102,18 @@ update_framebuffer_state( struct st_context *st )
>            st_update_renderbuffer_surface(st, strb);
>         }
>         pipe_surface_reference(&framebuffer->zsbuf, strb->surface);
> +      if (strb->surface) {
> +         width  = MAX2(width,  strb->surface->width);
> +         height = MAX2(height, strb->surface->height);
> +      }
>      }
>      else {
>         strb = st_renderbuffer(fb->Attachment[BUFFER_STENCIL].Renderbuffer);
>         if (strb) {
>            assert(strb->surface);
>            pipe_surface_reference(&framebuffer->zsbuf, strb->surface);
> +         width  = MAX2(width,  strb->surface->width);
> +         height = MAX2(height, strb->surface->height);
>         }
>         else
>            pipe_surface_reference(&framebuffer->zsbuf, NULL);
> @@ -122,6 +130,13 @@ update_framebuffer_state( struct st_context *st )
>      }
>   #endif
>
> +   /*
> +    * framebuffer->width should match fb->Weight, but for PIPE_TEXTURE_1D_ARRAY
> +    * fb->Height has the number of layers as opposed to height.
> +    */
> +   framebuffer->width  = width;
> +   framebuffer->height = height;
> +
>      cso_set_framebuffer(st->cso_context, framebuffer);
>   }

I think this is going to change the behavior for a framebuffer 
consisting of ordinary 2D surfaces but of mixed sizes.

In _mesa_test_framebuffer_completeness() we compute the 
gl_framebuffer:width/height as the min of the surfaces' dimensions.  So 
the original update_framebuffer_state() code set 
pipe_framebuffer_state::width/height to the min of the surfaces' dimensions.

Now, pipe_framebuffer_state::width/height is getting the max 
width/height of all attached surfaces.

I'm not sure how/if all the gallium drivers use the 
pipe_framebuffer_state::width/height params but they'll be getting 
different numbers now with mixed-sized framebuffers.

I'm not sure if that's a problem.

Maybe the real issue is in _mesa_test_framebuffer_completeness().  Maybe 
we should do something special with 1D texture array heights there?

-Brian
Am 03.04.2014 16:57, schrieb jfonseca@vmware.com:
> From: José Fonseca <jfonseca@vmware.com>
> 
> This prevents buffer overflow w/ llvmpipe when running piglit
> 
>   bin/gl-3.2-layered-rendering-clear-color-all-types 1d_array single_level -fbo -auto
> ---
>  src/mesa/state_tracker/st_atom_framebuffer.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/state_tracker/st_atom_framebuffer.c b/src/mesa/state_tracker/st_atom_framebuffer.c
> index 4c4f839..f8eb1f0 100644
> --- a/src/mesa/state_tracker/st_atom_framebuffer.c
> +++ b/src/mesa/state_tracker/st_atom_framebuffer.c
> @@ -52,13 +52,13 @@ update_framebuffer_state( struct st_context *st )
>     struct pipe_framebuffer_state *framebuffer = &st->state.framebuffer;
>     struct gl_framebuffer *fb = st->ctx->DrawBuffer;
>     struct st_renderbuffer *strb;
> +   unsigned width = 0;
> +   unsigned height = 0;
>     GLuint i;
>  
>     st_flush_bitmap_cache(st);
>  
>     st->state.fb_orientation = st_fb_orientation(fb);
> -   framebuffer->width = fb->Width;
> -   framebuffer->height = fb->Height;
>  
>     /*printf("------ fb size %d x %d\n", fb->Width, fb->Height);*/
>  
> @@ -81,6 +81,8 @@ update_framebuffer_state( struct st_context *st )
>  
>           if (strb->surface) {
>              pipe_surface_reference(&framebuffer->cbufs[i], strb->surface);
> +            width  = MAX2(width,  strb->surface->width);
> +            height = MAX2(height, strb->surface->height);
>           }
>           strb->defined = GL_TRUE; /* we'll be drawing something */
>        }
> @@ -100,12 +102,18 @@ update_framebuffer_state( struct st_context *st )
>           st_update_renderbuffer_surface(st, strb);
>        }
>        pipe_surface_reference(&framebuffer->zsbuf, strb->surface);
> +      if (strb->surface) {
> +         width  = MAX2(width,  strb->surface->width);
> +         height = MAX2(height, strb->surface->height);
> +      }
>     }
>     else {
>        strb = st_renderbuffer(fb->Attachment[BUFFER_STENCIL].Renderbuffer);
>        if (strb) {
>           assert(strb->surface);
>           pipe_surface_reference(&framebuffer->zsbuf, strb->surface);
> +         width  = MAX2(width,  strb->surface->width);
> +         height = MAX2(height, strb->surface->height);
>        }
>        else
>           pipe_surface_reference(&framebuffer->zsbuf, NULL);
> @@ -122,6 +130,13 @@ update_framebuffer_state( struct st_context *st )
>     }
>  #endif
>  
> +   /*
> +    * framebuffer->width should match fb->Weight, but for PIPE_TEXTURE_1D_ARRAY
> +    * fb->Height has the number of layers as opposed to height.
> +    */
> +   framebuffer->width  = width;
> +   framebuffer->height = height;
> +
>     cso_set_framebuffer(st->cso_context, framebuffer);
>  }
>  
> 

Do you really need all that additional MAX2 code given that the
calculated fb->Width/Height already did that to determine the values? I
guess though it can't hurt, and otherwise you'd hae to check the
dimension of the furst populated attachment.
So, Reviewed-by: Roland Scheidegger <sroland@vmware.com>

I think though longer term it would a MUCH better idea to not treat 1d
array textures as 2d textures _anywhere_ in core mesa (except when
dictated by the API), as far as I can tell it just requires everyone
(from classic drivers / meta code to st) to do workarounds everywhere
(or forget about it and face some weird bugs as a consequence...). A 1d
array texture is just very different to a 2d texture.

Roland
Am 03.04.2014 17:19, schrieb Brian Paul:
> On 04/03/2014 08:57 AM, jfonseca@vmware.com wrote:
>> From: José Fonseca <jfonseca@vmware.com>
>>
>> This prevents buffer overflow w/ llvmpipe when running piglit
>>
>>    bin/gl-3.2-layered-rendering-clear-color-all-types 1d_array
>> single_level -fbo -auto
>> ---
>>   src/mesa/state_tracker/st_atom_framebuffer.c | 19 +++++++++++++++++--
>>   1 file changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/state_tracker/st_atom_framebuffer.c
>> b/src/mesa/state_tracker/st_atom_framebuffer.c
>> index 4c4f839..f8eb1f0 100644
>> --- a/src/mesa/state_tracker/st_atom_framebuffer.c
>> +++ b/src/mesa/state_tracker/st_atom_framebuffer.c
>> @@ -52,13 +52,13 @@ update_framebuffer_state( struct st_context *st )
>>      struct pipe_framebuffer_state *framebuffer = &st->state.framebuffer;
>>      struct gl_framebuffer *fb = st->ctx->DrawBuffer;
>>      struct st_renderbuffer *strb;
>> +   unsigned width = 0;
>> +   unsigned height = 0;
>>      GLuint i;
>>
>>      st_flush_bitmap_cache(st);
>>
>>      st->state.fb_orientation = st_fb_orientation(fb);
>> -   framebuffer->width = fb->Width;
>> -   framebuffer->height = fb->Height;
>>
>>      /*printf("------ fb size %d x %d\n", fb->Width, fb->Height);*/
>>
>> @@ -81,6 +81,8 @@ update_framebuffer_state( struct st_context *st )
>>
>>            if (strb->surface) {
>>               pipe_surface_reference(&framebuffer->cbufs[i],
>> strb->surface);
>> +            width  = MAX2(width,  strb->surface->width);
>> +            height = MAX2(height, strb->surface->height);
>>            }
>>            strb->defined = GL_TRUE; /* we'll be drawing something */
>>         }
>> @@ -100,12 +102,18 @@ update_framebuffer_state( struct st_context *st )
>>            st_update_renderbuffer_surface(st, strb);
>>         }
>>         pipe_surface_reference(&framebuffer->zsbuf, strb->surface);
>> +      if (strb->surface) {
>> +         width  = MAX2(width,  strb->surface->width);
>> +         height = MAX2(height, strb->surface->height);
>> +      }
>>      }
>>      else {
>>         strb =
>> st_renderbuffer(fb->Attachment[BUFFER_STENCIL].Renderbuffer);
>>         if (strb) {
>>            assert(strb->surface);
>>            pipe_surface_reference(&framebuffer->zsbuf, strb->surface);
>> +         width  = MAX2(width,  strb->surface->width);
>> +         height = MAX2(height, strb->surface->height);
>>         }
>>         else
>>            pipe_surface_reference(&framebuffer->zsbuf, NULL);
>> @@ -122,6 +130,13 @@ update_framebuffer_state( struct st_context *st )
>>      }
>>   #endif
>>
>> +   /*
>> +    * framebuffer->width should match fb->Weight, but for
>> PIPE_TEXTURE_1D_ARRAY
>> +    * fb->Height has the number of layers as opposed to height.
>> +    */
>> +   framebuffer->width  = width;
>> +   framebuffer->height = height;
>> +
>>      cso_set_framebuffer(st->cso_context, framebuffer);
>>   }
> 
> I think this is going to change the behavior for a framebuffer
> consisting of ordinary 2D surfaces but of mixed sizes.
> 
> In _mesa_test_framebuffer_completeness() we compute the
> gl_framebuffer:width/height as the min of the surfaces' dimensions.  So
> the original update_framebuffer_state() code set
> pipe_framebuffer_state::width/height to the min of the surfaces'
> dimensions.
> 
> Now, pipe_framebuffer_state::width/height is getting the max
> width/height of all attached surfaces.
> 
> I'm not sure how/if all the gallium drivers use the
> pipe_framebuffer_state::width/height params but they'll be getting
> different numbers now with mixed-sized framebuffers.
> 
> I'm not sure if that's a problem.
Oh yes I missed that in my review I'm required to retract my R-b :-).
I think starting with fb->Width/Height and then using MIN2 would work
(but retain all the ugliness).


> 
> Maybe the real issue is in _mesa_test_framebuffer_completeness().  Maybe
> we should do something special with 1D texture array heights there?
> 
Might work, but ultimately I think it should really work all just
automatically. Treating 1d and 2d array textures differently is just a
real pain (that is 1d array textures having height of layers, which
leads to rb having that height, which ultimately means the fb has that
height). If anything, I think it should be at least fixed up at rb
level, not just fb.

Roland
Am 03.04.2014 17:35, schrieb Roland Scheidegger:
> Am 03.04.2014 17:19, schrieb Brian Paul:
>> On 04/03/2014 08:57 AM, jfonseca@vmware.com wrote:
>>> From: José Fonseca <jfonseca@vmware.com>
>>>
>>> This prevents buffer overflow w/ llvmpipe when running piglit
>>>
>>>    bin/gl-3.2-layered-rendering-clear-color-all-types 1d_array
>>> single_level -fbo -auto
>>> ---
>>>   src/mesa/state_tracker/st_atom_framebuffer.c | 19 +++++++++++++++++--
>>>   1 file changed, 17 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/mesa/state_tracker/st_atom_framebuffer.c
>>> b/src/mesa/state_tracker/st_atom_framebuffer.c
>>> index 4c4f839..f8eb1f0 100644
>>> --- a/src/mesa/state_tracker/st_atom_framebuffer.c
>>> +++ b/src/mesa/state_tracker/st_atom_framebuffer.c
>>> @@ -52,13 +52,13 @@ update_framebuffer_state( struct st_context *st )
>>>      struct pipe_framebuffer_state *framebuffer = &st->state.framebuffer;
>>>      struct gl_framebuffer *fb = st->ctx->DrawBuffer;
>>>      struct st_renderbuffer *strb;
>>> +   unsigned width = 0;
>>> +   unsigned height = 0;
>>>      GLuint i;
>>>
>>>      st_flush_bitmap_cache(st);
>>>
>>>      st->state.fb_orientation = st_fb_orientation(fb);
>>> -   framebuffer->width = fb->Width;
>>> -   framebuffer->height = fb->Height;
>>>
>>>      /*printf("------ fb size %d x %d\n", fb->Width, fb->Height);*/
>>>
>>> @@ -81,6 +81,8 @@ update_framebuffer_state( struct st_context *st )
>>>
>>>            if (strb->surface) {
>>>               pipe_surface_reference(&framebuffer->cbufs[i],
>>> strb->surface);
>>> +            width  = MAX2(width,  strb->surface->width);
>>> +            height = MAX2(height, strb->surface->height);
>>>            }
>>>            strb->defined = GL_TRUE; /* we'll be drawing something */
>>>         }
>>> @@ -100,12 +102,18 @@ update_framebuffer_state( struct st_context *st )
>>>            st_update_renderbuffer_surface(st, strb);
>>>         }
>>>         pipe_surface_reference(&framebuffer->zsbuf, strb->surface);
>>> +      if (strb->surface) {
>>> +         width  = MAX2(width,  strb->surface->width);
>>> +         height = MAX2(height, strb->surface->height);
>>> +      }
>>>      }
>>>      else {
>>>         strb =
>>> st_renderbuffer(fb->Attachment[BUFFER_STENCIL].Renderbuffer);
>>>         if (strb) {
>>>            assert(strb->surface);
>>>            pipe_surface_reference(&framebuffer->zsbuf, strb->surface);
>>> +         width  = MAX2(width,  strb->surface->width);
>>> +         height = MAX2(height, strb->surface->height);
>>>         }
>>>         else
>>>            pipe_surface_reference(&framebuffer->zsbuf, NULL);
>>> @@ -122,6 +130,13 @@ update_framebuffer_state( struct st_context *st )
>>>      }
>>>   #endif
>>>
>>> +   /*
>>> +    * framebuffer->width should match fb->Weight, but for
>>> PIPE_TEXTURE_1D_ARRAY
>>> +    * fb->Height has the number of layers as opposed to height.
>>> +    */
>>> +   framebuffer->width  = width;
>>> +   framebuffer->height = height;
>>> +
>>>      cso_set_framebuffer(st->cso_context, framebuffer);
>>>   }
>>
>> I think this is going to change the behavior for a framebuffer
>> consisting of ordinary 2D surfaces but of mixed sizes.
>>
>> In _mesa_test_framebuffer_completeness() we compute the
>> gl_framebuffer:width/height as the min of the surfaces' dimensions.  So
>> the original update_framebuffer_state() code set
>> pipe_framebuffer_state::width/height to the min of the surfaces'
>> dimensions.
>>
>> Now, pipe_framebuffer_state::width/height is getting the max
>> width/height of all attached surfaces.
>>
>> I'm not sure how/if all the gallium drivers use the
>> pipe_framebuffer_state::width/height params but they'll be getting
>> different numbers now with mixed-sized framebuffers.
>>
>> I'm not sure if that's a problem.
> Oh yes I missed that in my review I'm required to retract my R-b :-).
> I think starting with fb->Width/Height and then using MIN2 would work
> (but retain all the ugliness).
> 
> 
>>
>> Maybe the real issue is in _mesa_test_framebuffer_completeness().  Maybe
>> we should do something special with 1D texture array heights there?
>>
> Might work, but ultimately I think it should really work all just
> automatically. Treating 1d and 2d array textures differently is just a
> real pain (that is 1d array textures having height of layers, which
> leads to rb having that height, which ultimately means the fb has that
> height). If anything, I think it should be at least fixed up at rb
> level, not just fb.
> 

To elaborate a bit why this easily leading to bugs, here's another
oddity: the MaxNumLayers field (which we don't use in gallium, maybe we
should) also does its calculation based on the amount of layers from
each rb attachment. It will set this to 6 for cube maps, and
att->Renderbuffer->Depth otherwise. Guess what Depth is for 1d array
textures... Now this would only affect intel drivers and it's possible
there's some more workarounds for that too somewhere, but it's just
insanity.

Roland
----- Original Message -----
> On 04/03/2014 08:57 AM, jfonseca@vmware.com wrote:
> > From: José Fonseca <jfonseca@vmware.com>
> >
> > This prevents buffer overflow w/ llvmpipe when running piglit
> >
> >    bin/gl-3.2-layered-rendering-clear-color-all-types 1d_array single_level
> >    -fbo -auto
> > ---
> >   src/mesa/state_tracker/st_atom_framebuffer.c | 19 +++++++++++++++++--
> >   1 file changed, 17 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/mesa/state_tracker/st_atom_framebuffer.c
> > b/src/mesa/state_tracker/st_atom_framebuffer.c
> > index 4c4f839..f8eb1f0 100644
> > --- a/src/mesa/state_tracker/st_atom_framebuffer.c
> > +++ b/src/mesa/state_tracker/st_atom_framebuffer.c
> > @@ -52,13 +52,13 @@ update_framebuffer_state( struct st_context *st )
> >      struct pipe_framebuffer_state *framebuffer = &st->state.framebuffer;
> >      struct gl_framebuffer *fb = st->ctx->DrawBuffer;
> >      struct st_renderbuffer *strb;
> > +   unsigned width = 0;
> > +   unsigned height = 0;
> >      GLuint i;
> >
> >      st_flush_bitmap_cache(st);
> >
> >      st->state.fb_orientation = st_fb_orientation(fb);
> > -   framebuffer->width = fb->Width;
> > -   framebuffer->height = fb->Height;
> >
> >      /*printf("------ fb size %d x %d\n", fb->Width, fb->Height);*/
> >
> > @@ -81,6 +81,8 @@ update_framebuffer_state( struct st_context *st )
> >
> >            if (strb->surface) {
> >               pipe_surface_reference(&framebuffer->cbufs[i],
> >               strb->surface);
> > +            width  = MAX2(width,  strb->surface->width);
> > +            height = MAX2(height, strb->surface->height);
> >            }
> >            strb->defined = GL_TRUE; /* we'll be drawing something */
> >         }
> > @@ -100,12 +102,18 @@ update_framebuffer_state( struct st_context *st )
> >            st_update_renderbuffer_surface(st, strb);
> >         }
> >         pipe_surface_reference(&framebuffer->zsbuf, strb->surface);
> > +      if (strb->surface) {
> > +         width  = MAX2(width,  strb->surface->width);
> > +         height = MAX2(height, strb->surface->height);
> > +      }
> >      }
> >      else {
> >         strb =
> >         st_renderbuffer(fb->Attachment[BUFFER_STENCIL].Renderbuffer);
> >         if (strb) {
> >            assert(strb->surface);
> >            pipe_surface_reference(&framebuffer->zsbuf, strb->surface);
> > +         width  = MAX2(width,  strb->surface->width);
> > +         height = MAX2(height, strb->surface->height);
> >         }
> >         else
> >            pipe_surface_reference(&framebuffer->zsbuf, NULL);
> > @@ -122,6 +130,13 @@ update_framebuffer_state( struct st_context *st )
> >      }
> >   #endif
> >
> > +   /*
> > +    * framebuffer->width should match fb->Weight, but for
> > PIPE_TEXTURE_1D_ARRAY
> > +    * fb->Height has the number of layers as opposed to height.
> > +    */
> > +   framebuffer->width  = width;
> > +   framebuffer->height = height;
> > +
> >      cso_set_framebuffer(st->cso_context, framebuffer);
> >   }

> I think this is going to change the behavior for a framebuffer
> consisting of ordinary 2D surfaces but of mixed sizes.
> 
> In _mesa_test_framebuffer_completeness() we compute the
> gl_framebuffer:width/height as the min of the surfaces' dimensions.  So
> the original update_framebuffer_state() code set
> pipe_framebuffer_state::width/height to the min of the surfaces' dimensions.
> 
> Now, pipe_framebuffer_state::width/height is getting the max
> width/height of all attached surfaces.
> 
> I'm not sure how/if all the gallium drivers use the
> pipe_framebuffer_state::width/height params but they'll be getting
> different numbers now with mixed-sized framebuffers.
> 
> I'm not sure if that's a problem.

Yep, you're right. 

> Maybe the real issue is in _mesa_test_framebuffer_completeness().  Maybe
> we should do something special with 1D texture array heights there?

I'm not sure. _mesa_test_framebuffer_completeness() is used by non-gallium drivers too, which may be expect the GL semantics of numLayers in height.

Anyway, piglit with llvmpipe is hosed in many places (lots of tests deadlone). I'll need to force MESA_GL_VERSION_OVERRIDE=3.0 on the automated tests for the time being until we iron them out.

Thanks for the review.

Jose