Merge lp:~bi0ha2ard/stellarium/timefix into lp:stellarium

[Alexander Wolf]

Felix Zeltner proposed improvements for time syncing with operating system for minimize drift of time when Stellarium worked long time.

[Alexander Wolf (alexwolf)]

Looks like this patch is good but it need for testing (2-3 days for working Stellarium) :)

review: Approve

[Fabien Chéreau (xalioth)]

lastTimeChangeJDay variable is not necessary, simply do JDay += (StelApp::getTotalRunTime() - lastTimeChangeTime) / ONE_OVER_JD_SECOND

Also be careful with variable naming: lastTimeChangeTime -> timeOfLastJDayUpdate

StelCore::resetSync() can then be replaced by a simpler
timeOfLastJDayUpdate = StelApp::getTotalRunTime();

Isn't it?

review: Needs Fixing

[Felix Z. (bi0ha2ard)]

> lastTimeChangeJDay variable is not necessary, simply do JDay +=
> (StelApp::getTotalRunTime() - lastTimeChangeTime) / ONE_OVER_JD_SECOND
>
> Also be careful with variable naming: lastTimeChangeTime ->
> timeOfLastJDayUpdate
>
> StelCore::resetSync() can then be replaced by a simpler
> timeOfLastJDayUpdate = StelApp::getTotalRunTime();
>
> Isn't it?

No, lastTimeChangeJDay is necessary, otherwise you'd have to set lastTimeChangeTime every time updateTime() is called and you get the same behaviour as before because StelApp::getTotalRunTime() - lastTimeChangeTime would be the same as deltaTime.
This means you would be adding up small slices of time as before hence introducing one of the errors again.

[lp:~bi0ha2ard/stellarium/timefix]

6569. By bi0ha2ard on 2014-02-13

Variable names changed

[Fabien Chéreau (xalioth)]

OK I see. OK then. Try at least to name the variables with more meaningful
names!
Fab

On Thu, Feb 13, 2014 at 5:20 PM, Felix Z. <email address hidden>wrote:

> > lastTimeChangeJDay variable is not necessary, simply do JDay +=
> > (StelApp::getTotalRunTime() - lastTimeChangeTime) / ONE_OVER_JD_SECOND
> >
> > Also be careful with variable naming: lastTimeChangeTime ->
> > timeOfLastJDayUpdate
> >
> > StelCore::resetSync() can then be replaced by a simpler
> > timeOfLastJDayUpdate = StelApp::getTotalRunTime();
> >
> > Isn't it?
>
> No, lastTimeChangeJDay is necessary, otherwise you'd have to set
> lastTimeChangeTime every time updateTime() is called and you get the same
> behaviour as before because StelApp::getTotalRunTime() - lastTimeChangeTime
> would be the same as deltaTime.
> This means you would be adding up small slices of time as before hence
> introducing one of the errors again.
> --
> https://code.launchpad.net/~bi0ha2ard/stellarium/timefix/+merge/206209
> You are reviewing the proposed merge of lp:~bi0ha2ard/stellarium/timefix
> into lp:stellarium.
>

[Felix Z. (bi0ha2ard)]

OK, variable names changed in next commit.

> OK I see. OK then. Try at least to name the variables with more meaningful
> names!
> Fab
>
>
> On Thu, Feb 13, 2014 at 5:20 PM, Felix Z. <email address hidden>wrote:
>
> > > lastTimeChangeJDay variable is not necessary, simply do JDay +=
> > > (StelApp::getTotalRunTime() - lastTimeChangeTime) / ONE_OVER_JD_SECOND
> > >
> > > Also be careful with variable naming: lastTimeChangeTime ->
> > > timeOfLastJDayUpdate
> > >
> > > StelCore::resetSync() can then be replaced by a simpler
> > > timeOfLastJDayUpdate = StelApp::getTotalRunTime();
> > >
> > > Isn't it?
> >
> > No, lastTimeChangeJDay is necessary, otherwise you'd have to set
> > lastTimeChangeTime every time updateTime() is called and you get the same
> > behaviour as before because StelApp::getTotalRunTime() - lastTimeChangeTime
> > would be the same as deltaTime.
> > This means you would be adding up small slices of time as before hence
> > introducing one of the errors again.
> > --
> > https://code.launchpad.net/~bi0ha2ard/stellarium/timefix/+merge/206209
> > You are reviewing the proposed merge of lp:~bi0ha2ard/stellarium/timefix
> > into lp:stellarium.
> >

[Fabien Chéreau (xalioth)]

ok, thanks!

review: Approve

=== modified file 'src/core/StelCore.cpp'
--- src/core/StelCore.cpp	2014-02-10 11:06:35 +0000
+++ src/core/StelCore.cpp	2014-02-13 17:24:14 +0000
@@ -53,6 +53,7 @@
 const double StelCore::JD_MINUTE=0.00069444444444444444444;
 const double StelCore::JD_HOUR  =0.041666666666666666666;
 const double StelCore::JD_DAY   =1.;
+const double StelCore::ONE_OVER_JD_SECOND = 24 * 60 * 60;
 
 
 StelCore::StelCore() : movementMgr(NULL), geodesicGrid(NULL), currentProjectionType(ProjectionStereographic), position(NULL), timeSpeed(JD_SECOND), JDay(0.)
@@ -771,6 +772,7 @@
 void StelCore::setJDay(double JD)
 {
 	JDay=JD;
+	resetSync();
 }
 
 double StelCore::getJDay() const
@@ -781,6 +783,7 @@
 void StelCore::setMJDay(double MJD)
 {
 	JDay=MJD+2400000.5;
+	resetSync();
 }
 
 double StelCore::getMJDay() const
@@ -800,7 +803,9 @@
 
 void StelCore::setTimeRate(double ts)
 {
-	timeSpeed=ts; emit timeRateChanged(timeSpeed);
+	timeSpeed=ts;
+	resetSync();
+	emit timeRateChanged(timeSpeed);
 }
 
 double StelCore::getTimeRate() const
@@ -1223,7 +1228,15 @@
 // Increment time
 void StelCore::updateTime(double deltaTime)
 {
-	JDay+=timeSpeed*deltaTime;
+	if (getRealTimeSpeed())
+	{
+		// Get rid of the error from the 1 /
+		JDay = JDayOfLastJDayUpdate + (StelApp::getTotalRunTime() - secondsOfLastJDayUpdate) / ONE_OVER_JD_SECOND;
+	}
+	else
+	{
+		JDay = JDayOfLastJDayUpdate + (StelApp::getTotalRunTime() - secondsOfLastJDayUpdate) * timeSpeed;
+	}
 
 	// Fix time limits to -100000 to +100000 to prevent bugs
 	if (JDay>38245309.499988) JDay = 38245309.499988;
@@ -1249,6 +1262,12 @@
 	solsystem->computePositions(getJDay(), position->getHomePlanet()->getHeliocentricEclipticPos());
 }
 
+void StelCore::resetSync()
+{
+	JDayOfLastJDayUpdate = getJDay();
+	secondsOfLastJDayUpdate = StelApp::getTotalRunTime();
+}
+
 void StelCore::setStartupTimeMode(const QString& s)
 {
 	startupTimeMode = s;

=== modified file 'src/core/StelCore.hpp'
--- src/core/StelCore.hpp	2013-12-27 09:25:21 +0000
+++ src/core/StelCore.hpp	2014-02-13 17:24:14 +0000
@@ -259,6 +259,7 @@
 	static const double JD_MINUTE;
 	static const double JD_HOUR;
 	static const double JD_DAY;
+	static const double ONE_OVER_JD_SECOND;
 
 	//! Get the sidereal time shifted by the observer longitude
 	//! @return the local sidereal time in radian
@@ -534,6 +535,7 @@
 
 	void updateTransformMatrices();
 	void updateTime(double deltaTime);
+	void resetSync();
 
 	// Matrices used for every coordinate transfo
 	Mat4d matHeliocentricEclipticToAltAz;      // Transform from heliocentric ecliptic (Vsop87) to observer-centric altazimuthal coordinate
@@ -559,6 +561,8 @@
 	double presetSkyTime;
 	QTime initTodayTime;
 	QString startupTimeMode;
+	double secondsOfLastJDayUpdate;         // Time in seconds when the time rate or time last changed
+	double JDayOfLastJDayUpdate;         // JDay when the time rate or time last changed
 
 	// Variables for custom equation of Delta-T
 	Vec3f deltaTCustomEquationCoeff;