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

[Roland Scheidegger]

Am 10.06.2013 16:31, schrieb Zack Rusin:
> 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
written

> just work.
> 
> 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;
> +   }
> +
>     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 */
> 

I'm not entirely sure there's no odd opengl case where this logic
wouldn't work but provided there's no piglit regressions this looks good
to me.

Roland