Merge branch 'fix-joburg' into calamares

Resolve a long-standing annoyance. With the new model for TimeZones
and nicer data structures, along with consistent find-methods,
we can spot-patch TZ data to handle special cases of bad timezones
being assigned to obviously-otherwise locations.

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 );
+    QFETCH( double, latitude );
-    QVERIFY( zone );
+    QFETCH( double, longitude );
-    QCOMPARE( zone->zone(), QStringLiteral( "London" ) );
+    QFETCH( QString, name );
 
-
+    const auto* zone = zones.find( latitude, longitude );
-    // Tarawa is close to "the other side of the world" from London
-    zone = zones.find( 0.0, 179.0 );
     QVERIFY( zone );
-    QCOMPARE( zone->zone(), QStringLiteral( "Tarawa" ) );
+    QCOMPARE( zone->zone(), name );
+}
 
-    zone = zones.find( 0.0, -179.0 );
+void
-    QVERIFY( zone );
+LocaleTests::testLocationLookup2()
-    QCOMPARE( zone->zone(), QStringLiteral( "Tarawa" ) );
+{
+    // 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 );
 }
 
 

src/libcalamares/locale/TimeZone.cpp

@@ -110,12 +110,8 @@ 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 ) )
-    {
-        QTextStream in( &file );
     while ( !in.atEnd() )
     {
         QString line = in.readLine().trimmed().split( '#', SplitKeepEmptyParts ).first().trimmed();
@@ -186,8 +182,31 @@ loadTZData( RegionVector& regions, ZoneVector& zones )
         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.
      */
-
+    auto distance = [&]( const TimeZoneData* zone ) -> double {
-    double largestDifference = 720.0;
-    const TimeZoneData* closest = nullptr;
-
-    for ( const auto* zone : m_private->m_zones )
-    {
         // 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 )
+        return latitudeDifference + longitudeDifference;
-        {
+    };
-            largestDifference = latitudeDifference + longitudeDifference;
+
-            closest = zone;
+    return find( distance );
-        }
-    }
-    return closest;
 }
 
 QObject*

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;
 

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;
+    int mX = event->pos().x();
-    const TimeZoneData* closest = nullptr;
+    int mY = event->pos().y();
-    for ( auto it = m_zonesData->begin(); it; ++it )
+    auto distance = [&]( const CalamaresUtils::Locale::TimeZoneData* zone ) {
-    {
-        const auto* zone = *it;
-        if ( zone )
-        {
         QPoint locPos = TimeZoneImageList::getLocationPosition( zone->longitude(), zone->latitude() );
+        return double( abs( mX - locPos.x() ) + abs( mY - locPos.y() ) );
+    };
 
-            if ( ( abs( mX - locPos.x() ) + abs( mY - locPos.y() ) < abs( mX - nX ) + abs( mY - nY ) ) )
+    const auto* closest = m_zonesData->find( distance );
-            {
-                closest = zone;
-                nX = locPos.x();
-                nY = locPos.y();
-            }
-        }
-    }
-
     if ( closest )
     {
         // Set zone image and repaint widget