<div dir="ltr"><div><div><div><div><div><div><div><div><div><div><div><div><div>(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).<br>
</div><div><br>Hello all--<br><br></div>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 <a href="https://github.com/stereotype441/mesa.git">https://github.com/stereotype441/mesa.git</a>).  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:<br>
<br></div>- Texturing.<br></div><div>- Built-in uniform variables.<br></div>- 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.<br>
</div>- Mixing user-defined geometry shaders with fixed-function fragment shaders.<br></div></div>- 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).<br>
</div>- Color clamping.<br><br></div>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:<br>
<br></div>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.<br>
<br></div></div>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.<br>
<br>That in turn creates a problem because it prevents declarations like this from working:<br><br>varying in float foo[gl_VerticesIn];<br><br></div><div>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.:<br>
<br></div><div>varying in float foo[];<br></div><div><br>Similar changes are made for 2D inputs such as gl_TexCoordIn.<br><br></div><div>That in turn is a problem because it prevents code like this from working:<br><br></div>
<div>varying in float foo[gl_VerticesIn];<br></div><div>varying out float bar;<br></div><div>void main()<br>{<br></div><div>  for (int i = 0; i < gl_VerticesIn; i++) {<br></div><div>    bar = foo[i]; /* problem */<br></div>
<div>    EmitVertex();<br>  }<br>}<br><br></div><div>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.<br>
<br></div><div>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:<br>
<br></div><div>varing in float foo[];<br></div><div>varying out float bar;<br></div><div>void main()<br>{<br></div><div>  for (int i = 0; i < gl_VerticesIn; i++) {<br></div><div>    bar = foo[i]; /* illegal: i must be a constant because foo is unsized */<br>
</div><div>    EmitVertex();<br>  }<br>}<br><br></div><div>Conversely, code like this fails, even though it is allowed by ARB_geometry_shader4:<br><br></div><div>varying in float foo[gl_VerticesIn + 1 - 1];<br><br></div><div>
And so does this:<br></div><div><br></div>int main()<br><div>{<br></div><div>  float tmp[gl_VerticesIn];<br>}<br><br></div><div>Note that none of these problems applies to GLSL 1.50 style geometry shaders.<br></div><div><br>
</div><div>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).<br>
<br>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.<br>
<br></div><div>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.<br>
<br>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 :)<br>
<br></div><div>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.<br>
</div><div><br>I believe this is the right way to go, but I'm open to any comments anyone has on this plan.  Thanks in advance!<br><br>Paul<br></div></div></div>