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/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?
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.
