[Mesa-dev] [PATCH 8/8] draw: implement proper primitive assembler as a pipeline stage

Brian Paul brianp at vmware.com
Fri Aug 2 07:30:38 PDT 2013


Just minor nits...

On 08/02/2013 12:28 AM, Zack Rusin wrote:
> we used to have a face primitive assembler that we ran after if
> the gs was missing but we had adjacency primitives in the pipeline,
> lets convert it to a pipeline stage, which allows us to use it
> to inject outputs (primitive id) into the vertices. it's also
> a lot cleaner because the decomposition is already handled for us.
>
> Signed-off-by: Zack Rusin <zackr at vmware.com>
> ---
>   src/gallium/auxiliary/Makefile.sources             |    2 +-
>   src/gallium/auxiliary/draw/draw_context.c          |    1 +
>   src/gallium/auxiliary/draw/draw_pipe.c             |    4 +
>   src/gallium/auxiliary/draw/draw_pipe.h             |    5 +
>   src/gallium/auxiliary/draw/draw_pipe_ia.c          |  253 ++++++++++++++++++++
>   src/gallium/auxiliary/draw/draw_pipe_validate.c    |   15 +-
>   src/gallium/auxiliary/draw/draw_prim_assembler.c   |  225 -----------------
>   src/gallium/auxiliary/draw/draw_prim_assembler.h   |   62 -----
>   .../auxiliary/draw/draw_prim_assembler_tmp.h       |   31 ---
>   src/gallium/auxiliary/draw/draw_private.h          |    1 +
>   .../auxiliary/draw/draw_pt_fetch_shade_pipeline.c  |   18 +-
>   .../draw/draw_pt_fetch_shade_pipeline_llvm.c       |   18 +-
>   12 files changed, 283 insertions(+), 352 deletions(-)
>   create mode 100644 src/gallium/auxiliary/draw/draw_pipe_ia.c
>   delete mode 100644 src/gallium/auxiliary/draw/draw_prim_assembler.c
>   delete mode 100644 src/gallium/auxiliary/draw/draw_prim_assembler.h
>   delete mode 100644 src/gallium/auxiliary/draw/draw_prim_assembler_tmp.h
>
> diff --git a/src/gallium/auxiliary/Makefile.sources b/src/gallium/auxiliary/Makefile.sources
> index acbcef7..ee93e8b 100644
> --- a/src/gallium/auxiliary/Makefile.sources
> +++ b/src/gallium/auxiliary/Makefile.sources
> @@ -13,6 +13,7 @@ C_SOURCES := \
>   	draw/draw_pipe_clip.c \
>   	draw/draw_pipe_cull.c \
>   	draw/draw_pipe_flatshade.c \
> +        draw/draw_pipe_ia.c \
>   	draw/draw_pipe_offset.c \
>   	draw/draw_pipe_pstipple.c \
>   	draw/draw_pipe_stipple.c \
> @@ -23,7 +24,6 @@ C_SOURCES := \
>   	draw/draw_pipe_vbuf.c \
>   	draw/draw_pipe_wide_line.c \
>   	draw/draw_pipe_wide_point.c \
> -	draw/draw_prim_assembler.c \
>   	draw/draw_pt.c \
>   	draw/draw_pt_emit.c \
>   	draw/draw_pt_fetch.c \
> diff --git a/src/gallium/auxiliary/draw/draw_context.c b/src/gallium/auxiliary/draw/draw_context.c
> index 8bf3596..bbb2904 100644
> --- a/src/gallium/auxiliary/draw/draw_context.c
> +++ b/src/gallium/auxiliary/draw/draw_context.c
> @@ -555,6 +555,7 @@ draw_get_shader_info(const struct draw_context *draw)
>   void
>   draw_prepare_shader_outputs(struct draw_context *draw)
>   {
> +   draw_ia_prepare_outputs(draw, draw->pipeline.ia);
>      draw_unfilled_prepare_outputs(draw, draw->pipeline.unfilled);
>   }
>
> diff --git a/src/gallium/auxiliary/draw/draw_pipe.c b/src/gallium/auxiliary/draw/draw_pipe.c
> index f1ee6cb..8140299 100644
> --- a/src/gallium/auxiliary/draw/draw_pipe.c
> +++ b/src/gallium/auxiliary/draw/draw_pipe.c
> @@ -49,6 +49,7 @@ boolean draw_pipeline_init( struct draw_context *draw )
>      draw->pipeline.clip      = draw_clip_stage( draw );
>      draw->pipeline.flatshade = draw_flatshade_stage( draw );
>      draw->pipeline.cull      = draw_cull_stage( draw );
> +   draw->pipeline.ia        = draw_ia_stage( draw );
>      draw->pipeline.validate  = draw_validate_stage( draw );
>      draw->pipeline.first     = draw->pipeline.validate;
>
> @@ -61,6 +62,7 @@ boolean draw_pipeline_init( struct draw_context *draw )
>          !draw->pipeline.clip ||
>          !draw->pipeline.flatshade ||
>          !draw->pipeline.cull ||
> +       !draw->pipeline.ia ||
>          !draw->pipeline.validate)
>         return FALSE;
>
> @@ -95,6 +97,8 @@ void draw_pipeline_destroy( struct draw_context *draw )
>         draw->pipeline.flatshade->destroy( draw->pipeline.flatshade );
>      if (draw->pipeline.cull)
>         draw->pipeline.cull->destroy( draw->pipeline.cull );
> +   if (draw->pipeline.ia)
> +      draw->pipeline.ia->destroy( draw->pipeline.ia );
>      if (draw->pipeline.validate)
>         draw->pipeline.validate->destroy( draw->pipeline.validate );
>      if (draw->pipeline.aaline)
> diff --git a/src/gallium/auxiliary/draw/draw_pipe.h b/src/gallium/auxiliary/draw/draw_pipe.h
> index 70c286f..70822a4 100644
> --- a/src/gallium/auxiliary/draw/draw_pipe.h
> +++ b/src/gallium/auxiliary/draw/draw_pipe.h
> @@ -91,7 +91,10 @@ extern struct draw_stage *draw_stipple_stage( struct draw_context *context );
>   extern struct draw_stage *draw_wide_line_stage( struct draw_context *context );
>   extern struct draw_stage *draw_wide_point_stage( struct draw_context *context );
>   extern struct draw_stage *draw_validate_stage( struct draw_context *context );
> +extern struct draw_stage *draw_ia_stage(struct draw_context *context);
>
> +boolean draw_ia_stage_required(const struct draw_context *context,
> +                               unsigned prim);
>
>   extern void draw_free_temp_verts( struct draw_stage *stage );
>   extern boolean draw_alloc_temp_verts( struct draw_stage *stage, unsigned nr );
> @@ -105,6 +108,8 @@ void draw_pipe_passthrough_point(struct draw_stage *stage, struct prim_header *h
>
>   void draw_unfilled_prepare_outputs(struct draw_context *context,
>                                      struct draw_stage *stage);
> +void draw_ia_prepare_outputs(struct draw_context *context,
> +                             struct draw_stage *stage);
>
>
>   /**
> diff --git a/src/gallium/auxiliary/draw/draw_pipe_ia.c b/src/gallium/auxiliary/draw/draw_pipe_ia.c
> new file mode 100644
> index 0000000..9bdab8f
> --- /dev/null
> +++ b/src/gallium/auxiliary/draw/draw_pipe_ia.c
> @@ -0,0 +1,253 @@
> +/**************************************************************************
> + *
> + * Copyright 2013 VMware
> + * All Rights Reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sub license, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> + * next paragraph) shall be included in all copies or substantial portions
> + * of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
> + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT.
> + * IN NO EVENT SHALL TUNGSTEN GRAPHICS AND/OR ITS SUPPLIERS BE LIABLE FOR
> + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
> + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
> + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> + *
> + **************************************************************************/
> +
> +/**
> + * \brief  Used to decompose adjacency primitives and inject the prim id
> + */
> +
> +#include "util/u_math.h"
> +#include "util/u_memory.h"
> +#include "pipe/p_defines.h"
> +#include "draw_pipe.h"
> +#include "draw_fs.h"
> +#include "draw_gs.h"
> +
> +
> +struct ia_stage {
> +   struct draw_stage stage;
> +   int primid_slot;
> +   unsigned primid;
> +};
> +
> +
> +static INLINE struct ia_stage *ia_stage( struct draw_stage *stage )
> +{
> +   return (struct ia_stage *)stage;
> +}

Reformat:

static INLINE struct ia_stage *
ia_stage(struct draw_stage *stage)
{
    return (struct ia_stage *)stage;
}


> +
> +
> +static void
> +inject_primid(struct draw_stage *stage,
> +              struct prim_header *header,
> +              unsigned num_verts)
> +{
> +   struct ia_stage *ia = ia_stage(stage);
> +   unsigned slot = ia->primid_slot;
> +   unsigned i;
> +   unsigned primid = ia->primid;
> +
> +   /* In case the backend doesn't care about it */
> +   if (slot < 0) {
> +      return;
> +   }
> +
> +   for (i = 0; i < num_verts; ++i) {
> +      struct vertex_header *v = header->v[i];
> +      /*memcpy(&v->data[slot][0], &primid, sizeof(primid));
> +      memcpy(&v->data[slot][1], &primid, sizeof(primid));
> +      memcpy(&v->data[slot][2], &primid, sizeof(primid));
> +      memcpy(&v->data[slot][3], &primid, sizeof(primid));*/
> +      v->data[slot][0] = primid;
> +      v->data[slot][1] = primid;
> +      v->data[slot][2] = primid;
> +      v->data[slot][3] = primid;

MSVC is probably going to warn about these int->float assignments.  So 
maybe add a cast.


> +   }
> +   ++ia->primid;
> +}
> +
> +
> +static void
> +ia_point(struct draw_stage *stage,
> +         struct prim_header *header)
> +{
> +   inject_primid(stage, header, 1);
> +   stage->next->point(stage->next, header);
> +}
> +
> +static void
> +ia_line(struct draw_stage *stage,
> +        struct prim_header *header)
> +{
> +   inject_primid(stage, header, 2);
> +   stage->next->line(stage->next, header);
> +}
> +
> +static void
> +ia_tri(struct draw_stage *stage,
> +       struct prim_header *header)
> +{
> +   inject_primid(stage, header, 3);
> +   stage->next->tri(stage->next, header);
> +}
> +
> +static void
> +ia_first_point(struct draw_stage *stage,
> +               struct prim_header *header)
> +{
> +   struct ia_stage *ia = ia_stage(stage);
> +
> +   if (ia->primid_slot >= 0) {
> +      stage->point = ia_point;
> +   } else {
> +      stage->point = draw_pipe_passthrough_point;
> +   }
> +
> +   stage->point(stage, header);
> +}
> +
> +static void
> +ia_first_line(struct draw_stage *stage,
> +              struct prim_header *header)
> +{
> +   struct ia_stage *ia = ia_stage(stage);
> +
> +   if (ia->primid_slot >= 0) {
> +      stage->line = ia_line;
> +   } else {
> +      stage->line = draw_pipe_passthrough_line;
> +   }
> +
> +   stage->line(stage, header);
> +}
> +
> +static void
> +ia_first_tri(struct draw_stage *stage,
> +             struct prim_header *header)
> +{
> +   struct ia_stage *ia = ia_stage(stage);
> +
> +   if (ia->primid_slot >= 0) {
> +      stage->tri = ia_tri;
> +   } else {
> +      stage->tri = draw_pipe_passthrough_tri;
> +   }
> +
> +   stage->tri(stage, header);
> +}
> +
> +
> +static void
> +ia_flush(struct draw_stage *stage, unsigned flags)
> +{
> +   stage->point = ia_first_point;
> +   stage->line = ia_first_line;
> +   stage->tri = ia_first_tri;
> +   stage->next->flush(stage->next, flags);
> +}
> +
> +
> +static void
> +ia_reset_stipple_counter(struct draw_stage *stage)
> +{
> +   stage->next->reset_stipple_counter(stage->next);
> +}
> +
> +
> +static void
> +ia_destroy(struct draw_stage *stage)
> +{
> +   draw_free_temp_verts(stage);
> +   FREE(stage);
> +}
> +
> +
> +static boolean
> +needs_primid(const struct draw_context *draw)
> +{
> +   const struct draw_fragment_shader *fs = draw->fs.fragment_shader;
> +   const struct draw_geometry_shader *gs = draw->gs.geometry_shader;
> +   if (fs && fs->info.uses_primid) {
> +      return !gs || !gs->info.uses_primid;
> +   }
> +   return FALSE;
> +}
> +
> +boolean
> +draw_ia_stage_required(const struct draw_context *draw, unsigned prim)
> +{
> +   const struct draw_geometry_shader *gs = draw->gs.geometry_shader;
> +   if (needs_primid(draw)) {
> +      return TRUE;
> +   }
> +
> +   if (gs) {
> +      return FALSE;
> +   }
> +
> +   switch (prim) {
> +   case PIPE_PRIM_LINES_ADJACENCY:
> +   case PIPE_PRIM_LINE_STRIP_ADJACENCY:
> +   case PIPE_PRIM_TRIANGLES_ADJACENCY:
> +   case PIPE_PRIM_TRIANGLE_STRIP_ADJACENCY:
> +      return TRUE;
> +   default:
> +      return FALSE;
> +   }
> +}
> +
> +void
> +draw_ia_prepare_outputs(struct draw_context *draw,
> +                        struct draw_stage *stage)
> +{
> +   struct ia_stage *ia = ia_stage(stage);
> +   if (needs_primid(draw)) {
> +      ia->primid_slot = draw_alloc_extra_vertex_attrib(
> +         stage->draw, TGSI_SEMANTIC_PRIMID, 0);
> +   } else {
> +      ia->primid_slot = -1;
> +   }
> +   ia->primid = 0;
> +}
> +
> +struct draw_stage *
> +draw_ia_stage(struct draw_context *draw)
> +{
> +   struct ia_stage *ia = CALLOC_STRUCT(ia_stage);
> +   if (ia == NULL)
> +      goto fail;
> +
> +   ia->stage.draw = draw;
> +   ia->stage.name = "ia";
> +   ia->stage.next = NULL;
> +   ia->stage.point = ia_first_point;
> +   ia->stage.line = ia_first_line;
> +   ia->stage.tri = ia_first_tri;
> +   ia->stage.flush = ia_flush;
> +   ia->stage.reset_stipple_counter = ia_reset_stipple_counter;
> +   ia->stage.destroy = ia_destroy;
> +
> +   if (!draw_alloc_temp_verts(&ia->stage, 0))
> +      goto fail;
> +
> +   return &ia->stage;
> +
> +fail:
> +   if (ia)
> +      ia->stage.destroy(&ia->stage);
> +
> +   return NULL;
> +}
> diff --git a/src/gallium/auxiliary/draw/draw_pipe_validate.c b/src/gallium/auxiliary/draw/draw_pipe_validate.c
> index 2dbb95c..1ce89eb 100644
> --- a/src/gallium/auxiliary/draw/draw_pipe_validate.c
> +++ b/src/gallium/auxiliary/draw/draw_pipe_validate.c
> @@ -53,6 +53,7 @@ static boolean triangles( unsigned prim )
>      return prim >= PIPE_PRIM_TRIANGLES;
>   }
>
> +
>   /**
>    * Default version of a function to check if we need any special
>    * pipeline stages, or whether prims/verts can go through untouched.
> @@ -76,6 +77,13 @@ draw_need_pipeline(const struct draw_context *draw,
>                                             prim );
>      }
>
> +   /* If we need to decompose the primitives or inject
> +    * primitive id information then we have to run
> +    * the pipeline */

Closing */ on next line.

> +   if (draw_ia_stage_required(draw, prim)) {
> +      return TRUE;
> +   }
> +
>      /* Don't have to worry about triangles turning into lines/points
>       * and triggering the pipeline, because we have to trigger the
>       * pipeline *anyway* if unfilled mode is active.
> @@ -280,7 +288,12 @@ static struct draw_stage *validate_pipeline( struct draw_stage *stage )
>         next = draw->pipeline.clip;
>      }
>
> -
> +   /* Input assembler */
> +   if (draw_ia_stage_required(draw, draw->pt.prim)) {
> +      draw->pipeline.ia->next = next;
> +      next = draw->pipeline.ia;
> +   }
> +
>      draw->pipeline.first = next;
>
>      if (0) {

For the whole series:
Reviewed-by: Brian Paul <brianp at vmware.com>




More information about the mesa-dev mailing list