diff --git a/src/libcalamares/locale/Tests.cpp b/src/libcalamares/locale/Tests.cpp index e498ac039..b755a61a5 100644 --- a/src/libcalamares/locale/Tests.cpp +++ b/src/libcalamares/locale/Tests.cpp @@ -52,7 +52,9 @@ private Q_SLOTS: void testComplexZones(); void testTZLookup(); void testTZIterator(); + void testLocationLookup_data(); void testLocationLookup(); + void testLocationLookup2(); }; LocaleTests::LocaleTests() {} @@ -390,24 +392,67 @@ LocaleTests::testTZIterator() QCOMPARE( ( *zones.begin() )->zone(), QStringLiteral( "Abidjan" ) ); } +void +LocaleTests::testLocationLookup_data() +{ + QTest::addColumn< double >( "latitude" ); + QTest::addColumn< double >( "longitude" ); + QTest::addColumn< QString >( "name" ); + + QTest::newRow( "London" ) << 50.0 << 0.0 << QString( "London" ); + QTest::newRow( "Tarawa E" ) << 0.0 << 179.0 << QString( "Tarawa" ); + QTest::newRow( "Tarawa W" ) << 0.0 << -179.0 << QString( "Tarawa" ); + + QTest::newRow( "Johannesburg" ) << -26.0 << 28.0 << QString( "Johannesburg" ); // South Africa + QTest::newRow( "Maseru" ) << -29.0 << 27.0 << QString( "Maseru" ); // Lesotho + QTest::newRow( "Windhoek" ) << -22.0 << 17.0 << QString( "Windhoek" ); // Namibia + QTest::newRow( "Port Elisabeth" ) << -33.0 << 25.0 << QString( "Johannesburg" ); // South Africa + QTest::newRow( "Cape Town" ) << -33.0 << 18.0 << QString( "Johannesburg" ); // South Africa +} + void LocaleTests::testLocationLookup() { const CalamaresUtils::Locale::ZonesModel zones; - const auto* zone = zones.find( 50.0, 0.0 ); - QVERIFY( zone ); - QCOMPARE( zone->zone(), QStringLiteral( "London" ) ); + QFETCH( double, latitude ); + QFETCH( double, longitude ); + QFETCH( QString, name ); - - // Tarawa is close to "the other side of the world" from London - zone = zones.find( 0.0, 179.0 ); + const auto* zone = zones.find( latitude, longitude ); QVERIFY( zone ); - QCOMPARE( zone->zone(), QStringLiteral( "Tarawa" ) ); + QCOMPARE( zone->zone(), name ); +} - zone = zones.find( 0.0, -179.0 ); - QVERIFY( zone ); - QCOMPARE( zone->zone(), QStringLiteral( "Tarawa" ) ); +void +LocaleTests::testLocationLookup2() +{ + // Official + // ZA -2615+02800 Africa/Johannesburg + // Spot patch + // "ZA -3230+02259 Africa/Johannesburg\n"; + + const CalamaresUtils::Locale::ZonesModel zones; + const auto* zone = zones.find( -26.15, 28.00 ); + QCOMPARE( zone->zone(), QString( "Johannesburg" ) ); + // The TZ data sources use minutes-and-seconds notation, + // so "2615" is 26 degrees, 15 minutes, and 15 minutes is + // one-quarter of a degree. + QCOMPARE( zone->latitude(), -26.25 ); + QCOMPARE( zone->longitude(), 28.00 ); + + // Elsewhere in South Africa + const auto* altzone = zones.find( -32.0, 22.0 ); + QCOMPARE( altzone, zone ); // same pointer + QCOMPARE( altzone->zone(), QString( "Johannesburg" ) ); + QCOMPARE( altzone->latitude(), -26.25 ); + QCOMPARE( altzone->longitude(), 28.00 ); + + altzone = zones.find( -29.0, 27.0 ); + QCOMPARE( altzone->zone(), QString( "Maseru" ) ); + // -2928, that's -29 and 28/60 of a degree, is almost half, but we don't want + // to fall foul of variations in double-precision + QCOMPARE( trunc( altzone->latitude() * 1000.0 ), -29466 ); } diff --git a/src/libcalamares/locale/TimeZone.cpp b/src/libcalamares/locale/TimeZone.cpp index e2779281f..ddc705548 100644 --- a/src/libcalamares/locale/TimeZone.cpp +++ b/src/libcalamares/locale/TimeZone.cpp @@ -110,84 +110,103 @@ RegionData::tr() const } static void -loadTZData( RegionVector& regions, ZoneVector& zones ) +loadTZData( RegionVector& regions, ZoneVector& zones, QTextStream& in ) { - QFile file( TZ_DATA_FILE ); - if ( file.open( QIODevice::ReadOnly | QIODevice::Text ) ) + while ( !in.atEnd() ) { - QTextStream in( &file ); - while ( !in.atEnd() ) + QString line = in.readLine().trimmed().split( '#', SplitKeepEmptyParts ).first().trimmed(); + if ( line.isEmpty() ) { - QString line = in.readLine().trimmed().split( '#', SplitKeepEmptyParts ).first().trimmed(); - if ( line.isEmpty() ) - { - continue; - } - - QStringList list = line.split( QRegExp( "[\t ]" ), SplitSkipEmptyParts ); - if ( list.size() < 3 ) - { - continue; - } - - QStringList timezoneParts = list.at( 2 ).split( '/', SplitSkipEmptyParts ); - if ( timezoneParts.size() < 2 ) - { - continue; - } - - QString region = timezoneParts.first().trimmed(); - if ( region.isEmpty() ) - { - continue; - } - - QString countryCode = list.at( 0 ).trimmed(); - if ( countryCode.size() != 2 ) - { - continue; - } - - timezoneParts.removeFirst(); - QString zone = timezoneParts.join( '/' ); - if ( zone.length() < 2 ) - { - continue; - } - - QString position = list.at( 1 ); - int cooSplitPos = position.indexOf( QRegExp( "[-+]" ), 1 ); - double latitude; - double longitude; - if ( cooSplitPos > 0 ) - { - latitude = getRightGeoLocation( position.mid( 0, cooSplitPos ) ); - longitude = getRightGeoLocation( position.mid( cooSplitPos ) ); - } - else - { - continue; - } - - // Now we have region, zone, country, lat and longitude - const RegionData* existingRegion = nullptr; - for ( const auto* p : regions ) - { - if ( p->key() == region ) - { - existingRegion = p; - break; - } - } - if ( !existingRegion ) - { - regions.append( new RegionData( region ) ); - } - zones.append( new TimeZoneData( region, zone, countryCode, latitude, longitude ) ); + continue; } + + QStringList list = line.split( QRegExp( "[\t ]" ), SplitSkipEmptyParts ); + if ( list.size() < 3 ) + { + continue; + } + + QStringList timezoneParts = list.at( 2 ).split( '/', SplitSkipEmptyParts ); + if ( timezoneParts.size() < 2 ) + { + continue; + } + + QString region = timezoneParts.first().trimmed(); + if ( region.isEmpty() ) + { + continue; + } + + QString countryCode = list.at( 0 ).trimmed(); + if ( countryCode.size() != 2 ) + { + continue; + } + + timezoneParts.removeFirst(); + QString zone = timezoneParts.join( '/' ); + if ( zone.length() < 2 ) + { + continue; + } + + QString position = list.at( 1 ); + int cooSplitPos = position.indexOf( QRegExp( "[-+]" ), 1 ); + double latitude; + double longitude; + if ( cooSplitPos > 0 ) + { + latitude = getRightGeoLocation( position.mid( 0, cooSplitPos ) ); + longitude = getRightGeoLocation( position.mid( cooSplitPos ) ); + } + else + { + continue; + } + + // Now we have region, zone, country, lat and longitude + const RegionData* existingRegion = nullptr; + for ( const auto* p : regions ) + { + if ( p->key() == region ) + { + existingRegion = p; + break; + } + } + if ( !existingRegion ) + { + regions.append( new RegionData( region ) ); + } + zones.append( new TimeZoneData( region, zone, countryCode, latitude, longitude ) ); } } +/** @brief Extra, fake, timezones + * + * The timezone locations in zone.tab are not always very useful, + * given Calamares's standard "nearest zone" algorithm: for instance, + * in most locations physically in the country of South Africa, + * Maseru (the capital of Lesotho, and location for timezone Africa/Maseru) + * is closer than Johannesburg (the location for timezone Africa/Johannesburg). + * + * The algorithm picks the wrong place. This is for instance annoying + * when clicking on Cape Town, you get Maseru, and to get Johannesburg + * you need to click somewhere very carefully north of Maserru. + * + * These alternate zones are used to introduce "extra locations" + * into the timezone database, in order to influence the closest-location + * algorithm. Lines are formatted just like in zone.tab: remember the \n + */ +static const char altZones[] = + /* This extra zone is north-east of Karoo National park, + * and means that Western Cape province and a good chunk of + * Northern- and Eastern- Cape provinces get pulled in to Johannesburg. + * Bloemfontein is still closer to Maseru than either correct zone, + * but this is a definite improvement. + */ + "ZA -3230+02259 Africa/Johannesburg\n"; class Private : public QObject { @@ -195,13 +214,27 @@ class Private : public QObject public: RegionVector m_regions; ZoneVector m_zones; + ZoneVector m_altZones; //< Extra locations for zones Private() { m_regions.reserve( 12 ); // reasonable guess m_zones.reserve( 452 ); // wc -l /usr/share/zoneinfo/zone.tab - loadTZData( m_regions, m_zones ); + // Load the official timezones + { + QFile file( TZ_DATA_FILE ); + if ( file.open( QIODevice::ReadOnly | QIODevice::Text ) ) + { + QTextStream in( &file ); + loadTZData( m_regions, m_zones, in ); + } + } + // Load the alternate zones (see documentation at altZones) + { + QTextStream in( altZones ); + loadTZData( m_regions, m_altZones, in ); + } std::sort( m_regions.begin(), m_regions.end(), []( const RegionData* lhs, const RegionData* rhs ) { return lhs->key() < rhs->key(); @@ -336,6 +369,37 @@ ZonesModel::find( const QString& region, const QString& zone ) const return nullptr; } +STATICTEST const TimeZoneData* +find( double startingDistance, const ZoneVector& zones, const std::function< double( const TimeZoneData* ) >& distanceFunc ) +{ + double smallestDistance = startingDistance; + const TimeZoneData* closest = nullptr; + + for ( const auto* zone : zones ) + { + double thisDistance = distanceFunc( zone ); + if ( thisDistance < smallestDistance ) + { + closest = zone; + smallestDistance = thisDistance; + } + } + return closest; +} + +const TimeZoneData* +ZonesModel::find( const std::function< double( const TimeZoneData* ) >& distanceFunc ) const +{ + const auto* officialZone = CalamaresUtils::Locale::find( 1000000.0, m_private->m_zones, distanceFunc ); + const auto* altZone = CalamaresUtils::Locale::find( distanceFunc( officialZone ), m_private->m_altZones, distanceFunc ); + + // If nothing was closer than the official zone already was, altZone is + // nullptr; but if there is a spot-patch, then we need to re-find + // the zone by name, since we want to always return pointers into + // m_zones, not into the alternative spots. + return altZone ? find( altZone->region(), altZone->zone() ) : officialZone; +} + const TimeZoneData* ZonesModel::find( double latitude, double longitude ) const { @@ -344,12 +408,7 @@ ZonesModel::find( double latitude, double longitude ) const * either N/S or E/W equal to any other; this obviously * falls apart at the poles. */ - - double largestDifference = 720.0; - const TimeZoneData* closest = nullptr; - - for ( const auto* zone : m_private->m_zones ) - { + auto distance = [&]( const TimeZoneData* zone ) -> double { // Latitude doesn't wrap around: there is nothing north of 90 double latitudeDifference = abs( zone->latitude() - latitude ); @@ -368,13 +427,10 @@ ZonesModel::find( double latitude, double longitude ) const longitudeDifference = abs( westerly - easterly ); } - if ( latitudeDifference + longitudeDifference < largestDifference ) - { - largestDifference = latitudeDifference + longitudeDifference; - closest = zone; - } - } - return closest; + return latitudeDifference + longitudeDifference; + }; + + return find( distance ); } QObject* diff --git a/src/libcalamares/locale/TimeZone.h b/src/libcalamares/locale/TimeZone.h index 1d4b4a8ff..cc0c35ea2 100644 --- a/src/libcalamares/locale/TimeZone.h +++ b/src/libcalamares/locale/TimeZone.h @@ -167,6 +167,17 @@ public: Iterator begin() const { return Iterator( m_private ); } + /** @brief Look up TZ data based on an arbitrary distance function + * + * This is a generic method that can define distance in whatever + * coordinate system is wanted; returns the zone with the smallest + * distance. The @p distanceFunc must return "the distance" for + * each zone. It would be polite to return something non-negative. + * + * Note: not a slot, because the parameter isn't moc-able. + */ + const TimeZoneData* find( const std::function< double( const TimeZoneData* ) >& distanceFunc ) const; + public Q_SLOTS: /** @brief Look up TZ data based on its name. * @@ -176,7 +187,10 @@ public Q_SLOTS: /** @brief Look up TZ data based on the location. * - * Returns the nearest zone to the given lat and lon. + * Returns the nearest zone to the given lat and lon. This is a + * convenience function for calling find(), below, with a standard + * distance function based on the distance between the given + * location (lat and lon) and each zone's given location. */ const TimeZoneData* find( double latitude, double longitude ) const; diff --git a/src/modules/locale/timezonewidget/timezonewidget.cpp b/src/modules/locale/timezonewidget/timezonewidget.cpp index b1d3cfeaa..778f0535d 100644 --- a/src/modules/locale/timezonewidget/timezonewidget.cpp +++ b/src/modules/locale/timezonewidget/timezonewidget.cpp @@ -190,28 +190,15 @@ TimeZoneWidget::mousePressEvent( QMouseEvent* event ) { return; } - // Set nearest location - int nX = 999999, mX = event->pos().x(); - int nY = 999999, mY = event->pos().y(); - using namespace CalamaresUtils::Locale; - const TimeZoneData* closest = nullptr; - for ( auto it = m_zonesData->begin(); it; ++it ) - { - const auto* zone = *it; - if ( zone ) - { - QPoint locPos = TimeZoneImageList::getLocationPosition( zone->longitude(), zone->latitude() ); - - if ( ( abs( mX - locPos.x() ) + abs( mY - locPos.y() ) < abs( mX - nX ) + abs( mY - nY ) ) ) - { - closest = zone; - nX = locPos.x(); - nY = locPos.y(); - } - } - } + int mX = event->pos().x(); + int mY = event->pos().y(); + auto distance = [&]( const CalamaresUtils::Locale::TimeZoneData* zone ) { + QPoint locPos = TimeZoneImageList::getLocationPosition( zone->longitude(), zone->latitude() ); + return double( abs( mX - locPos.x() ) + abs( mY - locPos.y() ) ); + }; + const auto* closest = m_zonesData->find( distance ); if ( closest ) { // Set zone image and repaint widget