[Intel-gfx] [Mesa-dev] [PATCH 1/5] intel gen4/5: fix GL_VERTEX_PROGRAM_TWO_SIDE.

[Paul Berry]

On 17 July 2012 01:57, Olivier Galibert <galibert at pobox.com> wrote:

> On Mon, Jul 16, 2012 at 08:43:17PM -0700, Paul Berry wrote:
> > Also, I'm not convinced that #3 is necessary.  Is there something in the
> > spec that dictates this behaviour?  My reading of the spec is that if the
> > vertex shader writes to gl_BackColor but not glFrontColor, then the
> > contents of gl_Color in the fragment shader is undefined.
>
> Oh, I remember why I did that in the first place.  All the front/back
> piglit tests only write the appropriate color slot and not the other
> one.
>
> The language is annoying:
>   The following built-in vertex output variables are available, but
> deprecated. A particular one should be
>   written to if any functionality in a corresponding fragment shader or
> fixed pipeline uses it or state derived
>   from it. Otherwise, behavior is undefined.
>   out vec4 gl_FrontColor;
>   out vec4 gl_BackColor;
>   out vec4 gl_FrontSecondaryColor;
>   out vec4 gl_BackSecondaryColor;
>   [...]
>
> One could argue that you don't "use" gl_FrontColor if all your
> polygons are back-facing.  Dunno.  Do you consider all of the twoside
> piglit tests buggy?  We can fix *that*.
>
>   OG.
>
>
You have a good point about the piglit tests.  It's weird that they don't
set gl_FrontColor, but I can't bring myself to consider it a bug.  Sigh, I
was really hoping we could consider it undefined to write to gl_BackColor
and not gl_FrontColor, but I guess it's reasonable for someone to expect
that if they only write to gl_BackColor, and they only render backfacing
polygons, they should get a sensible value in the fragment shader.

If possible, I would still like to think of a way to address this situation
that (a) doesn't require modifying both fragment shader back-ends and the
SF program, and (b) helps all Mesa drivers, not just Intel Gen4-5.
Especially because I suspect we may have bugs in Gen6-7 related to this
situation.  Would you be happy with one of the following two alternatives?

1. In the GLSL front-end, if we detect that a vertex shader writes to
gl_BackColor but not gl_FrontColor, then automatically insert
"gl_FrontColor = 0;" into the shader.  This will guarantee that whenever
gl_BackColor is written, gl_FrontColor is too.

2. In the function brw_compute_vue_map(), assign a VUE slot for
VERT_RESULT_COL0 whenever *either* VERT_RESULT_COL0 or VERT_RESULT_BFC0 is
used.  This will guarantee that we always have a VUE slot available for
front color, so we don't have to be as tricky in the FS and SF code.

Note that alternative 2 leaves the contents of gl_FrontColor undefined if
the vertex shader doesn't write to it.  Although I appreciate what you're
saying about security leaks due to reads out of place, I think that in this
case the security risk is negligible, since the garbage value that winds up
in gl_FrontColor will just be a value out of some random register in the
vertex shader, not a random value out of memory and not a value belonging
to some other process.

This morning I'll try to ask some other Intel folks for their opinion on
the subject.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20120717/ee05aa06/attachment.html>