[Mesa-dev] [PATCH 5/5] draw: remove clip_vertex from vertex header
Jose Fonseca
jfonseca at vmware.com
Mon Dec 14 07:20:06 PST 2015
On 12/12/15 02:31, 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:
>
This code is outside my comfort zone, but I didn't spot anything wrong
in the series.
Reviewed-by: Jose Fonseca <jfonseca at vmware.com>
More information about the mesa-dev
mailing list