[Piglit] [Mesa-dev] [PATCH 00/18] Multi-stream support for geometry shaders

[Jordan Justen]

On Mon, Jun 16, 2014 at 1:05 AM, Samuel Iglesias Gonsálvez
<siglesias at igalia.com> wrote:
> On Sun, 2014-06-15 at 19:18 -0400, Ilia Mirkin wrote:
>> On Wed, Jun 11, 2014 at 12:31 PM, Jordan Justen <jljusten at gmail.com> wrote:
>> > On Wed, Jun 11, 2014 at 2:49 AM, Iago Toral <itoral at igalia.com> wrote:
>> >> Hi Chris,
>> >>
>> >> thanks for the quick review!
>> >>
>> >> On Wed, 2014-06-11 at 21:45 +1200, Chris Forbes wrote:
>> >>> I sent comments on patches 1, 3, 5, 9, 11, 16, 18
>> >>
>> >> We will look into these.
>> >>
>> >>> Patches 2, 4, 6-8, 10, 12-15, 17 are:
>> >>>
>> >>> Reviewed-by: Chris Forbes <chrisf at ijw.co.nz>
>> >>>
>> >>> You should also include a patch to docs/GL3.txt marking off this
>> >>> subfeature for i965 :)
>> >>>
>> >>>
>> >>> Do you have a bunch of piglits which exercise all the tricky corners
>> >>> of this? I see a few tests
>> >>> which require the existence of the stream layout qualifier, but not
>> >>> covering all the behavior.
>> >>
>> >> No, so far we have been testing this with a standalone program. We will
>> >> check what already exists in piglit and add missing test cases.
>> >
>> > This is the only piglit test that I know about:
>> > http://patchwork.freedesktop.org/patch/19224/
>> >
>> > Note that it passed on nvidia, but failed on catalyst.
>>
>> I just ran that test against this code (+ a few changes to add support
>> in gallium) and I got:
>>
>> $ MESA_EXTENSION_OVERRIDE=GL_ARB_gpu_shader5
>> bin/arb_gpu_shader5-xfb-streams -fbo -auto
>> Failed to compile geometry shader: 0:3(1): error: invalid input layout
>> qualifiers used
>>
>> source:
>> #version 150
>> #extension GL_ARB_gpu_shader5 : enable
>> layout(points, invocations = 32) in;
>> layout(points, max_vertices = 4) out;
>> out float stream0_0_out;
>> layout(stream = 1) out vec2 stream1_0_out;
>> layout(stream = 2) out float stream2_0_out;
>> layout(stream = 3) out vec3 stream3_0_out;
>> layout(stream = 1) out vec3 stream1_1_out;
>> layout(stream = 2) out vec4 stream2_1_out;
>> void main() {
>>   gl_Position = gl_in[0].gl_Position;
>>   stream0_0_out = 1.0 + gl_InvocationID;
>>   EmitVertex();
>>   EndPrimitive();
>>   stream3_0_out = vec3(12.0 + gl_InvocationID, 13.0 + gl_InvocationID,
>>                        14.0 + gl_InvocationID);
>>   EmitStreamVertex(3);
>>   EndStreamPrimitive(3);
>>   stream2_0_out = 7.0 + gl_InvocationID;
>>   stream2_1_out = vec4(8.0 + gl_InvocationID, 9.0 + gl_InvocationID,
>>                        10.0 + gl_InvocationID, 11.0 + gl_InvocationID);
>>   EmitStreamVertex(2);
>>   EndStreamPrimitive(2);
>>   stream1_0_out = vec2(2.0 + gl_InvocationID, 3.0 + gl_InvocationID);
>>   stream1_1_out = vec3(4.0 + gl_InvocationID, 5.0 + gl_InvocationID,
>>                        6.0 + gl_InvocationID);
>>   EmitStreamVertex(1);
>>   EndStreamPrimitive(1);
>> }PIGLIT: {'result': 'fail' }
>>
>> I guess it doesn't like
>>
>> "layout(points, invocations=32) in"? Not sure. I could also have
>> messed something up... doing a bisect over your patches blames
>>
>> "glsl: Add parsing support for multi-stream output in geometry shaders."
>>
>> Before that, it goes until the first stream=1 layout and fails there
>> (which is expected).
>>
>>   -ilia
>
> I will take a look at it.

Does anyone see any bugs in the test?

If not, perhaps I should just push it to piglit.

It never got a reviewed-by, but it certainly had time for review. :)

Thanks,

-Jordan