[Mesa-dev] [PATCH] draw: clean up d3d style point clipping

Jose Fonseca jfonseca at vmware.com
Mon Jan 20 07:27:31 PST 2014


Looks good to me AFAICT.

Jose

----- Original Message -----
> From: Roland Scheidegger <sroland at vmware.com>
> 
> Instead of skipping x/y clipping completely if there's point_tri_clip points
> use guard band clipping. This should be easier (previously we could not
> disable
> generating the x/y bits in the clip mask for llvm path, hence requiring
> custom
> clip path), and it also allows us to enable this for tris-as-points more
> easily
> too (this would require custom tri clip filtering too otherwise). Moreover,
> some unexpected things could have happen if there's a NaN or just a huge
> number
> in some tri-turned-point, as the driver's rasterizer would need to deal with
> it
> and that might well lead to undefined behavior in typical rasterizers (which
> need to convert these numbers to fixed point). Using a guardband should hence
> be more robust, while "usually" guaranteeing the same results. (Only
> "usually"
> because unlike hw guardbands draw guardband is always just twice the vp size,
> hence small vp but large points could still lead to different results.)
> Unfortunately because the clipmask generated is completely unaffected by
> guard
> band clipping, we still need a custom clip stage for points (but not for
> tris,
> as the actual clipping there takes guard band into account).
> ---
>  src/gallium/auxiliary/draw/draw_context.c          |    8 ++---
>  src/gallium/auxiliary/draw/draw_pipe_clip.c        |   34
>  ++++++++++++++------
>  src/gallium/auxiliary/draw/draw_private.h          |    2 +-
>  .../auxiliary/draw/draw_pt_fetch_shade_pipeline.c  |   15 +++++----
>  .../draw/draw_pt_fetch_shade_pipeline_llvm.c       |    8 +++--
>  5 files changed, 42 insertions(+), 25 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/draw/draw_context.c
> b/src/gallium/auxiliary/draw/draw_context.c
> index 9b5bcb5..842fdd3 100644
> --- a/src/gallium/auxiliary/draw/draw_context.c
> +++ b/src/gallium/auxiliary/draw/draw_context.c
> @@ -262,10 +262,10 @@ static void update_clip_flags( struct draw_context
> *draw )
>                     draw->rasterizer && draw->rasterizer->depth_clip);
>     draw->clip_user = draw->rasterizer &&
>                       draw->rasterizer->clip_plane_enable != 0;
> -   draw->clip_points_xy = draw->clip_xy &&
> -                          (!draw->driver.bypass_clip_points ||
> -                          (draw->rasterizer &&
> -                          !draw->rasterizer->point_tri_clip));
> +   draw->guard_band_points_xy = draw->guard_band_xy ||
> +                                (draw->driver.bypass_clip_points &&
> +                                (draw->rasterizer &&
> +                                 draw->rasterizer->point_tri_clip));
>  }
>  
>  /**
> diff --git a/src/gallium/auxiliary/draw/draw_pipe_clip.c
> b/src/gallium/auxiliary/draw/draw_pipe_clip.c
> index adfa4b6..9b339ae 100644
> --- a/src/gallium/auxiliary/draw/draw_pipe_clip.c
> +++ b/src/gallium/auxiliary/draw/draw_pipe_clip.c
> @@ -615,19 +615,33 @@ clip_point( struct draw_stage *stage,
>        stage->next->point( stage->next, header );
>  }
>  
> +
>  /*
>   * Clip points but ignore the first 4 (xy) clip planes.
> - * (This is necessary because we don't generate a different shader variant
> - * just for points hence xy clip bits are still generated. This is not
> really
> - * optimal because of the extra calculations both in generating clip masks
> - * and executing the clip stage but it gets the job done.)
> + * (Because the generated clip mask is completely unaffacted by guard band,
> + * we still need to manually evaluate the x/y planes if they are outside
> + * the guard band and not just outside the vp.)
>   */
>  static void
> -clip_point_no_xy( struct draw_stage *stage,
> -                  struct prim_header *header )
> +clip_point_guard_xy( struct draw_stage *stage,
> +                     struct prim_header *header )
>  {
> -   if ((header->v[0]->clipmask & 0xfffffff0) == 0)
> -      stage->next->point( stage->next, header );
> +   unsigned clipmask = header->v[0]->clipmask;
> +   if ((clipmask & 0xffffffff) == 0)
> +      stage->next->point(stage->next, header);
> +   else if ((clipmask & 0xfffffff0) == 0) {
> +      while (clipmask) {
> +         const unsigned plane_idx = ffs(clipmask)-1;
> +         clipmask &= ~(1 << plane_idx);  /* turn off this plane's bit */
> +         /* TODO: this should really do proper guardband clipping,
> +          * currently just throw out infs/nans.
> +          */
> +         if (util_is_inf_or_nan(header->v[0]->clip[0]) ||
> +             util_is_inf_or_nan(header->v[0]->clip[1]))
> +            return;
> +      }
> +      stage->next->point(stage->next, header);
> +   }
>  }
>  
>  
> @@ -636,7 +650,7 @@ static void
>  clip_first_point( struct draw_stage *stage,
>                    struct prim_header *header )
>  {
> -   stage->point = stage->draw->clip_points_xy ? clip_point :
> clip_point_no_xy;
> +   stage->point = stage->draw->guard_band_points_xy ? clip_point_guard_xy :
> clip_point;
>     stage->point(stage, header);
>  }
>  
> @@ -662,7 +676,7 @@ clip_line( struct draw_stage *stage,
>  
>  static void
>  clip_tri( struct draw_stage *stage,
> -	  struct prim_header *header )
> +          struct prim_header *header )
>  {
>     unsigned clipmask = (header->v[0]->clipmask |
>                          header->v[1]->clipmask |
> diff --git a/src/gallium/auxiliary/draw/draw_private.h
> b/src/gallium/auxiliary/draw/draw_private.h
> index 5bcb8a8..7f9b26c 100644
> --- a/src/gallium/auxiliary/draw/draw_private.h
> +++ b/src/gallium/auxiliary/draw/draw_private.h
> @@ -232,7 +232,7 @@ struct draw_context
>     boolean clip_z;
>     boolean clip_user;
>     boolean guard_band_xy;
> -   boolean clip_points_xy;
> +   boolean guard_band_points_xy;
>  
>     boolean force_passthrough; /**< never clip or shade */
>  
> 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 ab8a0c6..67c0c16 100644
> --- a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c
> +++ b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline.c
> @@ -60,7 +60,7 @@ struct fetch_pipeline_middle_end {
>   */
>  static void fetch_pipeline_prepare( struct draw_pt_middle_end *middle,
>                                      unsigned prim,
> -				    unsigned opt,
> +                                    unsigned opt,
>                                      unsigned *max_vertices )
>  {
>     struct fetch_pipeline_middle_end *fpme = (struct
>     fetch_pipeline_middle_end *)middle;
> @@ -72,8 +72,10 @@ static void fetch_pipeline_prepare( struct
> draw_pt_middle_end *middle,
>  
>     const unsigned gs_out_prim = (gs ? gs->output_primitive :
>                                   u_assembled_prim(prim));
> -   unsigned nr = MAX2( vs->info.num_inputs,
> -		       draw_total_vs_outputs(draw) );
> +   unsigned nr = MAX2(vs->info.num_inputs,
> +                      draw_total_vs_outputs(draw));
> +   unsigned point_clip = draw->rasterizer->fill_front ==
> PIPE_POLYGON_MODE_POINT ||
> +                         gs_out_prim == PIPE_PRIM_POINTS;
>  
>     if (gs) {
>        nr = MAX2(nr, gs->info.num_outputs + 1);
> @@ -97,18 +99,17 @@ static void fetch_pipeline_prepare( struct
> draw_pt_middle_end *middle,
>      */
>     fpme->vertex_size = sizeof(struct vertex_header) + nr * 4 *
>     sizeof(float);
>  
> -
>  
>     draw_pt_fetch_prepare( fpme->fetch,
>                            vs->info.num_inputs,
>                            fpme->vertex_size,
>                            instance_id_index );
>     draw_pt_post_vs_prepare( fpme->post_vs,
> -                            gs_out_prim == PIPE_PRIM_POINTS ?
> -                                           draw->clip_points_xy :
> draw->clip_xy,
> +                            draw->clip_xy,
>                              draw->clip_z,
>                              draw->clip_user,
> -                            draw->guard_band_xy,
> +                            point_clip ? draw->guard_band_points_xy :
> +                                         draw->guard_band_xy,
>                              draw->identity_viewport,
>                              draw->rasterizer->clip_halfz,
>                              (draw->vs.edgeflag_output ? TRUE : FALSE) );
> 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 bca658f..a1a99dc 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
> @@ -143,6 +143,8 @@ llvm_middle_end_prepare( struct draw_pt_middle_end
> *middle,
>        u_assembled_prim(in_prim);
>     const unsigned nr = MAX2(vs->info.num_inputs,
>                              draw_total_vs_outputs(draw));
> +   unsigned point_clip = draw->rasterizer->fill_front ==
> PIPE_POLYGON_MODE_POINT ||
> +                         out_prim == PIPE_PRIM_POINTS;
>  
>     fpme->input_prim = in_prim;
>     fpme->opt = opt;
> @@ -155,11 +157,11 @@ llvm_middle_end_prepare( struct draw_pt_middle_end
> *middle,
>  
>  
>     draw_pt_post_vs_prepare( fpme->post_vs,
> -                            out_prim == PIPE_PRIM_POINTS ?
> -                                        draw->clip_points_xy :
> draw->clip_xy,
> +                            draw->clip_xy,
>                              draw->clip_z,
>                              draw->clip_user,
> -                            draw->guard_band_xy,
> +                            point_clip ? draw->guard_band_points_xy :
> +                                         draw->guard_band_xy,
>                              draw->identity_viewport,
>                              draw->rasterizer->clip_halfz,
>                              (draw->vs.edgeflag_output ? TRUE : FALSE) );
> --
> 1.7.9.5
> 


More information about the mesa-dev mailing list