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

[Jose Fonseca]

On 27/09/17 15:07, Roland Scheidegger wrote:
> 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.

I'm still not following.  Are we talking about 
pipe_rasterizer_state::clip_plane_enable controlling TGSI_SEMANTIC_CLIPDIST?

I thought these have nothing to do with one another. 
pipe_rasterizer_state::clip_plane_enable should control 
legacy/fixed-fuction user clip planes.

If the OpenGL state tracker needs to translate GLSL shaders that write 
gl_ClipDistance but where the clip plane is disabled, the solution is 
simple: just create a shader variant that omits the 
TGSI_SEMANTIC_CLIPDIST in question, or writes an constant to it.

Like it was mentioned, it should be extremely rare for apps to emit 
gl_ClipDistance yet disable the clip planes, hence the shader variants 
should be extremely rare.

It's not just D3D10/11 that doesn't have flags to disable clip 
distances: I can't find them in Vulkan spec neither.  And while I know 
few/none want to build Vulkan drivers atop Gallium interface I think 
it's still useful to decide what deserves to be in Gallium interface or not.

So in short, llvmpipe is fine, please let's fix the state tracker instead.

This way one day we can just get rid of UCP altogether.

Jose