[Piglit] [PATCH] Split accuracy test to allow new multisample tests utilize this code

[Paul Berry]

On 7 May 2012 13:28, Anuj Phogat <anuj.phogat at gmail.com> wrote:

> On Mon, May 7, 2012 at 11:53 AM, Paul Berry <stereotype441 at gmail.com>
> wrote:
> >
> > On 4 May 2012 15:02, Anuj Phogat <anuj.phogat at gmail.com> 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 <anuj.phogat at gmail.com>
> >
> >
> > 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20120507/3b144521/attachment.htm>