Am 03.04.2014 17:19, schrieb Brian Paul:
> On 04/03/2014 08:57 AM, jfons...@vmware.com wrote:
>> From: José Fonseca <jfons...@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
