Code review comment for lp:~smspillaz/compiz-core/compiz-core.fix_880707.2.test

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

> The correct test cases for bug 880707 would look something like this:
>
> 1. Test composite's ability to adapt to slow plugins:
> (a) Enable hasVSync in opengl (or anywhere else like a tester plugin)
> (b) Call CompositeScreen::damageScreen/damageRegion several times.
> (c) Verify the time between (b) and CompositeScreen::paint being called is
> less than 2 milliseconds.

That's fairly straightforward then. I'll write a case for that (although, the unit test that I wrote excersizes slightly similar codepaths, but it doesn't really excersize the case where repaints are queued based on work done by plugins rather than on a regular bases.).

I'll write a case for that.

>
> 2. Test for slow plugins that would cause vsync problems if composite was not
> fixed:
> (a) Measure the time taken by the calls to glPaintOutput in
> PrivateGLScreen::paintOutputs.
> (b) Verify the time is less than 2 milliseconds.

I think the idea here is that in cases where plugins are being slow, you move to the next half-rate of frame timing, so your refresh rate would be 30 Hz instead of 60 Hz if you start dropping below 60 Hz.

>
> I don't think it's realistic to expect any test cases like this before the fix
> for bug 880707 is committed. It would require a whole new test architecture
> based on a fully functioning compiz process.

I think it is safe to have at least, a testcase which shows defined behaviour for the frame timing at least, so that we have the ability to catch regressions of things change, no ?

« Back to merge proposal