On Mon, Jun 16, 2014 at 1:20 PM, Jordan Justen <jljus...@gmail.com> wrote: > 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. :)
I'm not qualified to review, but it passed on nvidia blob in my tests. You can add my Tested-by if it helps. -ilia _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
