[Piglit] [PATCH v2 0/2] Proposed alternative refactor of multisample tests

[Anuj Phogat]

On Tue, May 8, 2012 at 7:42 AM, Paul Berry <stereotype441 at gmail.com> wrote:
> This patch series contains an alternative proposal to the patch "Split
> accuracy test to allow new multisample tests utilize this code" from
> 5/4, along the lines Anuj and I discussed over email yesterday.  Anuj,
> let me know if you think this alternative is reasonable.  I went ahead
> and left you as the author of the patch, but I would be equally happy
> taking over authorship if you'd prefer.
>
> Patch 2/2 contains the turn-on-off test, with the necessary
> modifications to work with the new refactor.  I have the impression
> that this test is still a work in progress--is that right, Anuj?
Test is complete only for color buffer testing.

>I got that impression because (a) the test ignores its <test_type>
> command line argument (it always tests in "color" mode), (b) the
> "small" variant of the test fails on my nVidia reference platform, and
> (c) the patch doesn't add the test to all.tests.
I kept the <test_type> option to accommodate depth and stencil testing
in future. "small" variant is untested and irrelevant for this test case. So,
 can be removed. I missed adding it to all.tests.

I'll make these changes along with few updated comments in turn-on-off.c
once you push  [PATCH v2 1/2].
OR
I can push both the patches together along with suggested changes.

> For what it's worth, I think it would be ok to remove the command-line
> options for the turn-on-off test (hardcoding to num_samples=4,
> test_type=color, small=false, and depthstencil=true) since varying
> those options is unlikely to expose any bugs that wouldn't already be
> caught by the accuracy test.
I agree. command line options are not required at the moment.

> [PATCH v2 1/2] Split accuracy test to allow new multisample tests utilize this code
> [PATCH v2 2/2] Add test to turn on/off MSAA in a FBO
Thanks for accommodating the requested features while re-factoring.
I am fine with both the patches.