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

2011-10-03 Thread Ian Romanick

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 

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

2011-10-03 Thread Paul Berry
On 3 October 2011 17:46, Ian Romanick i...@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 

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

2011-09-30 Thread Kenneth Graunke
On 09/27/2011 11:05 AM, Paul Berry wrote:
 This patch implements proper support for gl_ClipVertex by causing the
 new VS backend to populate the clip distance VUE slots using
 VERT_RESULT_CLIP_VERTEX when appropriate, and by using the
 untransformed clip planes in ctx-Transform.EyeUserPlane rather than
 the transformed clip planes in ctx-Transform._ClipUserPlane.
 
 When fixed functionality is in use the driver needs to do the old
 behavior (clip based on VERT_RESULT_HPOS and
 ctx-Transform._ClipUserPlane).  This happens automatically because we
 use the old VS backend for fixed functionality.
 
 Fixes the following Piglit tests on i965 Gen6:
 - vs-clip-vertex-const-accept
 - vs-clip-vertex-const-reject
 - vs-clip-vertex-different-from-position
 - vs-clip-vertex-equal-to-position
 - vs-clip-vertex-homogeneity
 - vs-clip-based-on-position
 - vs-clip-based-on-position-homogeneity
 - clip-plane-transformation clipvert_pos
 - clip-plane-transformation pos_clipvert
 - clip-plane-transformation pos
 ---
  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |   31 ++-
  src/mesa/drivers/dri/i965/brw_vs.c |8 +-
  2 files changed, 36 insertions(+), 3 deletions(-)
 
 diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp 
 b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
 index e5eda22..f335a86 100644
 --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
 +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
 @@ -553,7 +553,16 @@ vec4_visitor::setup_uniform_clipplane_values()
   this-userplane[compacted_clipplane_index] = dst_reg(UNIFORM, 
 this-uniforms);
   this-userplane[compacted_clipplane_index].type = 
 BRW_REGISTER_TYPE_F;
   for (int j = 0; j  4; ++j) {
 -c-prog_data.param[this-uniforms * 4 + j] = 
 ctx-Transform._ClipUserPlane[i][j];
 +/* For fixed functionality shaders, we need to clip based on
 + * ctx-Transform._ClipUserPlane (which has been transformed by
 + * Mesa core into clip coordinates).  For user-supplied vertex
 + * shaders, we need to use the untransformed clip planes in
 + * ctx-Transform.EyeUserPlane.  Since vec4_visitor is currently
 + * only used for user-supplied vertex shaders, we can hardcode
 + * this to EyeUserPlane for now.
 + */
 +c-prog_data.param[this-uniforms * 4 + j]
 +   = ctx-Transform.EyeUserPlane[i][j];

So, we trade support for fixed function clipping for gl_ClipVertex
clipping?  That seems really unfortunate.  I know we don't use the new
VS backend for fixed function today, but we will.

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?

Yeah, I know it's untested, and untested code = broken code, and all
that, but...if you already know what you need to do...why not just do it?

   }
   ++compacted_clipplane_index;
   ++this-uniforms;
 @@ -1840,9 +1849,27 @@ vec4_visitor::emit_clip_distances(struct brw_reg reg, 
 int offset)
return;
 }
  
 +   /* From the GLSL 1.30 spec, section 7.1 (Vertex Shader Special Variables):
 +*
 +* 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.
 +*
 +* This function is only called if the shader didn't write to
 +* gl_ClipDistance.  Accordingly, we use gl_ClipVertex to perform clipping
 +* if the user wrote to it; otherwise we use gl_Position.
 +*/
 +   gl_vert_result clip_vertex = VERT_RESULT_CLIP_VERTEX;
 +   if (!(c-prog_data.outputs_written
 +  BITFIELD64_BIT(VERT_RESULT_CLIP_VERTEX))) {
 +  clip_vertex = VERT_RESULT_HPOS;
 +   }
 +
 for (int i = 0; i + offset  c-key.nr_userclip  i  4; ++i) {
emit(DP4(dst_reg(brw_writemask(reg, 1  i)),
 -   src_reg(output_reg[VERT_RESULT_HPOS]),
 +   src_reg(output_reg[clip_vertex]),
 src_reg(this-userplane[i + offset])));
 }
  }
 diff --git a/src/mesa/drivers/dri/i965/brw_vs.c 
 b/src/mesa/drivers/dri/i965/brw_vs.c
 index 93c6838..4fd260f 100644
 --- a/src/mesa/drivers/dri/i965/brw_vs.c
 +++ b/src/mesa/drivers/dri/i965/brw_vs.c
 @@ -137,11 +137,17 @@ brw_compute_vue_map(struct brw_vue_map *vue_map,
 /* The hardware doesn't care about the rest of the vertex outputs, so just
  * assign them contiguously.  Don't reassign outputs that already have a
  * slot.
 +*
 +* Also, don't assign a slot for 

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

2011-09-30 Thread Paul Berry
On 29 September 2011 23:16, Kenneth Graunke kenn...@whitecape.org wrote:

 On 09/27/2011 11:05 AM, Paul Berry wrote:
  This patch implements proper support for gl_ClipVertex by causing the
  new VS backend to populate the clip distance VUE slots using
  VERT_RESULT_CLIP_VERTEX when appropriate, and by using the
  untransformed clip planes in ctx-Transform.EyeUserPlane rather than
  the transformed clip planes in ctx-Transform._ClipUserPlane.
 
  When fixed functionality is in use the driver needs to do the old
  behavior (clip based on VERT_RESULT_HPOS and
  ctx-Transform._ClipUserPlane).  This happens automatically because we
  use the old VS backend for fixed functionality.
 
  Fixes the following Piglit tests on i965 Gen6:
  - vs-clip-vertex-const-accept
  - vs-clip-vertex-const-reject
  - vs-clip-vertex-different-from-position
  - vs-clip-vertex-equal-to-position
  - vs-clip-vertex-homogeneity
  - vs-clip-based-on-position
  - vs-clip-based-on-position-homogeneity
  - clip-plane-transformation clipvert_pos
  - clip-plane-transformation pos_clipvert
  - clip-plane-transformation pos
  ---
   src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |   31
 ++-
   src/mesa/drivers/dri/i965/brw_vs.c |8 +-
   2 files changed, 36 insertions(+), 3 deletions(-)
 
  diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
 b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
  index e5eda22..f335a86 100644
  --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
  +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
  @@ -553,7 +553,16 @@ vec4_visitor::setup_uniform_clipplane_values()
this-userplane[compacted_clipplane_index] = dst_reg(UNIFORM,
 this-uniforms);
this-userplane[compacted_clipplane_index].type =
 BRW_REGISTER_TYPE_F;
for (int j = 0; j  4; ++j) {
  -c-prog_data.param[this-uniforms * 4 + j] =
 ctx-Transform._ClipUserPlane[i][j];
  +/* For fixed functionality shaders, we need to clip based on
  + * ctx-Transform._ClipUserPlane (which has been transformed
 by
  + * Mesa core into clip coordinates).  For user-supplied
 vertex
  + * shaders, we need to use the untransformed clip planes in
  + * ctx-Transform.EyeUserPlane.  Since vec4_visitor is
 currently
  + * only used for user-supplied vertex shaders, we can
 hardcode
  + * this to EyeUserPlane for now.
  + */
  +c-prog_data.param[this-uniforms * 4 + j]
  +   = ctx-Transform.EyeUserPlane[i][j];

 So, we trade support for fixed function clipping for gl_ClipVertex
 clipping?  That seems really unfortunate.  I know we don't use the new
 VS backend for fixed function today, but we will.


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?


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 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 

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

2011-09-30 Thread Eric Anholt
On Fri, 30 Sep 2011 01:09:32 -0700, Paul Berry stereotype...@gmail.com wrote:
Non-text part: multipart/mixed
Non-text part: multipart/alternative
 On 29 September 2011 23:16, Kenneth Graunke kenn...@whitecape.org wrote:
 
  On 09/27/2011 11:05 AM, Paul Berry wrote:
  So, we trade support for fixed function clipping for gl_ClipVertex
  clipping?  That seems really unfortunate.  I know we don't use the new
  VS backend for fixed function today, but we will.
 
 
 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 :)

ctx-Shader.CurrentVertexProgram is the vertex shader, if enabled.

If not, ctx-VertexProgram._Enabled tells us if a vertex program is in
use, and ctx-VertexProgram.Current is that program.

Otherwise, you're in fixed function.

ctx-VertexProgram._Current points at one of those three.


pgpBmk8oC2U5K.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2011-09-30 Thread Paul Berry
On 30 September 2011 10:04, Eric Anholt e...@anholt.net wrote:

 On Fri, 30 Sep 2011 01:09:32 -0700, Paul Berry stereotype...@gmail.com
 wrote:
 Non-text part: multipart/mixed
 Non-text part: multipart/alternative
  On 29 September 2011 23:16, Kenneth Graunke kenn...@whitecape.org
 wrote:
 
   On 09/27/2011 11:05 AM, Paul Berry wrote:
   So, we trade support for fixed function clipping for gl_ClipVertex
   clipping?  That seems really unfortunate.  I know we don't use the new
   VS backend for fixed function today, but we will.
  
 
  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 :)

 ctx-Shader.CurrentVertexProgram is the vertex shader, if enabled.

 If not, ctx-VertexProgram._Enabled tells us if a vertex program is in
 use, and ctx-VertexProgram.Current is that program.

 Otherwise, you're in fixed function.

 ctx-VertexProgram._Current points at one of those three.


Ok, thanks.  I'll send out a v2 patch that is more future proof.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2011-09-27 Thread Paul Berry
This patch implements proper support for gl_ClipVertex by causing the
new VS backend to populate the clip distance VUE slots using
VERT_RESULT_CLIP_VERTEX when appropriate, and by using the
untransformed clip planes in ctx-Transform.EyeUserPlane rather than
the transformed clip planes in ctx-Transform._ClipUserPlane.

When fixed functionality is in use the driver needs to do the old
behavior (clip based on VERT_RESULT_HPOS and
ctx-Transform._ClipUserPlane).  This happens automatically because we
use the old VS backend for fixed functionality.

Fixes the following Piglit tests on i965 Gen6:
- vs-clip-vertex-const-accept
- vs-clip-vertex-const-reject
- vs-clip-vertex-different-from-position
- vs-clip-vertex-equal-to-position
- vs-clip-vertex-homogeneity
- vs-clip-based-on-position
- vs-clip-based-on-position-homogeneity
- clip-plane-transformation clipvert_pos
- clip-plane-transformation pos_clipvert
- clip-plane-transformation pos
---
 src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp |   31 ++-
 src/mesa/drivers/dri/i965/brw_vs.c |8 +-
 2 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp 
b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
index e5eda22..f335a86 100644
--- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
+++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
@@ -553,7 +553,16 @@ vec4_visitor::setup_uniform_clipplane_values()
  this-userplane[compacted_clipplane_index] = dst_reg(UNIFORM, 
this-uniforms);
  this-userplane[compacted_clipplane_index].type = BRW_REGISTER_TYPE_F;
  for (int j = 0; j  4; ++j) {
-c-prog_data.param[this-uniforms * 4 + j] = 
ctx-Transform._ClipUserPlane[i][j];
+/* For fixed functionality shaders, we need to clip based on
+ * ctx-Transform._ClipUserPlane (which has been transformed by
+ * Mesa core into clip coordinates).  For user-supplied vertex
+ * shaders, we need to use the untransformed clip planes in
+ * ctx-Transform.EyeUserPlane.  Since vec4_visitor is currently
+ * only used for user-supplied vertex shaders, we can hardcode
+ * this to EyeUserPlane for now.
+ */
+c-prog_data.param[this-uniforms * 4 + j]
+   = ctx-Transform.EyeUserPlane[i][j];
  }
  ++compacted_clipplane_index;
  ++this-uniforms;
@@ -1840,9 +1849,27 @@ vec4_visitor::emit_clip_distances(struct brw_reg reg, 
int offset)
   return;
}
 
+   /* From the GLSL 1.30 spec, section 7.1 (Vertex Shader Special Variables):
+*
+* 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.
+*
+* This function is only called if the shader didn't write to
+* gl_ClipDistance.  Accordingly, we use gl_ClipVertex to perform clipping
+* if the user wrote to it; otherwise we use gl_Position.
+*/
+   gl_vert_result clip_vertex = VERT_RESULT_CLIP_VERTEX;
+   if (!(c-prog_data.outputs_written
+  BITFIELD64_BIT(VERT_RESULT_CLIP_VERTEX))) {
+  clip_vertex = VERT_RESULT_HPOS;
+   }
+
for (int i = 0; i + offset  c-key.nr_userclip  i  4; ++i) {
   emit(DP4(dst_reg(brw_writemask(reg, 1  i)),
-   src_reg(output_reg[VERT_RESULT_HPOS]),
+   src_reg(output_reg[clip_vertex]),
src_reg(this-userplane[i + offset])));
}
 }
diff --git a/src/mesa/drivers/dri/i965/brw_vs.c 
b/src/mesa/drivers/dri/i965/brw_vs.c
index 93c6838..4fd260f 100644
--- a/src/mesa/drivers/dri/i965/brw_vs.c
+++ b/src/mesa/drivers/dri/i965/brw_vs.c
@@ -137,11 +137,17 @@ brw_compute_vue_map(struct brw_vue_map *vue_map,
/* The hardware doesn't care about the rest of the vertex outputs, so just
 * assign them contiguously.  Don't reassign outputs that already have a
 * slot.
+*
+* Also, don't assign a slot for VERT_RESULT_CLIP_VERTEX, since it is
+* unsupported in pre-GEN6, and in GEN6+ the vertex shader converts it into
+* clip distances.
 */
for (int i = 0; i  VERT_RESULT_MAX; ++i) {
   if ((outputs_written  BITFIELD64_BIT(i)) 
   vue_map-vert_result_to_slot[i] == -1) {
- assign_vue_slot(vue_map, i);
+ if (i != VERT_RESULT_CLIP_VERTEX) {
+assign_vue_slot(vue_map, i);
+ }
   }
}
 }
-- 
1.7.6.2

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev