On 7 May 2012 13:28, 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 Mon, May 7, 2012 at 11:53 AM, Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> wrote:<br>
><br>
> On 4 May 2012 15:02, Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com">anuj.phogat@gmail.com</a>> wrote:<br>
>><br>
>> This patch splits accuracy.cpp in to three files: common.h, common.cpp<br>
>> and accuracy.c.<br>
>><br>
>> common.cpp defines the functions which can be utilized to develop new<br>
>> multisample test cases. Functions can be utilized to:<br>
>> - Draw a test image to default framebuffer.<br>
>> - Initialize a FBO with specified samples count.<br>
>> - Draw a test image to FBO.<br>
>> - Draw a reference image.<br>
>> - Verify the accuracy of multisample antialiasing in FBO.<br>
>><br>
>> Signed-off-by: Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com">anuj.phogat@gmail.com</a>><br>
><br>
><br>
> 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?<br>
</div>I provided the C interface in sync with my plans to develop new<br>
multisample test cases using C. There are no specific benefits of this<br>
choice.<br>
<div class="im"><br>
> 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.<br>
><br>
</div>I agree. That would require some extra efforts.<br>
<div class="im"><br>
> Unless there's something I'm missing, I would prefer if we took an approach like this to sharing code:<br>
><br>
> - Move the class declarations from accuracy.cpp into common.h.<br>
> - Move the member function implementations from accuracy.cpp into common.cpp.<br>
> - 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.<br>
> - Leave accuracy.cpp as a C++ program.<br>
><br>
</div>I initially re factored using a similar approach. But later thoughts<br>
of providing a "C" interface overwhelmed me.<br>
I am fine with your suggestion and comfortable developing new tests in C++.<br>
<div class="im"><br>
> If you're interested, I'd be willing to take a stab at this approach and send it to the list.<br>
><br>
</div>Can you modify few member functions as per testing requirements of<br>
turn-on-off.c ([PATCH] Add test to turn on/off MSAA in a FBO) ?<br>
e.g define test_fbo, provide an option of rendering test image to<br>
default fb, using render buffer as color attachment for 0 sample<br>
count, re initializing test_fbo with specified sample_count.<br>
I am fine with making above listed changes to re factored<br>
accuracy.cpp, if you face any issues understanding exact requirements.<br></blockquote><div><br>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.<br>
</div></div>