Am 28.09.2017 um 17:53 schrieb Jose Fonseca: > On 28/09/17 16:29, Roland Scheidegger wrote: >> 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. > > Ok.. > > So when implementing D3D10 we just need to ensure clip enable is always > set to all ones? I don't think that will work - the (gl and presumably gallium) semantics are that enabled clip planes must be written or the results are undefined. I suppose we could cheat there too (in draw) and take the insersection of enabled and written clip dists, but otherwise we'd have to enable only these clip planes a shader actually writes...
Roland _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
