On 07/19/2013 11:48 AM, Paul Berry wrote:
(TL;DR: geometry shaders are humming along, but because of a hitch I've
run into, I'm going to change gears and implement GLSL 1.50-style
geometry shaders first rather than ARB_geometry_shader4 functionality.
This means some piglit tests will need to be rewritten).

Hello all--

As some of you may have noticed, I've spent the last several weeks
writing a number of geometry shader tests and fixinging my "gs" branch
(on https://github.com/stereotype441/mesa.git).  I'm happy to report
that it's working better than ever: I now have the following features
working in geometry shaders on Intel drivers:

- Texturing.
- Built-in uniform variables.
- Interactions with ARB_shader_texture_lod,
ARB_shading_language_packing, ARB_texture_cube_map_array,
ARB_texture_multisample, ARB_texture_rectangle,
ARB_uniform_buffer_object, and EXT_texture_array.
- Mixing user-defined geometry shaders with fixed-function fragment shaders.
- gl_PointSizeIn, gl_PrimitiveIDIn, gl_PrimitiveID, gl_ClipVertex,
gl_ClipDistanceIn, and gl_ClipDistance (including several important
corner cases of gl_ClipDistance that were previously broken, such as
bulk assignment).
- Color clamping.

I've done all of this work using the ARB_geometry_shader4 version of
geometry shaders; my intention was to get that working first, then
introduce additional code to support GLSL 1.50-style geometry shaders.
This was partly motivated by the fact that Bryan Cain's initial work on
geometry shaders was done using ARB_geometry_shader4, and partly
motivated by the fact that it's our standard development practice in
Mesa to develop features as extensions first, and then only support a
core feature once all of the extensions it includes have been finished.
However, I've now run into hitch that's making me want to change course
and implement GLSL-1.50 style geometry shaders first.  It's a bit of a
long story so buckle up:

In GLSL 1.50, the way you specify the input primitive type for a
geometry shader is using a layout qualifier in the shader source code,
so it is known at compile time.  Whereas in ARB_geometry_shader4, the
way you specify the input primitive type is using
glProgramParameteriARB() prior to linking.  This means that the input
primitive type isn't known at compile time.

That's a problem because ARB_geometry_shader4 provides for a read-only
built-in constant called gl_VerticesIn, which is the number of vertices
in each geometry shader input primitive (e.g. if the input primitive
type is "triangles", gl_VerticesIn is 3).  Mesa does not currently
support constants whose value is determined at link time, since constant
folding, determination of explicit array sizes, and a lot of
constant-related error checking happen at compile time.  To work around
this, the "gs" branch currently treats gl_VerticesIn as a varying input
during compilation, and then at link time replaces any references to it
with the appropriate constant value.

Oh dear.  That's... awful.

There is another possible work-around for this problem, but I don't know that it's any better. gl_VerticesIn has a limited set of possible value: 1, 2, 3, 4, and 6. We could ast-to-hir for each of those values. We'd then pick the "right" one link time. There would also be some weirdness with cases were only a subset compile (e.g., because the shader does something like 'float foo[gl_VerticesIn-2];' which would fail to compile when gl_VerticesIn <= 2).

Before committing to this route we should:

1. Survey known apps (e.g., on Steam) to see if there are any apps that try to use OpenGL 3.0 + GL_ARB_geometry_shader4.

2. Determine whether there is any hardware supported by Mesa that will never get OpenGL 3.0 (due to, say, one missing feature), but might get GLSL 1.30 + GL_ARB_geometry_shader4.

Just going for 3.2 and punting on the extension for now seems like the right way to go, but I also don't want to paint ourselves into a "we have to rearchitect the world" kind of corner. :(

That in turn creates a problem because it prevents declarations like
this from working:

varying in float foo[gl_VerticesIn];

since array sizes must be integral-constant expressions.  To work around
this, the "gs" branch currently modifies the parser so that the above
declaration is treated as equivalent to an unsized array, i.e.:

varying in float foo[];

Similar changes are made for 2D inputs such as gl_TexCoordIn.

That in turn is a problem because it prevents code like this from working:

varying in float foo[gl_VerticesIn];
varying out float bar;
void main()
{
   for (int i = 0; i < gl_VerticesIn; i++) {
     bar = foo[i]; /* problem */
     EmitVertex();
   }
}

since the GLSL spec stipulates that it is only legal to index into an
unsized array using an integral constant expression.  To work around
that, this stipulation is suspended when ARB_geometry_shader4 is in use.

At this point, we have three layers of workarounds being applied because
of one crazy feature in ARB_geometry_shader4 that doesn't exist in GLSL
1.50 style geometry shaders.  Some of these workarounds have unintended
consequences that aren't easy to fix.  For example, the following shader
is accepted, even though it should be rejected:

varing in float foo[];
varying out float bar;
void main()
{
   for (int i = 0; i < gl_VerticesIn; i++) {
     bar = foo[i]; /* illegal: i must be a constant because foo is
unsized */
     EmitVertex();
   }
}

Conversely, code like this fails, even though it is allowed by
ARB_geometry_shader4:

varying in float foo[gl_VerticesIn + 1 - 1];

And so does this:

int main()
{
   float tmp[gl_VerticesIn];
}

Note that none of these problems applies to GLSL 1.50 style geometry
shaders.

I'm not comfortable with this situation.  I would far prefer to remove
the workarounds, and instead have Mesa postpone compilation of
ARB_geometry_shader4-style geometry shaders until link time, when the
value of gl_VerticesIn is known.  That would make the code cleaner, and
eliminate all of the corner cases that are currently broken, at the
expense of making certain compile errors reported at link time rather
than compile time (competing implementations, such as Nvidia's
proprietary driver, have this behaviour already).

However, that's going to be a fair amount of effort, and it's effort
that doesn't really push us towards GL 3.2 (which is the primary reason
I'm interested in working on geometry shaders to begin with).  So
instead, I'm going to shift gears and modify my "gs" branch so that it
implements GLSL 1.50 style geometry shaders rather than
ARB_geometry_shader4 style geometry shaders.  Once GLSL 1.50 is working
and committed upstream, I'll consider working on ARB_geometry_shader4
again, if there is interest.

An additional advantage of shifting gears to GLSL 1.50 style geometry
shaders is that we no longer have to worry about interactions between
geometry shaders and compatibility-only features (since Mesa currently
only implements the core profile of GL 3.1 and above).  It also means
that we don't have to worry about subtle bugs creeping in because of
ARB_geometry_shader4's use of 2D arrays (a lot of code in src/glsl
assumes that arrays are always 1D, and I haven't yet convinced myself
that this assumption has been removed when necessary for geometry
shaders).  Finally, it's possible that a high enough percentage of the
users of geometry shaders will do so through via GLSL 1.50, so it may
never be necessary to implement ARB_geometry_shader4 at all.

One impact of this course change is that many of the existing piglit
tests which validate geometry shaders currently use
ARB_geometry_shader4.  We'll need to write counterparts to these tests
that use GLSL 1.50.  Intel has a few interns working for us this
summer--I think I may try to talk them into doing that work :)

I've taken a snapshot of the current state of my "gs" branch and called
it "gs-arb", so if we later find out this was a bad idea, we can switch
back to plan A without much penalty.

I believe this is the right way to go, but I'm open to any comments
anyone has on this plan.  Thanks in advance!

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

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

Reply via email to