On 8 May 2012 22:23, Anuj Phogat <span dir="ltr"><<a href="mailto:anuj.phogat@gmail.com" target="_blank">anuj.phogat@gmail.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">On Tue, May 8, 2012 at 7:42 AM, Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> wrote:<br>
> This patch series contains an alternative proposal to the patch "Split<br>
> accuracy test to allow new multisample tests utilize this code" from<br>
> 5/4, along the lines Anuj and I discussed over email yesterday. Anuj,<br>
> let me know if you think this alternative is reasonable. I went ahead<br>
> and left you as the author of the patch, but I would be equally happy<br>
> taking over authorship if you'd prefer.<br>
><br>
> Patch 2/2 contains the turn-on-off test, with the necessary<br>
> modifications to work with the new refactor. I have the impression<br>
> that this test is still a work in progress--is that right, Anuj?<br>
</div>Test is complete only for color buffer testing.<br>
<div class="im"><br>
>I got that impression because (a) the test ignores its <test_type><br>
> command line argument (it always tests in "color" mode), (b) the<br>
> "small" variant of the test fails on my nVidia reference platform, and<br>
> (c) the patch doesn't add the test to all.tests.<br>
</div>I kept the <test_type> option to accommodate depth and stencil testing<br>
in future. "small" variant is untested and irrelevant for this test case. So,<br>
can be removed. I missed adding it to all.tests.<br>
<br>
I'll make these changes along with few updated comments in turn-on-off.c<br>
once you push [PATCH v2 1/2].<br>
OR<br>
I can push both the patches together along with suggested changes.<br>
<div class="im"><br>
> For what it's worth, I think it would be ok to remove the command-line<br>
> options for the turn-on-off test (hardcoding to num_samples=4,<br>
> test_type=color, small=false, and depthstencil=true) since varying<br>
> those options is unlikely to expose any bugs that wouldn't already be<br>
> caught by the accuracy test.<br>
</div>I agree. command line options are not required at the moment.<br>
<div class="im"><br>
> [PATCH v2 1/2] Split accuracy test to allow new multisample tests utilize this code<br>
> [PATCH v2 2/2] Add test to turn on/off MSAA in a FBO<br>
</div>Thanks for accommodating the requested features while re-factoring.<br>
I am fine with both the patches.<br>
</blockquote></div><br>Ok, thanks. I've pushed patch 1/2. I'll let you handle the other one.<br>