[Piglit] [PATCH 1/2] GLX_OML_sync_control: Add new tests.
Eric Anholt
eric at anholt.net
Mon Oct 22 10:45:08 PDT 2012
Ian Romanick <idr at freedesktop.org> writes:
> On 10/19/2012 11:13 AM, Eric Anholt wrote:
>> I was going to go touch this code in Mesa, then I realised that we
>> didn't have a single test for it.
>>
>> v2: Rebase on Chad's BUILD_* sedjob, and explicitly test various swap intervals.
>
> A couple comments below...
>> diff --git a/tests/spec/glx_oml_sync_control/swapbuffersmsc-divisor-zero.c b/tests/spec/glx_oml_sync_control/swapbuffersmsc-divisor-zero.c
>> new file mode 100644
>> index 0000000..247c130
>> --- /dev/null
>> +++ b/tests/spec/glx_oml_sync_control/swapbuffersmsc-divisor-zero.c
>> + glXGetSyncValuesOML(dpy, win, &start_ust, &start_msc, &start_sbc);
>> + if (start_sbc != 0) {
>> + fprintf(stderr,
>> + "Initial SBC for the window should be 0, was %lld\n",
>> + (long long)start_sbc);
>> + piglit_report_result(PIGLIT_FAIL);
>> + }
>
> Since this appears in every test, maybe make a
> get_and_validate_initial_sync_values helper function?
In 2 out of the 4 tests, and refactoring would be at least as many LOC
and less code locality. No thanks.
>> diff --git a/tests/spec/glx_oml_sync_control/swapbuffersmsc-return.c b/tests/spec/glx_oml_sync_control/swapbuffersmsc-return.c
>> new file mode 100644
>> index 0000000..92f34ea
>> --- /dev/null
>> +++ b/tests/spec/glx_oml_sync_control/swapbuffersmsc-return.c
>> + do {
>> + glXGetSyncValuesOML(dpy, win, &start_ust, &start_msc, &start_sbc);
>> +
>> + /* Wait for the MSC to be at least equal to target,
>> + * with no divisor trickery.
>> + */
>> + target_msc = start_msc + 5;
>> + glXWaitForMscOML(dpy, win, target_msc, 0, 0,
>> + &wait_ust, &wait_msc, &wait_sbc);
>> +
>> + if (i++ >= 5) {
>> + fprintf(stderr,
>> + "current_msc is returning bad values "
>> + "(%lld, start value was %lld\n",
>> + (long long)start_msc,
>> + (long long)wait_msc);
>> + piglit_report_result(PIGLIT_FAIL);
>> + }
>> + } while (wait_msc < start_msc);
>
> There's something about this loop that I don't grok. Why try 5 times to
> see if glXWaitForMscOML can wait 5 vblanks? How can wait_msc ever be
> less than start_msc?
It was just the wrapping test. I rewrote it to be more like
swapbuffersmsc-divisor-zero for clarity.
>> + if (wait_msc < target_msc) {
>> + fprintf(stderr,
>> + "glXWaitForMscOML() returned msc of %lld, "
>> + "expected >= %lld\n",
>> + (long long)wait_msc,
>> + (long long)target_msc);
>> + }
>> +
>> + glXGetSyncValuesOML(dpy, win,
>> + ¤t_ust, ¤t_msc, ¤t_sbc);
>> +
>> + if (current_msc < target_msc) {
>
> Maybe 'if (current_msc < wait_msc)' instead?
I ended up moving this into the wrapping test block in the rewrite.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20121022/e100ca5b/attachment.pgp>
More information about the Piglit
mailing list