[Mesa-dev] [PATCH 3/3] draw: make sure viewport index is fetched from leading vertex

Brian Paul brianp at vmware.com
Wed May 29 09:01:54 PDT 2013


On 05/25/2013 12:12 AM, Zack Rusin wrote:
> Viewport index should only be used on a per primitive basis, so
> instead of fetching it from each vertex, potentially making each
> vertex in a primitive use a different viewport index, which is
> obviously broken, make sure that we only fetch from the first
> vertex in the primitive making the viewport index the same
> for the entire primtive.
>
> Signed-off-by: Zack Rusin <zackr at vmware.com>
> ---
>   src/gallium/auxiliary/draw/draw_cliptest_tmp.h     |   25 ++++++++----
>   src/gallium/auxiliary/draw/draw_pipe_clip.c        |   40 +++++++++++++-------
>   src/gallium/auxiliary/draw/draw_pt.h               |    3 +-
>   .../auxiliary/draw/draw_pt_fetch_shade_pipeline.c  |    2 +-
>   .../draw/draw_pt_fetch_shade_pipeline_llvm.c       |    2 +-
>   src/gallium/auxiliary/draw/draw_pt_post_vs.c       |    9 +++--
>   6 files changed, 53 insertions(+), 28 deletions(-)
>
> diff --git a/src/gallium/auxiliary/draw/draw_cliptest_tmp.h b/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
> index 6e4a247..2f2b88e 100644
> --- a/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
> +++ b/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
> @@ -26,7 +26,8 @@
>    **************************************************************************/
>
>   static boolean TAG(do_cliptest)( struct pt_post_vs *pvs,
> -                                 struct draw_vertex_info *info )
> +                                 struct draw_vertex_info *info,
> +                                 const struct draw_prim_info *prim_info )
>   {
>      struct vertex_header *out = info->verts;
>      /* const */ float (*plane)[4] = pvs->draw->plane;
> @@ -42,6 +43,9 @@ static boolean TAG(do_cliptest)( struct pt_post_vs *pvs,
>      bool have_cd = false;
>      unsigned viewport_index_output =
>         draw_current_shader_viewport_index_output(pvs->draw);
> +   int viewport_index =
> +      draw_current_shader_uses_viewport_index(pvs->draw) ?
> +      *((unsigned*)out->data[viewport_index_output]): 0;
>
>      cd[0] = draw_current_shader_clipdistance_output(pvs->draw, 0);
>      cd[1] = draw_current_shader_clipdistance_output(pvs->draw, 1);
> @@ -51,14 +55,19 @@ static boolean TAG(do_cliptest)( struct pt_post_vs *pvs,
>
>      for (j = 0; j < info->count; j++) {
>         float *position = out->data[pos];
> -      int viewport_index =
> -         draw_current_shader_uses_viewport_index(pvs->draw) ?
> -         *((unsigned*)out->data[viewport_index_output]): 0;
>         unsigned mask = 0x0;
> -      const float *scale = pvs->draw->viewports[
> -         DRAW_CLAMP_VIEWPORT_IDX(viewport_index) ].scale;
> -      const float *trans = pvs->draw->viewports[
> -         DRAW_CLAMP_VIEWPORT_IDX(viewport_index)].translate;
> +      float *scale = pvs->draw->viewports[0].scale;
> +      float *trans = pvs->draw->viewports[0].translate;
> +      if (draw_current_shader_uses_viewport_index(pvs->draw)) {
> +         unsigned verts_per_prim = u_vertices_per_prim(prim_info->prim);
> +         /* only change the viewport_index for the leading vertex */
> +         if (!(j % verts_per_prim)) {
> +            viewport_index = *((unsigned*)out->data[viewport_index_output]);
> +            viewport_index = DRAW_CLAMP_VIEWPORT_IDX(viewport_index);
> +         }
> +         scale = pvs->draw->viewports[viewport_index].scale;
> +         trans = pvs->draw->viewports[viewport_index].translate;
> +      }
>
>         initialize_vertex_header(out);
>
> diff --git a/src/gallium/auxiliary/draw/draw_pipe_clip.c b/src/gallium/auxiliary/draw/draw_pipe_clip.c
> index 3b82be9..ff11e36 100644
> --- a/src/gallium/auxiliary/draw/draw_pipe_clip.c
> +++ b/src/gallium/auxiliary/draw/draw_pipe_clip.c
> @@ -118,15 +118,14 @@ static void interp( const struct clip_stage *clip,
>   		    struct vertex_header *dst,
>   		    float t,
>   		    const struct vertex_header *out,
> -		    const struct vertex_header *in )
> +		    const struct vertex_header *in,
> +                    unsigned viewport_index )
>   {
>      const unsigned nr_attrs = draw_current_shader_outputs(clip->stage.draw);
>      const unsigned pos_attr = draw_current_shader_position_output(clip->stage.draw);
>      const unsigned clip_attr = draw_current_shader_clipvertex_output(clip->stage.draw);
>      unsigned j;
>      float t_nopersp;
> -   unsigned viewport_index_output =
> -      draw_current_shader_viewport_index_output(clip->stage.draw);
>
>      /* Vertex header.
>       */
> @@ -145,16 +144,11 @@ static void interp( const struct clip_stage *clip,
>       * new window coordinates:
>       */
>      {
> -      int viewport_index =
> -         draw_current_shader_uses_viewport_index(clip->stage.draw) ?
> -         *((unsigned*)in->data[viewport_index_output]) : 0;
>         const float *pos = dst->pre_clip_pos;
>         const float *scale =
> -         clip->stage.draw->viewports[
> -            DRAW_CLAMP_VIEWPORT_IDX(viewport_index)].scale;
> +         clip->stage.draw->viewports[viewport_index].scale;
>         const float *trans =
> -         clip->stage.draw->viewports[
> -            DRAW_CLAMP_VIEWPORT_IDX(viewport_index)].translate;
> +         clip->stage.draw->viewports[viewport_index].translate;
>         const float oow = 1.0f / pos[3];
>
>         dst->data[pos_attr][0] = pos[0] * oow * scale[0] + trans[0];
> @@ -332,11 +326,20 @@ do_clip_tri( struct draw_stage *stage,
>      boolean bEdges[MAX_CLIPPED_VERTICES];
>      boolean *inEdges = aEdges;
>      boolean *outEdges = bEdges;
> +   unsigned viewport_index_output =
> +      draw_current_shader_viewport_index_output(clipper->stage.draw);
> +   int viewport_index = 0;
>
>      inlist[0] = header->v[0];
>      inlist[1] = header->v[1];
>      inlist[2] = header->v[2];
>
> +   if (draw_current_shader_uses_viewport_index(clipper->stage.draw)) {
> +      const struct vertex_header *leading_vertex = inlist[0];
> +      viewport_index = *((unsigned*)leading_vertex->data[viewport_index_output]);
> +      viewport_index = DRAW_CLAMP_VIEWPORT_IDX(viewport_index);
> +   }
> +
>      if (DEBUG_CLIP) {
>         const float *v0 = header->v[0]->clip;
>         const float *v1 = header->v[1]->clip;
> @@ -411,7 +414,7 @@ do_clip_tri( struct draw_stage *stage,
>   		* know dp != dp_prev from DIFFERENT_SIGNS, above.
>   		*/
>   	       float t = dp / (dp - dp_prev);
> -	       interp( clipper, new_vert, t, vert, vert_prev );
> +	       interp( clipper, new_vert, t, vert, vert_prev, viewport_index );
>   	
>   	       /* Whether or not to set edge flag for the new vert depends
>                   * on whether it's a user-defined clipping plane.  We're
> @@ -432,7 +435,7 @@ do_clip_tri( struct draw_stage *stage,
>   	       /* Coming back in.
>   		*/
>   	       float t = dp_prev / (dp_prev - dp);
> -	       interp( clipper, new_vert, t, vert_prev, vert );
> +	       interp( clipper, new_vert, t, vert_prev, vert, viewport_index );
>
>   	       /* Copy starting vert's edgeflag:
>   		*/
> @@ -505,6 +508,15 @@ do_clip_line( struct draw_stage *stage,
>      float t0 = 0.0F;
>      float t1 = 0.0F;
>      struct prim_header newprim;
> +   unsigned viewport_index_output =
> +      draw_current_shader_viewport_index_output(clipper->stage.draw);
> +   int viewport_index = 0;
> +
> +   if (draw_current_shader_uses_viewport_index(clipper->stage.draw)) {
> +      const struct vertex_header *leading_vertex = v0;
> +      viewport_index = *((unsigned*)leading_vertex->data[viewport_index_output]);
> +      viewport_index = DRAW_CLAMP_VIEWPORT_IDX(viewport_index);
> +   }

Could this chunk of code (also seen above) be put in a helper function 
that returns the viewport_index?  It seems that viewport_index_output 
only needs to be fetched when draw_current_shader_uses_viewport_index().


>
>      while (clipmask) {
>         const unsigned plane_idx = ffs(clipmask)-1;
> @@ -528,7 +540,7 @@ do_clip_line( struct draw_stage *stage,
>      }
>
>      if (v0->clipmask) {
> -      interp( clipper, stage->tmp[0], t0, v0, v1 );
> +      interp( clipper, stage->tmp[0], t0, v0, v1, viewport_index );
>         copy_flat(stage, stage->tmp[0], v0);
>         newprim.v[0] = stage->tmp[0];
>      }
> @@ -537,7 +549,7 @@ do_clip_line( struct draw_stage *stage,
>      }
>
>      if (v1->clipmask) {
> -      interp( clipper, stage->tmp[1], t1, v1, v0 );
> +      interp( clipper, stage->tmp[1], t1, v1, v0, viewport_index );
>         newprim.v[1] = stage->tmp[1];
>      }
>      else {
> diff --git a/src/gallium/auxiliary/draw/draw_pt.h b/src/gallium/auxiliary/draw/draw_pt.h
> index dca8368..29e5499 100644
> --- a/src/gallium/auxiliary/draw/draw_pt.h
> +++ b/src/gallium/auxiliary/draw/draw_pt.h
> @@ -225,7 +225,8 @@ struct pt_fetch *draw_pt_fetch_create( struct draw_context *draw );
>   struct pt_post_vs;
>
>   boolean draw_pt_post_vs_run( struct pt_post_vs *pvs,
> -			     struct draw_vertex_info *info );
> +			     struct draw_vertex_info *info,
> +                             const struct draw_prim_info *prim_info );
>
>   void draw_pt_post_vs_prepare( struct pt_post_vs *pvs,
>   			      boolean clip_xy,
> diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c
> index ea2a5d6..6d1bd11 100644
> --- a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c
> +++ b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c
> @@ -323,7 +323,7 @@ static void fetch_pipeline_generic( struct draw_pt_middle_end *middle,
>       */
>      if (draw_current_shader_position_output(draw) != -1) {
>
> -      if (draw_pt_post_vs_run( fpme->post_vs, vert_info ))
> +      if (draw_pt_post_vs_run( fpme->post_vs, vert_info, prim_info ))
>         {
>            opt |= PT_PIPELINE;
>         }
> diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c
> index 5d2993e..ecb7a6b 100644
> --- a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c
> +++ b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c
> @@ -417,7 +417,7 @@ llvm_pipeline_generic( struct draw_pt_middle_end *middle,
>       */
>      if (draw_current_shader_position_output(draw) != -1) {
>         if ((opt & PT_SHADE) && gshader) {
> -         clipped = draw_pt_post_vs_run( fpme->post_vs, vert_info );
> +         clipped = draw_pt_post_vs_run( fpme->post_vs, vert_info, prim_info );
>         }
>         if (clipped) {
>            opt |= PT_PIPELINE;
> diff --git a/src/gallium/auxiliary/draw/draw_pt_post_vs.c b/src/gallium/auxiliary/draw/draw_pt_post_vs.c
> index 0212656..d717d05 100644
> --- a/src/gallium/auxiliary/draw/draw_pt_post_vs.c
> +++ b/src/gallium/auxiliary/draw/draw_pt_post_vs.c
> @@ -27,6 +27,7 @@
>
>   #include "util/u_memory.h"
>   #include "util/u_math.h"
> +#include "util/u_prim.h"
>   #include "pipe/p_context.h"
>   #include "draw/draw_context.h"
>   #include "draw/draw_private.h"
> @@ -48,7 +49,8 @@ struct pt_post_vs {
>      unsigned flags;
>
>      boolean (*run)( struct pt_post_vs *pvs,
> -                   struct draw_vertex_info *info );
> +                   struct draw_vertex_info *info,
> +                   const struct draw_prim_info *prim_info );
>   };
>
>   static INLINE void
> @@ -115,9 +117,10 @@ dot4(const float *a, const float *b)
>
>
>   boolean draw_pt_post_vs_run( struct pt_post_vs *pvs,
> -			     struct draw_vertex_info *info )
> +			     struct draw_vertex_info *info,
> +                             const struct draw_prim_info *prim_info )
>   {
> -   return pvs->run( pvs, info );
> +   return pvs->run( pvs, info, prim_info );
>   }
>

Reviewed-by: Brian Paul <brianp at vmware.com>


More information about the mesa-dev mailing list