[Mesa-dev] [PATCH 2/2] i965 Gen6: Implement gl_ClipVertex.

[Paul Berry]

On 3 October 2011 17:46, Ian Romanick <idr at freedesktop.org> wrote:

> On 09/30/2011 01:09 AM, Paul Berry wrote:
>
>  My intention was never to give up support for fixed function clipping.
>>  I just don't know how to tell, from within the vertex shader backend,
>> whether the shader we're compiling is an application-defined GLSL shader
>> or Mesa's built-in fixed function vertex shader.  Since at the moment we
>> use the old VS backend for fixed function, and the new VS backend for
>> application-defined GLSL shaders, I figured I could dodge the question
>> for now by putting the fixed-function logic in the old VS backend and
>> the non-fixed-function logic in the new VS backend.  Unfortunately your
>> eyes were too sharp for me to get away with that dodge :)
>>
>>
>>    Couldn't you just do:
>>    const bool clip_vertex = c->prog_data.outputs_written &
>>    BITFIELD64_BIT(VERT_RESULT_**CLIP_VERTEX);
>>
>>    c->prog_data.param[this->**uniforms * 4 + j] =
>>       clip_vertex ? ctx->Transform.EyeUserPlane[i]**[j]
>>                   : ctx->Transform._ClipUserPlane[**i][j];
>>
>>    ...or is outputs_written not available at this point in time?
>>
>
> What follows is a really good summary of the reasons gl_ClipVertex is a
> horrible idea that needed to die in a fire. :)
>
> ARB_vertex_program works around most of this disaster by only allowing user
> clip planes with "position invariant" vertex shaders (i.e., use
> fixed-function to do the transformation).
>
> NV_vertex_program works around it by adding the o[HPOS] output, which is
> analogous to gl_ClipVertex.
>
> OpenGL ES 2.0 works around all of this by removing user clip planes
> altogether.
>
>
>  Yes, outputs_written is available at this point in time.  But I'm not
>> certain whether this code would be correct.  The question hinges on how
>> we interpret a subtle ambiguity in the GLSL 1.30 spec: what happens in
>> the case where clipping is in use, but the application-supplied vertex
>> shader doesn't write to either gl_ClipVertex or gl_ClipDistance?
>>  Accompany me, if you dare, into ambiguous spec land:
>>
>> GL 2.1, GL 3.0, GLSL 1.10, and GLSL 1.20 all say that the behavior is
>> undefined if the vertex shader writes to neither gl_ClipVertex nor
>> gl_ClipDistance.  But GLSL 1.30 says this: "If a linked set of shaders
>> forming the vertex stage contains no static write to gl_ClipVertex or
>> gl_ClipDistance, but the application has requested clipping against user
>> clip planes through the API, then the coordinate written to gl_Position
>> is used for comparison against the user clip planes."  The subtle
>>
>
> By application of the "odd man out" rule, the GLSL 1.30 spec is wrong and,
> as you have found, unimplementable.  Page 69 (page 85 of the PDF) of the
> OpenGL 3.0 spec says:
>
>    "If gl_ClipVertex is not written by the vertex shader, its value
>    is undefined, which implies that the results of clipping to any
>    client-defined clip planes are also undefined."
>
> In my book, that trumps whatever the GLSL spec says.
>
>
>  ambiguity is: when using gl_Position for comparison against the user
>> clip planes, should we transform it from clip coordinates to eye
>> coordinates before comparing it with the user clip planes?  Or
>> equivalently, should we transform the user clip planes from eye
>> coordinates to clip coordinates before comparing them with gl_Position?
>>  (The second, equivalent form of the question is the form that is
>> relevant to Mesa; our answer determines whether we should upload
>> ctx->Transform.EyeUserPlane or ctx->Transform._ClipUserPlane)**.
>>
>> If the answer is "yes, the coordinates should be transformed", then we
>> should use ctx->Transform.EyeUserPlane when the shader writes to
>> gl_ClipVertex and ctx->Transform._ClipUserPlane when it doesn't.  In
>> that case your suggestion would work fine, and the code I submitted is
>> wrong.  But if the answer is "no, the coordinates should not be
>> transformed", then we need to use ctx->Transform.EyeUserPlane for
>> application-provided vertex shaders, and ctx->Transform._ClipUserPlane
>> for Mesa's built-in fixed function vertex shader.
>>
>> IMHO, the correct answer is "no, the coordinates should not be
>> transformed".  I'm basing this on discussions I had with Chad last
>> Wednesday while I was writing the clip-plane-transformation Piglit test.
>>  But I'm by no means certain.  Here are the arguments I can think of
>> both for and against doing the coordinate transformation:
>>
>> Argument against: If the spec writers intended for a coordinate
>> transformation to be used when using gl_Position to substitute for
>> gl_ClipVertex, surely they would have specifically said this in the
>> spec.  They didn't.
>>
>> Argument against: GL 2.1 and GL 3.0 say "The user must ensure that the
>> clip vertex and client-defined clip planes are defined in the same
>> coordinate space."  This seems to heavily imply that there is no
>> preferred coordinate space for gl_ClipVertex; the application may use
>> whatever coordinate space it desires, provided that it specifies user
>> clip planes in the same coordinate space.  So who is to say that we
>> should transform from clip coordinates to eye coordinates when using
>> gl_Position to substitute for gl_ClipVertex?  Since the specs never
>> explicitly say what coordinate space gl_ClipVertex is intended to be in,
>> the only sensible interpretation is to do no coordinate transformation
>> at all.
>>
>> Argument for: In the description of ClipPlane() in GL 2.1 and GL 3.0, it
>> is clear that the clip planes specified by the application are meant to
>> be translated from object coordinates to eye coordinates at the time the
>> clip planes are specified (this must be the case, because the spec says
>> we use the model-view matrix to do this transformation).  Since
>> gl_ClipVertex's sole purpose is to be used for comparison against user
>> clip planes, and the user clip planes are clearly meant to be stored in
>> eye coordinates, gl_ClipVertex is clearly meant to be in eye
>> coordinates.  Therefore when using gl_Position to substitute for
>> gl_ClipVertex, we should transform from clip coordinates to eye
>> coordinates.
>>
>> Argument for: The shader "void main() { gl_Position =
>> gl_ModelViewProjectionMatrix * gl_Vertex; }" ought to behave as much
>> like fixed functionality as possible.  Since clip planes are interpreted
>> in eye coordinates in fixed functionality, when using gl_Position to
>> substitute for gl_ClipVertex, we really ought to transform from clip
>> coordinates to eye coordinates.
>>
>
> As far as the OpenGL spec is concerned, there is no such thing as Mesa's
> _ClipUserPlane.  It's just an optimization used by Mesa to avoid explicitly
> having the equivalent of gl_Position and gl_ClipVertex in the fixed-function
> pipeline.  Outside that one use, you can pretend that it isn't there at all.
>  If the GLSL 1.30 spec is correct, and that's a big "if," then we'd have to
> use EyeUserPlane.
>
>
>  Incidentally, I was hoping I could resolve this question by trying some
>> experiments on my nVidia-based system at home, and seeing what it did.
>>  Unfortunately, what it does if you don't write to gl_ClipVertex is:
>> fail to clip at all.  So if it's any consolation, no matter what we do
>> we'll be in better conformance with the specs than nVidia :)
>>
>
> Given what the GL spec says, I think that's valid.  Unless AMD, Apple, and
> Intel's Windows drivers all do some other identical behavior, I'd support
> doing this as well.
>

Can I assume that you would also be ok with some other behavior, since in
your book, the GLSL 1.30 spec is the odd man out and therefore the behavior
is undefined?  The reason I ask is because the patches I've already
submitted to the list (and which have already been positively reviewed)
implement a different behavior than nVidia's drivers, and in fact
considering the way we do clipping in Mesa it would actually be extra work
to replicate nVidia's "fail to clip at all" behavior.  If it's all the same
to you I'd rather just go ahead with the behavior that I've implemented
rather than go to extra work to be like nVidia's drivers for this silly
corner case.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20111003/a8d055da/attachment-0001.htm>