[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,
>> +			    &current_ust, &current_msc, &current_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