[Mesa-dev] [PATCH 5/5] draw: remove clip_vertex from vertex header

Brian Paul brianp at vmware.com
Mon Dec 14 07:17:21 PST 2015


The series looks good to me.  Thanks for digging into this.

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



On 12/11/2015 07:31 PM, sroland at vmware.com wrote:
> From: Roland Scheidegger <sroland at vmware.com>
>
> vertex header had both clip_pos and clip_vertex.
> We only really need one (clip_pos) because the draw llvm shader would
> overwrite the position output from the vs with the viewport transformed.
> However, we don't really need the second one, which was only really used
> for gl_ClipVertex - if the shader didn't have that the values were just
> duplicated to both clip_pos and clip_vertex. So, just use this from the vs
> output instead when we actually need it.
> Also change clip debug to output both the data from clip_pos and the
> clipVertex output (if available).
> Makes some things more complex, some things less complex, but seems more
> easy to understand what clipping actually does (and what values it uses
> to do its magic).
> ---
>   src/gallium/auxiliary/draw/draw_cliptest_tmp.h |  1 -
>   src/gallium/auxiliary/draw/draw_llvm.c         | 21 ++------
>   src/gallium/auxiliary/draw/draw_llvm.h         |  4 --
>   src/gallium/auxiliary/draw/draw_pipe_clip.c    | 67 +++++++++++++++++++-------
>   src/gallium/auxiliary/draw/draw_private.h      |  1 -
>   5 files changed, 54 insertions(+), 40 deletions(-)
>
> diff --git a/src/gallium/auxiliary/draw/draw_cliptest_tmp.h b/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
> index 06c82c3..c57e5f4 100644
> --- a/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
> +++ b/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
> @@ -91,7 +91,6 @@ static boolean TAG(do_cliptest)( struct pt_post_vs *pvs,
>            }
>
>            for (i = 0; i < 4; i++) {
> -            out->clip_vertex[i] = clipvertex[i];
>               out->clip_pos[i] = position[i];
>            }
>
> diff --git a/src/gallium/auxiliary/draw/draw_llvm.c b/src/gallium/auxiliary/draw/draw_llvm.c
> index 934f452..a966e45 100644
> --- a/src/gallium/auxiliary/draw/draw_llvm.c
> +++ b/src/gallium/auxiliary/draw/draw_llvm.c
> @@ -378,14 +378,13 @@ static LLVMTypeRef
>   create_jit_vertex_header(struct gallivm_state *gallivm, int data_elems)
>   {
>      LLVMTargetDataRef target = gallivm->target;
> -   LLVMTypeRef elem_types[4];
> +   LLVMTypeRef elem_types[3];
>      LLVMTypeRef vertex_header;
>      char struct_name[24];
>
>      util_snprintf(struct_name, 23, "vertex_header%d", data_elems);
>
>      elem_types[DRAW_JIT_VERTEX_VERTEX_ID]  = LLVMIntTypeInContext(gallivm->context, 32);
> -   elem_types[DRAW_JIT_VERTEX_CLIP_VERTEX]  = LLVMArrayType(LLVMFloatTypeInContext(gallivm->context), 4);
>      elem_types[DRAW_JIT_VERTEX_CLIP_POS]  = LLVMArrayType(LLVMFloatTypeInContext(gallivm->context), 4);
>      elem_types[DRAW_JIT_VERTEX_DATA]  = LLVMArrayType(elem_types[1], data_elems);
>
> @@ -407,9 +406,6 @@ create_jit_vertex_header(struct gallivm_state *gallivm, int data_elems)
>         DRAW_JIT_VERTEX_VERTEX_ID);
>      */
>      (void) target; /* silence unused var warning for non-debug build */
> -   LP_CHECK_MEMBER_OFFSET(struct vertex_header, clip_vertex,
> -                          target, vertex_header,
> -                          DRAW_JIT_VERTEX_CLIP_VERTEX);
>      LP_CHECK_MEMBER_OFFSET(struct vertex_header, clip_pos,
>                             target, vertex_header,
>                             DRAW_JIT_VERTEX_CLIP_POS);
> @@ -1019,7 +1015,7 @@ store_clip(struct gallivm_state *gallivm,
>              const struct lp_type vs_type,
>              LLVMValueRef io_ptr,
>              LLVMValueRef (*outputs)[TGSI_NUM_CHANNELS],
> -           boolean clip_vertex, int idx)
> +           int idx)
>   {
>      LLVMBuilderRef builder = gallivm->builder;
>      LLVMValueRef soa[4];
> @@ -1046,14 +1042,8 @@ store_clip(struct gallivm_state *gallivm,
>      soa[2] = LLVMBuildLoad(builder, outputs[idx][2], ""); /*z0 z1 .. zn*/
>      soa[3] = LLVMBuildLoad(builder, outputs[idx][3], ""); /*w0 w1 .. wn*/
>
> -   if (clip_vertex) {
> -      for (i = 0; i < vs_type.length; i++) {
> -         clip_ptrs[i] = draw_jit_header_clip_vertex(gallivm, io_ptrs[i]);
> -      }
> -   } else {
> -      for (i = 0; i < vs_type.length; i++) {
> -         clip_ptrs[i] = draw_jit_header_clip_pos(gallivm, io_ptrs[i]);
> -      }
> +   for (i = 0; i < vs_type.length; i++) {
> +      clip_ptrs[i] = draw_jit_header_clip_pos(gallivm, io_ptrs[i]);
>      }
>
>      lp_build_transpose_aos(gallivm, vs_type, soa, soa);
> @@ -1771,8 +1761,7 @@ draw_llvm_generate(struct draw_llvm *llvm, struct draw_llvm_variant *variant,
>
>         if (pos != -1 && cv != -1) {
>            /* store original positions in clip before further manipulation */
> -         store_clip(gallivm, vs_type, io, outputs, TRUE, key->clip_user ? cv : pos);
> -         store_clip(gallivm, vs_type, io, outputs, FALSE, pos);
> +         store_clip(gallivm, vs_type, io, outputs, pos);
>
>            /* do cliptest */
>            if (enable_cliptest) {
> diff --git a/src/gallium/auxiliary/draw/draw_llvm.h b/src/gallium/auxiliary/draw/draw_llvm.h
> index c40df1c..f617a29 100644
> --- a/src/gallium/auxiliary/draw/draw_llvm.h
> +++ b/src/gallium/auxiliary/draw/draw_llvm.h
> @@ -104,7 +104,6 @@ enum {
>
>   enum {
>      DRAW_JIT_VERTEX_VERTEX_ID = 0,
> -   DRAW_JIT_VERTEX_CLIP_VERTEX,
>      DRAW_JIT_VERTEX_CLIP_POS,
>      DRAW_JIT_VERTEX_DATA
>   };
> @@ -162,9 +161,6 @@ enum {
>   #define draw_jit_header_id(_gallivm, _ptr)              \
>      lp_build_struct_get_ptr(_gallivm, _ptr, DRAW_JIT_VERTEX_VERTEX_ID, "id")
>
> -#define draw_jit_header_clip_vertex(_gallivm, _ptr) \
> -   lp_build_struct_get_ptr(_gallivm, _ptr, DRAW_JIT_VERTEX_CLIP_VERTEX, "clip_vertex")
> -
>   #define draw_jit_header_clip_pos(_gallivm, _ptr) \
>      lp_build_struct_get_ptr(_gallivm, _ptr, DRAW_JIT_VERTEX_CLIP_POS, "clip_pos")
>
> diff --git a/src/gallium/auxiliary/draw/draw_pipe_clip.c b/src/gallium/auxiliary/draw/draw_pipe_clip.c
> index 95616bf..67d8eca 100644
> --- a/src/gallium/auxiliary/draw/draw_pipe_clip.c
> +++ b/src/gallium/auxiliary/draw/draw_pipe_clip.c
> @@ -60,6 +60,7 @@ struct clip_stage {
>
>      unsigned pos_attr;
>      boolean have_clipdist;
> +   int cv_attr;
>
>      /* List of the attributes to be constant interpolated. */
>      uint num_const_attribs;
> @@ -151,7 +152,10 @@ static void interp(const struct clip_stage *clip,
>
>      /* Interpolate the clip-space coords.
>       */
> -   interp_attr(dst->clip_vertex, t, in->clip_vertex, out->clip_vertex);
> +   if (clip->cv_attr >= 0) {
> +      interp_attr(dst->data[clip->cv_attr], t,
> +                  in->data[clip->cv_attr], out->data[clip->cv_attr]);
> +   }
>      /* interpolate the clip-space position */
>      interp_attr(dst->clip_pos, t, in->clip_pos, out->clip_pos);
>
> @@ -215,9 +219,9 @@ static void interp(const struct clip_stage *clip,
>    * Triangle is considered null/empty if its area is equal to zero.
>    */
>   static inline boolean
> -is_tri_null(struct draw_context *draw, const struct prim_header *header)
> +is_tri_null(const struct clip_stage *clip, const struct prim_header *header)
>   {
> -   const unsigned pos_attr = draw_current_shader_position_output(draw);
> +   const unsigned pos_attr = clip->pos_attr;
>      float x1 = header->v[1]->data[pos_attr][0] - header->v[0]->data[pos_attr][0];
>      float y1 = header->v[1]->data[pos_attr][1] - header->v[0]->data[pos_attr][1];
>      float z1 = header->v[1]->data[pos_attr][2] - header->v[0]->data[pos_attr][2];
> @@ -243,6 +247,7 @@ static void emit_poly(struct draw_stage *stage,
>                         unsigned n,
>                         const struct prim_header *origPrim)
>   {
> +   const struct clip_stage *clipper = clip_stage(stage);
>      struct prim_header header;
>      unsigned i;
>      ushort edge_first, edge_middle, edge_last;
> @@ -282,7 +287,7 @@ static void emit_poly(struct draw_stage *stage,
>            header.v[2] = inlist[0];  /* the provoking vertex */
>         }
>
> -      tri_null = is_tri_null(stage->draw, &header);
> +      tri_null = is_tri_null(clipper, &header);
>         /* If we generated a triangle with an area, aka. non-null triangle,
>          * or if the previous triangle was also null then skip all subsequent
>          * null triangles */
> @@ -307,11 +312,18 @@ static void emit_poly(struct draw_stage *stage,
>            debug_printf("Clipped tri: (flat-shade-first = %d)\n",
>                         stage->draw->rasterizer->flatshade_first);
>            for (j = 0; j < 3; j++) {
> -            debug_printf("  Vert %d: clip: %f %f %f %f\n", j,
> -                         header.v[j]->clip_vertex[0],
> -                         header.v[j]->clip_vertex[1],
> -                         header.v[j]->clip_vertex[2],
> -                         header.v[j]->clip_vertex[3]);
> +            debug_printf("  Vert %d: clip pos: %f %f %f %f\n", j,
> +                         header.v[j]->clip_pos[0],
> +                         header.v[j]->clip_pos[1],
> +                         header.v[j]->clip_pos[2],
> +                         header.v[j]->clip_pos[3]);
> +            if (clipper->cv_attr >= 0) {
> +               debug_printf("  Vert %d: cv: %f %f %f %f\n", j,
> +                            header.v[j]->data[clipper->cv_attr][0],
> +                            header.v[j]->data[clipper->cv_attr][1],
> +                            header.v[j]->data[clipper->cv_attr][2],
> +                            header.v[j]->data[clipper->cv_attr][3]);
> +            }
>               for (k = 0; k < draw_num_shader_outputs(stage->draw); k++) {
>                  debug_printf("  Vert %d: Attr %d:  %f %f %f %f\n", j, k,
>                               header.v[j]->data[k][0],
> @@ -321,7 +333,7 @@ static void emit_poly(struct draw_stage *stage,
>               }
>            }
>         }
> -      stage->next->tri( stage->next, &header );
> +      stage->next->tri(stage->next, &header);
>      }
>   }
>
> @@ -360,10 +372,14 @@ static inline float getclipdist(const struct clip_stage *clipper,
>      } else {
>         /*
>          * legacy user clip planes or gl_ClipVertex
> -       * (clip will contain clipVertex output if available, pos otherwise).
>          */
>         plane = clipper->plane[plane_idx];
> -      dp = dot4(vert->clip_vertex, plane);
> +      if (clipper->cv_attr >= 0) {
> +         dp = dot4(vert->data[clipper->cv_attr], plane);
> +      }
> +      else {
> +         dp = dot4(vert->clip_pos, plane);
> +      }
>      }
>      return dp;
>   }
> @@ -410,13 +426,22 @@ do_clip_tri(struct draw_stage *stage,
>      viewport_index = draw_viewport_index(clipper->stage.draw, prov_vertex);
>
>      if (DEBUG_CLIP) {
> -      const float *v0 = header->v[0]->clip_vertex;
> -      const float *v1 = header->v[1]->clip_vertex;
> -      const float *v2 = header->v[2]->clip_vertex;
> -      debug_printf("Clip triangle:\n");
> +      const float *v0 = header->v[0]->clip_pos;
> +      const float *v1 = header->v[1]->clip_pos;
> +      const float *v2 = header->v[2]->clip_pos;
> +      debug_printf("Clip triangle pos:\n");
>         debug_printf(" %f, %f, %f, %f\n", v0[0], v0[1], v0[2], v0[3]);
>         debug_printf(" %f, %f, %f, %f\n", v1[0], v1[1], v1[2], v1[3]);
>         debug_printf(" %f, %f, %f, %f\n", v2[0], v2[1], v2[2], v2[3]);
> +      if (clipper->cv_attr >= 0) {
> +         const float *v0 = header->v[0]->data[clipper->cv_attr];
> +         const float *v1 = header->v[1]->data[clipper->cv_attr];
> +         const float *v2 = header->v[2]->data[clipper->cv_attr];
> +         debug_printf("Clip triangle cv:\n");
> +         debug_printf(" %f, %f, %f, %f\n", v0[0], v0[1], v0[2], v0[3]);
> +         debug_printf(" %f, %f, %f, %f\n", v1[0], v1[1], v1[2], v1[3]);
> +         debug_printf(" %f, %f, %f, %f\n", v2[0], v2[1], v2[2], v2[3]);
> +      }
>      }
>
>      /*
> @@ -565,7 +590,7 @@ do_clip_tri(struct draw_stage *stage,
>
>         /* Emit the polygon as triangles to the setup stage:
>          */
> -      emit_poly( stage, inlist, inEdges, n, header );
> +      emit_poly(stage, inlist, inEdges, n, header);
>      }
>   }
>
> @@ -577,7 +602,7 @@ do_clip_line(struct draw_stage *stage,
>                struct prim_header *header,
>                unsigned clipmask)
>   {
> -   const struct clip_stage *clipper = clip_stage( stage );
> +   const struct clip_stage *clipper = clip_stage(stage);
>      struct vertex_header *v0 = header->v[0];
>      struct vertex_header *v1 = header->v[1];
>      struct vertex_header *prov_vertex;
> @@ -792,6 +817,12 @@ clip_init_state(struct draw_stage *stage)
>
>      clipper->pos_attr = draw_current_shader_position_output(draw);
>      clipper->have_clipdist = draw_current_shader_num_written_clipdistances(draw) > 0;
> +   if (draw_current_shader_clipvertex_output(draw) != clipper->pos_attr) {
> +      clipper->cv_attr = (int)draw_current_shader_clipvertex_output(draw);
> +   }
> +   else {
> +      clipper->cv_attr = -1;
> +   }
>
>      /* We need to know for each attribute what kind of interpolation is
>       * done on it (flat, smooth or noperspective).  But the information
> diff --git a/src/gallium/auxiliary/draw/draw_private.h b/src/gallium/auxiliary/draw/draw_private.h
> index 7040d75..8774beb 100644
> --- a/src/gallium/auxiliary/draw/draw_private.h
> +++ b/src/gallium/auxiliary/draw/draw_private.h
> @@ -89,7 +89,6 @@ struct vertex_header {
>      unsigned pad:1;
>      unsigned vertex_id:16;
>
> -   float clip_vertex[4];
>      float clip_pos[4];
>
>      /* This will probably become float (*data)[4] soon:
>



More information about the mesa-dev mailing list