[Piglit] [PATCH 4/4] ext_shader_samples_identical: Simple fragment shader rendering test

Ian Romanick idr at freedesktop.org
Thu Nov 19 13:45:50 PST 2015


On 11/19/2015 09:59 AM, Neil Roberts wrote:
> Ian Romanick <idr at freedesktop.org> writes:
> 
>> +# Group EXT_shader_samples_identical
>> +with profile.group_manager(
>> +        PiglitGLTest,
>> +        grouptools.join('spec', 'EXT_shader_samples_identical')) as g:
>> +    g(['ext_shader_samples_identical', '2'])
>> +    g(['ext_shader_samples_identical', '4'])
>> +    g(['ext_shader_samples_identical', '8'])
>> +    g(['ext_shader_samples_identical', '16'])
> 
> It might be nice to iterate over MSAA_SAMPLE_COUNTS instead of
> hardcoding the sample counts.

There are a couple other places in all.py that use a hardcoded list.
I'll change those two.

>> +	glUseProgram(draw_prog);
>> +	glVertexAttribPointer(PIGLIT_ATTRIB_POS, 2, GL_FLOAT,
>> +			      GL_FALSE, 0, tri_verts);
>> +	glEnableVertexAttribArray(PIGLIT_ATTRIB_POS);
>> +	glDrawArrays(GL_TRIANGLES, 0, 3);
>> +	glDisableVertexAttribArray(PIGLIT_ATTRIB_POS);
> 
> It could be good to use piglit_draw_rect_from_arrays here and maybe draw
> a diamond shape or something instead of a triangle. That way it would be
> less code and it would be easier to port to a core profile because it
> wouldn't rely on being able to skip using a VBO and VAO. It should just
> work because you are using the standard Piglit names for your
> attributes.

Yeah, that's a good suggestion.  This is part of the "Much code borrowed
from tests/spec/arb_texture_multisample/texelfetch.c." :)  I'll change
the arb_texture_multisample test too.

I'll have v2 out in a couple hours.

>> +	glVertexAttribPointer(PIGLIT_ATTRIB_POS, 2, GL_FLOAT,
>> +			      GL_FALSE, 0, quad_verts);
>> +	glVertexAttribPointer(PIGLIT_ATTRIB_TEX, 2, GL_FLOAT,
>> +			      GL_FALSE, 0, quad_texcoords);
>> +	glEnableVertexAttribArray(PIGLIT_ATTRIB_POS);
>> +	glEnableVertexAttribArray(PIGLIT_ATTRIB_TEX);
>> +	glDrawArrays(GL_TRIANGLE_FAN, 0, 4);
>> +	glDisableVertexAttribArray(PIGLIT_ATTRIB_POS);
>> +	glDisableVertexAttribArray(PIGLIT_ATTRIB_TEX);
> 
> Same here, I think it could be good to use piglit_draw_rect_tex.
> 
> Otherwise seems like a neat test.
> 
> Regards,
> - Neil
> 



More information about the Piglit mailing list