On 7 May 2012 13:28, Anuj Phogat <[email protected]> wrote: > On Mon, May 7, 2012 at 11:53 AM, Paul Berry <[email protected]> > wrote: > > > > On 4 May 2012 15:02, Anuj Phogat <[email protected]> wrote: > >> > >> This patch splits accuracy.cpp in to three files: common.h, common.cpp > >> and accuracy.c. > >> > >> common.cpp defines the functions which can be utilized to develop new > >> multisample test cases. Functions can be utilized to: > >> - Draw a test image to default framebuffer. > >> - Initialize a FBO with specified samples count. > >> - Draw a test image to FBO. > >> - Draw a reference image. > >> - Verify the accuracy of multisample antialiasing in FBO. > >> > >> Signed-off-by: Anuj Phogat <[email protected]> > > > > > > It seems like you've gone to a bunch of effort to provide a C interface > (rather than a C++ interface) between the accuracy test and common.cpp, so > that the accuracy test can be written in straight C. Can you comment on > the benefits of this? > I provided the C interface in sync with my plans to develop new > multisample test cases using C. There are no specific benefits of this > choice. > > > I see two downsides: a. it forces us to introduce extra wrapper > functions that manipulate global state, and that doesn't seem desirable. > b. if we later decide to add a test that re-uses part of the common > structure but not all of it (e.g. by adding a new TestPattern-derived > class), we won't be able to do so easily. > > > I agree. That would require some extra efforts. > > > Unless there's something I'm missing, I would prefer if we took an > approach like this to sharing code: > > > > - Move the class declarations from accuracy.cpp into common.h. > > - Move the member function implementations from accuracy.cpp into > common.cpp. > > - Leave the global variable Test *test = NULL; in accuracy.cpp, so that > the global state is confined to the individual test file, and isn't part of > the interface between accuracy and common. > > - Leave accuracy.cpp as a C++ program. > > > I initially re factored using a similar approach. But later thoughts > of providing a "C" interface overwhelmed me. > I am fine with your suggestion and comfortable developing new tests in C++. > > > If you're interested, I'd be willing to take a stab at this approach and > send it to the list. > > > Can you modify few member functions as per testing requirements of > turn-on-off.c ([PATCH] Add test to turn on/off MSAA in a FBO) ? > e.g define test_fbo, provide an option of rendering test image to > default fb, using render buffer as color attachment for 0 sample > count, re initializing test_fbo with specified sample_count. > I am fine with making above listed changes to re factored > accuracy.cpp, if you face any issues understanding exact requirements. >
Ok, thanks. Since I objected to your refactor, I feel like it's only fair for me to put together an alternative. I'll get my proposal out to the list as soon as I can.
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
