Merge lp:~vanvugt/compiz-core/fix-880707.2 into lp:compiz-core/0.9.5

Proposed by Daniel van Vugt
Status: Merged
Approved by: Sam Spilsbury
Approved revision: 2903
Merged at revision: 2915
Proposed branch: lp:~vanvugt/compiz-core/fix-880707.2
Merge into: lp:compiz-core/0.9.5
Diff against target: 408 lines (+89/-150)
3 files modified
plugins/composite/src/privates.h (+2/-7)
plugins/composite/src/screen.cpp (+63/-131)
plugins/opengl/src/screen.cpp (+24/-12)
To merge this branch: bzr merge lp:~vanvugt/compiz-core/fix-880707.2
Reviewer Review Type Date Requested Status
Sam Spilsbury Approve
Compiz Maintainers Pending
Tim Penhey Pending
Review via email: mp+83472@code.launchpad.net

This proposal supersedes a proposal from 2011-11-14.

Commit message

Fix inaccurate frame timing causing tearing and stuttering (LP: #880707) (LP: #92599) (LP: #798868) (LP: #876575) (LP: #755841) (LP: #891744)

Description of the change

Introduced "tickless" frame timing to the composite plugin. This means we no longer need to poll for repaints. Composite only wakes up when a repaint is required and it's the right time to do so. This not only improves vsync accuracy, but also reduces CPU usage and power consumption.

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote : Posted in a previous version of this proposal

> bool scheduled, painting, reschedule;

Many coding standards say only one variable declaration per line. I'm not entirely sure what the compiz-core one says.

Also, please prefer initializer lists to setting variables in the constructor.

+ scheduled = false;
+ painting = false;
+ reschedule = false;

These should all be in the initializer list.

review: Needs Fixing
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

I disagree...

That file already contains multiple variables per line. So I am not violating the existing standard.

And I tried using initializer lists, but gcc gave warnings about variable initialization order. Those =false lines were the only way I could make the warnings go away.

Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

222 + /*
223 + * Note the use of delay = 1 instead of 0, even though 0 would be better.
224 + * A delay of zero is presently broken due to CompTimer bugs;
225 + * 1. Infinite loop in CompTimeoutSource::callback when a zero
226 + * timer is set.
227 + * 2. Priority set too high in CompTimeoutSource::CompTimeoutSource
228 + * causing the glib main event loop to be starved of X events.'

Just use the delay of zero and mark the other branches as prerequisites to this one :)

Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

Oh, it might also be good to get this code into unit tests, since regressions that could be introduced on frame timing would end up very difficult to debug down the road.

Feel free to send me an email about it, and I'll work on it next week. Thanks for all this, its excellent :)

Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

> Just use the delay of zero and mark the other branches as prerequisites to
> this one :)

I tested a delay of zero. While it made a huge difference in the old composite code (and exposed the CompTimer bugs), using 0 instead of 1 makes no perceivable performance difference in this new version (tested with fixes for CompTimer applied). So maybe my comment is a bit wrong and should not say "even though 0 would be better". In fact, in theory, a delay of zero would increase CPU usage. And since it provides no perceivable benefit any more, I don't think we should aim to change it to zero.

Also, creating dependencies on other branches when you don't need to is not ideal. I would prefer to keep things simple as they are...

Revision history for this message
Tim Penhey (thumper) wrote : Posted in a previous version of this proposal

The reason you get warnings in the initializer list is that the member variables are initialized in the order that they are declared in the class. The initializer list expects that the order in the list is the same as the order that they are defined. If you don't, it is possible that a developer would assume that something earlier in the initializer list was initialized when in fact it wasn't.

Sam, what does the compiz coding standards say about initializer lists (if anything)?

Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Awesome. Testing with NVIDIA-280, this fix appears to solve bug 92599 and bug 888039.

Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Found a problem when testing with "nouveau". Nouveau advertises support for GLX_SGI_video_sync, however it is quietly disabled by default and returns immediately. This causes the code in this proposal to attempt to run unthrottled at 1000 FPS, which obviously is a little laggy.

The workaround for nouveau is in /etc/X11/xorg.conf:
Section "Device"
 Identifier "My Graphics"
 Option "GLXVBlank" "on"
EndSection

However, I will have to build a workaround into this proposal so that users don't need to edit xorg.conf.

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote : Posted in a previous version of this proposal

Or - query upstream as to why it is disabled? Maybe it's something we
can safely distropatch, they already have it in the pipes?

Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

I will research the issue with nouveau separately. However it's now clear that we need to occasionally expect graphics drivers to be a little broken. Compiz should be able to handle it without becoming unusable. I will look at adding detection for such driver problems into compiz (opengl plugin?) today.

Revision history for this message
Daniel van Vugt (vanvugt) wrote : Posted in a previous version of this proposal

Interestingly, I only realized today that the fix for bug 763005 is a prerequisite for this proposal. It probably wouldn't work otherwise. Lucky the fix for 763005 is already committed...

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

This proposal also appears to solve LP: #798868 and LP: #876575.

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

This will be merged as soon as the testing branch in https://code.launchpad.net/~smspillaz/compiz-core/compiz-core.fix_880707.2.test/+merge/82961 is complete

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

(I also don't like working around driver bugs inside of the opengl plugin since these can break if the driver developers change something. If you need to add a workaround, add it to the workarounds plugin by replacing the GL::getVideoSync function pointer with your work-around wrapper)

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I'm generalizing the "driver bug workaround" today because similar code is needed for all drivers in some circumstances.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Done. The changes to plugins/opengl/src/screen.cpp are no longer just for nouveau, but now important for all users.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Major improvement today. Reduced tearing enough to also resolve bug 755841.

Revision history for this message
Omer Akram (om26er) wrote :

Daniel, is there a specific reason this change won't make into Precise? I have noticed real positive improvements on on Intel(smooth minimize animations), nvidia-current (no tearing) and nouvea (no tearing)

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Omer, you misinterpreted my comment in bug 880707. What I meant was that this fix is so complex that it won't go into any oneiric updates. But getting it into precise is my absolute top priority.

I agree; the number of improvements this branch brings is much larger than I ever envisioned. I never even noticed the stuttering bugs until I saw they were fixed by this code. It was a nice surprise.

We do absolutely want everyone to have these improvements by the time precise is released. :)

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

I'm going to have another look at how we can do automated testing from this branch again tomorrow.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

I already did automate testing... using an army of testers now subscribed to ppa:vanvugt/compiz ;)

OK, maybe you meant a different kind of automated...

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

Indeed :)

I've started updating lp:~smspillaz/compiz-core/fix-880707.2.test with the stuff we discussed there earlier. Namely testing for phase (eg, that it doesn't take > 4ms to get from a vblank begin fence to finishing a swapbuffers / paint) and also testing that we don't fall out of phase when we are throttling the framerate with the composite plugin.

I don't really think its possible to be testing for an exact fix to bug 880707 without instantiating a separate opengl context and running out constructs in there, but doing that is going to be far too much effort than its worth really.

Please let me know of any other parts of the composite plugin's code that you think can be tested.

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

(and thank you for setting up the PPA)

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Comments in the other branch.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

More work may be required on this proposal. Some anecdotal evidence from a minority of testers, and a brief test on a slow netbook, both suggest that this proposal can make performance worse. I will need to look into this.

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

Sure. I'll keep working on testcases for the current stuff we've got and rebase on any other work you come up with when that happens.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Fixed the slow-motion animations problem in r2900.

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

I agree with fixing the plugins for what its worth.

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

Hey Daniel,

I had a think about the problem that you were encountering with the plugins. As you said earlier, the plugins are expecting a situation where msSinceLastPaint is going to be reflective of optimalRedrawTime, but with the tickless algorithm, msSinceLastPaint could be quite large. On slower systems, if it took > optimalRedrawTime to actually process a paint, then forcing msSinceLastPaint to be optimalRedrawTime would result in slow animations since frame skipping would not work correctly.

Since the only case we really wanted to cover was the case where the screen went from idle to active, what do you think about doing something like this:

    struct timeval tv;
    int timeDiff;

    gettimeofday (&tv, 0);

    timeDiff = TIMEVALDIFF (&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 && !reschedule)
 timeDiff = mOptimalRedrawTime;

    ...

At least on your branch, reschedule is going to be set every time a plugin requests a redraw in donePaint or addWindowDamage (which is the correct way to do it), so that means that in those cases, msSinceLastPaint is always going to be accurate. And where we have switched from inactive to active, msSinceLastPaint is always going to be mOptimalRedrawTime (in fact, it could be zero, but that might require further testing).

I've updated my changes to reflect this.

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Your suggestion looks incorrect. priv->reschedule is always false there. I don't think it will work.

Best to use my r2900 below. Mine has been tested for a few days, whereas yours would appear to probably not work (or compile in that form :)

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

I forgot to mention - the check would have to be moved to before priv->reschedule has been set to false (I've roughly transposed the code from my branch to yours)

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

(It works quite well here, and I've tested it with all the plugins)

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

I think that checking if the timeDiff is more than 100 only really works around the original problem which I outlined earlier as well. There's a case where some really slow plugin could take more than 100 ms per frame to process some animation, which would result in perceived slowness in its animation code ;-)

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

"I forgot to mention - the check would have to be moved to before priv->reschedule has been set to false (I've roughly transposed the code from my branch to yours)"

That still makes no sense in my mind. But if you can reproduce the slow animations bug consistently, and have tested and verified your fix works then go nuts. If however this is another code change you're proposing without actually testing it, then please reconsider.

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

On Thu, 22 Dec 2011, Daniel van Vugt wrote:

> "I forgot to mention - the check would have to be moved to before priv->reschedule has been set to false (I've roughly transposed the code from my branch to yours)"
>
> That still makes no sense in my mind. But if you can reproduce the slow animations bug consistently, and have tested and verified your fix works then go nuts. If however this is another code change you're proposing without actually testing it, then please reconsider.

I'm running it right now and haven't run into any problems whatsoever. I
can probably give it a try on some of my other systems as well, but I put
some cases in to force it to run slowly and it still works then.

> --
> https://code.launchpad.net/~vanvugt/compiz-core/fix-880707.2/+merge/83472
> You are requested to review the proposed merge of lp:~vanvugt/compiz-core/fix-880707.2 into lp:compiz-core.
>

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Yep, a simple sleep or something to make painting slow should allow you to reproduce the problem and then verify you're actually fixing it.

In my real-world example, I have an old netbook which takes between 2-4 frames to paint each frame (up to 64ms) during animations. So it's easy to reproduce "slow motion animations" on a machine that only achieves 15-30 FPS.

Oddly enough, as I have mentioned before, this "slow" machine gets a perfect 60 FPS with "Force full screen redraw (buffer swap) on repaint" enabled. But only when using this branch. The difference is staggering.

lp:~vanvugt/compiz-core/fix-880707.2 updated
2901. By Daniel van Vugt

Merge with upstream changes (lp:compiz-core).

2902. By Daniel van Vugt

Merge upstream changes.

2903. By Daniel van Vugt

Merge from upstream.

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

OK, I think there's been enough discussion here.

I'm confident with the changes this branch is introducing, and think we just need to land it now. I have created an entire testing framework for it, which can be found at lp:~smspillaz/compiz-core/compiz-core.fix_880707.2.test . That test tests for both phase and period timings. On a period of aggressive vblank scheduling for 70 seconds, that test passes each time on my machine.

The testing branch will need to land a little bit after the main one does, once Daniel is ok with it, but ideally it needs to be in before feature freeze.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'plugins/composite/src/privates.h'
--- plugins/composite/src/privates.h 2011-10-15 11:00:51 +0000
+++ plugins/composite/src/privates.h 2012-01-16 10:07:25 +0000
@@ -64,7 +64,7 @@
6464
65 void detectRefreshRate ();65 void detectRefreshRate ();
6666
67 int getTimeToNextRedraw (struct timeval *tv);67 void scheduleRepaint ();
6868
69 public:69 public:
7070
@@ -95,13 +95,9 @@
95 int overlayWindowCount;95 int overlayWindowCount;
9696
97 struct timeval lastRedraw;97 struct timeval lastRedraw;
98 int nextRedraw;
99 int redrawTime;98 int redrawTime;
100 int optimalRedrawTime;99 int optimalRedrawTime;
101 int frameStatus;100 bool scheduled, painting, reschedule;
102 int timeMult;
103 bool idle;
104 int timeLeft;
105101
106 bool slowAnimations;102 bool slowAnimations;
107103
@@ -110,7 +106,6 @@
110 compiz::composite::PaintHandler *pHnd;106 compiz::composite::PaintHandler *pHnd;
111107
112 CompositeFPSLimiterMode FPSLimiterMode;108 CompositeFPSLimiterMode FPSLimiterMode;
113 int frameTimeAccumulator;
114109
115 CompWindowList withDestroyedWindows;110 CompWindowList withDestroyedWindows;
116};111};
117112
=== modified file 'plugins/composite/src/screen.cpp'
--- plugins/composite/src/screen.cpp 2012-01-12 17:49:40 +0000
+++ plugins/composite/src/screen.cpp 2012-01-16 10:07:25 +0000
@@ -279,17 +279,14 @@
279 exposeRects (),279 exposeRects (),
280 windowPaintOffset (0, 0),280 windowPaintOffset (0, 0),
281 overlayWindowCount (0),281 overlayWindowCount (0),
282 nextRedraw (0),
283 redrawTime (1000 / 50),282 redrawTime (1000 / 50),
284 optimalRedrawTime (1000 / 50),283 optimalRedrawTime (1000 / 50),
285 frameStatus (0),284 scheduled (false),
286 timeMult (1),285 painting (false),
287 idle (true),286 reschedule (false),
288 timeLeft (0),
289 slowAnimations (false),287 slowAnimations (false),
290 pHnd (NULL),288 pHnd (NULL),
291 FPSLimiterMode (CompositeFPSLimiterModeDefault),289 FPSLimiterMode (CompositeFPSLimiterModeDefault),
292 frameTimeAccumulator (0),
293 withDestroyedWindows ()290 withDestroyedWindows ()
294{291{
295 gettimeofday (&lastRedraw, 0);292 gettimeofday (&lastRedraw, 0);
@@ -422,10 +419,6 @@
422419
423 showOutputWindow ();420 showOutputWindow ();
424421
425 priv->paintTimer.start
426 (boost::bind (&CompositeScreen::handlePaintTimeout, this),
427 priv->optimalRedrawTime);
428
429 return true;422 return true;
430}423}
431424
@@ -469,11 +462,9 @@
469void462void
470CompositeScreen::damageScreen ()463CompositeScreen::damageScreen ()
471{464{
472 if (priv->damageMask == 0)
473 priv->paintTimer.setTimes (priv->paintTimer.minLeft ());
474
475 priv->damageMask |= COMPOSITE_SCREEN_DAMAGE_ALL_MASK;465 priv->damageMask |= COMPOSITE_SCREEN_DAMAGE_ALL_MASK;
476 priv->damageMask &= ~COMPOSITE_SCREEN_DAMAGE_REGION_MASK;466 priv->damageMask &= ~COMPOSITE_SCREEN_DAMAGE_REGION_MASK;
467 priv->scheduleRepaint ();
477}468}
478469
479void470void
@@ -482,9 +473,6 @@
482 if (priv->damageMask & COMPOSITE_SCREEN_DAMAGE_ALL_MASK)473 if (priv->damageMask & COMPOSITE_SCREEN_DAMAGE_ALL_MASK)
483 return;474 return;
484475
485 if (priv->damageMask == 0)
486 priv->paintTimer.setTimes (priv->paintTimer.minLeft ());
487
488 priv->damage += region;476 priv->damage += region;
489 priv->damageMask |= COMPOSITE_SCREEN_DAMAGE_REGION_MASK;477 priv->damageMask |= COMPOSITE_SCREEN_DAMAGE_REGION_MASK;
490478
@@ -495,15 +483,14 @@
495483
496 if (priv->damage.numRects () > 100)484 if (priv->damage.numRects () > 100)
497 damageScreen ();485 damageScreen ();
486 priv->scheduleRepaint ();
498}487}
499488
500void489void
501CompositeScreen::damagePending ()490CompositeScreen::damagePending ()
502{491{
503 if (priv->damageMask == 0)
504 priv->paintTimer.setTimes (priv->paintTimer.minLeft ());
505
506 priv->damageMask |= COMPOSITE_SCREEN_DAMAGE_PENDING_MASK;492 priv->damageMask |= COMPOSITE_SCREEN_DAMAGE_PENDING_MASK;
493 priv->scheduleRepaint ();
507}494}
508495
509unsigned int496unsigned int
@@ -673,6 +660,7 @@
673 mOptions[CompositeOptions::DetectRefreshRate].value ().set (false);660 mOptions[CompositeOptions::DetectRefreshRate].value ().set (false);
674 screen->setOptionForPlugin ("composite", "refresh_rate", value);661 screen->setOptionForPlugin ("composite", "refresh_rate", value);
675 mOptions[CompositeOptions::DetectRefreshRate].value ().set (true);662 mOptions[CompositeOptions::DetectRefreshRate].value ().set (true);
663 optimalRedrawTime = redrawTime = 1000 / value.i ();
676 }664 }
677 else665 else
678 {666 {
@@ -693,77 +681,47 @@
693 priv->FPSLimiterMode = newMode;681 priv->FPSLimiterMode = newMode;
694}682}
695683
696int684void
697PrivateCompositeScreen::getTimeToNextRedraw (struct timeval *tv)685PrivateCompositeScreen::scheduleRepaint ()
698{686{
699 int diff;687 if (painting)
700688 {
701 diff = compiz::core::timer::timeval_diff (tv, &lastRedraw);689 reschedule = true;
702690 return;
703 /* handle clock rollback */691 }
704 if (diff < 0)692
705 diff = 0;693 if (scheduled)
706 694 return;
707 bool hasVSyncBehavior =695
708 (FPSLimiterMode == CompositeFPSLimiterModeVSyncLike ||696 scheduled = true;
709 (pHnd && pHnd->hasVSync ()));697
710698 int delay;
711 if (idle || hasVSyncBehavior)699 if (FPSLimiterMode == CompositeFPSLimiterModeVSyncLike ||
712 {700 (pHnd && pHnd->hasVSync ()))
713 if (timeMult > 1)701 {
714 {702 delay = 1;
715 frameStatus = -1;
716 redrawTime = optimalRedrawTime;
717 timeMult--;
718 }
719 }703 }
720 else704 else
721 {705 {
722 int next;706 struct timeval now;
723 if (diff > redrawTime)707 gettimeofday (&now, 0);
724 {708 int elapsed = compiz::core::timer::timeval_diff (&now, &lastRedraw);
725 if (frameStatus > 0)709 if (elapsed < 0)
726 frameStatus = 0;710 elapsed = 0;
727711 delay = elapsed < optimalRedrawTime ? optimalRedrawTime - elapsed : 1;
728 next = optimalRedrawTime * (timeMult + 1);
729 if (diff > next)
730 {
731 frameStatus--;
732 if (frameStatus < -1)
733 {
734 timeMult++;
735 redrawTime = diff = next;
736 }
737 }
738 }
739 else if (diff < redrawTime)
740 {
741 if (frameStatus < 0)
742 frameStatus = 0;
743
744 if (timeMult > 1)
745 {
746 next = optimalRedrawTime * (timeMult - 1);
747 if (diff < next)
748 {
749 frameStatus++;
750 if (frameStatus > 4)
751 {
752 timeMult--;
753 redrawTime = next;
754 }
755 }
756 }
757 }
758 }712 }
759 713 /*
760 if (diff >= redrawTime)714 * Note the use of delay = 1 instead of 0, even though 0 would be better.
761 return 1;715 * A delay of zero is presently broken due to CompTimer bugs;
762716 * 1. Infinite loop in CompTimeoutSource::callback when a zero
763 if (hasVSyncBehavior)717 * timer is set.
764 return (redrawTime - diff) * 0.7;718 * 2. Priority set too high in CompTimeoutSource::CompTimeoutSource
765719 * causing the glib main event loop to be starved of X events.
766 return redrawTime - diff;720 * Fixes for both of these issues are being worked on separately.
721 */
722 paintTimer.start
723 (boost::bind (&CompositeScreen::handlePaintTimeout, cScreen),
724 delay);
767}725}
768726
769int727int
@@ -782,8 +740,9 @@
782CompositeScreen::handlePaintTimeout ()740CompositeScreen::handlePaintTimeout ()
783{741{
784 struct timeval tv;742 struct timeval tv;
785 int timeToNextRedraw;
786743
744 priv->painting = true;
745 priv->reschedule = false;
787 gettimeofday (&tv, 0);746 gettimeofday (&tv, 0);
788747
789 if (priv->damageMask)748 if (priv->damageMask)
@@ -798,21 +757,18 @@
798 /* handle clock rollback */757 /* handle clock rollback */
799 if (timeDiff < 0)758 if (timeDiff < 0)
800 timeDiff = 0;759 timeDiff = 0;
801760 /*
802 if (priv->slowAnimations)761 * Now that we use a "tickless" timing algorithm, timeDiff could be
803 {762 * very large if the screen is truely idle.
804 int msSinceLastPaint;763 * However plugins expect the old behaviour where timeDiff is rarely
805764 * larger than the frame rate (optimalRedrawTime).
806 if (priv->FPSLimiterMode == CompositeFPSLimiterModeDisabled)765 * So enforce this to keep animations timed correctly and smooth...
807 msSinceLastPaint = 1;766 */
808 else767 if (timeDiff > 100)
809 msSinceLastPaint =768 timeDiff = priv->optimalRedrawTime;
810 priv->idle ? 2 : (timeDiff * 2) / priv->redrawTime;769
811770 priv->redrawTime = timeDiff;
812 preparePaint (msSinceLastPaint);771 preparePaint (priv->slowAnimations ? 1 : timeDiff);
813 }
814 else
815 preparePaint (priv->idle ? priv->redrawTime : timeDiff);
816772
817 /* substract top most overlay window region */773 /* substract top most overlay window region */
818 if (priv->overlayWindowCount)774 if (priv->overlayWindowCount)
@@ -876,39 +832,15 @@
876 break;832 break;
877 }833 }
878 }834 }
879
880 priv->idle = false;
881 }
882 else
883 {
884 priv->idle = true;
885 }835 }
886836
887 priv->lastRedraw = tv;837 priv->lastRedraw = tv;
888 gettimeofday (&tv, 0);838 priv->painting = false;
889839 priv->scheduled = false;
890 if (priv->FPSLimiterMode == CompositeFPSLimiterModeDisabled)840 if (priv->reschedule)
891 {841 priv->scheduleRepaint ();
892 const int msToReturn1After = 100;842
893843 return false;
894 priv->frameTimeAccumulator += priv->redrawTime;
895 if (priv->frameTimeAccumulator > msToReturn1After)
896 {
897 priv->frameTimeAccumulator %= msToReturn1After;
898 timeToNextRedraw = 1;
899 }
900 else
901 timeToNextRedraw = 0;
902 }
903 else
904 timeToNextRedraw = priv->getTimeToNextRedraw (&tv);
905
906 if (priv->idle)
907 priv->paintTimer.setTimes (timeToNextRedraw, MAXSHORT);
908 else
909 priv->paintTimer.setTimes (timeToNextRedraw);
910
911 return true;
912}844}
913845
914void846void
915847
=== modified file 'plugins/opengl/src/screen.cpp'
--- plugins/opengl/src/screen.cpp 2012-01-09 15:12:20 +0000
+++ plugins/opengl/src/screen.cpp 2012-01-16 10:07:25 +0000
@@ -78,6 +78,9 @@
7878
79 bool canDoSaturated = false;79 bool canDoSaturated = false;
80 bool canDoSlightlySaturated = false;80 bool canDoSlightlySaturated = false;
81
82 unsigned int vsyncCount = 0;
83 unsigned int unthrottledFrames = 0;
81}84}
8285
83CompOutput *targetOutput = NULL;86CompOutput *targetOutput = NULL;
@@ -1077,15 +1080,29 @@
1077void1080void
1078waitForVideoSync ()1081waitForVideoSync ()
1079{1082{
1083 GL::unthrottledFrames++;
1080 if (GL::waitVideoSync)1084 if (GL::waitVideoSync)
1081 {1085 {
1082 // Don't wait twice. Just in case.1086 // Don't wait twice. Just in case.
1083 if (GL::swapInterval)1087 if (GL::swapInterval)
1084 (*GL::swapInterval) (0);1088 (*GL::swapInterval) (0);
10851089
1090 /*
1091 * While glXSwapBuffers/glXCopySubBufferMESA are meant to do a
1092 * flush before they blit, it is best to not let that happen.
1093 * Because that flush would occur after GL::waitVideoSync, causing
1094 * a delay and the final blit to be slightly out of sync resulting
1095 * in tearing. So we need to do a glFinish before we wait for
1096 * vsync, to absolutely minimize tearing.
1097 */
1098 glFinish ();
1099
1086 // Docs: http://www.opengl.org/registry/specs/SGI/video_sync.txt1100 // Docs: http://www.opengl.org/registry/specs/SGI/video_sync.txt
1087 unsigned int frameno;1101 unsigned int oldCount = GL::vsyncCount;
1088 (*GL::waitVideoSync) (1, 0, &frameno);1102 (*GL::waitVideoSync) (1, 0, &GL::vsyncCount);
1103
1104 if (GL::vsyncCount != oldCount)
1105 GL::unthrottledFrames = 0;
1089 }1106 }
1090}1107}
10911108
@@ -1094,7 +1111,10 @@
1094{1111{
1095 // Docs: http://www.opengl.org/registry/specs/SGI/swap_control.txt1112 // Docs: http://www.opengl.org/registry/specs/SGI/swap_control.txt
1096 if (GL::swapInterval)1113 if (GL::swapInterval)
1114 {
1097 (*GL::swapInterval) (sync ? 1 : 0);1115 (*GL::swapInterval) (sync ? 1 : 0);
1116 GL::unthrottledFrames++;
1117 }
1098 else if (sync)1118 else if (sync)
1099 waitForVideoSync ();1119 waitForVideoSync ();
1100}1120}
@@ -1177,15 +1197,6 @@
11771197
1178 targetOutput = &screen->outputDevs ()[0];1198 targetOutput = &screen->outputDevs ()[0];
11791199
1180 glFlush ();
1181
1182 /*
1183 * FIXME: Actually fix the composite plugin to be more efficient;
1184 * If GL::swapInterval == NULL && GL::waitVideoSync != NULL then the
1185 * composite plugin should not be imposing any framerate restriction
1186 * (ie. blocking the CPU) at all. Because the framerate will be controlled
1187 * and optimized here:
1188 */
1189 if (mask & COMPOSITE_SCREEN_DAMAGE_ALL_MASK)1200 if (mask & COMPOSITE_SCREEN_DAMAGE_ALL_MASK)
1190 {1201 {
1191 /*1202 /*
@@ -1257,7 +1268,8 @@
1257bool1268bool
1258PrivateGLScreen::hasVSync ()1269PrivateGLScreen::hasVSync ()
1259{1270{
1260 return (GL::waitVideoSync && optionGetSyncToVblank ());1271 return GL::waitVideoSync && optionGetSyncToVblank () &&
1272 GL::unthrottledFrames < 5;
1261}1273}
12621274
1263bool1275bool

Subscribers

People subscribed via source and target branches