[Mesa-dev] [PATCH] glsl/gs: Fix transform feedback of gl_ClipDistance.

[Paul Berry]

On 24 October 2013 00:13, Pohjolainen, Topi <topi.pohjolainen at intel.com>wrote:

> On Wed, Oct 23, 2013 at 01:08:42PM -0700, Paul Berry wrote:
> > Since gl_ClipDistance is lowered from an array of floats to an array
> > of vec4's during compilation, transform feedback has special logic to
> > keep track of the pre-lowered array size so that attempting to perform
> > transform feedback on gl_ClipDistance produces a result with the
> > correct size.
> >
> > Previously, this special logic always consulted the vertex shader's
> > size for gl_ClipDistance.  This patch fixes it so that it uses the
> > geometry shader's size for gl_ClipDistance when a geometry shader is
> > in use.
> >
> > Fixes piglit test spec/glsl-1.50/transform-feedback-type-and-size.
> > ---
> >  src/glsl/link_varyings.cpp | 2 +-
> >  src/glsl/linker.cpp        | 2 ++
> >  src/mesa/main/mtypes.h     | 6 ++++++
> >  3 files changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
> > index 4ba6d8a..c503645 100644
> > --- a/src/glsl/link_varyings.cpp
> > +++ b/src/glsl/link_varyings.cpp
> > @@ -328,7 +328,7 @@ tfeedback_decl::assign_location(struct gl_context
> *ctx,
> >        const unsigned vector_elements =
> >           this->matched_candidate->type->fields.array->vector_elements;
> >        unsigned actual_array_size = this->is_clip_distance_mesa ?
> > -         prog->Vert.ClipDistanceArraySize :
> > +         prog->LastClipDistanceArraySize :
> >           this->matched_candidate->type->array_size();
> >
> >        if (this->is_subscripted) {
> > diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> > index b23c31a..d8f655c 100644
> > --- a/src/glsl/linker.cpp
> > +++ b/src/glsl/linker.cpp
> > @@ -2100,6 +2100,7 @@ link_shaders(struct gl_context *ctx, struct
> gl_shader_program *prog)
> >        validate_vertex_shader_executable(prog, sh);
> >        if (!prog->LinkStatus)
> >        goto done;
> > +      prog->LastClipDistanceArraySize =
> prog->Vert.ClipDistanceArraySize;
> >
> >        _mesa_reference_shader(ctx,
> &prog->_LinkedShaders[MESA_SHADER_VERTEX],
> >                            sh);
> > @@ -2132,6 +2133,7 @@ link_shaders(struct gl_context *ctx, struct
> gl_shader_program *prog)
> >        validate_geometry_shader_executable(prog, sh);
> >        if (!prog->LinkStatus)
> >        goto done;
> > +      prog->LastClipDistanceArraySize =
> prog->Geom.ClipDistanceArraySize;
> >
> >        _mesa_reference_shader(ctx,
> &prog->_LinkedShaders[MESA_SHADER_GEOMETRY],
> >                            sh);
> > diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> > index 6374e8c..bc7dea4 100644
> > --- a/src/mesa/main/mtypes.h
> > +++ b/src/mesa/main/mtypes.h
> > @@ -2476,6 +2476,12 @@ struct gl_shader_program
> >     unsigned NumUserUniformStorage;
> >     struct gl_uniform_storage *UniformStorage;
> >
> > +   /**
> > +    * Size of the gl_ClipDistance array that is output from the last
> pipeline
> > +    * stage before the geometry shader.
>
> Can you explain the "before the geometry shader" part? This is used by the
> transform feedback mechanism and hence represents the size of the varying
> output
> by the geometry shader (if present of course), right? This comment in turn
> refers to the varying output by vertex shader and _read_ by geometry
> shader.
>

Oops.  I meant to say "...output from the last pipeline stage before the
fragment shader".  Is that clearer?

In other words, for now, it's the geometry shader (if present), and the
vertex shader otherwise.  But in the future, when we add tessellation
shaders, it'll be:

- the size of gl_ClipDistance output by the geometry shader, if present.
- Otherwise, the size of gl_ClipDistance output by the tessellation
evaluation shader, if present.
- Otherwise, the size of gl_ClipDistance output by the tessellation control
shader, if present.
- Otherwise, the size of gl_ClipDistance output by the vertex shader.


>
> > +    */
> > +   GLuint LastClipDistanceArraySize;
> > +
> >     struct gl_uniform_block *UniformBlocks;
> >     unsigned NumUniformBlocks;
> >
> > --
> > 1.8.4.1
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131024/baedb31d/attachment.html>