[Piglit] default piglit window size change

[Chad Versace]

On 11/04/2012 03:58 PM, Brian Paul wrote:
> On 11/04/2012 01:11 PM, Eric Anholt wrote:
>> Brian Paul<brianp at vmware.com>  writes:
>>
>>> On 11/02/2012 10:52 AM, Paul Berry wrote:
>>>> On 2 November 2012 09:32, Brian Paul<brianp at vmware.com
>>>> <mailto:brianp at vmware.com>>  wrote:
>>>>
>>>>
>>>>      I think I've brought this up in the past.  On Windows, the minimum
>>>>      width of a window is 116 pixels (at least in my current Win7
>>>>      setup). A lot of piglit tests create 100x100 pixels and when
>>>>      piglit creates the window, Win7 resizes it to 116x100.  The
>>>>      framework code detects this change and reports:
>>>>
>>>>      Got spurious window resize in automatic run (100,100 to 116,100)
>>>>      PIGLIT: {'result': 'warn'}
>>>>
>>>>      So a bunch of tests get the 'warn' result instead of being run
>>>>      properly.  I think this changed at some point during the recent
>>>>      framework/waffle overhaul.
>>>>
>>>>      My proposal for this is to set a default window size of 150x150 in
>>>>      the piglit-framework-gl.h header:
>>>>
>>>>      --- a/tests/util/piglit-framework-__gl.h
>>>>      +++ b/tests/util/piglit-framework-__gl.h
>>>>      @@ -216,6 +216,13 @@ piglit_gl_test_run(int argc, char *argv[],
>>>>
>>>>                \
>>>>                        config.init = piglit_init;          \
>>>>                        config.display = piglit_display;          \
>>>>      +         \
>>>>      +                /* Default window size.  Note: Win7's min window
>>>>      width */    \
>>>>      +               /* seems to be 116 pixels.  When the window size
>>>>      is */       \
>>>>      +                /* unexpectedly resized, tests are marked as
>>>>      "WARN". */      \
>>>>      +                /* Let's use a larger default to avoid that. */
>>>>             \
>>>>      +                config.window_width = 150;         \
>>>>      +                config.window_height = 150;
>>>>
>>>>        #define PIGLIT_GL_TEST_CONFIG_END          \
>>>>
>>>>
>>>>      And then remove these config lines from a bunch of tests:
>>>>
>>>>              config.window_width = 100;
>>>>              config.window_height = 100;
>>>>
>>>>      Of course, I'll have to inspect these tests to make sure they
>>>>      don't rely on that specific window size, but I bet few if any of
>>>>      them do.
>>>>
>>>>      This change fixes things here.
>>>>
>>>>      Sound OK?
>>>>
>>>>      -Brian
>>>>
>>>>
>>>> Sounds reasonable to me.  I like the idea of having a default window
>>>> size, since a lot of piglit tests just paint the whole window with the
>>>> same color, so for those tests the size is pretty much irrelevant :)
>>>
>>> And there's quite a few tests that don't draw anything at all.
>>>
>>> I'll post my first batch of patches soon.  There'll be more as I get time.
>>
>> I don't like how this turned out, really.  On our side, we'd like to see
>> tests that don't care about size rendered to smaller buffers, since we
>> run tests under simulation for HW bringup.  I don't like losing the
>> information that the test author had about requested size, even though
>> in many cases the author just copy and pasted.

Eric, it should be possible to satisfy your requirement of "use small window
sizes for hw bringup" and Brian's requirement of "use bigger window sizes
for MS Windows". Just let the default values of
piglit_gl_test_config::window_{width,height} be 0. The framework can then
postpone the decision of what the default window size is until runtime.
For Windows, we can have an ifdef that sets it to 150,150. For linux,
we can make it smaller. For hw bringup, we can set an environment var
or a command line flag that makes the windows absurdly small. This
should be very simple to implement.

>> There are two things I'd like to see done with regards to test size:
> 
> Hmm, I really don't have time to investigate and implement a more elaborate
> solution right now.  And I think Chad should definitely be involved in any such
> changes.  Chad, do you have time to look into this in the near future?

Yes. I can look at implementing Eric's two suggestions. Regardless of that,
though, ...
 
> I'd really like to move forward with my changes as-is so we can get things
> working on Windows.  I'd be happy to help with the follow-on changes if we
> decide to do something fancier.

... I agree that you should move forward with your changes as-is. I can
then begin looking into doing runtime selection of default window size.

>> - Have a "I don't draw anything" flag (or maybe just ask for 1x1) in
>>    tests that don't need the window system buffer.

Eric, this is on my todo list. It should be easy, so I'll bump up its
priority.

>> - Convert the remaining tests that aren't specific to window system
>>    buffer interactions to run with an FBO as the framebuffer.  This
>>    avoids window systems (including gnome-shell) screwing with our window
>>    size, and should improve performance slightly if we take advantage of
>>    surfaceless contexts.  Now that swapbuffers has been replaced, I think
>>    this should be easy.

This sounds like a good idea, but I haven't looked into it yet. Again, though,
I don't want to see Brian's fixes blocked because we want a new feature,
especially since the new feature is not incompatible with the fixes.

> One problem I have with this is losing the ability to see what's drawn.  That
> can be super helpful for debugging a failure.
> 
> Could the piglit_present_results() function be modified to copy the FBO's image
> to the window when we're not in -auto mode?  Or if some other new flag is set?

Brian, if I understand your question correctly, this is not a problem. Tests
that are capable of running in fbo mode are still capable of running in windowed
mode. That is, `./test-name -auto` runs the test in old-fashioned window mode
and `./test-name -auto -fbo` runs it in invisible fbo mode.

> But still, it's important to test window rendering too because of the upside
> down orientation difference with windows vs. FBOs.  In the past we've certainly
> had bugs in that area, and probably will in the future too.  If the 'general'
> tests were run both in window and FBO mode that'd probably catch most of the
> orientation bugs.

This feature---add an option to the testrunner to run all fbo-capable tests in
window and fbo mode---seems straight forward to do in the python layer. Add a new
property to the python test class that indicates that the test is capable of
running in fbo-mode. If the run-twice option is set and a test is capable of fbo-mode,
then the python framework runs the test twice, with and without the -fbo cmd line arg.