[Mesa-dev] [PATCH] gallium: do not automatically enable clip planes when a vertex shader writes to gl_ClipDistance

[Roland Scheidegger]

Am 27.09.2017 um 09:13 schrieb Olivier Lauffenburger:
> Software rasterizer and LLVM contain code to enable clipping as soon as
> a vertex shader writes to gl_ClipDistance, even if the corresponding
> clip planes are disabled.
> GLSL specification states that "Values written into gl_ClipDistance for
> planes that are not enabled have no effect."
> The actual behavior is thus non-conformant.
> 
> This patch removes the code that automagically enables user clipping
> planes even if they are disabled.
> 
> Signed-off-by: Olivier Lauffenburger <o.lauffenburger at topsolid.com>

FWIW that code is there because you can't disable clip distances with
d3d10 - if you write them in the shader, they're enabled (d3d9 didn't
have clip distances, just old user clip planes, which of course have
enable bits). They are very similar to cull distances there (which you
can't disable with gl neither).
I suppose we cheated there a bit... I might even have realized it wasn't
quite GL conformant when we did this, but it didn't cause piglit
regressions then (I guess it's very rare a shader actually declares clip
distance outputs but does not enable them).
This was introduced back in June 2013:
https://lists.freedesktop.org/archives/mesa-dev/2013-June/040559.html

So with this removed, I suppose we need to add a workaround in our code
(which is indeed rather unfortunate). But I don't see another
(reasonable) way to make it gl conformant.
If however there's still no piglit test exercising this, there should be
one.

> ---
>  src/gallium/auxiliary/draw/draw_cliptest_tmp.h | 7 -------
>  src/gallium/auxiliary/draw/draw_llvm.c         | 5 -----
>  2 files changed, 12 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/draw/draw_cliptest_tmp.h b/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
> index dd57c5678e..129ee7230d 100644
> --- a/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
> +++ b/src/gallium/auxiliary/draw/draw_cliptest_tmp.h
> @@ -57,13 +57,6 @@ static boolean TAG(do_cliptest)( struct pt_post_vs *pvs,
>     if (cd[0] != pos || cd[1] != pos)
>        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)) {
> -      flags |= DO_CLIP_USER;
> -      ucp_enable = (1 << num_written_clipdistance) - 1;
> -   }
> -
>     assert(pos != -1);
>     for (j = 0; j < info->count; j++) {
>        float *position = out->data[pos];
> diff --git a/src/gallium/auxiliary/draw/draw_llvm.c b/src/gallium/auxiliary/draw/draw_llvm.c
> index 203572010f..80b1b48f21 100644
> --- a/src/gallium/auxiliary/draw/draw_llvm.c
> +++ b/src/gallium/auxiliary/draw/draw_llvm.c
> @@ -1162,11 +1162,6 @@ generate_clipmask(struct draw_llvm *llvm,
>     if (cd[0] != pos || cd[1] != pos)
>        have_cd = true;
>  
> -   if (num_written_clipdistance && !clip_user) {
> -      clip_user = true;
> -      ucp_enable = (1 << num_written_clipdistance) - 1;
> -   }
> -
>     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 */
>