On 9 May 2012 21:04, Anuj Phogat <[email protected]> wrote: > > > > I included this as sub test (only for color buffer blits) in a patch > posted on May 7: > > [PATCH] Add test to verify glBlitFramebuffer with multisample > framebuffer objects. > > In this patch, I am just clearing the multisample buffers to unique > colors to do the testing. > > This patch definitely require an update after the recent splitting of > accuracy test. I would > > love to see your comments to improve this patch. > > > > Your patch looks better to me as it includes testing of depth and > stencil buffer blits as well. > > Keeping the clear color version of same test has no added advantage. So, > I will exclude this > > test case from the list of sub tests in above mentioned patch. >
Sorry, I forgot you had a test out on the mailing list that overlapped some of my test. I'll review your glBlitFramebuffer patch in more detail later this morning. My initial impression is that you're right--my test overlaps the functionality of test_blit_ms_ms_matching_samples() (and is more thorough, since it blits a complex pattern rather than a solid color). Definitely keep your other tests, though--it's good to check that GL errors are generated in the correct circumstances, and that no data is copied when a GL error occurs. > > > > Reviewed-by: Anuj Phogat <[email protected]> > > If you prefer to keep all multisample blitting cases in a single test, > I am happy to merge the two patches > and send it to the mailing list: > 1. [PATCH] Add test to verify glBlitFramebuffer with multisample > framebuffer objects. > 2. [PATCH 1/2] Add a test of MSAA-to-MSAA blits. > > OR > > I will split my patch and post rest of the blitting cases as separate > tests. > In general I prefer splitting tests up, as long as it can be done without duplicating a lot of code. In my experience small tests that just do a few things are easier to debug than large tests that do a lot. > > Thanks > Anuj >
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
