On Mon, Jun 16, 2014 at 1:05 AM, Samuel Iglesias Gonsálvez <sigles...@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 <jljus...@gmail.com> wrote: >> > On Wed, Jun 11, 2014 at 2:49 AM, Iago Toral <ito...@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 <chr...@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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev