[Mesa-dev] [PATCH 2/5] draw: enable user plane clipping when clipdistance is used

[Brian Paul]

On 06/10/2013 08:31 AM, Zack Rusin wrote:
> Draw depended on clip_plane_enable being set in the rasterizer
> to use clipdistance registers for clipping. That's really
> unfriendly because it requires that rasterizer state to have
> variants for every shader out there. Instead of depending on
> the rasterizer lets extract the info from the available state:
> if a shader writes clipdistance then we need to use it and we
> need to clip using a number of planes equal to the number
> of writen clipdistance components. This way clipdistances
> just work.

Is this a gallium interface policy that should be documented?


>
> Signed-off-by: Zack Rusin <zackr at vmware.com>
> ---
>   src/gallium/auxiliary/draw/draw_cliptest_tmp.h |   21 ++++++++++++++++-----
>   src/gallium/auxiliary/draw/draw_llvm.c         |    8 ++++++++
>   2 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/src/gallium/auxiliary/draw/draw_cliptest_tmp.h b/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
> index 7a385c8..6736dba 100644
> --- a/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
> +++ b/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
> @@ -35,8 +35,8 @@ static boolean TAG(do_cliptest)( struct pt_post_vs *pvs,
>      const unsigned cv = draw_current_shader_clipvertex_output(pvs->draw);
>      unsigned cd[2];
>      const unsigned ef = pvs->draw->vs.edgeflag_output;
> -   const unsigned ucp_enable = pvs->draw->rasterizer->clip_plane_enable;
> -   const unsigned flags = (FLAGS);
> +   unsigned ucp_enable = pvs->draw->rasterizer->clip_plane_enable;
> +   unsigned flags = (FLAGS);
>      unsigned need_pipeline = 0;
>      unsigned j;
>      unsigned i;
> @@ -46,12 +46,24 @@ static boolean TAG(do_cliptest)( struct pt_post_vs *pvs,
>      int viewport_index =
>         draw_current_shader_uses_viewport_index(pvs->draw) ?
>         *((unsigned*)out->data[viewport_index_output]): 0;
> +   int num_written_clipdistance =
> +      draw_current_shader_num_written_clipdistances(pvs->draw);
>
>      cd[0] = draw_current_shader_clipdistance_output(pvs->draw, 0);
>      cd[1] = draw_current_shader_clipdistance_output(pvs->draw, 1);
>
>      if (cd[0] != pos || cd[1] != pos)
> -     have_cd = true;
> +      have_cd = true;
> +
> +   /* If clipdistance semantic has been written by the shader
> +    * that means we're expected to do 'user plane clipping' */
> +   if (num_written_clipdistance && !(flags & DO_CLIP_USER)) {
> +      unsigned i;
> +      flags |= DO_CLIP_USER;
> +      ucp_enable = 0;
> +      for (i = 0; i < num_written_clipdistance; ++i)
> +         ucp_enable |= 1 << i;
> +   }
>
>      for (j = 0; j < info->count; j++) {
>         float *position = out->data[pos];
> @@ -111,8 +123,7 @@ static boolean TAG(do_cliptest)( struct pt_post_vs *pvs,
>
>            if (flags & DO_CLIP_USER) {
>               unsigned ucp_mask = ucp_enable;
> -            int num_written_clipdistance =
> -               draw_current_shader_num_written_clipdistances(pvs->draw);
> +
>               while (ucp_mask) {
>                  unsigned plane_idx = ffs(ucp_mask)-1;
>                  ucp_mask &= ~(1 << plane_idx);
> diff --git a/src/gallium/auxiliary/draw/draw_llvm.c b/src/gallium/auxiliary/draw/draw_llvm.c
> index 4a71955..82f4c20 100644
> --- a/src/gallium/auxiliary/draw/draw_llvm.c
> +++ b/src/gallium/auxiliary/draw/draw_llvm.c
> @@ -1146,6 +1146,14 @@ generate_clipmask(struct draw_llvm *llvm,
>      if (cd[0] != pos || cd[1] != pos)
>         have_cd = true;
>
> +   if (num_written_clipdistance && !clip_user) {
> +      unsigned i;
> +      clip_user = true;
> +      ucp_enable = 0;
> +      for (i = 0; i < num_written_clipdistance; ++i)
> +         ucp_enable |= 1 << i;

I think this can be done without a loop:

   ucp_enable = (1 << num_written_clipdistance) - 1;

Same thing earlier in the code.


> +   }
> +
>      mask = lp_build_const_int_vec(gallivm, i32_type, 0);
>      temp = lp_build_const_int_vec(gallivm, i32_type, 0);
>      zero = lp_build_const_vec(gallivm, f32_type, 0);         /* 0.0f 0.0f 0.0f 0.0f */
>

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