Merge lp:~smspillaz/compiz-core/compiz-core.fix-880707-test-v3 into lp:compiz-core/0.9.5

[Sam Spilsbury]

Separated out the composite plugin's paint scheduler into a separate class. Created a testcase using libdrm.

Expected output:

./plugins/composite/tests/paintscheduler/compiz_paintscheduler_test
DEBUG: attempting to load platform i915 ... failed
DEBUG: attempting to load platform radeon ... failed
DEBUG: attempting to load platform nouveau ... success
DEBUG: average vblank wait time was 33.51
DEBUG: average frame rate 29.8418 Hz
TEST: unthrottled vblank timings time 33.51 within threshold of 10
DEBUG: median absolute deviation of the periods was 0
DEBUG: st. dev of the phases was 0.365006
PASS: unthrottled vblank timings
DEBUG: average vblank wait time was 21.01
DEBUG: average frame rate 47.5964 Hz
TEST: no vsync 50 Hz refresh rate time 21.01 within threshold of 10
DEBUG: median absolute deviation of the periods was 0
DEBUG: st. dev of the phases was 0
PASS: no vsync 50 Hz refresh rate
DEBUG: attempting to load platform i915 ... failed
DEBUG: attempting to load platform radeon ... failed
DEBUG: attempting to load platform nouveau ... success
DEBUG: average vblank wait time was 33.7067
DEBUG: average frame rate 29.6677 Hz
TEST: vsync 50 Hz refresh rate time 33.7067 within threshold of 10
DEBUG: median absolute deviation of the periods was 0
DEBUG: st. dev of the phases was 0.47914
PASS: vsync 50 Hz refresh rate
DEBUG: average vblank wait time was 33.9467
DEBUG: average frame rate 29.458 Hz
TEST: no vsync 20 Hz refresh rate time 33.9467 within threshold of 10
DEBUG: median absolute deviation of the periods was 0
DEBUG: st. dev of the phases was 0
PASS: no vsync 20 Hz refresh rate
DEBUG: attempting to load platform i915 ... failed
DEBUG: attempting to load platform radeon ... failed
DEBUG: attempting to load platform nouveau ... success
DEBUG: average vblank wait time was 49.9367
DEBUG: average frame rate 20.0254 Hz
TEST: vsync 30 Hz refresh rate time 49.9367 within threshold of 10
DEBUG: median absolute deviation of the periods was 0
DEBUG: st. dev of the phases was 0.234331
PASS: vsync 30 Hz refresh rate
DEBUG: average vblank wait time was 11.0133
DEBUG: average frame rate 90.799 Hz
TEST: no vsync 100 Hz refresh rate time 11.0133 within threshold of 10
DEBUG: median absolute deviation of the periods was 0
DEBUG: st. dev of the phases was 0
PASS: no vsync 100 Hz refresh rate
DEBUG: attempting to load platform i915 ... failed
DEBUG: attempting to load platform radeon ... failed
DEBUG: attempting to load platform nouveau ... success
DEBUG: average vblank wait time was 16.89
DEBUG: average frame rate 59.2066 Hz
TEST: vsync 100 Hz refresh rate time 16.89 within threshold of 10
DEBUG: median absolute deviation of the periods was 0
DEBUG: st. dev of the phases was 0.359418
PASS: vsync 100 Hz refresh rate

[Daniel van Vugt (vanvugt)]

I think this proposal is going too far.

You started with my proposal which is: 345 lines (+65/-145) 3 files modified
and turned it into: 1175 lines (+758/-195) 12 files modified

If such a large volume of code is required just to add a test case, at the expense of future maintainability and readability, then I suggest the test case should not exist. Less code will lead to fewer bugs and better maintainability/readability in the long run.

review: Disapprove

[Daniel van Vugt (vanvugt)]

Also, in terms of class design my personal rule is "if the class name ends in 'er' then you probably need to redesign it". Generally if you can come up with classes that are nouns that represent actual conceptual objects, and not actions (not 'er'), then everything will fit together more elegantly.

[Tim Penhey (thumper)]

If the purpose of the test is to ensure correct behaviour, I can't see what the problem is.

Sure, adding tests adds code to the system, but if the purpose of that code is to make sure that the change does what it says it does, then it is good code.

In systems with complete test coverage, it is often usual to have more test code than the code that is being tested.

[Daniel van Vugt (vanvugt)]

I agree test cases can be any size, and most of the new lines of code are just tests. It's mostly just the introduction of a scheduler class that is rubbing my design sensibilities the wrong way. Especially when compared to how relatively simple and elegant lp:~vanvugt/compiz-core/fix-880707.2 is presently.

Also, test-paintscheduler seems to depend on having DRM-based graphics drivers installed and graphics active on the build machine. Won't that only work for people building compiz in a graphical environment, and only work for people with open source graphics drivers?

[Sam Spilsbury (smspillaz)]

>
> Also, test-paintscheduler seems to depend on having DRM-based graphics drivers
> installed and graphics active on the build machine. Won't that only work for
> people building compiz in a graphical environment, and only work for people
> with open source graphics drivers?

Unfortunately yes. I've updated the test proposal so that it just skips the vblanking test on hardware where DRM is available. I'll need to check the nvidia or ati documentation a little later on how to do hardware vblanks, but they might not expose such information.

The scheduler class is necessary so that we can unit test it. Its not possible to instansiate CompositeScreen outside of the compiz process since it requires core to be initialized.

[Sam Spilsbury (smspillaz)]

(Unfortunately the new upstream rules means that I can't merge this until there is a testcase for it. If you have any other ideas on testcases or class separations I'd be glad to take them :) The test can't be running inside compiz though)

[Sam Spilsbury (smspillaz)]

Also noted that I need to add some #ifdefs to skip the drm stuff if its not available.

[Sam Spilsbury (smspillaz)]

> >
> > Also, test-paintscheduler seems to depend on having DRM-based graphics
> drivers
> > installed and graphics active on the build machine. Won't that only work for
> > people building compiz in a graphical environment, and only work for people
> > with open source graphics drivers?
>
> Unfortunately yes. I've updated the test proposal so that it just skips the
> vblanking test on hardware where DRM is available. I'll need to check the
> nvidia or ati documentation a little later on how to do hardware vblanks, but
> they might not expose such information.
>

That should read, where DRM is *not* available.

[Daniel van Vugt (vanvugt)]

This is tricky. You can't actually test that you've fixed the vsync problems unless you test it using GLX vsync calls that the opengl plugin uses. And even then, you will not notice regressions caused by high latencies in plugins (unity for bug 880707) without also executing all the plugins' glPaintOutput methods etc.

So yes, the test cases probably test the new PaintScheduler class very well. But no, the tests have very little to do with verifying bug 880707 or any other vsync problems in reality. As such, I have to reject test-paintscheduler and PaintScheduler right now because they have nothing to do with verifying the fix for bug 880707 or any similar vsync regressions which could occur in plugins in future.

[Daniel van Vugt (vanvugt)]

Also, the test cases seem to work by verifying the vblank period time with a tolerance of 10ms. Bug 880707 had nothing to do with the period/frequency, the period was always correct thanks to CompTimer. Bug 880707 was caused by poor _phase_ timing and the tolerance seems to be a max 2-3ms. It's testing the wrong code and using the wrong method... :(

[Sam Spilsbury (smspillaz)]

> Also, the test cases seem to work by verifying the vblank period time with a
> tolerance of 10ms. Bug 880707 had nothing to do with the period/frequency, the
> period was always correct thanks to CompTimer. Bug 880707 was caused by poor
> _phase_ timing and the tolerance seems to be a max 2-3ms. It's testing the
> wrong code and using the wrong method... :(

Thanks. I'll have a look into fixing it a little later this week then.

As for using GLX to do it, I'm open to that idea, although I wanted to form a testcase for this outside of opengl since there might be other uses for it.

[Sam Spilsbury (smspillaz)]

> This is tricky. You can't actually test that you've fixed the vsync problems
> unless you test it using GLX vsync calls that the opengl plugin uses. And even
> then, you will not notice regressions caused by high latencies in plugins
> (unity for bug 880707) without also executing all the plugins' glPaintOutput
> methods etc.
>

Yes. This is a test for the PaintScheduler class. I don't think its a comprehensive test for bug 880707, and in reality, the fix for that is to write less slow plugins or move to some kind of multithreaded architecture (which I don't think we'll be doing any time soon) ;-)

[Daniel van Vugt (vanvugt)]

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.

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 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.

[Daniel van Vugt (vanvugt)]

Very importantly, the above test cases could be run with and without the fixes for bug 880707, to show the test cases themselves are correct.

[Sam Spilsbury (smspillaz)]

> 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 ?

[Daniel van Vugt (vanvugt)]

The frame rate doesn't halve, because the old composite code kind-of-partially adjusted for slow rendering. That would be a period/frequency problem but this was a problem with the phase. The core of bug 880707 was that composite was always out of phase with the hardware vblank, but using the right frequency/framerate:

Composite: --*----*----*----*----*----*
Hardware : *----*----*----*----*----*

So composite changed the front buffer roughly 1/4 of the way through the vrefresh, which was about 4ms too late. Hence in my case all the tearing is visible in the top half of the screen.

Please don't test for framerate in any test cases relating to bug 880707. It's not related.

[Daniel van Vugt (vanvugt)]

Also, these proposed test cases should not require a 'scheduler' class at all... ?

It's better if the test cases and glue don't conflict when merged with the fix for bug 880707, so you can add and remove the fix to verify the test case passes with it, and fails without it.

[Sam Spilsbury (smspillaz)]

On Tue, Nov 22, 2011 at 8:45 PM, Daniel van Vugt <email address hidden> wrote:

> The frame rate doesn't halve, because the old composite code
> kind-of-partially adjusted for slow rendering. That would be a
> period/frequency problem but this was a problem with the phase. The core of
> bug 880707 was that composite was always out of phase with the hardware
> vblank, but using the right frequency/framerate:
>
> Composite: --*----*----*----*----*----*
> Hardware : *----*----*----*----*----*
>
> So composite changed the front buffer roughly 1/4 of the way through the
> vrefresh, which was about 4ms too late. Hence in my case all the tearing is
> visible in the top half of the screen.
>
>
Yes, so the test needs to be one where we measure time from the vblank
event to some function which would be like glXSwapBuffers. My understanding
is that the scheduler is supposed to work like:

Damage -> PreparePaint (timeDiff being the diff from the last Damage) ->
Paint Dispatch -> Wait vblank -> Update front buffer -> Check if another
paint needs to be dispatched immediately or sleep until one does.

I can adjust the tests to be based around that case to ensure that we can
schedule paints whenever we want to (random) but the actual part where we
update the front buffer never happens outside within 3ms outside of the
vblank event.

(My apologies for the delay, I want to make sure that I'm really sure of
the correct behaviour and that we can verify the correct behaviour before I
merge it)

> Please don't test for framerate in any test cases relating to bug 880707.
> It's not related.
>
>
I know it isn't strictly related here, however it is the correct behaviour
in this case (halving the frame rate rather than going below 60Hz which
would put you out of phase)

>
> --
>
> https://code.launchpad.net/~smspillaz/compiz-core/compiz-core.fix_880707.2.test/+merge/82961<https://code.launchpad.net/%7Esmspillaz/compiz-core/compiz-core.fix_880707.2.test/+merge/82961>
> You are the owner of
> lp:~smspillaz/compiz-core/compiz-core.fix_880707.2.test.
>

--
Sam Spilsbury

[Sam Spilsbury (smspillaz)]

On Tue, Nov 22, 2011 at 8:57 PM, Daniel van Vugt <email address hidden> wrote:

> Also, these proposed test cases should not require a 'scheduler' class at
> all... ?
>
> It's better if the test cases and glue don't conflict when merged with the
> fix for bug 880707, so you can add and remove the fix to verify the test
> case passes with it, and fails without it.
>

The test case is there for catching regressions. I may need to change this
code in future. The reason for having a separate class is because this is
the architecture I'm trying to move compiz towards (eg, having separate
worker classes with fewer dependencies on the entire system so that
individual algorithms can be tested separately)

> --
>
> https://code.launchpad.net/~smspillaz/compiz-core/compiz-core.fix_880707.2.test/+merge/82961<https://code.launchpad.net/%7Esmspillaz/compiz-core/compiz-core.fix_880707.2.test/+merge/82961>
> You are the owner of
> lp:~smspillaz/compiz-core/compiz-core.fix_880707.2.test.
>

--
Sam Spilsbury

[Daniel van Vugt (vanvugt)]

Vblank is not an "event". It is an interval of time with a distinct beginning and an end. It could even be a millisecond or 2 in length. I think part of the cause of this bug is that glXWaitVideoSyncSGI returns immediately if we're already in vblank (with intel at least), meaning you have no idea how much more vblank time you have left to render the frame before tearing occurs. That's why my fix is aimed at starting the glXWaitVideoSyncSGI ASAP to ensure we are near the start of the vblanking interval before we update the front buffer.

[http://en.wikipedia.org/wiki/Vertical_blanking_interval]
[http://www.opengl.org/registry/specs/SGI/video_sync.txt]

I also note that if your test cases don't use GLX then you can remove the dependency on DRM. And even if you do use GLX then you should still remove the DRM code. Because we have no guarantee that the aforementioned behaviour of glXWaitVideoSyncSGI matches that of DRM. It's likely to even be different between driver manufacturers. So best to just use timers and no DRM code.

As for the style issues... please try to avoid naming classes with an 'er' suffix. Consider it a challenge. I think you'll find your resulting designs more logical and elegant if you do.

Please do take your time. The original fix took me several days solid of testing and refinement. Part of the reason why I'm distressed to see it get convoluted so quickly :(

[Daniel van Vugt (vanvugt)]

An example of a better class name/design: Clutter uses a class called Timeline for scheduling animations...

http://www.gtkmm.org/docs/cluttermm-0.9/docs/reference/html/classClutter_1_1Timeline.html

[Sam Spilsbury (smspillaz)]

On Wed, Nov 23, 2011 at 11:24 AM, Daniel van Vugt <email address hidden> wrote:
>
> Vblank is not an "event". It is an interval of time with a distinct beginning and an end. It could even be a millisecond or 2 in length. I think part of the cause of this bug is that glXWaitVideoSyncSGI returns immediately if we're already in vblank (with intel at least), meaning you have no idea how much more vblank time you have left to render the frame before tearing occurs.

Ah. So on libdrm at least this is implemented as an event rather than
a kind of poll-waiting thing. I agree its awkward, though, this is the
only GL independent implementation I know of (wanted to test the logic
of the code outside of OpenGL).

>
> That's why my fix is aimed at starting the glXWaitVideoSyncSGI ASAP to ensure we are near the start of the vblanking interval before we update the front buffer.
>
> [http://en.wikipedia.org/wiki/Vertical_blanking_interval]
> [http://www.opengl.org/registry/specs/SGI/video_sync.txt]
>
> I also note that if your test cases don't use GLX then you can remove the dependency on DRM. And even if you do use GLX then you should still remove the DRM code. Because we have no guarantee that the aforementioned behaviour of glXWaitVideoSyncSGI matches that of DRM. It's likely to even be different between driver manufacturers. So best to just use timers and no DRM code.

I'd be more concerned actually if the implementation with
glXWaitVideoSyncSGI was different depending on the driver codebases -
it *should* block until the vblank period happens rather than doing
anything else. I recognize that on some drivers this would be a
problem like you said because if the vertical blanking period is
already underway then there will be no blocking and an immediate
return. (This is ironically where using libdrm would help somewhat,
because this code is setup in such a way that the paint dispatch
thread *always* blocks until the *next* vertical blanking period, even
if we were in one already, but obviously we can't do that because its
not portable).

This code some something similar (although in a somewhat more
convoluted way). It spawns a thread to watch libdrm events, and the
paint thread waits on the event watcher thread to wake it up whenever
the vblanking period starts (eg, kernel gave us a vblank event).

So it's something like:

Thread 1: Timer dispatch -> Calculate time diff -> preparePaint ->
paint -> wait for vblank ..............! -> fill front buffer ->
repeat
Thread 2: Handle libdrm event ... -> ... -> ...
-----------------------------------------------------------------
vblank -> ! -> repeat

Of course, now we're discussing the implementation detail of the testcase ;-)

libdrm here of course ,serves as a testcase to make sure that Thread 1
is doing the right thing. You could replace thread 2 with anything,
even a dummy thing which wakes up every * ms to allow Thread 1 to be
unblocked, simulating the vblank blocking period. What is important in
the testcase is that regardless of how many times Thread 1 needs to do
work, it is always doing the critical fill-front-buffer stage within
3ms of being woken up. If thread 1 is was taking forever to do its
work, then ...

[Daniel van Vugt (vanvugt)]

Whoa, too many words. But I think the gist is that you understand :)

I will be verifying your next proposal by actually testing not only the bug is still fixed, but also that the test cases do actually verify bug 880707. I will be doing this by finding ways to reintroduce the bug and then verify your test cases fail under those conditions. And of course verifying that the test cases pass when the fix is enabled. Please save time by testing this yourself before proposing a new version...

[Daniel van Vugt (vanvugt)]

It appears the half framerate problem does happen with some drivers. I can reproduce the half framerate issue with NVIDIA-280 on a GT210 card, and think that bug 888039 covers this. The good news is that (as expected) the half framerate issue is indeed solved by lp:~vanvugt/compiz-core/fix-880707.2. And the older cause of low NVIDIA frame rates - bug 92599 - seems to be solved (avoided) by the fix too.

[Daniel van Vugt (vanvugt)]

To clarify... with NVIDIA-280... the bug is very obvious and easy to see as half-frame-rate if you have enabled "Workarounds>Force full screen redraws (buffer swap) on repaint". If you don't have that enabled, then you are likely to mostly see tearing instead of a lower frame rate.

[Sam Spilsbury (smspillaz)]

Will have another look at this tomorrow.

[Daniel van Vugt (vanvugt)]

Please base this on my branch, and retarget for lp:compiz-core. While I'm happy to review and test it, I am probably not going to be totally confident having all these new changes committed in my name. So let's make it yours.

And please don't do the final submission until at least next week. Because this week is crazy and I'd rather not have the option of even thinking about it.

[Daniel van Vugt (vanvugt)]

Also, please don't resubmit until the work in progress described in my proposal is completed.

[Sam Spilsbury (smspillaz)]

Sure no problem.

One thing I was going to ask based on something I came across while testing this. The composite plugin has an option to throttle frame rates to some specified rate, eg 1 to 200 frames per second. However, with the current code this does not happen if the paint handler is doing vsync. I think it should be possible to limit the fps to a certain number while still doing vsync, but I'm wondering if this is the correct behaviour. Let me know when you have time to.

[Daniel van Vugt (vanvugt)]

It was intentional that the throttling doesn't work with vsync. Because you could easily break the nice smooth vsync by having a bad frame rate value detected or set manually. I didn't want anyone to shoot themselves in the foot and log new bugs because they had been fiddling with the wrong settings. If you want a specific frame rate other than that of the monitor then you need to turn vsync off. I don't think it's a bad thing...

[Sam Spilsbury (smspillaz)]

Ah, I was thinking more along the lines of:

If you have a monitor with a refresh rate of N, then the actual refresh rate with vsync on while throttled is going to be N / 2 until N falls under the maximum refresh rate. The phase in this case would still be in-sync with the monitor since the actual swapbuffers / copysubbuffermesa calls are going to happen within that 4ms window of the vertical blank period anyways.

If functionality like that doesn't make sense I'm happy to just drop it. The only use-case I can think of really is where the user intentionally wanted to throttle the compositor framerate in order to save battery life at the cost of smoother animations or video.

[Daniel van Vugt (vanvugt)]

Right now it would cause a regression of bug 92599 which is not acceptable. That's one good reason to think about how to do it as a separate future enhancement. Even then it's not important at all.

I think the existing compiz CPU issues (mostly discussed in bug 803943) are more important with respect to power savings.

[Sam Spilsbury (smspillaz)]

Ah right, the refresh rate detection. That isn't much fun. I think that refresh rate detection should be removed anyways and the "refresh rate" option should actually be a cap on how much your screen is allowed to redraw per second even with vsync on. And that number should be fairly high, since the vblank period of most monitors is usually around 16ms.

[Sam Spilsbury (smspillaz)]

(The high cpu usage in the case back then was mostly with unity when blurs were being updated constantly, I'm not really sure if that bug is particularly valid anymore, or at least, while testing things this time around the cpu usage while idle is nil and while active its around 5%)

[Daniel van Vugt (vanvugt)]

A maximum refresh rate option would be non-trivial to get right if you're also trying to sync to vblank. And if it can be treated as a separate activity, then I think it should be. We don't need this branch getting any more complicated than it already is.

[Sam Spilsbury (smspillaz)]

I got it working in about 100~ lines of code :) I can probably move it to a separate branch if need be. For now its in this one.

[Daniel van Vugt (vanvugt)]

100 lines of code is very large (and risky) for such a simple feature. Or am I misinterpreting something?

[Sam Spilsbury (smspillaz)]

It was mostly a case of exposing the vsync api from the paint handlers to the paint scheduler, and requiring another vsync wait if we were exceeding the maximum frame rate. So mostly shuffling code around.

[Sam Spilsbury (smspillaz)]

Any more ideas on this Daniel? I can't merge your original branch without unit tests, but I can make a PPA of this if need be.

[Daniel van Vugt (vanvugt)]

Yes, a PPA if possible please. I'm trying to reduce the amount of Ubuntu work I do. But I'm happy to test all the relevant bugs with a bunch of different graphics cards if you make it quick and easy for me :)

[Daniel van Vugt (vanvugt)]

Though if your changes are compatible with the oneiric code, as mine are, then I can merge them myself... ?

[Sam Spilsbury (smspillaz)]

Hey Daniel,

Packages to test this can be found in ppa://smspillaz/880707

[Daniel van Vugt (vanvugt)]

Are you sure?...
"This PPA does not contain any packages yet. Find more information about how to upload packages in the PPA help page."

[Sam Spilsbury (smspillaz)]

Ah, typo when uploading, try it now https://launchpad.net/~smspillaz/+archive/880707

[Daniel van Vugt (vanvugt)]

The PPA is still missing the binary packages. It says they're not scheduled to start building for another 2 (amd64) and 8 (i386) hours respectively. That's strange... The builds should have been queued immediately. I will wait.

In future please ensure the binaries are built and tested before handing them over to someone else. It's always possible you can make simple mistakes (missing/wrong fixes), that ideally you want to find in the PPA before other people get stuck on them.

[Daniel van Vugt (vanvugt)]

A couple of conflicts, which weren't too hard to fix. But then it was full of build failures. Too many failures...

Please pull from my branch again, then fix the conflicts and build failures.

review: Needs Fixing

[Sam Spilsbury (smspillaz)]

Conflicts fixed.

[Daniel van Vugt (vanvugt)]

The target branch is now "finished". Please proposed for merging into lp:compiz-core instead.

[Sam Spilsbury (smspillaz)]

Whoops. Fixed.

[Sam Spilsbury (smspillaz)]

(A note that I will change this to use google test and the testing infrastructure in compiz once this is merged)

[lp:~smspillaz/compiz-core/compiz-core.fix-880707-test-v3]

2919. By Sam Spilsbury on 2012-01-20

Merged from lp:~thomas-voss/compiz-core/fix-bad-get-and-missing-symbols

[Alan Griffiths (alan-griffiths)]

I really can't tell if the code has the intended effect, but it largely looks reasonable.

I presume from the comment trail that vanvugh will verify?

BTW the ARRAY_SIZE is better written as a simple template:

template<typename Type, size_t N>
inline size_t length_of(Type (&)[N]) { return N; }

review: Abstain

[Daniel van Vugt (vanvugt)]

It appears this got merged without anyone fixing the build failures :(

Unfortunate, because I had planned on reviewing this proposal again this week.

I'll try to fix the errors as part of bug 921406.

=== modified file 'cmake/CompizPlugin.cmake'
--- cmake/CompizPlugin.cmake	2012-01-12 06:48:58 +0000
+++ cmake/CompizPlugin.cmake	2012-01-20 14:46:26 +0000
@@ -342,7 +342,7 @@
             ${${_PLUGIN}_PKG_INCDIRS}
             ${${_PLUGIN}_INCDIRS}
             ${COMPIZ_INCLUDE_DIRS}
-            ${CMAKE_PREFIX_PATH}/include
+            ${PLUGIN_PREFIX}/include
             ${CMAKE_INCLUDE_PATH}
 	    ${${_PLUGIN}_MOD_INCLUDE_DIRS}
 	    ${CORE_MOD_INCLUDE_DIRS}
@@ -353,16 +353,20 @@
 		      GLOBAL
 		      PROPERTY ${_PLUGIN}_MOD_LIBRARY_DIRS)
 
+	set (SYSTEM_LINK_DIRS
+	     "/usr/lib"
+	     "/usr/lib32"
+	     "/usr/lib64"
+	     "/usr/lib/${CMAKE_LIBRARY_ARCHITECTURE}")
+
 	link_directories (
             ${COMPIZ_LINK_DIRS}
             ${${_PLUGIN}_PKG_LIBDIRS}
             ${${_PLUGIN}_LIBDIRS}
             ${PLUGIN_LIBDIR}
             ${COMPIZ_LIBDIR}/compiz
-            ${CMAKE_PREFIX_PATH}/lib
-            ${CMAKE_PREFIX_PATH}/lib32
-            ${CMAKE_PREFIX_PATH}/lib64
 	    ${${_PLUGIN}_MOD_LIBRARY_DIRS}
+            ${SYSTEM_LINK_DIRS}
 	)
 
 	add_library (
 
=== modified file 'plugins/composite/CMakeLists.txt'
--- plugins/composite/CMakeLists.txt	2011-03-11 12:15:30 +0000
+++ plugins/composite/CMakeLists.txt	2012-01-20 14:46:26 +0000
@@ -3,3 +3,5 @@
 include (CompizPlugin)
 
 compiz_plugin (composite)
+
+add_subdirectory (tests)
 
=== modified file 'plugins/composite/composite.xml.in'
--- plugins/composite/composite.xml.in	2009-08-08 06:00:14 +0000
+++ plugins/composite/composite.xml.in	2012-01-20 14:46:26 +0000
@@ -8,16 +8,11 @@
 		<_short>Slow Animations</_short>
 		<_long>Toggle use of slow animations</_long>
 	    </option>
-	    <option name="detect_refresh_rate" type="bool">
-		<_short>Detect Refresh Rate</_short>
-		<_long>Automatic detection of refresh rate</_long>
-		<default>true</default>
-	    </option>
-	    <option name="refresh_rate" type="int">
-		<_short>Refresh Rate</_short>
-		<_long>The rate at which the screen is redrawn (times/second)</_long>
-		<default>50</default>
-		<min>1</min>
+	    <option name="refresh_rate_cap" type="int">
+		<_short>Refresh Rate Cap</_short>
+		<_long>Maximum allowed refresh rate (set -1 to force VSync usage)</_long>
+		<default>200</default>
+		<min>-1</min>
 		<max>200</max>
 	    </option>
 	    <option name="unredirect_fullscreen_windows" type="bool">
 
=== modified file 'plugins/composite/include/composite/composite.h'
--- plugins/composite/include/composite/composite.h	2012-01-18 16:26:45 +0000
+++ plugins/composite/include/composite/composite.h	2012-01-20 14:46:26 +0000
@@ -37,6 +37,7 @@
 #include "core/output.h"
 #include "core/screen.h"
 #include "core/wrapsystem.h"
+#include "composite/fpslimiter.h"
 
 #define COMPOSITE_SCREEN_DAMAGE_PENDING_MASK (1 << 0)
 #define COMPOSITE_SCREEN_DAMAGE_REGION_MASK  (1 << 1)
@@ -80,14 +81,6 @@
  */
 #define PAINT_SCREEN_NO_BACKGROUND_MASK            (1 << 6)
 
-
-typedef enum
-{
-    CompositeFPSLimiterModeDisabled = 0,
-    CompositeFPSLimiterModeDefault,
-    CompositeFPSLimiterModeVSyncLike
-} CompositeFPSLimiterMode;
-
 class PrivateCompositeScreen;
 class PrivateCompositeWindow;
 class CompositeScreen;
@@ -106,6 +99,10 @@
 			       const CompRegion    &region) = 0;
 
     virtual bool hasVSync () { return false; };
+    /* Return true if we are allowed to wait some more
+     * for framerate throttling */
+    virtual bool waitVSync (unsigned int mask) { return false; }
+    virtual void syncBuffers (unsigned int        mask) = 0;
 
     virtual void prepareDrawing () {};
     virtual bool compositingActive () { return false; };
@@ -173,10 +170,6 @@
     public CompOption::Class
 {
     public:
-
-
-
-    public:
 	CompositeScreen (CompScreen *s);
 	~CompositeScreen ();
 
 
=== added file 'plugins/composite/include/composite/fpslimiter.h'
--- plugins/composite/include/composite/fpslimiter.h	1970-01-01 00:00:00 +0000
+++ plugins/composite/include/composite/fpslimiter.h	2012-01-20 14:46:26 +0000
@@ -0,0 +1,38 @@
+/*
+ * Copyright © 2008 Dennis Kasprzyk
+ * Copyright © 2007 Novell, Inc.
+ *
+ * Permission to use, copy, modify, distribute, and sell this software
+ * and its documentation for any purpose is hereby granted without
+ * fee, provided that the above copyright notice appear in all copies
+ * and that both that copyright notice and this permission notice
+ * appear in supporting documentation, and that the name of
+ * Dennis Kasprzyk not be used in advertising or publicity pertaining to
+ * distribution of the software without specific, written prior permission.
+ * Dennis Kasprzyk makes no representations about the suitability of this
+ * software for any purpose. It is provided "as is" without express or
+ * implied warranty.
+ *
+ * DENNIS KASPRZYK DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN
+ * NO EVENT SHALL DENNIS KASPRZYK BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+ * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ * Authors: Dennis Kasprzyk <[e-mail]>
+ *          David Reveman <[e-mail]>
+ */
+
+#ifndef _COMPIZ_COMPOSITE_FPSLIMITER_H
+#define _COMPIZ_COMPOSITE_FPSLIMITER_H
+
+typedef enum
+{
+    CompositeFPSLimiterModeDisabled = 0,
+    CompositeFPSLimiterModeDefault,
+    CompositeFPSLimiterModeVSyncLike
+} CompositeFPSLimiterMode;
+
+#endif
 
=== modified file 'plugins/composite/src/composite.cpp'
--- plugins/composite/src/composite.cpp	2012-01-18 16:26:45 +0000
+++ plugins/composite/src/composite.cpp	2012-01-20 14:46:26 +0000
@@ -57,35 +57,6 @@
 }
 
 bool
-PrivateCompositeScreen::setOption (const CompString  &name,
-				   CompOption::Value &value)
-{
-    unsigned int index;
-
-    bool rv = CompositeOptions::setOption (name, value);
-
-    if (!rv || !CompOption::findOption (getOptions (), name, &index))
-	return false;
-
-    switch (index) {
-	case CompositeOptions::DetectRefreshRate:
-	    if (optionGetDetectRefreshRate ())
-		detectRefreshRate ();
-	    break;
-	case CompositeOptions::RefreshRate:
-	    if (optionGetDetectRefreshRate ())
-		return false;
-	    redrawTime = 1000 / optionGetRefreshRate ();
-	    optimalRedrawTime = redrawTime;
-	    break;
-	default:
-	    break;
-    }
-
-    return rv;
-}
-
-bool
 CompositePluginVTable::init ()
 {
     if (!CompPlugin::checkPluginABI ("core", CORE_ABIVERSION))
 
=== added file 'plugins/composite/src/paintscheduler.cpp'
--- plugins/composite/src/paintscheduler.cpp	1970-01-01 00:00:00 +0000
+++ plugins/composite/src/paintscheduler.cpp	2012-01-20 14:46:26 +0000
@@ -0,0 +1,189 @@
+/*
+ * Copyright © 2008 Dennis Kasprzyk
+ * Copyright © 2007 Novell, Inc.
+ *
+ * Permission to use, copy, modify, distribute, and sell this software
+ * and its documentation for any purpose is hereby granted without
+ * fee, provided that the above copyright notice appear in all copies
+ * and that both that copyright notice and this permission notice
+ * appear in supporting documentation, and that the name of
+ * Dennis Kasprzyk not be used in advertising or publicity pertaining to
+ * distribution of the software without specific, written prior permission.
+ * Dennis Kasprzyk makes no representations about the suitability of this
+ * software for any purpose. It is provided "as is" without express or
+ * implied warranty.
+ *
+ * DENNIS KASPRZYK DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN
+ * NO EVENT SHALL DENNIS KASPRZYK BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+ * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ * Authors: Dennis Kasprzyk <[e-mail]>
+ *          David Reveman <[e-mail]>
+ *          Daniel van Vugt <[e-mail]>
+ *          Sam Spilsbury <[e-mail]>
+ */
+
+#include "paintscheduler.h"
+#include <boost/bind.hpp>
+
+compiz::composite::scheduler::PaintScheduler::PaintScheduler (PaintSchedulerDispatchBase *b) :
+    mSchedulerState (0),
+    mRedrawTime (1000 / 50),
+    mOptimalRedrawTime (1000 / 50),
+    mFPSLimiterMode (CompositeFPSLimiterModeDefault),
+    mDispatchBase (b)
+{
+    gettimeofday (&mLastRedraw, 0);
+}
+
+compiz::composite::scheduler::PaintScheduler::~PaintScheduler ()
+{
+    mPaintTimer.stop ();
+}
+
+bool
+compiz::composite::scheduler::PaintScheduler::schedule ()
+{
+    int 			    delay = 1;
+
+    if (!mDispatchBase->schedulerCompositingActive ())
+	return false;
+
+    if (mSchedulerState & paintSchedulerPainting)
+    {
+	mSchedulerState |= paintSchedulerReschedule;
+	return false;
+    }
+
+    if (mSchedulerState & paintSchedulerScheduled)
+	return false;
+
+    mSchedulerState |= paintSchedulerScheduled;
+
+    if (mFPSLimiterMode == CompositeFPSLimiterModeVSyncLike ||
+	(mDispatchBase->schedulerHasVsync ()))
+    {
+	delay = 1;
+    }
+    else
+    {
+	struct timeval now;
+	gettimeofday (&now, 0);
+	int elapsed = compiz::core::timer::timeval_diff (&now, &mLastRedraw);
+	if (elapsed < 0)
+	    elapsed = 0;
+ 	delay = elapsed < mOptimalRedrawTime ? mOptimalRedrawTime - elapsed + 1 : 1;
+    }
+    /*
+     * Note the use of delay = 1 instead of 0, even though 0 would be better.
+     * A delay of zero is presently broken due to CompTimer bugs;
+     *    1. Infinite loop in CompTimeoutSource::callback when a zero
+     *       timer is set.
+     *    2. Priority set too high in CompTimeoutSource::CompTimeoutSource
+     *       causing the glib main event loop to be starved of X events.
+     * Fixes for both of these issues are being worked on separately.
+     */
+
+    mPaintTimer.start
+	(boost::bind (&compiz::composite::scheduler::PaintScheduler::dispatch, this),
+	delay);
+
+    return false;
+}
+
+int
+compiz::composite::scheduler::PaintScheduler::getRedrawTime ()
+{
+    return mRedrawTime;
+}
+
+int
+compiz::composite::scheduler::PaintScheduler::getOptimalRedrawTime ()
+{
+    return mOptimalRedrawTime;
+}
+
+bool
+compiz::composite::scheduler::PaintScheduler::dispatch ()
+{
+    struct timeval                  tv;
+    int                             timeDiff;
+
+    gettimeofday (&tv, 0);
+
+    timeDiff = compiz::core::timer::timeval_diff (&tv, &mLastRedraw);
+
+    /* handle clock rollback */
+
+    if (timeDiff < 0)
+	timeDiff = 0;  
+
+    /*
+     * Now that we use a "tickless" timing algorithm, timeDiff could be
+     * very large if the screen is truely idle.
+     * However plugins expect the old behaviour where timeDiff is never
+     * larger than the frame rate (optimalRedrawTime).
+     * So enforce this to keep animations timed correctly and smooth...
+     */
+
+    if (timeDiff > mOptimalRedrawTime &&
+	!(mSchedulerState & paintSchedulerReschedule))
+	timeDiff = mOptimalRedrawTime;
+
+    mSchedulerState |= paintSchedulerPainting;
+    mSchedulerState &= ~paintSchedulerReschedule;   
+
+    mRedrawTime = timeDiff;
+
+    mDispatchBase->prepareScheduledPaint (timeDiff);
+    mDispatchBase->paintScheduledPaint ();
+
+    mLastRedraw = tv;
+    
+    /* Throttle to the allowed refresh rate */
+    if (mFPSLimiterMode == CompositeFPSLimiterModeDefault)
+    {
+	if (mDispatchBase->schedulerHasVsync ())
+	{
+	    do
+	    {
+		/* If throttling is allowed, ensure that
+		 * timeDiff is always >= mOptimalRedrawTime
+		 * by waiting for another video-sync */
+		if (mDispatchBase->syncScheduledPaint ())
+		{
+		    struct timeval stv;
+		    gettimeofday (&stv, 0);
+		    timeDiff = compiz::core::timer::timeval_diff (&stv, &mLastRedraw);
+		}
+		else
+		    timeDiff = mOptimalRedrawTime;
+	    }
+	    while (timeDiff < mOptimalRedrawTime);
+	}
+    }
+    else if (mFPSLimiterMode == CompositeFPSLimiterModeVSyncLike)
+    {
+	if (mDispatchBase->schedulerHasVsync ())
+	    mDispatchBase->syncScheduledPaint ();
+    }
+
+    mDispatchBase->doneScheduledPaint ();
+    mSchedulerState &= ~(paintSchedulerPainting | paintSchedulerScheduled);
+
+    if (mSchedulerState & paintSchedulerReschedule)
+	schedule ();
+
+    return false;
+}
+
+void
+compiz::composite::scheduler::PaintScheduler::setRefreshRate (unsigned int rate)
+{
+    mOptimalRedrawTime = mRedrawTime =
+	static_cast <int> (1000 / static_cast <float> (rate));
+}
 
=== added file 'plugins/composite/src/paintscheduler.h'
--- plugins/composite/src/paintscheduler.h	1970-01-01 00:00:00 +0000
+++ plugins/composite/src/paintscheduler.h	2012-01-20 14:46:26 +0000
@@ -0,0 +1,96 @@
+/*
+ * Copyright © 2008 Dennis Kasprzyk
+ * Copyright © 2007 Novell, Inc.
+ *
+ * Permission to use, copy, modify, distribute, and sell this software
+ * and its documentation for any purpose is hereby granted without
+ * fee, provided that the above copyright notice appear in all copies
+ * and that both that copyright notice and this permission notice
+ * appear in supporting documentation, and that the name of
+ * Dennis Kasprzyk not be used in advertising or publicity pertaining to
+ * distribution of the software without specific, written prior permission.
+ * Dennis Kasprzyk makes no representations about the suitability of this
+ * software for any purpose. It is provided "as is" without express or
+ * implied warranty.
+ *
+ * DENNIS KASPRZYK DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN
+ * NO EVENT SHALL DENNIS KASPRZYK BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+ * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ * Authors: Dennis Kasprzyk <[e-mail]>
+ *          David Reveman <[e-mail]>
+ */
+
+#include <core/timer.h>
+#include <composite/fpslimiter.h>
+
+#ifndef _COMPIZ_COMPOSITE_PAINTSCHEDULER_H
+#define _COMPIZ_COMPOSITE_PAINTSCHEDULER_H
+
+namespace compiz
+{
+namespace composite
+{
+namespace scheduler
+{
+
+const unsigned int paintSchedulerScheduled = 1 << 0;
+const unsigned int paintSchedulerPainting = 1 << 1;
+const unsigned int paintSchedulerReschedule = 1 << 2;
+
+class PaintSchedulerDispatchBase
+{
+    public:
+
+	virtual void prepareScheduledPaint (unsigned int timeDiff) = 0;
+	virtual void paintScheduledPaint () = 0;
+	virtual bool syncScheduledPaint () = 0;
+	virtual void doneScheduledPaint () = 0;
+	virtual bool schedulerCompositingActive () = 0;
+	virtual bool schedulerHasVsync () = 0;
+};
+
+class PaintScheduler
+{
+    public:
+
+	PaintScheduler (PaintSchedulerDispatchBase *);
+	~PaintScheduler ();
+
+	bool schedule ();
+	void setFPSLimiterMode (CompositeFPSLimiterMode mode);
+	CompositeFPSLimiterMode getFPSLimiterMode ();
+
+	int getRedrawTime ();
+	int getOptimalRedrawTime ();
+
+	void setRefreshRate (unsigned int);
+
+    protected:
+
+	bool dispatch ();
+
+    private:
+
+	unsigned int   mSchedulerState;
+
+	struct timeval mLastRedraw;
+	int            mRedrawTime;
+	int            mOptimalRedrawTime;
+
+	CompositeFPSLimiterMode mFPSLimiterMode;
+
+	CompTimer mPaintTimer;
+
+	PaintSchedulerDispatchBase *mDispatchBase;
+};
+
+}
+}
+}
+
+#endif
 
=== modified file 'plugins/composite/src/privates.h'
--- plugins/composite/src/privates.h	2011-11-14 07:41:22 +0000
+++ plugins/composite/src/privates.h	2012-01-20 14:46:26 +0000
@@ -31,6 +31,8 @@
 #include <composite/composite.h>
 #include <core/atoms.h>
 
+#include "paintscheduler.h"
+
 #include "composite_options.h"
 
 #if COMPOSITE_MAJOR > 0 || COMPOSITE_MINOR > 2
@@ -41,17 +43,17 @@
 extern CompPlugin::VTable *compositeVTable;
 
 extern CompWindow *lastDamagedWindow;
+	
 
 class PrivateCompositeScreen :
     ScreenInterface,
+    public compiz::composite::scheduler::PaintSchedulerDispatchBase,
     public CompositeOptions
 {
     public:
 	PrivateCompositeScreen (CompositeScreen *cs);
 	~PrivateCompositeScreen ();
 
-	bool setOption (const CompString &name, CompOption::Value &value);
-
 	void outputChangeNotify ();
 
 	void handleEvent (XEvent *event);
@@ -64,7 +66,19 @@
 
 	void detectRefreshRate ();
 
-	void scheduleRepaint ();
+    protected:
+
+	void prepareScheduledPaint (unsigned int timeDiff);
+
+	void paintScheduledPaint ();
+
+	bool syncScheduledPaint ();
+
+	void doneScheduledPaint ();
+
+	bool schedulerCompositingActive ();
+
+	bool schedulerHasVsync ();
 
     public:
 
@@ -94,18 +108,10 @@
 
 	int overlayWindowCount;
 
-	struct timeval lastRedraw;
-	int            redrawTime;
-	int            optimalRedrawTime;
-	bool           scheduled, painting, reschedule;
-
 	bool slowAnimations;
 
-	CompTimer paintTimer;
-
-	compiz::composite::PaintHandler *pHnd;
-
-	CompositeFPSLimiterMode FPSLimiterMode;
+	compiz::composite::PaintHandler 	     *pHnd;
+	compiz::composite::scheduler::PaintScheduler scheduler;
 
 	CompWindowList withDestroyedWindows;
 };
 
=== modified file 'plugins/composite/src/screen.cpp'
--- plugins/composite/src/screen.cpp	2012-01-19 04:07:41 +0000
+++ plugins/composite/src/screen.cpp	2012-01-20 14:46:26 +0000
@@ -175,16 +175,6 @@
 		    }
 		}
 	    }
-	    else if (randrExtension &&
-		     event->type == randrEvent + RRScreenChangeNotify)
-	    {
-		XRRScreenChangeNotifyEvent *rre;
-
-		rre = (XRRScreenChangeNotifyEvent *) event;
-
-		if (screen->root () == rre->root)
-		    detectRefreshRate ();
-	    }
 	    break;
     }
 }
@@ -259,8 +249,6 @@
 
 CompositeScreen::~CompositeScreen ()
 {
-    priv->paintTimer.stop ();
-
 #ifdef USE_COW
     if (useCow)
 	XCompositeReleaseOverlayWindow (screen->dpy (),
@@ -279,21 +267,17 @@
     exposeRects (),
     windowPaintOffset (0, 0),
     overlayWindowCount (0),
-    redrawTime (1000 / 50),
-    optimalRedrawTime (1000 / 50),
-    scheduled (false),
-    painting (false),
-    reschedule (false),
     slowAnimations (false),
     pHnd (NULL),
-    FPSLimiterMode (CompositeFPSLimiterModeDefault),
+    scheduler (this),
     withDestroyedWindows ()
 {
-    gettimeofday (&lastRedraw, 0);
     // wrap outputChangeNotify
     ScreenInterface::setHandler (screen);
 
     optionSetSlowAnimationsKeyInitiate (CompositeScreen::toggleSlowAnimations);
+    optionSetRefreshRateCapNotify (boost::bind (&PrivateCompositeScreen::detectRefreshRate,
+						this));
 }
 
 PrivateCompositeScreen::~PrivateCompositeScreen ()
@@ -445,7 +429,6 @@
 				    CompositeRedirectManual);
 
     priv->pHnd = NULL;
-    priv->paintTimer.stop ();
 
     hideOutputWindow ();
 }
@@ -464,7 +447,7 @@
 {
     priv->damageMask |= COMPOSITE_SCREEN_DAMAGE_ALL_MASK;
     priv->damageMask &= ~COMPOSITE_SCREEN_DAMAGE_REGION_MASK;
-    priv->scheduleRepaint ();
+    priv->scheduler.schedule ();
 }
 
 void
@@ -483,14 +466,14 @@
 
     if (priv->damage.numRects () > 100)
        damageScreen ();
-    priv->scheduleRepaint ();
+    priv->scheduler.schedule ();
 }
 
 void
 CompositeScreen::damagePending ()
 {
     priv->damageMask |= COMPOSITE_SCREEN_DAMAGE_PENDING_MASK;
-    priv->scheduleRepaint ();
+    priv->scheduler.schedule ();
 }
 
 unsigned int
@@ -635,204 +618,145 @@
 void
 PrivateCompositeScreen::detectRefreshRate ()
 {
-    if (!noDetection &&
-	optionGetDetectRefreshRate ())
-    {
-	CompString        name;
-	CompOption::Value value;
-
-	value.set ((int) 0);
-
-	if (screen->XRandr ())
-	{
-	    XRRScreenConfiguration *config;
-
-	    config  = XRRGetScreenInfo (screen->dpy (),
-					screen->root ());
-	    value.set ((int) XRRConfigCurrentRate (config));
-
-	    XRRFreeScreenConfigInfo (config);
-	}
-
-	if (value.i () == 0)
-	    value.set ((int) 50);
-
-	mOptions[CompositeOptions::DetectRefreshRate].value ().set (false);
-	screen->setOptionForPlugin ("composite", "refresh_rate", value);
-	mOptions[CompositeOptions::DetectRefreshRate].value ().set (true);
-	optimalRedrawTime = redrawTime = 1000 / value.i ();
-    }
+    unsigned int rate = optionGetRefreshRateCap ();
+
+    if (rate)
+        scheduler.setRefreshRate (rate);
     else
-    {
-	redrawTime = 1000 / optionGetRefreshRate ();
-	optimalRedrawTime = redrawTime;
-    }
+	scheduler.setFPSLimiterMode (CompositeFPSLimiterModeVSyncLike);
 }
 
 CompositeFPSLimiterMode
 CompositeScreen::FPSLimiterMode ()
 {
-    return priv->FPSLimiterMode;
+    return priv->scheduler.getFPSLimiterMode ();
 }
 
 void
 CompositeScreen::setFPSLimiterMode (CompositeFPSLimiterMode newMode)
 {
-    priv->FPSLimiterMode = newMode;
-}
-
-void
-PrivateCompositeScreen::scheduleRepaint ()
-{
-    if (painting)
-    {
-	reschedule = true;
-	return;
-    }
-
-    if (scheduled)
-	return;
-
-    scheduled = true;
-
-    int delay;
-    if (FPSLimiterMode == CompositeFPSLimiterModeVSyncLike ||
-	(pHnd && pHnd->hasVSync ()))
-    {
-	delay = 1;
+    priv->scheduler.setFPSLimiterMode (newMode);
+}
+
+void
+PrivateCompositeScreen::prepareScheduledPaint (unsigned int timeDiff)
+{
+    if (pHnd)
+	pHnd->prepareDrawing ();
+
+    cScreen->preparePaint (slowAnimations ? 1 : timeDiff);
+}
+
+bool
+PrivateCompositeScreen::schedulerCompositingActive ()
+{
+    return pHnd ? pHnd->compositingActive () : false;
+}
+
+bool
+PrivateCompositeScreen::schedulerHasVsync ()
+{
+    return pHnd ? pHnd->hasVSync () : false;
+}
+
+void
+PrivateCompositeScreen::paintScheduledPaint ()
+{
+    int mask;
+
+    /* substract top most overlay window region */
+    if (overlayWindowCount)
+    {
+	for (CompWindowList::reverse_iterator rit =
+	     screen->windows ().rbegin ();
+	     rit != screen->windows ().rend (); rit++)
+        {
+	    CompWindow *w = (*rit);
+
+	    if (w->destroyed () || w->invisible ())
+		continue;
+
+	    if (!CompositeWindow::get (w)->redirected ())
+		damage -= w->region ();
+
+	    break;
+	}
+
+	if (damageMask & COMPOSITE_SCREEN_DAMAGE_ALL_MASK)
+	{
+	    damageMask &= ~COMPOSITE_SCREEN_DAMAGE_ALL_MASK;
+	    damageMask |= COMPOSITE_SCREEN_DAMAGE_REGION_MASK;
+	}
+    }
+
+    tmpRegion = damage & screen->region ();
+
+    if (damageMask & COMPOSITE_SCREEN_DAMAGE_REGION_MASK)
+    {
+	if (tmpRegion == screen->region ())
+	    cScreen->damageScreen ();
+    }
+
+    mask = damageMask;
+
+    damage = CompRegion ();
+    damageMask = 0;
+
+    CompOutput::ptrList outputs (0);
+
+    if (optionGetForceIndependentOutputPainting () ||
+	!screen->hasOverlappingOutputs ())
+    {
+	foreach (CompOutput &o, screen->outputDevs ())
+	    outputs.push_back (&o);
     }
     else
+	outputs.push_back (&screen->fullscreenOutput ());
+
+    cScreen->paint (outputs, mask);
+
+    damageMask = mask;
+}
+
+bool
+PrivateCompositeScreen::syncScheduledPaint ()
+{
+    if (pHnd)
+	return pHnd->waitVSync (damageMask);
+
+    return false;
+}
+
+void
+PrivateCompositeScreen::doneScheduledPaint ()
+{
+    if (pHnd)
+	pHnd->syncBuffers (damageMask);
+
+    damageMask = 0;
+
+    cScreen->donePaint ();
+
+    foreach (CompWindow *w, screen->windows ())
     {
-	struct timeval now;
-	gettimeofday (&now, 0);
-	int elapsed = compiz::core::timer::timeval_diff (&now, &lastRedraw);
-	if (elapsed < 0)
-	    elapsed = 0;
- 	delay = elapsed < optimalRedrawTime ? optimalRedrawTime - elapsed : 1;
+	if (w->destroyed ())
+	{
+	    CompositeWindow::get (w)->addDamage ();
+	    break;
+	}
     }
-
-    paintTimer.start
-	(boost::bind (&CompositeScreen::handlePaintTimeout, cScreen),
-	delay);
 }
 
 int
 CompositeScreen::redrawTime ()
 {
-    return priv->redrawTime;
+    return priv->scheduler.getRedrawTime ();
 }
 
 int
 CompositeScreen::optimalRedrawTime ()
 {
-    return priv->optimalRedrawTime;
-}
-
-bool
-CompositeScreen::handlePaintTimeout ()
-{
-    struct      timeval tv;
-
-    priv->painting = true;
-    priv->reschedule = false;
-    gettimeofday (&tv, 0);
-
-    if (priv->damageMask)
-    {
-	int         timeDiff;
-
-	if (priv->pHnd)
-	    priv->pHnd->prepareDrawing ();
-
-	timeDiff = compiz::core::timer::timeval_diff (&tv, &priv->lastRedraw);
-
-	/* handle clock rollback */
-	if (timeDiff < 0)
-	    timeDiff = 0;
-	/*
-	 * Now that we use a "tickless" timing algorithm, timeDiff could be
-	 * very large if the screen is truely idle.
-	 * However plugins expect the old behaviour where timeDiff is rarely
-	 * larger than the frame rate (optimalRedrawTime).
-	 * So enforce this to keep animations timed correctly and smooth...
-	 */
-	if (timeDiff > 100)
-	    timeDiff = priv->optimalRedrawTime;
-
-	priv->redrawTime = timeDiff;
-	preparePaint (priv->slowAnimations ? 1 : timeDiff);
-
-	/* substract top most overlay window region */
-	if (priv->overlayWindowCount)
-	{
-	    for (CompWindowList::reverse_iterator rit =
-		 screen->windows ().rbegin ();
-	         rit != screen->windows ().rend (); rit++)
-	    {
-		CompWindow *w = (*rit);
-
-		if (w->destroyed () || w->invisible ())
-		    continue;
-
-		if (!CompositeWindow::get (w)->redirected ())
-		    priv->damage -= w->region ();
-
-		break;
-	    }
-
-	    if (priv->damageMask & COMPOSITE_SCREEN_DAMAGE_ALL_MASK)
-	    {
-		priv->damageMask &= ~COMPOSITE_SCREEN_DAMAGE_ALL_MASK;
-		priv->damageMask |= COMPOSITE_SCREEN_DAMAGE_REGION_MASK;
-	    }
-	}
-
-	priv->tmpRegion = priv->damage & screen->region ();
-
-	if (priv->damageMask & COMPOSITE_SCREEN_DAMAGE_REGION_MASK)
-	{
-	    if (priv->tmpRegion == screen->region ())
-		damageScreen ();
-	}
-
-	priv->damage = CompRegion ();
-
-	int mask = priv->damageMask;
-	priv->damageMask = 0;
-
-	CompOutput::ptrList outputs (0);
-
-	if (priv->optionGetForceIndependentOutputPainting ()
-	    || !screen->hasOverlappingOutputs ())
-	{
-	    foreach (CompOutput &o, screen->outputDevs ())
-		outputs.push_back (&o);
-	}
-	else
-	    outputs.push_back (&screen->fullscreenOutput ());
-
-	paint (outputs, mask);
-
-
-	donePaint ();
-
-	foreach (CompWindow *w, screen->windows ())
-	{
-	    if (w->destroyed ())
-	    {
-		CompositeWindow::get (w)->addDamage ();
-		break;
-	    }
-	}
-    }
-
-    priv->lastRedraw = tv;
-    priv->painting = false;
-    priv->scheduled = false;
-    if (priv->reschedule)
-	priv->scheduleRepaint ();
-
-    return false;
+    return priv->scheduler.getOptimalRedrawTime ();
 }
 
 void
 
=== added directory 'plugins/composite/tests'
=== added file 'plugins/composite/tests/CMakeLists.txt'
--- plugins/composite/tests/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ plugins/composite/tests/CMakeLists.txt	2012-01-20 14:46:26 +0000
@@ -0,0 +1,1 @@
+add_subdirectory (paintscheduler)
 
=== added directory 'plugins/composite/tests/paintscheduler'
=== added file 'plugins/composite/tests/paintscheduler/CMakeLists.txt'
--- plugins/composite/tests/paintscheduler/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ plugins/composite/tests/paintscheduler/CMakeLists.txt	2012-01-20 14:46:26 +0000
@@ -0,0 +1,36 @@
+include (FindPkgConfig)
+
+pkg_check_modules (COMPIZ_PAINTSCHEDULER REQUIRED dri libdrm glibmm-2.4)
+
+find_library( DRM_LIBRARY drm PATHS /usr/lib/${CMAKE_LIBRARY_ARCHITECTURE} NO_DEFAULT_PATH )
+
+include_directories (${COMPIZ_INCLUDE_DIRS}
+		     ${COMPIZ_PAINTSCHEDULER_INCLUDE_DIRS}
+		     ${compiz_SOURCE_DIR}/include
+		     ${compiz_BINARY_DIR}
+		     ${compiz_BINARY_DIR}/generated
+		     ${compiz_SOURCE_DIR}/src
+		     ${compiz_SOURCE_DIR}/src/timer/src
+		     ${compiz_SOURCE_DIR}/src/timer/include
+		     ../../include
+		     ../../src)
+
+#message (${DRM_LIBRARY})
+
+# FIXME: Hardcoding drm like this is stupid
+# but due to a bug in CMake we have to do it
+add_executable (compiz_paintscheduler_test
+		../../src/paintscheduler.cpp
+		${compiz_SOURCE_DIR}/src/timer/src/timer.cpp
+		${compiz_SOURCE_DIR}/src/timer/src/timeouthandler.cpp
+		test-paintscheduler.cpp
+		test-paintscheduler-set-drmvblanktype.c)
+
+target_link_libraries (compiz_paintscheduler_test
+		       ${COMPIZ_LIBRARIES} m pthread dl ${DRM_LIBRARY})
+
+set_target_properties (compiz_paintscheduler_test PROPERTIES
+			LINK_FLAGS "-L/usr/lib/${CMAKE_LIBRARY_ARCHITECTURE}")
+
+add_test (test-paintscheduler
+	  ${CMAKE_CURRENT_BINARY_DIR}/compiz_paintscheduler_test)
 
=== added file 'plugins/composite/tests/paintscheduler/test-paintscheduler-set-drmvblanktype.c'
--- plugins/composite/tests/paintscheduler/test-paintscheduler-set-drmvblanktype.c	1970-01-01 00:00:00 +0000
+++ plugins/composite/tests/paintscheduler/test-paintscheduler-set-drmvblanktype.c	2012-01-20 14:46:26 +0000
@@ -0,0 +1,6 @@
+#include "test-paintscheduler-set-drmvblanktype.h"
+
+void setDRMVBlankType (drmVBlankSeqType *t, unsigned int f)
+{
+    *t = f;
+}
 
=== added file 'plugins/composite/tests/paintscheduler/test-paintscheduler-set-drmvblanktype.h'
--- plugins/composite/tests/paintscheduler/test-paintscheduler-set-drmvblanktype.h	1970-01-01 00:00:00 +0000
+++ plugins/composite/tests/paintscheduler/test-paintscheduler-set-drmvblanktype.h	2012-01-20 14:46:26 +0000
@@ -0,0 +1,4 @@
+#include <xf86drm.h>
+
+void
+setDRMVBlankType (drmVBlankSeqType *t, unsigned int f);
 
=== added file 'plugins/composite/tests/paintscheduler/test-paintscheduler.cpp'
--- plugins/composite/tests/paintscheduler/test-paintscheduler.cpp	1970-01-01 00:00:00 +0000
+++ plugins/composite/tests/paintscheduler/test-paintscheduler.cpp	2012-01-20 14:46:26 +0000
@@ -0,0 +1,702 @@
+/*
+ * Copyright © 2011 Canonical Ltd.
+ *
+ * Permission to use, copy, modify, distribute, and sell this software
+ * and its documentation for any purpose is hereby granted without
+ * fee, provided that the above copyright notice appear in all copies
+ * and that both that copyright notice and this permission notice
+ * appear in supporting documentation, and that the name of
+ * Dennis Kasprzyk not be used in advertising or publicity pertaining to
+ * distribution of the software without specific, written prior permission.
+ * Dennis Kasprzyk makes no representations about the suitability of this
+ * software for any purpose. It is provided "as is" without express or
+ * implied warranty.
+ *
+ * DENNIS KASPRZYK DISCLAIMS ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN
+ * NO EVENT SHALL DENNIS KASPRZYK BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS
+ * OF USE, DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT,
+ * NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION
+ * WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ * drm_vblank handling code adapted from libdrm
+ * Copyright 2008 Tungsten Graphics
+ *   Jakob Bornecrantz <[e-mail]>
+ * Copyright 2008 Intel Corporation
+ *   Jesse Barnes <[e-mail]>
+ *
+ * Authors: Sam Spilsbury <[e-mail]>
+ */
+
+#include <iostream>
+#include <string>
+#include <glib.h>
+#include <core/timeouthandler.h>
+#include <paintscheduler.h>
+#include <privatetimeouthandler.h>
+#include <privatetimer.h>
+#include <privatetimeoutsource.h>
+
+#include <assert.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <unistd.h>
+#include <string.h>
+#include <errno.h>
+#include <sys/poll.h>
+#include <sys/time.h>
+
+#include <cmath>
+
+extern "C"
+{
+#include "test-paintscheduler-set-drmvblanktype.h"
+}
+#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
+
+using namespace compiz::composite::scheduler;
+
+class DummyPaintDispatch;
+
+void pass (const std::string &message)
+{
+    std::cout << "PASS: " << message << std::endl;
+}
+
+void fail (const std::string &message)
+{
+    std::cout << "FAIL: " << message << std::endl;
+    exit (1);
+}
+
+class Worker
+{
+    public:
+
+	/* Amount of time it takes to complete some work in ms */
+	static void setWorkFactor (unsigned int workFactor) { mWorkFactor = workFactor; }
+	static void work ()
+	{
+	    struct timespec ts;
+
+	    ts.tv_sec = 0;
+	    ts.tv_nsec = 1000000 * mWorkFactor;
+
+	    nanosleep (&ts, NULL);
+	}
+
+    private:
+
+	static unsigned int mWorkFactor;
+};
+
+unsigned int Worker::mWorkFactor = 0;
+
+class VBlankWaiter
+{
+    public:
+
+	typedef timeval time_value;
+
+	VBlankWaiter (DummyPaintDispatch *owner, unsigned int n);
+	virtual ~VBlankWaiter ();
+
+	virtual bool waitVBlank () = 0;
+	virtual bool hasVSync () = 0;
+
+	bool checkTimings (float req, float threshold);
+	float averagePeriod ();
+	float averagePhase ();
+
+	bool addPaintStart (unsigned int time);
+	void addPaintDone (const VBlankWaiter::time_value &);
+
+    protected:
+
+	void addVBlank (const VBlankWaiter::time_value &);
+
+	std::vector <float> periods;
+	std::vector <VBlankWaiter::time_value> paintPeriods;
+	std::vector <VBlankWaiter::time_value> blankPeriods;
+	unsigned int        timingCount;
+	unsigned int	    paintCount;
+	DummyPaintDispatch  *owner;
+};
+
+class DRMVBlankWaiter :
+    public VBlankWaiter
+{
+    public:
+
+	DRMVBlankWaiter (DummyPaintDispatch *owner, unsigned int n);
+	~DRMVBlankWaiter ();
+
+	bool waitVBlank ();
+	bool hasVSync ();
+
+	class DRMInfo
+	{
+	    public:
+
+		DRMInfo (DRMVBlankWaiter *w) :
+		    self (w)
+		{
+		}
+
+		DRMVBlankWaiter *self;
+	};
+
+    protected:
+
+	static gpointer
+	drmEventHandlerThread (gpointer);
+
+	static void
+	vblankHandler (int          fd,
+		       unsigned int frame,
+		       unsigned int sec,
+		       unsigned int usec,
+		       void         *data);
+    private:
+
+	drmVBlank       mVbl;
+	drmEventContext evctx;
+	int             drmFD;
+	DRMInfo         *info;
+	GThread    	*eventThread;
+	GMutex		eventMutex;
+	GCond		eventCondition;
+	bool            pendingVBlank;
+};
+
+class SleepVBlankWaiter :
+    public VBlankWaiter
+{
+    public:
+
+	SleepVBlankWaiter (DummyPaintDispatch *, unsigned int);
+	~SleepVBlankWaiter ();
+
+	bool waitVBlank ();
+	bool hasVSync () { return true; }
+
+	static VBlankWaiter::time_value start_time;
+
+    private:
+
+	const static int vblank_ms_period = 60;
+};
+
+VBlankWaiter::time_value SleepVBlankWaiter::start_time;
+
+class NilVBlankWaiter :
+    public VBlankWaiter
+{
+    public:
+
+	NilVBlankWaiter (DummyPaintDispatch *, unsigned int);
+	~NilVBlankWaiter ();
+
+	bool waitVBlank ();
+
+	bool hasVSync () { return false; }
+};
+
+class DummyPaintDispatch :
+    public PaintSchedulerDispatchBase
+{
+    public:
+
+	DummyPaintDispatch (Glib::RefPtr <Glib::MainLoop> loop);
+
+	void prepareScheduledPaint (unsigned int timeDiff)
+	{
+	    success = waiter->addPaintStart (timeDiff);
+	}
+
+	void paintScheduledPaint ()
+	{
+	    Worker::work ();
+	}
+
+	bool syncScheduledPaint ()
+	{
+	    if (waiter && !isDone ())
+		return waiter->waitVBlank ();
+	    else
+		return false;
+	}
+
+	void doneScheduledPaint ()
+	{
+	    VBlankWaiter::time_value tv;
+	    gettimeofday (&tv, NULL);
+
+	    waiter->addPaintDone (tv);
+
+	    if (waiter && success)
+		sched.schedule ();
+	    else
+		ml->quit ();
+	}
+
+	bool schedulerCompositingActive ()
+	{
+	    return true;
+	}
+
+	bool schedulerHasVsync ()
+	{
+	    if (isDone ())
+		return false;
+
+	    if (waiter)
+		return waiter->hasVSync ();
+
+	    return true;
+	}
+
+	void setWaiter (VBlankWaiter *w)
+	{
+	    if (waiter)
+		delete waiter;
+
+	    waiter = w;
+	}
+
+	void setRefreshRate (unsigned int r)
+	{
+	    sched.setRefreshRate (r);
+	}
+
+	bool isDone () { return !(waiter && success); }
+
+    private:
+
+	PaintScheduler sched;
+	VBlankWaiter   *waiter;
+	bool           success;
+	Glib::RefPtr <Glib::MainLoop> ml;
+};
+
+float
+VBlankWaiter::averagePeriod ()
+{
+    float buf = 0.0f;
+
+    for (std::vector <float>::iterator it = periods.begin ();
+	 it != periods.end (); it++)
+    {
+	buf += (*it);
+    }
+
+    buf = buf / (periods.size ());
+
+    return buf;
+}
+
+/* Returns false if a value exists outside
+ * the threshold */
+bool
+VBlankWaiter::checkTimings (float req, float threshold)
+{
+    std::sort (periods.begin (), periods.end ());
+
+    float median = periods[periods.size () / 2];
+    std::vector <float> deviationPeriods (periods.size ());
+
+    unsigned int i = 0;
+
+    for (std::vector <float>::iterator it = periods.begin ();
+	it != periods.end (); it++, i++)
+	deviationPeriods[i] = fabsf ((*it) - median);
+
+    std::sort (deviationPeriods.begin (), deviationPeriods.end ());
+
+    float medianDeviation = deviationPeriods[deviationPeriods.size () / 2];
+
+    std::cout << "DEBUG: median absolute deviation of the periods was " << medianDeviation << std::endl;
+
+    if (medianDeviation > threshold)
+	return false;
+
+    /* No phase checks if there is no vblanking */
+    if (hasVSync ())
+    {
+
+	/* The phase needs to be sensitive to outliers
+	* so use the standard deviation with an acceptable
+	* deviation of 1 */
+
+	std::vector <VBlankWaiter::time_value>::iterator pit = paintPeriods.begin ();
+	std::vector <VBlankWaiter::time_value>::iterator vit = blankPeriods.begin ();
+	float sum = 0.0f;
+	float mean = 0.0f;
+
+	/* Check if any blank periods fall outside of -4ms
+	* of the paint period, that means we're out of phase. */
+
+	for (; vit != blankPeriods.end (); vit++, pit++)
+	    sum += ((pit->tv_usec - vit->tv_usec) / 1000.0f);
+
+	mean = sum / blankPeriods.size ();
+
+	pit = paintPeriods.begin ();
+	vit = blankPeriods.begin ();
+
+	sum = 0.0f;
+
+	for (; vit != blankPeriods.end (); vit++, pit++)
+	    sum += pow (((pit->tv_usec - vit->tv_usec) / 1000.0f) - mean, 2);
+
+	sum /= blankPeriods.size ();
+
+	std::cout << "DEBUG: st. dev of the phases was " << sqrt (sum) << std::endl;
+
+	if (sqrt (sum) > 0.025f)
+	    return false;
+    }
+
+    return true;
+}
+
+bool
+VBlankWaiter::addPaintStart (unsigned int time)
+{
+    if (timingCount < periods.size ())
+	periods[timingCount]= time;
+    else
+	return false;
+
+    ++timingCount;
+
+    return true;
+}
+
+void
+VBlankWaiter::addPaintDone (const VBlankWaiter::time_value &tv)
+{
+    if (paintCount < paintPeriods.size ())
+        paintPeriods[paintCount] = tv;
+    ++paintCount;
+}
+
+void
+VBlankWaiter::addVBlank (const VBlankWaiter::time_value &tv)
+{
+    /* There may be multiple vblanks per paint
+     * because of throttling, so use the last slot
+     * available for the paint period */
+    if (paintCount < blankPeriods.size ())
+	blankPeriods[paintCount] = tv;
+}
+
+VBlankWaiter::VBlankWaiter (DummyPaintDispatch *o, unsigned int n) :
+    periods (n),
+    paintPeriods (n),
+    blankPeriods (n),
+    timingCount (0),
+    paintCount (0),
+    owner (o)
+{
+}
+
+VBlankWaiter::~VBlankWaiter  ()
+{
+}
+
+SleepVBlankWaiter::SleepVBlankWaiter (DummyPaintDispatch *o, unsigned int n) :
+    VBlankWaiter (o, n)
+{
+}
+
+SleepVBlankWaiter::~SleepVBlankWaiter ()
+{
+}
+
+bool
+SleepVBlankWaiter::waitVBlank ()
+{
+    /* Wait until the next interval of 16ms */
+    VBlankWaiter::time_value tv;
+    gettimeofday (&tv, NULL);
+
+    unsigned long usecIntervalWait = 16000 - (tv.tv_usec % 16000);
+
+    struct timespec slp;
+
+    slp.tv_sec = 0;
+    slp.tv_nsec = usecIntervalWait * 1000;
+
+    nanosleep (&slp, NULL);
+
+    gettimeofday (&tv, NULL);
+
+    addVBlank (tv);
+
+    return timingCount < periods.size ();
+}
+
+void
+DRMVBlankWaiter::vblankHandler (int fd, unsigned int frame, unsigned int sec, unsigned int usc, void *data)
+{
+    drmVBlank vbl;
+    DRMInfo   *info = static_cast <DRMInfo *> (data);
+
+    setDRMVBlankType (&vbl.request.type, DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT);
+    vbl.request.sequence = 1;
+    vbl.request.signal = (unsigned long) data;
+
+    drmWaitVBlank (fd, &vbl);
+
+    g_mutex_lock (&info->self->eventMutex);
+
+    if (info->self->timingCount != info->self->periods.size ())
+	info->self->pendingVBlank = true;
+
+    g_cond_signal (&info->self->eventCondition);
+    g_mutex_unlock (&info->self->eventMutex);
+}
+
+gpointer
+DRMVBlankWaiter::drmEventHandlerThread (gpointer data)
+{
+    DRMVBlankWaiter *self = static_cast <DRMVBlankWaiter *> (data);
+
+    while (!self->owner->isDone ())
+    {
+	int    ret;
+	struct pollfd pfd;
+
+	pfd.fd = self->drmFD;
+	pfd.events = POLLIN | POLLHUP;
+	pfd.revents = 0;
+
+	ret = poll (&pfd, 1, -1);
+
+	if (ret <= 0)
+	{
+	    std::cout << "DEBUG: poll () failed" << std::endl;
+	    perror ("poll");
+	    return NULL;
+	}
+
+	ret = drmHandleEvent (self->drmFD, &self->evctx);
+	if (ret != 0)
+	{
+	    drmError (ret, "DRMVBlankWaiter::setupEventContextThread");
+	    return NULL;
+	}
+    }
+
+    return NULL;
+}
+
+DRMVBlankWaiter::DRMVBlankWaiter (DummyPaintDispatch *o, unsigned int n) :
+    VBlankWaiter (o, n),
+    info (new DRMInfo (this)),
+    pendingVBlank (false)
+{
+    const char  *modules[] = { "i915", "radeon", "nouveau", "vmwgfx" };
+
+    g_mutex_init (&eventMutex);
+    g_cond_init  (&eventCondition);
+
+    for (unsigned int i = 0; i < ARRAY_SIZE (modules); i++)
+    {
+	std::cout << "DEBUG: attempting to load platform " << modules[i];
+	drmFD = drmOpen (modules[i], NULL);
+
+	if (drmFD < 0)
+	    std::cout << " ... failed" << std::endl;
+	else
+	{
+	    std::cout << " ... success" << std::endl;;
+	    break;
+	}
+    }
+
+    if (drmFD < 0)
+	throw std::exception ();
+
+    memset (&mVbl, 0, sizeof (drmVBlank));
+    mVbl.request.type = DRM_VBLANK_RELATIVE;
+    mVbl.request.sequence = 0;
+
+    int ret = drmWaitVBlank (drmFD, &mVbl);
+
+    if (ret != 0)
+    {
+	drmError (ret, "DRMVBlankWaiter::DRMVBlankWaiter (RELATIVE)");
+	throw std::exception ();
+    }
+
+    setDRMVBlankType (&mVbl.request.type, DRM_VBLANK_RELATIVE | DRM_VBLANK_EVENT);
+    mVbl.request.sequence = 1;
+    mVbl.request.signal = (unsigned long) info;
+
+    ret = drmWaitVBlank (drmFD, &mVbl);
+
+    if (ret != 0)
+    {
+	drmError (ret, "DRMVBlankWaiter::DRMVBlankWaiter (RELATIVE EVENT)");
+	throw std::exception ();
+    }
+
+    memset (&evctx, 0, sizeof (drmEventContext));
+    evctx.version = DRM_EVENT_CONTEXT_VERSION;
+    evctx.vblank_handler = DRMVBlankWaiter::vblankHandler;
+    evctx.page_flip_handler = NULL;
+
+    eventThread = g_thread_try_new ("drm_wait_t", &DRMVBlankWaiter::drmEventHandlerThread, (void *) this, NULL);
+
+    if (!eventThread)
+    {
+	std::cout << "DEBUG: could not create thread" << std::endl;
+	throw std::exception ();
+    }
+}
+
+DRMVBlankWaiter::~DRMVBlankWaiter ()
+{
+    g_thread_join (eventThread);
+
+    delete info;
+
+    drmClose (drmFD);
+}
+
+NilVBlankWaiter::NilVBlankWaiter (DummyPaintDispatch *o, unsigned int n) :
+    VBlankWaiter (o, n)
+{
+}
+
+bool
+NilVBlankWaiter::waitVBlank ()
+{
+    /* Just add a new "vblank" */
+    VBlankWaiter::time_value tv;
+    gettimeofday (&tv, NULL);
+
+    addVBlank (tv);
+
+    return timingCount < periods.size ();
+}
+
+
+NilVBlankWaiter::~NilVBlankWaiter ()
+{
+}
+
+bool
+DRMVBlankWaiter::hasVSync ()
+{
+    return true;
+}
+
+bool
+DRMVBlankWaiter::waitVBlank ()
+{
+    VBlankWaiter::time_value tv;
+
+    g_mutex_lock (&eventMutex);
+    if (!pendingVBlank && !owner->isDone ())
+	g_cond_wait (&eventCondition, &eventMutex);
+
+    pendingVBlank = false;
+    g_mutex_unlock (&eventMutex);
+
+    gettimeofday (&tv, NULL);
+
+    addVBlank (tv);
+
+    return true;
+}
+
+DummyPaintDispatch::DummyPaintDispatch (Glib::RefPtr <Glib::MainLoop> mainloop) :
+    sched (this),
+    waiter (NULL),
+    success (true),
+    ml (mainloop)
+{
+    sched.schedule ();
+}
+
+bool doTest (const std::string &testName, int refreshRate, float workFactor, bool vsync)
+{
+    TimeoutHandler     *th = new TimeoutHandler ();
+    TimeoutHandler::SetDefault (th);
+
+    Glib::RefPtr <Glib::MainContext> ctx = Glib::MainContext::get_default ();
+    Glib::RefPtr <Glib::MainLoop> mainloop = Glib::MainLoop::create (ctx, false);
+    Glib::RefPtr <CompTimeoutSource> timeout = CompTimeoutSource::create (ctx);
+
+    DummyPaintDispatch *dpb = new DummyPaintDispatch (mainloop);
+    VBlankWaiter       *vbwaiter = NULL;
+
+    if (vsync)
+    {
+	try
+	{
+	    vbwaiter = new DRMVBlankWaiter (dpb, 200);
+	}
+	catch (std::exception &e)
+	{
+	    std::cout << "WARN: can't test DRM vblanking! Using hardcoded nanosleep () based waiter" << std::endl;
+	    vbwaiter = new SleepVBlankWaiter (dpb, 200);
+	}
+    }
+    else
+	vbwaiter = new NilVBlankWaiter (dpb, 200);
+
+    dpb->setWaiter (vbwaiter);
+    dpb->setRefreshRate (refreshRate);
+
+    Worker::setWorkFactor ((1000 / refreshRate) * workFactor);
+
+    std::cout << "INFO: " << testName << " with refresh rate of " << refreshRate << "Hz and work factor of " << workFactor << (vsync ? " with vertical sync" : " without vertical sync") << std::endl;
+
+    mainloop->run ();
+
+    float averagePeriod = vbwaiter->averagePeriod ();
+
+    std::cout << "DEBUG: average vblank wait time was " << averagePeriod << std::endl;
+    std::cout << "DEBUG: average frame rate " << 1000 / averagePeriod << " Hz" << std::endl;
+    std::cout << "TEST: " << testName << " time " << averagePeriod << " within threshold of " << 10.0f << std::endl;
+
+    if (vbwaiter->checkTimings (averagePeriod, 10.0f) &&
+	1000 / averagePeriod < refreshRate)
+	pass (testName);
+    else
+	fail (testName);
+
+    dpb->setWaiter (NULL);
+
+    delete dpb;
+    delete th;
+
+    return true;
+}
+   
+int main (void)
+{
+    gettimeofday (&SleepVBlankWaiter::start_time, NULL);
+
+    doTest ("unthrottled vblank timings", 60, 0.0f, true);
+    doTest ("no vsync 60 Hz refresh rate", 60, 0.0f, false);
+    doTest ("unthrottled vblank timings", 60, 0.8f, true);
+    doTest ("no vsync 60 Hz refresh rate", 60, 0.8f, false);
+    doTest ("vsync 50 Hz refresh rate", 50, 0.0f, true);
+    doTest ("no vsync 20 Hz refresh rate", 30, 0.0f,false);
+    doTest ("vsync 50 Hz refresh rate", 50, 1.6f, true);
+    doTest ("no vsync 20 Hz refresh rate", 30, 1.6f,false);
+    doTest ("vsync 30 Hz refresh rate", 30, 0.0f, true);
+    doTest ("no vsync 100 Hz refresh rate", 100, 0.0f, false);
+    doTest ("vsync 100 Hz refresh rate", 100, 0.0f, true);
+    doTest ("no vsync 100 Hz refresh rate", 100, 2.8f, false);
+    doTest ("vsync 100 Hz refresh rate", 100, 2.8f, true);
+
+    return 0;
+}
 
=== modified file 'plugins/opengl/include/opengl/opengl.h'
--- plugins/opengl/include/opengl/opengl.h	2012-01-12 06:48:58 +0000
+++ plugins/opengl/include/opengl/opengl.h	2012-01-20 14:46:26 +0000
@@ -102,6 +102,8 @@
     typedef int (*GLXWaitVideoSyncProc) (int	  divisor,
 					 int	  remainder,
 					 unsigned int *count);
+    typedef int (*GLXSwapIntervalProc) (int interval);
+
 
     #ifndef GLX_VERSION_1_3
     typedef struct __GLXFBConfigRec *GLXFBConfig;
@@ -165,6 +167,7 @@
     extern GLXCopySubBufferProc     copySubBuffer;
     extern GLXGetVideoSyncProc      getVideoSync;
     extern GLXWaitVideoSyncProc     waitVideoSync;
+    extern GLXSwapIntervalProc      swapInterval;
     extern GLXGetFBConfigsProc      getFBConfigs;
     extern GLXGetFBConfigAttribProc getFBConfigAttrib;
     extern GLXCreatePixmapProc      createPixmap;
 
=== modified file 'plugins/opengl/src/privates.h'
--- plugins/opengl/src/privates.h	2012-01-19 18:12:31 +0000
+++ plugins/opengl/src/privates.h	2012-01-20 14:46:26 +0000
@@ -67,6 +67,9 @@
 			   const CompRegion    &region);
 
 	bool hasVSync ();
+	bool waitVSync (unsigned int mask);
+
+	void syncBuffers (unsigned int        mask);
 
 	void prepareDrawing ();
 
@@ -115,6 +118,7 @@
 	GLXContext ctx;
 
 	CompRegion outputRegion;
+	CompRegion tmpRegion;
 
 	bool pendingCommands;
 
 
=== modified file 'plugins/opengl/src/screen.cpp'
--- plugins/opengl/src/screen.cpp	2012-01-12 06:48:58 +0000
+++ plugins/opengl/src/screen.cpp	2012-01-20 14:46:26 +0000
@@ -31,15 +31,13 @@
 #include <math.h>
 
 namespace GL {
-    typedef int (*GLXSwapIntervalProc) (int interval);
-
-    GLXSwapIntervalProc      swapInterval = NULL;
     GLXBindTexImageProc      bindTexImage = NULL;
     GLXReleaseTexImageProc   releaseTexImage = NULL;
     GLXQueryDrawableProc     queryDrawable = NULL;
     GLXCopySubBufferProc     copySubBuffer = NULL;
     GLXGetVideoSyncProc      getVideoSync = NULL;
     GLXWaitVideoSyncProc     waitVideoSync = NULL;
+    GLXSwapIntervalProc      swapInterval = NULL;
     GLXGetFBConfigsProc      getFBConfigs = NULL;
     GLXGetFBConfigAttribProc getFBConfigAttrib = NULL;
     GLXCreatePixmapProc      createPixmap = NULL;
@@ -1081,6 +1079,7 @@
 waitForVideoSync ()
 {
     GL::unthrottledFrames++;
+    // Docs: http://www.opengl.org/registry/specs/SGI/video_sync.txt
     if (GL::waitVideoSync)
     {
 	// Don't wait twice. Just in case.
@@ -1122,13 +1121,6 @@
 } // namespace GL
 
 void
-PrivateGLScreen::waitForVideoSync ()
-{
-    if (optionGetSyncToVblank ())
-        GL::waitForVideoSync ();
-}
-
-void
 PrivateGLScreen::paintOutputs (CompOutput::ptrList &outputs,
 			       unsigned int        mask,
 			       const CompRegion    &region)
@@ -1141,7 +1133,7 @@
 	    glClear (GL_COLOR_BUFFER_BIT);
     }
 
-    CompRegion tmpRegion (region);
+    tmpRegion = region;
 
     foreach (CompOutput *output, outputs)
     {
@@ -1196,7 +1188,11 @@
     }
 
     targetOutput = &screen->outputDevs ()[0];
+}
 
+bool
+PrivateGLScreen::waitVSync (unsigned int mask)
+{
     if (mask & COMPOSITE_SCREEN_DAMAGE_ALL_MASK)
     {
 	/*
@@ -1205,16 +1201,27 @@
 	 * Unfortunately it only works with glXSwapBuffers in most drivers.
 	 */
 	GL::controlSwapVideoSync (optionGetSyncToVblank ());
+	return false;
+    }
+    else
+    {
+	GL::waitForVideoSync ();
+	return true;
+    }
+}
+
+void
+PrivateGLScreen::syncBuffers (unsigned int        mask)
+{
+    if (mask & COMPOSITE_SCREEN_DAMAGE_ALL_MASK)
+    {
 	glXSwapBuffers (screen->dpy (), cScreen->output ());
     }
     else
     {
-	BoxPtr pBox;
-	int    nBox, y;
-
-	waitForVideoSync ();
-	pBox = const_cast <Region> (tmpRegion.handle ())->rects;
-	nBox = const_cast <Region> (tmpRegion.handle ())->numRects;
+	BoxPtr pBox = const_cast <Region> (tmpRegion.handle ())->rects;
+	int    nBox = const_cast <Region> (tmpRegion.handle ())->numRects;
+	int    y;
 
 	if (GL::copySubBuffer)
 	{
 
=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt	2012-01-18 17:05:12 +0000
+++ src/CMakeLists.txt	2012-01-20 14:46:26 +0000
@@ -59,7 +59,7 @@
     ${CORE_MOD_LIBRARY_DIRS}
 )
 
-add_library (compiz_core
+add_library (compiz_core SHARED
 	${CMAKE_CURRENT_SOURCE_DIR}/global.cpp
 	${CMAKE_CURRENT_SOURCE_DIR}/region.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/atoms.cpp
@@ -98,7 +98,7 @@
 	      PROPERTY CORE_MOD_LIBRARIES)
 
 target_link_libraries (
-    compiz_core 
+    compiz_core
     
     ${COMPIZ_LIBRARIES} 
     
@@ -115,6 +115,7 @@
 target_link_libraries (
     compiz 
     compiz_core
+    
     ${COMPIZ_LIBRARIES} 
     
     m 
@@ -128,6 +129,13 @@
 )
 
 install (
+    TARGETS compiz_core
+    RUNTIME DESTINATION ${COMPIZ_DESTDIR}${exec_prefix}
+  	LIBRARY DESTINATION ${COMPIZ_DESTDIR}${libdir}
+  	ARCHIVE DESTINATION ${COMPIZ_DESTDIR}${libdir}
+)
+
+install (
     TARGETS compiz
     DESTINATION ${COMPIZ_DESTDIR}${exec_prefix}
 )
 
=== modified file 'src/screen.cpp'
--- src/screen.cpp	2012-01-19 18:12:31 +0000
+++ src/screen.cpp	2012-01-20 14:46:26 +0000
@@ -791,15 +791,27 @@
 	    pListCount++;
     }
 
+	CompString lpName;
+	
     foreach (CompOption::Value &lp, list)
     {
 	bool skip = false;
-	if (lp.s () == "core")
+	
+	try 
+	{
+		lpName = lp.s();
+	} catch(const boost::bad_get&)
+	{
+		lpName.clear();		
+	}
+	
+	
+	if (lpName == "core")
 	    continue;
 
 	foreach (CompString &p, initialPlugins)
 	{
-	    if (p == lp.s ())
+	    if (p == lpName)
 	    {
 		skip = true;
 		break;
@@ -839,20 +851,39 @@
     {
 	std::list <CompString>::iterator it = initialPlugins.begin ();
 	bool				 skip = false;
-	if (opt.s () == "core")
-	    continue;
+	
+	try 
+	{
+		if (opt.s () == "core")
+	 	   continue;
+	} catch(const boost::bad_get&)
+	{
+	}
 
 	for (; it != initialPlugins.end (); it++)
 	{
-	    if ((*it) == opt.s ())
-	    {
-		skip = true;
-		break;
-	    }
+	        try
+		{
+			if ((*it) == opt.s())
+			{
+			    skip = true;
+			    break;
+			}
+		 } catch (...)
+		 {
+		 }
 	}
 
 	if (!skip)
-	    pList.at (j++).set (opt.s ());
+	{
+		try
+		{
+		    pList.at (j++).set (opt.s ());
+		} catch(const boost::bad_get&)
+		{
+		    j++;
+		}
+	}
     }
 
     assert (j == pList.size ());
@@ -902,7 +933,13 @@
 
 	if (p == 0 && !failedPush)
 	{
-	    p = CompPlugin::load (pList[i].s ().c_str ());
+	    try
+	    {
+		p = CompPlugin::load (pList[i].s ().c_str ());
+	    } catch (...)
+	    {
+	    }
+	    
 	    if (p)
 	    {
 		if (!CompPlugin::push (p))