summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAkarsh Simha <akarsh@kde.org>2016-09-26 03:01:08 (GMT)
committerAkarsh Simha <akarsh@kde.org>2016-09-29 02:20:26 (GMT)
commit6b9ddf72f775d45ff720858395ce316457714a25 (patch)
tree009d47f3975ae75e94c08b2b36b31d644562b95d
parent2029a90da256d5b8e4795520a18a848320311a78 (diff)
Improve use of CachingDms
Prevent throwing away cached trigonometric values.
-rw-r--r--kstars/skyobjects/skypoint.cpp18
-rw-r--r--kstars/skyobjects/skypoint.h4
-rw-r--r--kstars/skyobjects/starobject.cpp140
-rw-r--r--kstars/skyobjects/starobject.h1
4 files changed, 125 insertions, 38 deletions
diff --git a/kstars/skyobjects/skypoint.cpp b/kstars/skyobjects/skypoint.cpp
index 4452941..d6ba764 100644
--- a/kstars/skyobjects/skypoint.cpp
+++ b/kstars/skyobjects/skypoint.cpp
@@ -180,10 +180,10 @@ void SkyPoint::setFromEcliptic( const CachingDms *Obliquity, const dms& EcLong,
double sinDec = sinLat*cosObliq + cosLat*sinObliq*sinLong;
double y = sinLong*cosObliq - (sinLat/cosLat)*sinObliq;
- double RARad = atan2( y, cosLong );
- RA.setRadians( RARad );
+// double RARad = atan2( y, cosLong );
+ RA.setUsing_atan2( y, cosLong );
RA.reduce(); // FIXME: dms::reduce() doesn't work like this
- Dec.setRadians( asin(sinDec) );
+ Dec.setUsing_asin( sinDec );
}
void SkyPoint::precess( const KSNumbers *num ) {
@@ -408,8 +408,8 @@ void SkyPoint::precessFromAnyEpoch(long double jd0, long double jdf){
double cosRA, sinRA, cosDec, sinDec;
double v[3], s[3];
- RA.setD( RA0.Degrees() );
- Dec.setD( Dec0.Degrees() );
+ RA = RA0;
+ Dec = Dec0; // Is this necessary?
if (jd0 == jdf)
return;
@@ -469,11 +469,11 @@ void SkyPoint::precessFromAnyEpoch(long double jd0, long double jdf){
num.p2( 2, i )*s[2];
}
- RA.setRadians( atan2( v[1],v[0] ) );
- Dec.setRadians( asin( v[2] ) );
+ RA.setUsing_atan2( v[1], v[0] );
+ Dec.setUsing_asin( v[2] );
- if (RA.Degrees() < 0.0 )
- RA.setD( RA.Degrees() + 360.0 );
+ // FIXME: bad practice with CachingDms
+ RA.reduceToRange( dms::ZERO_TO_2PI );
return;
}
diff --git a/kstars/skyobjects/skypoint.h b/kstars/skyobjects/skypoint.h
index 7978f57..97a7b05 100644
--- a/kstars/skyobjects/skypoint.h
+++ b/kstars/skyobjects/skypoint.h
@@ -105,6 +105,7 @@ public:
*@param r catalog Right Ascension.
*/
inline void setRA0( dms r ) { RA0 = r; }
+ inline void setRA0( CachingDms r ) { RA0 = r; }
/** Overloaded member function, provided for convenience.
*It behaves essentially like the above function.
@@ -116,6 +117,7 @@ public:
*@param d catalog Declination.
*/
inline void setDec0( dms d ) { Dec0 = d; }
+ inline void setDec0( CachingDms d ) { Dec0 = d; }
/** Overloaded member function, provided for convenience.
*It behaves essentially like the above function.
@@ -127,6 +129,7 @@ public:
*@param r Right Ascension.
*/
inline void setRA( dms r ) { RA = r; }
+ inline void setRA( CachingDms r ) { RA = r; }
/** Overloaded member function, provided for convenience.
*It behaves essentially like the above function.
@@ -138,6 +141,7 @@ public:
*@param d Declination.
*/
inline void setDec( dms d ) { Dec = d; }
+ inline void setDec( CachingDms d ) { Dec = d; }
/** Overloaded member function, provided for convenience.
*It behaves essentially like the above function.
diff --git a/kstars/skyobjects/starobject.cpp b/kstars/skyobjects/starobject.cpp
index 8906ee4..22f5e7c 100644
--- a/kstars/skyobjects/starobject.cpp
+++ b/kstars/skyobjects/starobject.cpp
@@ -154,8 +154,8 @@ void StarObject::init( const starData *stardata )
setMag( stardata->mag / 100.0 );
setRA0( ra );
setDec0( dec );
- setRA( ra );
- setDec( dec );
+ setRA( ra0() );
+ setDec( dec0() );
SpType[0] = stardata->spec_type[0];
SpType[1] = stardata->spec_type[1];
PM_RA = stardata->dRA / 10.0;
@@ -274,31 +274,17 @@ void StarObject::updateCoords( const KSNumbers *num, bool , const CachingDms*, c
std::clock_t start, stop;
start = std::clock();
#endif
- double saveRA = ra0().Hours();
- double saveDec = dec0().Degrees();
-
- double newRA, newDec;
-
- bool properMotionCorrections = getIndexCoords( num, &newRA, &newDec );
+ CachingDms saveRA = ra0(), saveDec = dec0();
+ CachingDms newRA, newDec;
- // FIXME: Find a better way to do this without conditionals etc.
- // Having the conditional is good so we can use CachingDms on RA0 and Dec0
-
- // FIXME: This code is very inefficient as it destroys the
- // sine/cosine cache and repeatedly recreates it!
-
- if ( properMotionCorrections ) {
- newRA /= 15.0; // getIndexCoords returns in Degrees, while we want the RA in Hours
- setRA0( newRA );
- setDec0( newDec );
- }
+ getIndexCoords( num, newRA, newDec );
+ setRA0( newRA );
+ setDec0( newDec );
SkyPoint::updateCoords( num );
+ setRA0( saveRA );
+ setDec0( saveDec );
- if ( properMotionCorrections ) {
- setRA0( saveRA );
- setDec0( saveDec );
- }
#ifdef PROFILE_UPDATECOORDS
stop = std::clock();
updateCoordsCpuTime += double( stop - start )/double( CLOCKS_PER_SEC );
@@ -306,10 +292,107 @@ void StarObject::updateCoords( const KSNumbers *num, bool , const CachingDms*, c
#endif
}
+bool StarObject::getIndexCoords( const KSNumbers *num, CachingDms &ra, CachingDms &dec )
+{
+ static double pmms;
+
+ // =================== NOTE: CODE DUPLICATION ====================
+ // If you modify this, please also modify the other getIndexCoords
+ // ===============================================================
+ //
+ // Reason for code duplication is as follows:
+ //
+ // This method is designed to use CachingDms, i.e. we know we are
+ // going to use the sine and cosine of the returned values.
+ //
+ // The other method is designed to avoid CachingDms and try to
+ // compute as little trigonometry as possible when the ra/dec has
+ // to be returned in double (used in SkyMesh::indexStar() for
+ // example)
+ //
+ // Thus, the philosophy of writing code is different. Granted, we
+ // don't need to optimize for the smaller star catalogs (which use
+ // SkyMesh::indexStar()), but it is nevertheless a good idea,
+ // given that getIndexCoords() shows up in callgrind as one of the
+ // slightly more expensive operations.
+
+ // Old, Incorrect Proper motion Computation. We retain this in a
+ // comment because we might want to use it to come up with a
+ // linear approximation that's faster.
+ // double dra = pmRA() * num->julianMillenia() / ( cos( dec0().radians() ) * 3600.0 );
+ // double ddec = pmDec() * num->julianMillenia() / 3600.0;
+
+ // Proper Motion Correction should be implemented as motion along a great
+ // circle passing through the given (ra0, dec0) in a direction of
+ // atan2( pmRA(), pmDec() ) to an angular distance given by the Magnitude of
+ // PM times the number of Julian millenia since J2000.0
+
+ pmms = pmMagnitudeSquared();
+
+ if( std::isnan( pmms ) || pmms * num->julianMillenia() * num->julianMillenia() < 1. ) {
+ // Ignore corrections
+ ra = ra0();
+ dec = dec0();
+ return false;
+ }
+
+ double pm = pmMagnitude() * num->julianMillenia(); // Proper Motion in arcseconds
+
+ double dir0 = ( ( pm > 0 ) ? atan2( pmRA(), pmDec() ) : atan2( -pmRA(), -pmDec() ) ); // Bearing, in radian
+
+ ( pm < 0 ) && ( pm = -pm );
+
+ double dst = ( pm * M_PI / ( 180.0 * 3600.0 ) );
+ // double phi = M_PI / 2.0 - dec0().radians();
+
+
+ // Note: According to callgrind, dms::dms() + dms::setRadians()
+ // takes ~ 40 CPU cycles, whereas, the advantage afforded by using
+ // sincos() instead of sin() and cos() calls seems to be about 30
+ // CPU cycles.
+
+ // So it seems like it is not worth turning dir0 and dst into dms
+ // objects and using SinCos(). However, caching the values of sin
+ // and cos if we are going to reuse them avoids expensive (~120
+ // CPU cycle) recomputation!
+ CachingDms lat1, dtheta;
+ double sinDst = sin( dst ), cosDst = cos( dst );
+ lat1.setUsing_asin( dec0().sin() * cosDst + dec0().cos() * sinDst * cos( dir0 ) );
+ dtheta.setUsing_atan2( sin( dir0 ) * sinDst * dec0().cos(),
+ cosDst - dec0().sin() * lat1.sin() );
+
+ ra = ra0() + dtheta; // Use operator + to avoid trigonometry
+ dec = lat1; // Need variable lat1 because dec may refer to dec0, so cannot construct result in-place
+
+ // *ra = ra0().Degrees() + dra;
+ // *dec = dec0().Degrees() + ddec;
+ return true;
+}
+
+
bool StarObject::getIndexCoords( const KSNumbers *num, double *ra, double *dec )
{
static double pmms;
+ // =================== NOTE: CODE DUPLICATION ====================
+ // If you modify this, please also modify the other getIndexCoords
+ // ===============================================================
+ //
+ // Reason for code duplication is as follows:
+ //
+ // This method is designed to avoid CachingDms and try to compute
+ // as little trigonometry as possible when the ra/dec has to be
+ // returned in double (used in SkyMesh::indexStar() for example)
+ //
+ // The other method is designed to use CachingDms, i.e. we know we
+ // are going to use the sine and cosine of the returned values.
+ //
+ // Thus, the philosophy of writing code is different. Granted, we
+ // don't need to optimize for the smaller star catalogs (which use
+ // SkyMesh::indexStar()), but it is nevertheless a good idea,
+ // given that getIndexCoords() shows up in callgrind as one of the
+ // slightly more expensive operations.
+
// Old, Incorrect Proper motion Computation. We retain this in a
// comment because we might want to use it to come up with a
// linear approximation that's faster.
@@ -350,12 +433,11 @@ bool StarObject::getIndexCoords( const KSNumbers *num, double *ra, double *dec )
// and cos if we are going to reuse them avoids expensive (~120
// CPU cycle) recomputation!
dms lat1, dtheta;
- double sinDec0, cosDec0, sinDst = sin( dst ), cosDst = cos( dst );
- dec0().SinCos( sinDec0, cosDec0 );
- lat1.setRadians( asin( sinDec0 * cosDst +
- cosDec0 * sinDst * cos( dir0 ) ) );
- dtheta.setRadians( atan2( sin( dir0 ) * sinDst * cosDec0,
- cosDst - sinDec0 * lat1.sin() ) );
+ double sinDst = sin( dst ), cosDst = cos( dst );
+ double sinLat1 = dec0().sin() * cosDst + dec0().cos() * sinDst * cos( dir0 );
+ lat1.setRadians( asin( sinLat1 ) );
+ dtheta.setRadians( atan2( sin( dir0 ) * sinDst * dec0().cos(),
+ cosDst - dec0().sin() * sinLat1 ) );
// Using dms instead, to ensure that the numbers are in the right range.
dms finalRA( ra0().Degrees() + dtheta.Degrees() );
diff --git a/kstars/skyobjects/starobject.h b/kstars/skyobjects/starobject.h
index 56bd223..1f071b7 100644
--- a/kstars/skyobjects/starobject.h
+++ b/kstars/skyobjects/starobject.h
@@ -173,6 +173,7 @@ public:
* @return true if we changed the coordinates, false otherwise
* NOTE: ra and dec both in degrees.
*/
+ bool getIndexCoords( const KSNumbers *num, CachingDms &ra, CachingDms &dec );
bool getIndexCoords( const KSNumbers *num, double *ra, double *dec );
/** @short added for JIT updates from both StarComponent and ConstellationLines */