Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 101 additions & 71 deletions src/core/StelCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1351,40 +1351,55 @@ void StelCore::moveObserverTo(const StelLocation& target, double duration, doubl
emit locationChanged(getCurrentLocation());
}

QCache<size_t, qint64> StelCore::utcOffsetCache;

double StelCore::getUTCOffset(const double JD) const
{
int year, month, day, hour, minute, second;
StelUtils::getDateTimeFromJulianDay(JD, &year, &month, &day, &hour, &minute, &second);
// This method takes a significant amount of time. Try to cache a few entries.
// All these elements can influence UTCOffset, so we must include them in the cache.
// Presumably, offset does not change during a minute, so we can exclude seconds here.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like an unsafe assumption to me. What is the cost of keeping the seconds?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updates every second, not every minute.

struct UTCOffsetHashData {
int year;
int month;
int day;
int hour;
int minute;
//int second;
StelLocation *loc;
QTimeZone *tz;
} u;
int second;
StelUtils::getDateTimeFromJulianDay(JD, &u.year, &u.month, &u.day, &u.hour, &u.minute, &second);
// as analogous to second statement in getJDFromDate, nkerr
if ( year <= 0 )
if ( u.year <= 0 )
{
year = year - 1;
u.year = u.year - 1;
}
//getTime/DateFromJulianDay returns UTC time, not local time
QDateTime universal(QDate(year, month, day), QTime(hour, minute, second), Qt::UTC);
QDateTime universal(QDate(u.year, u.month, u.day), QTime(u.hour, u.minute, second), Qt::UTC);
if (!universal.isValid())
{
//qWarning() << "JD " << QString("%1").arg(JD) << " out of bounds of QT help with GMT shift, using current datetime";
// Assumes the GMT shift was always the same before year -4710
// NOTE: QDateTime has no year 0, and therefore likely different leap year rules.
// Under which circumstances do we get invalid universal?
universal = QDateTime(QDate(-4710, month, day), QTime(hour, minute, second), Qt::UTC);
universal = QDateTime(QDate(-4710, u.month, u.day), QTime(u.hour, u.minute, second), Qt::UTC);
}

#if defined(Q_OS_WIN)
if (abs(year)<3)
if (abs(u.year)<3)
{
// Mitigate a QTBUG on Windows (GH #594).
// This bug causes offset to be MIN_INT in January to March, 1AD.
// We assume a constant offset in this remote history,
// so we construct yet another date to get a valid offset.
// Application of the named time zones is inappropriate in any case.
universal = QDateTime(QDate(3, month, day), QTime(hour, minute, second), Qt::UTC);
universal = QDateTime(QDate(3, u.month, u.day), QTime(u.hour, u.minute, second), Qt::UTC);
}
#endif
StelLocation loc = getCurrentLocation();
QString tzName = getCurrentTimeZone();
QTimeZone tz(tzName.toUtf8());
const StelLocation &loc = getCurrentLocation();
const QString tzName(getCurrentTimeZone());
const QTimeZone tz(tzName.toUtf8());
// We must fight a bug in Qt6.2 on Linux. For some reason tz.isValid() is true even for our self-named zones LMST,LTST,system_default.
// We must use an intermediate Boolean which we set to false where needed.
bool tzValid=tz.isValid();
Expand All @@ -1395,83 +1410,98 @@ double StelCore::getUTCOffset(const double JD) const
qWarning() << "Invalid timezone: " << tzName;
}

qint64 shiftInSeconds = 0;
if (tzName=="system_default" || (loc.planetName=="Earth" && !tzValid && !QString("LMST LTST").contains(tzName)))
// Complete the cache object and hash it.
u.loc= & const_cast<StelLocation &>(loc);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is some ugliness here. Why do you keep the object by const reference and then violate this constness?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what various warnings made me do.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, this is the wrong way to read the warnings then :P They are supposed to make you improve the code, not equip it with kludges.

u.tz = & const_cast<QTimeZone&>(tz);
size_t dateLocHash=qHash(&u);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you hashing a pointer to a local variable?..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did I misunderstand the qHash() docs? Does this only hash the pointer, not its contents?
OK, that's not what I wanted... :-o
Do I need to write my own qHash function for the struct?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you need a custom hash function for every custom data type. This may be of help.


qint64 shiftInSeconds;
qint64 *shiftInSecondsCached=utcOffsetCache.object(dateLocHash);

if (shiftInSecondsCached) // found in cache
{
QDateTime local = universal.toLocalTime();
//Both timezones should be interpreted as UTC because secsTo() converts both
//times to UTC if their zones have different daylight saving time rules.
local.setTimeSpec(Qt::UTC);
shiftInSeconds = universal.secsTo(local);
if (abs(shiftInSeconds)>50000 || shiftInSeconds==INT_MIN)
{
qDebug() << "TZ system_default or invalid, At JD" << QString::number(JD, 'g', 11) << ", shift:" << shiftInSeconds;
}
shiftInSeconds=*shiftInSecondsCached;
}
else
{
// The first adoption of a standard time was on December 1, 1847 in Great Britain
if (tzValid && loc.planetName=="Earth" && (JD>=StelCore::TZ_ERA_BEGINNING || getUseCustomTimeZone()))
if (tzName==L1S("system_default") || (loc.planetName==L1S("Earth") && !tzValid && !QString("LMST LTST").contains(tzName)))
{
if (getUseDST())
shiftInSeconds = tz.offsetFromUtc(universal);
else
shiftInSeconds = tz.standardTimeOffset(universal);
if (abs(shiftInSeconds)>500000 || shiftInSeconds==INT_MIN)
QDateTime local = universal.toLocalTime();
//Both timezones should be interpreted as UTC because secsTo() converts both
//times to UTC if their zones have different daylight saving time rules.
local.setTimeSpec(Qt::UTC);
shiftInSeconds = universal.secsTo(local);
if (abs(shiftInSeconds)>50000 || shiftInSeconds==INT_MIN)
{
// Something very strange has happened. The Windows-only clause above already mitigated GH #594.
// Trigger this with a named custom TZ like Europe/Stockholm.
// Then try to wheel back some date in January-March from year 10 to 0. Instead of year 1, it jumps to 70,
// an offset of INT_MIN
qWarning() << "ERROR TRAPPED! --- Please submit a bug report with this logfile attached.";
qWarning() << "TZ" << tz << "valid, but at JD" << QString::number(JD, 'g', 11) << ", shift:" << shiftInSeconds;
qWarning() << "Universal reference date: " << universal.toString();
qDebug() << "TZ system_default or invalid, At JD" << QString::number(JD, 'g', 11) << ", shift:" << shiftInSeconds;
}
}
else
{
shiftInSeconds = qRound((loc.getLongitude()/15.f)*3600.f); // Local Mean Solar Time
}
if (tzName=="LTST")
shiftInSeconds += qRound(getSolutionEquationOfTime()*60);
}
//qDebug() << "ShiftInSeconds:" << shiftInSeconds;
#ifdef Q_OS_WIN
// A dirty hack for report: https://github.com/Stellarium/stellarium/issues/686
// TODO: switch to IANA TZ on all operating systems
if (tzName=="Europe/Volgograd")
shiftInSeconds = 4*3600; // UTC+04:00
#endif

// Extraterrestrial: Either use the configured Terrestrial timezone, or even a pseudo-LMST based on planet's rotation speed?
if (loc.planetName!="Earth")
{
if (tzValid && (JD>=StelCore::TZ_ERA_BEGINNING || getUseCustomTimeZone()))
{
if (getUseDST())
shiftInSeconds = tz.offsetFromUtc(universal);
// The first adoption of a standard time was on December 1, 1847 in Great Britain
if (tzValid && loc.planetName==L1S("Earth") && (JD>=StelCore::TZ_ERA_BEGINNING || getUseCustomTimeZone()))
{
if (getUseDST())
shiftInSeconds = tz.offsetFromUtc(universal);
else
shiftInSeconds = tz.standardTimeOffset(universal);
if (abs(shiftInSeconds)>500000 || shiftInSeconds==INT_MIN)
{
// Something very strange has happened. The Windows-only clause above already mitigated GH #594.
// Trigger this with a named custom TZ like Europe/Stockholm.
// Then try to wheel back some date in January-March from year 10 to 0. Instead of year 1, it jumps to 70,
// an offset of INT_MIN
qWarning() << "ERROR TRAPPED! --- Please submit a bug report with this logfile attached.";
qWarning() << "TZ" << tz << "valid, but at JD" << QString::number(JD, 'g', 11) << ", shift:" << shiftInSeconds;
qWarning() << "Universal reference date: " << universal.toString();
}
}
else
shiftInSeconds = tz.standardTimeOffset(universal);
if (shiftInSeconds==INT_MIN) // triggered error
{
// Something very strange has happened. The Windows-only clause above already mitigated GH #594.
// Trigger this with a named custom TZ like Europe/Stockholm.
// Then try to wheel back some date in January-March from year 10 to 0. Instead of year 1, it jumps to 70,
// an offset of INT_MIN
qWarning() << "ERROR TRAPPED! --- Please submit a bug report with this logfile attached.";
qWarning() << "TZ" << tz << "valid, but at JD" << QString::number(JD, 'g', 11) << ", shift:" << shiftInSeconds;
qWarning() << "Universal reference date: " << universal.toString();
shiftInSeconds = qRound((loc.getLongitude()/15.f)*3600.f); // Local Mean Solar Time
}
if (tzName==L1S("LTST"))
shiftInSeconds += qRound(getSolutionEquationOfTime()*60);
}
else
//qDebug() << "ShiftInSeconds:" << shiftInSeconds;
#ifdef Q_OS_WIN
// A dirty hack for report: https://github.com/Stellarium/stellarium/issues/686
// TODO: switch to IANA TZ on all operating systems
if (tzName==L1S("Europe/Volgograd"))
shiftInSeconds = 4*3600; // UTC+04:00
#endif

// Extraterrestrial: Either use the configured Terrestrial timezone, or even a pseudo-LMST based on planet's rotation speed?
if (loc.planetName!=L1S("Earth"))
{
// TODO: This should give "mean solar time" for any planet.
// Combine rotation and orbit, or (for moons) rotation and orbit of parent planet.
// LTST is even worse, needs equation of time for other planets.
shiftInSeconds = 0; // For now, give UT
if (tzValid && (JD>=StelCore::TZ_ERA_BEGINNING || getUseCustomTimeZone()))
{
if (getUseDST())
shiftInSeconds = tz.offsetFromUtc(universal);
else
shiftInSeconds = tz.standardTimeOffset(universal);
if (shiftInSeconds==INT_MIN) // triggered error
{
// Something very strange has happened. The Windows-only clause above already mitigated GH #594.
// Trigger this with a named custom TZ like Europe/Stockholm.
// Then try to wheel back some date in January-March from year 10 to 0. Instead of year 1, it jumps to 70,
// an offset of INT_MIN
qWarning() << "ERROR TRAPPED! --- Please submit a bug report with this logfile attached.";
qWarning() << "TZ" << tz << "valid, but at JD" << QString::number(JD, 'g', 11) << ", shift:" << shiftInSeconds;
qWarning() << "Universal reference date: " << universal.toString();
}
}
else
{
// TODO: This should give "mean solar time" for any planet.
// Combine rotation and orbit, or (for moons) rotation and orbit of parent planet.
// LTST is even worse, needs equation of time for other planets.
shiftInSeconds = 0; // For now, give UT
}
}
shiftInSecondsCached = new qint64(shiftInSeconds);
utcOffsetCache.insert(dateLocHash, shiftInSecondsCached);
}

return shiftInSeconds / 3600.0;
}

Expand Down
6 changes: 6 additions & 0 deletions src/core/StelCore.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "StelSkyDrawer.hpp"
#include "StelPropertyMgr.hpp"
#include "Dithering.hpp"
#include <QCache>
#include <QString>
#include <QStringList>
#include <QTime>
Expand Down Expand Up @@ -315,8 +316,13 @@ class StelCore : public QObject
const StelLocation& getCurrentLocation() const;
//! Get the UTC offset on the current location (in hours)
//! N.B. This is a rather costly operation. Re-use where possible!
//! In addition, this is now cached with 1-minute granularity.
double getUTCOffset(const double JD) const;
private:
// Cache is over qHash(&UTCOffsetHashData).
static QCache<size_t, qint64> utcOffsetCache;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why static?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I make this a per-object variable, it breaks many const-qualified functions. If there will ever be more than one core, they would just share the results. Or do you see problems with this?

Copy link
Contributor

@10110111 10110111 Sep 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I make this a per-object variable, it breaks many const-qualified functions.

This is exactly why mutable keyword was introduced: one of the uses for it is implementation of transparent caches.

If there will ever be more than one core, they would just share the results.

Hmm, this doesn't seem like a likely situation, but in this way the staticness might make sense, though there's some strangeness in the way you implement the cache that I can't tell whether it actually makes sense.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I forgot about mutable.


public:
QString getCurrentTimeZone() const;
void setCurrentTimeZone(const QString& tz);

Expand Down