[Piglit] [PATCH] piglit-fbo: check the proper extension dependency when attaching a color texture

[Alejandro Piñeiro]

On 15/01/16 21:48, Mark Janes wrote:
> Alejandro Piñeiro <apinheiro at igalia.com> writes:
>
>> On 15/01/16 13:07, Mark Janes wrote:
>>> This patch causes piglit.spec.ext_framebuffer_multisample.accuracy to
>>> fail on g33 and g965.  Is that what you expected?
>> No, I was not expecting any regression. It seemed a trivial change,
>> based on that attach_x was using, and it didn't raise any regression on
>> my haswell machine.
>>
>> Unfourtunately, I think that I don't have access to those old GPUs. The
>> more similar thing is an Ironlake. Could you confirm if it is working on
>> Ironlake? Could you share more details (stdout/stderr) of what it is
>> failing?
> It looks like the tests were previously skipping on these platforms.
> There may be platform limitations that prevent the tests from passing,
> now that they are enabled.

Yes, it seems that this patch didn't cause the test failing (so a
regression) but just exposing a failing test that was wrongly skipped
before. Having said so, as I mentioned, I don't have access to the
proper hw to confirm that.

>
> Sample output from one of the tests on g965:
> /tmp/build_root/m64/lib/piglit/bin/ext_framebuffer_multisample-accuracy all_samples depth_draw depthstencil -auto -fbo
> Pixels that should be unlit
>   count = 137813
>   RMS error = 0.241702
> Pixels that should be totally lit
>   count = 119798
>   RMS error = 0.568553
> The error threshold for unlit and totally lit pixels test is 0.016650
> Pixels that should be partially lit
>   count = 4533
>   RMS error = 0.593167
> The error threshold for partially lit pixels is 0.333000
> Samples = 0, Result = fail
>
>>
>>> Alejandro Piñeiro <apinheiro at igalia.com> writes:
>>>
>>>> attach_color_texture uses TEXTURE_RECTANGLE, so it should check
>>>> for GL_ARB_texture_rectangle.
>>>>
>>>> attach_multisample_color_texture uses multisample texture targets,
>>>> so it should check for GL_ARB_texture_multisample.
>>>>
>>>> Before this patch, the dependency check was wrongly switched.
>>>> ---
>>>>  tests/util/piglit-fbo.cpp | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/tests/util/piglit-fbo.cpp b/tests/util/piglit-fbo.cpp
>>>> index bc705aa..1c4cd6c 100644
>>>> --- a/tests/util/piglit-fbo.cpp
>>>> +++ b/tests/util/piglit-fbo.cpp
>>>> @@ -208,14 +208,14 @@ Fbo::try_setup(const FboConfig &new_config)
>>>>  		if (config.num_samples == 0) {
>>>>  
>>>>  			/* Attach textures as color attachments */
>>>> -			piglit_require_extension("GL_ARB_texture_multisample");
>>>> +			piglit_require_extension("GL_ARB_texture_rectangle");
>>>>  			for (int i = 0; i < config.num_tex_attachments; i++)
>>>>  				attach_color_texture(new_config, i);
>>>>  
>>>>  		} else {
>>>>  
>>>>  			/* Attach multisample textures as color attachments */
>>>> -			piglit_require_extension("GL_ARB_texture_rectangle");
>>>> +			piglit_require_extension("GL_ARB_texture_multisample");
>>>>  			for (int i = 0; i < config.num_tex_attachments; i++)
>>>>  				attach_multisample_color_texture(new_config, i);
>>>>  		}
>>>> -- 
>>>> 2.1.4
>>>>
>>>> _______________________________________________
>>>> Piglit mailing list
>>>> Piglit at lists.freedesktop.org
>>>> http://lists.freedesktop.org/mailman/listinfo/piglit
>> -- 
>> Alejandro Piñeiro (apinheiro at igalia.com)

-- 
Alejandro Piñeiro (apinheiro at igalia.com)