[Mesa-dev] [PATCH 2/2] draw: inject frontface info into wireframe outputs

Roland Scheidegger sroland at vmware.com
Wed Jul 31 18:58:57 PDT 2013


Am 31.07.2013 13:38, schrieb Zack Rusin:
> Draw module can decompose primitives into wireframe models, which
> is a fancy word for 'lines', unfortunately that decomposition means
> that we weren't able to preserve the original front-face info which
> could be derived from the original primitives (lines don't have a
> 'face'). To fix it allow draw module to inject a fake face semantic
> into outputs from which the backends can figure out the original
> frontfacing info of the primitives.
> 
> Signed-off-by: Zack Rusin <zackr at vmware.com>
> ---
>  src/gallium/auxiliary/draw/draw_context.c       |   43 ++++++++++++++++++++
>  src/gallium/auxiliary/draw/draw_context.h       |    6 +++
>  src/gallium/auxiliary/draw/draw_pipe.h          |    3 ++
>  src/gallium/auxiliary/draw/draw_pipe_unfilled.c |   49 +++++++++++++++++++++++
>  src/gallium/drivers/i915/i915_state_derived.c   |    2 +
>  src/gallium/drivers/llvmpipe/lp_context.h       |    3 ++
>  src/gallium/drivers/llvmpipe/lp_setup.c         |    1 +
>  src/gallium/drivers/llvmpipe/lp_setup_context.h |    1 +
>  src/gallium/drivers/llvmpipe/lp_setup_line.c    |   14 ++++++-
>  src/gallium/drivers/llvmpipe/lp_state_derived.c |    9 +++++
>  src/gallium/drivers/r300/r300_state_derived.c   |    1 +
>  src/gallium/drivers/softpipe/sp_state_derived.c |    2 +
>  src/gallium/drivers/svga/svga_swtnl_state.c     |    1 +
>  13 files changed, 133 insertions(+), 2 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/draw/draw_context.c b/src/gallium/auxiliary/draw/draw_context.c
> index 4a6ba1a..2e95b5c 100644
> --- a/src/gallium/auxiliary/draw/draw_context.c
> +++ b/src/gallium/auxiliary/draw/draw_context.c
> @@ -39,6 +39,7 @@
>  #include "util/u_helpers.h"
>  #include "util/u_prim.h"
>  #include "draw_context.h"
> +#include "draw_pipe.h"
>  #include "draw_vs.h"
>  #include "draw_gs.h"
>  
> @@ -540,6 +541,22 @@ draw_get_shader_info(const struct draw_context *draw)
>     }
>  }
>  
> +/**
> + * Prepare outputs slots from the draw module
> + *
> + * Certain parts of the draw module can emit additional
> + * outputs that can be quite useful to the backends, a good
> + * example of it is the process of decomposing primitives
> + * into wireframes (aka. lines) which normally would lose
> + * the face-side information, but using this method we can
> + * inject another shader output which passes the original
> + * face side information to the backend.
> + */
> +void
> +draw_prepare_shader_outputs(struct draw_context *draw)
> +{
> +   draw_unfilled_prepare_outputs(draw, draw->pipeline.unfilled);
> +}
>  
>  /**
>   * Ask the draw module for the location/slot of the given vertex attribute in
> @@ -973,3 +990,29 @@ draw_stats_clipper_primitives(struct draw_context *draw,
>        }
>     }
>  }
> +
> +
> +/**
> + * Returns true if the draw module will inject the frontface
> + * info into the outputs.
> + *
> + * Given the specified primitive and rasterizer state
> + * the function will figure out if the draw module
> + * will inject the front-face information into shader
> + * outputs. This is done to preserve the front-facing
> + * info when decomposing primitives into wireframes.
> + */
> +boolean
> +draw_will_inject_frontface(const struct draw_context *draw)
> +{
> +   unsigned reduced_prim = u_reduced_prim(draw->pt.prim);
> +   const struct pipe_rasterizer_state *rast = draw->rasterizer;
> +
> +   if (reduced_prim != PIPE_PRIM_TRIANGLES) {
> +      return FALSE;
> +   }
> +
> +   return (rast &&
> +           (rast->fill_front != PIPE_POLYGON_MODE_FILL ||
> +            rast->fill_back != PIPE_POLYGON_MODE_FILL));
> +}
> diff --git a/src/gallium/auxiliary/draw/draw_context.h b/src/gallium/auxiliary/draw/draw_context.h
> index 4a1b27e..0815047 100644
> --- a/src/gallium/auxiliary/draw/draw_context.h
> +++ b/src/gallium/auxiliary/draw/draw_context.h
> @@ -126,10 +126,16 @@ draw_install_pstipple_stage(struct draw_context *draw, struct pipe_context *pipe
>  struct tgsi_shader_info *
>  draw_get_shader_info(const struct draw_context *draw);
>  
> +void
> +draw_prepare_shader_outputs(struct draw_context *draw);
> +
>  int
>  draw_find_shader_output(const struct draw_context *draw,
>                          uint semantic_name, uint semantic_index);
>  
> +boolean
> +draw_will_inject_frontface(const struct draw_context *draw);
> +
>  uint
>  draw_num_shader_outputs(const struct draw_context *draw);
>  
> diff --git a/src/gallium/auxiliary/draw/draw_pipe.h b/src/gallium/auxiliary/draw/draw_pipe.h
> index 4792507..2e48b56 100644
> --- a/src/gallium/auxiliary/draw/draw_pipe.h
> +++ b/src/gallium/auxiliary/draw/draw_pipe.h
> @@ -102,6 +102,9 @@ void draw_pipe_passthrough_line(struct draw_stage *stage, struct prim_header *he
>  void draw_pipe_passthrough_point(struct draw_stage *stage, struct prim_header *header);
>  
>  
> +void draw_unfilled_prepare_outputs(struct draw_context *context,
> +                                   struct draw_stage *stage);
> +
>  
>  /**
>   * Get a writeable copy of a vertex.
> diff --git a/src/gallium/auxiliary/draw/draw_pipe_unfilled.c b/src/gallium/auxiliary/draw/draw_pipe_unfilled.c
> index d87741b..d8a603f 100644
> --- a/src/gallium/auxiliary/draw/draw_pipe_unfilled.c
> +++ b/src/gallium/auxiliary/draw/draw_pipe_unfilled.c
> @@ -47,6 +47,8 @@ struct unfilled_stage {
>      * and PIPE_POLYGON_MODE_POINT,
>      */
>     unsigned mode[2];
> +
> +   int face_slot;
>  };
>  
>  
> @@ -55,8 +57,31 @@ static INLINE struct unfilled_stage *unfilled_stage( struct draw_stage *stage )
>     return (struct unfilled_stage *)stage;
>  }
>  
> +static void
> +inject_front_face_info(struct draw_stage *stage,
> +                       struct prim_header *header)
> +{
> +   struct unfilled_stage *unfilled = unfilled_stage(stage);
> +   unsigned ccw = header->det < 0.0;
> +   boolean is_front_face = (
> +      (stage->draw->rasterizer->front_ccw && ccw) ||
> +      (!stage->draw->rasterizer->front_ccw && !ccw));
> +   unsigned slot = unfilled->face_slot;
> +   struct vertex_header *v0 = header->v[0];
> +   struct vertex_header *v1 = header->v[1];
> +   struct vertex_header *v2 = header->v[2];
> +
> +   /* In case the backend doesn't care about it */
> +   if (slot < 0) {
> +      return;
> +   }
>  
> +   v0->data[slot][0] = is_front_face;
> +   v1->data[slot][0] = is_front_face;
> +   v2->data[slot][0] = is_front_face;
> +}
>  
> +   
>  static void point( struct draw_stage *stage,
>  		   struct vertex_header *v0 )
>  {
> @@ -83,6 +108,8 @@ static void points( struct draw_stage *stage,
>     struct vertex_header *v1 = header->v[1];
>     struct vertex_header *v2 = header->v[2];
>  
> +   inject_front_face_info(stage, header);
> +
>     if ((header->flags & DRAW_PIPE_EDGE_FLAG_0) && v0->edgeflag) point( stage, v0 );
>     if ((header->flags & DRAW_PIPE_EDGE_FLAG_1) && v1->edgeflag) point( stage, v1 );
>     if ((header->flags & DRAW_PIPE_EDGE_FLAG_2) && v2->edgeflag) point( stage, v2 );
> @@ -99,6 +126,8 @@ static void lines( struct draw_stage *stage,
>     if (header->flags & DRAW_PIPE_RESET_STIPPLE)
>        stage->next->reset_stipple_counter( stage->next );
>  
> +   inject_front_face_info(stage, header);
> +
>     if ((header->flags & DRAW_PIPE_EDGE_FLAG_2) && v2->edgeflag) line( stage, v2, v0 );
>     if ((header->flags & DRAW_PIPE_EDGE_FLAG_0) && v0->edgeflag) line( stage, v0, v1 );
>     if ((header->flags & DRAW_PIPE_EDGE_FLAG_1) && v1->edgeflag) line( stage, v1, v2 );
> @@ -192,6 +221,26 @@ static void unfilled_destroy( struct draw_stage *stage )
>     FREE( stage );
>  }
>  
> +/*
> + * Try to allocate an output slot which we can use
> + * to preserve the front face information.
> + */
> +void
> +draw_unfilled_prepare_outputs( struct draw_context *draw,
> +                               struct draw_stage *stage )
> +{
> +   struct unfilled_stage *unfilled = unfilled_stage(stage);
> +   const struct pipe_rasterizer_state *rast = draw ? draw->rasterizer : 0;
> +   if (rast &&
> +       (rast->fill_front != PIPE_POLYGON_MODE_FILL ||
> +        rast->fill_back != PIPE_POLYGON_MODE_FILL)) {
> +      unfilled->face_slot = draw_alloc_extra_vertex_attrib(
> +         stage->draw, TGSI_SEMANTIC_FACE, 0);
> +   } else {
> +      unfilled->face_slot = -1;
> +   }
> +}
> +
>  
>  /**
>   * Create unfilled triangle stage.
> diff --git a/src/gallium/drivers/i915/i915_state_derived.c b/src/gallium/drivers/i915/i915_state_derived.c
> index e01f16e..e1d1840 100644
> --- a/src/gallium/drivers/i915/i915_state_derived.c
> +++ b/src/gallium/drivers/i915/i915_state_derived.c
> @@ -67,6 +67,8 @@ static void calculate_vertex_layout(struct i915_context *i915)
>     colors[0] = colors[1] = fog = needW = face = FALSE;
>     memset(&vinfo, 0, sizeof(vinfo));
>  
> +   draw_prepare_shader_outputs(i915->draw);
> +
>     /* Determine which fragment program inputs are needed.  Setup HW vertex
>      * layout below, in the HW-specific attribute order.
>      */
> diff --git a/src/gallium/drivers/llvmpipe/lp_context.h b/src/gallium/drivers/llvmpipe/lp_context.h
> index 9495e42..fc948a7 100644
> --- a/src/gallium/drivers/llvmpipe/lp_context.h
> +++ b/src/gallium/drivers/llvmpipe/lp_context.h
> @@ -122,6 +122,9 @@ struct llvmpipe_context {
>     /** Which geometry shader output slot contains layer */
>     int layer_slot;
>  
> +   /** A fake frontface output for unfilled primitives */
> +   int face_slot;
> +
>     /**< minimum resolvable depth value, for polygon offset */   
>     double mrd;
>     
> diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c b/src/gallium/drivers/llvmpipe/lp_setup.c
> index 65f61ed..5fde01f 100644
> --- a/src/gallium/drivers/llvmpipe/lp_setup.c
> +++ b/src/gallium/drivers/llvmpipe/lp_setup.c
> @@ -1053,6 +1053,7 @@ lp_setup_update_state( struct lp_setup_context *setup,
>        setup->psize = lp->psize_slot;
>        setup->viewport_index_slot = lp->viewport_index_slot;
>        setup->layer_slot = lp->layer_slot;
> +      setup->face_slot = lp->face_slot;
>  
>        assert(lp->dirty == 0);
>  
> diff --git a/src/gallium/drivers/llvmpipe/lp_setup_context.h b/src/gallium/drivers/llvmpipe/lp_setup_context.h
> index 89c23bb..ea1d0d6 100644
> --- a/src/gallium/drivers/llvmpipe/lp_setup_context.h
> +++ b/src/gallium/drivers/llvmpipe/lp_setup_context.h
> @@ -106,6 +106,7 @@ struct lp_setup_context
>     float psize;
>     unsigned viewport_index_slot;
>     unsigned layer_slot;
> +   unsigned face_slot;
>  
>     struct pipe_framebuffer_state fb;
>     struct u_rect framebuffer;
> diff --git a/src/gallium/drivers/llvmpipe/lp_setup_line.c b/src/gallium/drivers/llvmpipe/lp_setup_line.c
> index bfed59c..3b16163 100644
> --- a/src/gallium/drivers/llvmpipe/lp_setup_line.c
> +++ b/src/gallium/drivers/llvmpipe/lp_setup_line.c
> @@ -37,6 +37,7 @@
>  #include "lp_state_fs.h"
>  #include "lp_state_setup.h"
>  #include "lp_context.h"
> +#include "draw/draw_context.h"
>  
>  #define NUM_CHANNELS 4
>  
> @@ -45,6 +46,7 @@ struct lp_line_info {
>     float dx;
>     float dy;
>     float oneoverarea;
> +   boolean frontfacing;
>  
>     const float (*v1)[4];
>     const float (*v2)[4];
> @@ -214,7 +216,8 @@ static void setup_line_coefficients( struct lp_setup_context *setup,
>        case LP_INTERP_FACING:
>           for (i = 0; i < NUM_CHANNELS; i++)
>              if (usage_mask & (1 << i))
> -               constant_coef(setup, info, slot+1, 1.0, i);
> +               constant_coef(setup, info, slot+1,
> +                             info->frontfacing ? 1.0f : -1.0f, i);
I realize this was there before but technically this isn't correct (and
doesn't match what we're doing for tris) - only channel 0 is supposed to
represent sign, the rest are supposed to be 0,0,1. Also while not
required I think it would be nice if the same logic would be used for
calculating the constant coefficient as is used for tris, whatever that
is...

>           break;
>  
>        default:
> @@ -613,15 +616,22 @@ try_setup_line( struct lp_setup_context *setup,
>     plane[2].dcdx = y[2] - y[3];
>     plane[3].dcdx = y[3] - y[0];
>  
> +   if (draw_will_inject_frontface(lp_context->draw) &&
I think it's annoying you have to do these calls to determine if there's
a valid frontface here for each line instead of just per draw call but
it doesn't seem easy to avoid it.
Also, no love for llvmpipe point face? I realize d3d10 doesn't require
it but OpenGL (and IIRC d3d9) do.


> +       setup->face_slot > 0) {
> +      line->inputs.frontfacing = v1[setup->face_slot][0];
> +   } else {
> +      line->inputs.frontfacing = TRUE;
> +   }
> +   
>  
>     /* Setup parameter interpolants:
>      */
>     info.a0 = GET_A0(&line->inputs);
>     info.dadx = GET_DADX(&line->inputs);
>     info.dady = GET_DADY(&line->inputs);
> +   info.frontfacing = line->inputs.frontfacing;
>     setup_line_coefficients(setup, &info); 
>  
> -   line->inputs.frontfacing = TRUE;
>     line->inputs.disable = FALSE;
>     line->inputs.opaque = FALSE;
>     line->inputs.layer = layer;
> diff --git a/src/gallium/drivers/llvmpipe/lp_state_derived.c b/src/gallium/drivers/llvmpipe/lp_state_derived.c
> index dc80358..5a51b50 100644
> --- a/src/gallium/drivers/llvmpipe/lp_state_derived.c
> +++ b/src/gallium/drivers/llvmpipe/lp_state_derived.c
> @@ -53,6 +53,8 @@ compute_vertex_info(struct llvmpipe_context *llvmpipe)
>     int vs_index;
>     uint i;
>  
> +   draw_prepare_shader_outputs(llvmpipe->draw);
> +
>     llvmpipe->color_slot[0] = -1;
>     llvmpipe->color_slot[1] = -1;
>     llvmpipe->bcolor_slot[0] = -1;
> @@ -138,6 +140,13 @@ compute_vertex_info(struct llvmpipe_context *llvmpipe)
>        llvmpipe->layer_slot = 0;
>     }
>  
> +   /* Check for a fake front face for unfilled primitives*/
> +   vs_index = draw_find_shader_output(llvmpipe->draw,
> +                                      TGSI_SEMANTIC_FACE, 0);
> +   if (vs_index >= 0) {
> +      llvmpipe->face_slot = vinfo->num_attribs;
> +      draw_emit_vertex_attr(vinfo, EMIT_4F, INTERP_CONSTANT, vs_index);
> +   }
>  
>     draw_compute_vertex_size(vinfo);
>     lp_setup_set_vertex_info(llvmpipe->setup, vinfo);
> diff --git a/src/gallium/drivers/r300/r300_state_derived.c b/src/gallium/drivers/r300/r300_state_derived.c
> index 1013557..da86cd2 100644
> --- a/src/gallium/drivers/r300/r300_state_derived.c
> +++ b/src/gallium/drivers/r300/r300_state_derived.c
> @@ -1079,6 +1079,7 @@ void r300_update_derived_state(struct r300_context* r300)
>  
>          if (r300->draw) {
>              memset(&r300->vertex_info, 0, sizeof(struct vertex_info));
> +            draw_prepare_shader_outputs(r300->draw);
>              r300_draw_emit_all_attribs(r300);
>              draw_compute_vertex_size(&r300->vertex_info);
>              r300_swtcl_vertex_psc(r300);
> diff --git a/src/gallium/drivers/softpipe/sp_state_derived.c b/src/gallium/drivers/softpipe/sp_state_derived.c
> index 6115349..9aa97d2 100644
> --- a/src/gallium/drivers/softpipe/sp_state_derived.c
> +++ b/src/gallium/drivers/softpipe/sp_state_derived.c
> @@ -65,6 +65,8 @@ softpipe_get_vertex_info(struct softpipe_context *softpipe)
>  {
>     struct vertex_info *vinfo = &softpipe->vertex_info;
>  
> +   draw_prepare_shader_outputs(softpipe->draw);
> +
>     if (vinfo->num_attribs == 0) {
>        /* compute vertex layout now */
>        const struct tgsi_shader_info *fsInfo = &softpipe->fs_variant->info;
> diff --git a/src/gallium/drivers/svga/svga_swtnl_state.c b/src/gallium/drivers/svga/svga_swtnl_state.c
> index d744f18..d60af3f 100644
> --- a/src/gallium/drivers/svga/svga_swtnl_state.c
> +++ b/src/gallium/drivers/svga/svga_swtnl_state.c
> @@ -162,6 +162,7 @@ svga_swtnl_update_vdecl( struct svga_context *svga )
>     memset(vinfo, 0, sizeof(*vinfo));
>     memset(vdecl, 0, sizeof(vdecl));
>  
> +   draw_prepare_shader_outputs(draw);
>     /* always add position */
>     src = draw_find_shader_output(draw, TGSI_SEMANTIC_POSITION, 0);
>     draw_emit_vertex_attr(vinfo, EMIT_4F, INTERP_LINEAR, src);
> 

Looks like quite a heavy interface (and sort of silly to allocate 128
bits in the vertex data (so actually twice that for one line) for 1 bit
of information but given all our data passed on to the line/point funcs
are float4 I don't really see any other easy way neither), but seems all
necessary unfortunately. I guess another option would be to pass the
face info always along the vertex data no matter what (which would mean
all those additional calls for setting up outputs, determining if
there's a valid frontface etc. could go along with the storage needed)
for all primitives to the point/line/tri funcs but I'm not really
thrilled about that idea neither (passing it for tris so it doesn't have
to be recalculated may or may not be a good idea neither).

So this is ok by me.

Roland


More information about the mesa-dev mailing list