Am 28.09.2017 um 16:12 schrieb 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.lauffenbur...@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/04055
9.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?
Yes.
I thought these have nothing to do with one another.
pipe_rasterizer_state::clip_plane_enable should control
legacy/fixed-fuction user clip planes.
Nope. Every single driver (except those using draw) assumes this
also enables clip dists - this includes svga which translates
those away in the shader which aren't enabled.
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.
Well, it is easier to have an extra enable and having to add
additional rasterizer dependencies on state trackers which don't
have separate enable, rather than having to hack shaders with
state trackers which don't have them. Of course, svga does
exactly that but it's a bit annoying.
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.
Of course, newer apis won't have that - clearly the separate
enables just carried over from legacy GL.
So in short, llvmpipe is fine, please let's fix the state
tracker instead.
Well, we're really the only ones caring about non-gl gallium
state tracker, so I'm not sure it makes sense to impose the d3d10
semantics there. We just cheated in draw.
And actually, thinking about this, it's really not even possible
easily and cleanly doing this in the state tracker: you can pass
those clip dists to the next shader stage. If that's all
pre-fragment stage (vertex, geometry, tesselation), this is still
doable just annoying (must pass them unaltered between stages not
the last stage before rasterization), but it's impossible to pass
them to fragment stage (unless you'd use generic varying) if you
rewrite the shader to not include them (don't ask me if it works
correctly in svga, it might have code for this too).
So I believe there's really no other choice other than following
GL semantics there in gallium.
