Jamey Sharp <[email protected]> writes:

> From: TheoH <[email protected]>
>
> The existing tests don't demonstrate a variety of implementation bugs
> that we've observed in current drivers, so this is our attempt to
> demonstrate those bugs.

> diff --git a/tests/spec/glx_oml_sync_control/timing.c 
> b/tests/spec/glx_oml_sync_control/timing.c
> new file mode 100644
> index 0000000..decb352
> --- /dev/null
> +++ b/tests/spec/glx_oml_sync_control/timing.c
> @@ -0,0 +1,298 @@

> +     for (i = 0; i < loops; i++) {
> +             int64_t swapped_ust = 0xd0, swapped_msc = 0xd0, swapped_sbc = 
> 0xd0;
> +             int64_t new_ust = 0xd0, new_msc = 0xd0, new_sbc = 0xd0, 
> new_timestamp;
> +             int64_t expected_msc, target_sbc;
> +             int64_t target_msc = 0;
> +
> +             if (target_msc_delta) {
> +                     target_msc = last_msc + target_msc_delta;
> +             }
> +
> +             if (use_swapbuffers) {
> +                     glClearColor(0.0, 1.0, 0.0, 0.0);
> +                     glClear(GL_COLOR_BUFFER_BIT);
> +                     glFlush();
> +
> +                     target_sbc = glXSwapBuffersMscOML(dpy, win, target_msc, 
> divisor, msc_remainder);
> +                     if(target_sbc <= 0) {
> +                             fprintf(stderr, "glXSwapBuffersMscOML 
> failed\n");
> +                             return PIGLIT_FAIL;
> +                     }
> +                        /*ERROR: swapBuffersMsc calculates wrong target SBC 
> (i.e. not current sbc + 1) */
> +                     if(target_sbc != last_sbc + 1) {
> +                             fprintf(stderr, "glXSwapBuffersMscOML 
> calculated the wrong target sbc: expected %"PRId64" but got %"PRId64"\n", 
> last_sbc + 1, target_sbc);

We're not totally consistent on this, but I prefer
mostly-80-columns-wrapped code where reasonable.

> +             /* ERROR: non-monotonicity in ust/msc */
> +             if (new_ust < last_ust) {
> +                     fprintf(stderr, "non-monotonic UST went backward by 
> %"PRId64"\n", last_ust - new_ust);
> +                     result = PIGLIT_FAIL;
> +             }

I think you can drop the "/* ERROR" comments.  They're immediately
followed by a more detailed printf of the problem, anyway.

> +             if (new_msc > expected_msc) {
> +                     fprintf(stderr, "woke up %"PRId64" MSCs later than 
> expected\n", new_msc - expected_msc);
> +             }

You could use piglit_merge_result(&result, PIGLIT_WARN) here, so you'll
get a WARN output in the summary if this case failed but no others did.
(same feedback in a couple other places)

Attachment: pgp_uL0GgmQnN.pgp
Description: PGP signature

_______________________________________________
Piglit mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to