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

[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.

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

-Brian

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev