Address review feedback

qgis · Jan 6, 2021 · f2d5d5f4db164f24e20fb82717f2150cb4796ef2 · f2d5d5f
1 parent 5159081
commit f2d5d5f4db164f24e20fb82717f2150cb4796ef2
diff --git a/python/core/auto_generated/qgstemporalutils.sip.in b/python/core/auto_generated/qgstemporalutils.sip.in
@@ -62,6 +62,25 @@ support.
 
 :return: - ``True`` if the export was successful.
          - error: will be set to a descriptive error message if the export fails
+%End
+
+    static QDateTime calculateFrameTime( const QDateTime &start, const long long frame, double timeStep, QgsUnitTypes::TemporalUnit timeStepUnit );
+%Docstring
+Calculates the frame time for an animation.
+
+If mFrameTimeStep is fractional, then QgsInterval is used to determine the
+duration of the frame. This uses average durations for months and years.
+Otherwise, we use QDateTime to advance by the exact duration of the current
+month or year. So a time step of 1.5 months will result in a duration of 45
+days, but a time step of 1 month will result in a duration that depends upon
+the number of days in the current month.
+
+:param start: the start time of the animation.
+:param frame: the frame number
+:param timeStep: the time step for the animation
+:param timeStepUnit: the time step unit for the animation
+
+:return: The calculated datetime for the frame.
 %End
 };
 

diff --git a/src/core/qgstemporalnavigationobject.cpp b/src/core/qgstemporalnavigationobject.cpp
@@ -17,6 +17,7 @@
 
 #include "qgstemporalnavigationobject.h"
 #include "qgis.h"
+#include "qgstemporalutils.h"
 
 QgsTemporalNavigationObject::QgsTemporalNavigationObject( QObject *parent )
   : QgsTemporalController( parent )
@@ -75,7 +76,7 @@ QgsExpressionContextScope *QgsTemporalNavigationObject::createExpressionContextS
   scope->setVariable( QStringLiteral( "frame_rate" ), mFramesPerSecond, true );
   scope->setVariable( QStringLiteral( "frame_number" ), mCurrentFrameNumber, true );
   scope->setVariable( QStringLiteral( "frame_timestep" ), mFrameTimeStep, true );
-  scope->setVariable( QStringLiteral( "frame_timestepunit" ), mFrameTimeStepUnit, true );
+  scope->setVariable( QStringLiteral( "frame_timestep_unit" ), mFrameTimeStepUnit, true );
   scope->setVariable( QStringLiteral( "animation_start_time" ), mTemporalExtents.begin(), true );
   scope->setVariable( QStringLiteral( "animation_end_time" ), mTemporalExtents.end(), true );
   scope->setVariable( QStringLiteral( "animation_interval" ), mTemporalExtents.end() - mTemporalExtents.begin(), true );
@@ -91,70 +92,8 @@ QgsDateTimeRange QgsTemporalNavigationObject::dateTimeRangeForFrameNumber( long
 
   const long long nextFrame = frame + 1;
 
-  QDateTime begin;
-  QDateTime end;
-
-  // If mFrameTimeStep is fractional, we use QgsInterval to determine the
-  // duration of the frame, which uses average durations for months and years.
-  // Otherwise, we use QDateTime to advance by the exact duration of the current
-  // month or year. So a time step of 1.5 months will result in a duration of 45
-  // days, but a time step of 1 month will result in a duration that depends upon
-  // the number of days in the current month.
-  if ( mFrameTimeStepIsFractional )
-  {
-    double duration = QgsInterval( mFrameTimeStep, mFrameTimeStepUnit ).seconds();
-    begin = start.addSecs( frame * duration );
-    end = start.addSecs( nextFrame * duration );
-  }
-  else
-  {
-    switch ( mFrameTimeStepUnit )
-    {
-      case QgsUnitTypes::TemporalUnit::TemporalMilliseconds:
-        begin = start.addMSecs( frame * mFrameTimeStep );
-        end = start.addMSecs( nextFrame * mFrameTimeStep );
-        break;
-      case QgsUnitTypes::TemporalUnit::TemporalSeconds:
-        begin = start.addSecs( frame * mFrameTimeStep );
-        end = start.addSecs( nextFrame * mFrameTimeStep );
-        break;
-      case QgsUnitTypes::TemporalUnit::TemporalMinutes:
-        begin = start.addSecs( 60 * frame * mFrameTimeStep );
-        end = start.addSecs( 60 * nextFrame * mFrameTimeStep );
-        break;
-      case QgsUnitTypes::TemporalUnit::TemporalHours:
-        begin = start.addSecs( 3600 * frame * mFrameTimeStep );
-        end = start.addSecs( 3600 * nextFrame * mFrameTimeStep );
-        break;
-      case QgsUnitTypes::TemporalUnit::TemporalDays:
-        begin = start.addDays( frame * mFrameTimeStep );
-        end = start.addDays( nextFrame * mFrameTimeStep );
-        break;
-      case QgsUnitTypes::TemporalUnit::TemporalWeeks:
-        begin = start.addDays( 7 * frame * mFrameTimeStep );
-        end = start.addDays( 7 * nextFrame * mFrameTimeStep );
-        break;
-      case QgsUnitTypes::TemporalUnit::TemporalMonths:
-        begin = start.addMonths( frame * mFrameTimeStep );
-        end = start.addMonths( nextFrame * mFrameTimeStep );
-        break;
-      case QgsUnitTypes::TemporalUnit::TemporalYears:
-        begin = start.addYears( frame * mFrameTimeStep );
-        end = start.addYears( nextFrame * mFrameTimeStep );
-        break;
-      case QgsUnitTypes::TemporalUnit::TemporalDecades:
-        begin = start.addYears( 10 * frame * mFrameTimeStep );
-        end = start.addYears( 10 * nextFrame * mFrameTimeStep );
-        break;
-      case QgsUnitTypes::TemporalUnit::TemporalCenturies:
-        begin = start.addYears( 100 * frame * mFrameTimeStep );
-        end = start.addYears( 100 * nextFrame * mFrameTimeStep );
-        break;
-      case QgsUnitTypes::TemporalUnit::TemporalUnknownUnit:
-        Q_ASSERT( false );
-        break;
-    }
-  }
+  QDateTime begin = QgsTemporalUtils::calculateFrameTime( start, frame, mFrameTimeStep, mFrameTimeStepUnit );
+  QDateTime end = QgsTemporalUtils::calculateFrameTime( start, nextFrame, mFrameTimeStep, mFrameTimeStepUnit );
 
   QDateTime frameStart = begin;
 
@@ -249,8 +188,6 @@ long long QgsTemporalNavigationObject::currentFrameNumber() const
 void QgsTemporalNavigationObject::setFrameTimeStep( double timeStep )
 {
   mFrameTimeStep = timeStep;
-  double unused;
-  mFrameTimeStepIsFractional = fabs( modf( mFrameTimeStep, &unused ) ) > 0.00001;
   setCurrentFrameNumber( 0 );
 }
 

diff --git a/src/core/qgstemporalnavigationobject.h b/src/core/qgstemporalnavigationobject.h
@@ -325,7 +325,6 @@ class CORE_EXPORT QgsTemporalNavigationObject : public QgsTemporalController, pu
 
     //! Frame duration
     double mFrameTimeStep;
-    bool mFrameTimeStepIsFractional;
     QgsUnitTypes::TemporalUnit mFrameTimeStepUnit;
 
     //! Member for frame rate

diff --git a/src/core/qgstemporalutils.cpp b/src/core/qgstemporalutils.cpp
@@ -142,3 +142,58 @@ bool QgsTemporalUtils::exportAnimation( const QgsMapSettings &mapSettings, const
 
   return true;
 }
+
+
+QDateTime QgsTemporalUtils::calculateFrameTime( const QDateTime &start, const long long frame, const double timeStep, const QgsUnitTypes::TemporalUnit timeStepUnit )
+{
+
+  double unused;
+  bool isFractional = fabs( modf( timeStep, &unused ) ) > 0.00001;
+
+  if ( isFractional )
+  {
+    double duration = QgsInterval( timeStep, timeStepUnit ).seconds();
+    return start.addSecs( frame * duration );
+  }
+  else
+  {
+    switch ( timeStepUnit )
+    {
+      case QgsUnitTypes::TemporalUnit::TemporalMilliseconds:
+        return start.addMSecs( frame * timeStep );
+        break;
+      case QgsUnitTypes::TemporalUnit::TemporalSeconds:
+        return start.addSecs( frame * timeStep );
+        break;
+      case QgsUnitTypes::TemporalUnit::TemporalMinutes:
+        return start.addSecs( 60 * frame * timeStep );
+        break;
+      case QgsUnitTypes::TemporalUnit::TemporalHours:
+        return start.addSecs( 3600 * frame * timeStep );
+        break;
+      case QgsUnitTypes::TemporalUnit::TemporalDays:
+        return start.addDays( frame * timeStep );
+        break;
+      case QgsUnitTypes::TemporalUnit::TemporalWeeks:
+        return start.addDays( 7 * frame * timeStep );
+        break;
+      case QgsUnitTypes::TemporalUnit::TemporalMonths:
+        return start.addMonths( frame * timeStep );
+        break;
+      case QgsUnitTypes::TemporalUnit::TemporalYears:
+        return start.addYears( frame * timeStep );
+        break;
+      case QgsUnitTypes::TemporalUnit::TemporalDecades:
+        return start.addYears( 10 * frame * timeStep );
+        break;
+      case QgsUnitTypes::TemporalUnit::TemporalCenturies:
+        return start.addYears( 100 * frame * timeStep );
+        break;
+      case QgsUnitTypes::TemporalUnit::TemporalUnknownUnit:
+        Q_ASSERT( false );
+        return start;
+        break;
+    }
+  }
+}
+
diff --git a/src/core/qgstemporalutils.h b/src/core/qgstemporalutils.h
@@ -89,6 +89,25 @@ class CORE_EXPORT QgsTemporalUtils
      * \returns TRUE if the export was successful.
      */
     static bool exportAnimation( const QgsMapSettings &mapSettings, const AnimationExportSettings &settings, QString &error SIP_OUT, QgsFeedback *feedback = nullptr );
+
+    /**
+     * Calculates the frame time for an animation.
+     *
+     * If mFrameTimeStep is fractional, then QgsInterval is used to determine the
+     * duration of the frame. This uses average durations for months and years.
+     * Otherwise, we use QDateTime to advance by the exact duration of the current
+     * month or year. So a time step of 1.5 months will result in a duration of 45
+     * days, but a time step of 1 month will result in a duration that depends upon
+     * the number of days in the current month.
+     *
+     * \param start the start time of the animation.
+     * \param frame the frame number
+     * \param timeStep the time step for the animation
+     * \param timeStepUnit the time step unit for the animation
+     *
+     * \returns The calculated datetime for the frame.
+     */
+    static QDateTime calculateFrameTime( const QDateTime &start, const long long frame, double timeStep, QgsUnitTypes::TemporalUnit timeStepUnit );
 };
 
 

diff --git a/tests/src/core/testqgstemporalnavigationobject.cpp b/tests/src/core/testqgstemporalnavigationobject.cpp
@@ -198,9 +198,8 @@ void TestQgsTemporalNavigationObject::frameSettings()
 
   navigationObject->setFrameTimeStep( 1 );
   navigationObject->setFrameTimeStepUnit( QgsUnitTypes::TemporalHours );
-  QCOMPARE( navigationObject->frameTimeStep(), 1 );
-  QCOMPARE( static_cast<int>( navigationObject->frameTimeStepUnit() ),
-            static_cast<int>( QgsUnitTypes::TemporalHours ) );
+  QCOMPARE( navigationObject->frameTimeStep(), 1.0 );
+  QCOMPARE( navigationObject->frameTimeStepUnit(), QgsUnitTypes::TemporalHours );
 
   QCOMPARE( navigationObject->currentFrameNumber(), 0 );
   QCOMPARE( navigationObject->totalFrameCount(), 5 );
@@ -253,7 +252,7 @@ void TestQgsTemporalNavigationObject::expressionContext()
   std::unique_ptr< QgsExpressionContextScope > scope( object.createExpressionContextScope() );
   QCOMPARE( scope->variable( QStringLiteral( "frame_rate" ) ).toDouble(), 30.0 );
   QCOMPARE( scope->variable( QStringLiteral( "frame_timestep" ) ).value< double >(), 1.0 );
-  QCOMPARE( scope->variable( QStringLiteral( "frame_timestepunit" ) ).value< QgsUnitTypes::TemporalUnit >(), QgsUnitTypes::TemporalUnit::TemporalHours );
+  QCOMPARE( scope->variable( QStringLiteral( "frame_timestep_unit" ) ).value< QgsUnitTypes::TemporalUnit >(), QgsUnitTypes::TemporalUnit::TemporalHours );
   QCOMPARE( scope->variable( QStringLiteral( "frame_number" ) ).toInt(), 1 );
   QCOMPARE( scope->variable( QStringLiteral( "animation_start_time" ) ).toDateTime(), range.begin() );
   QCOMPARE( scope->variable( QStringLiteral( "animation_end_time" ) ).toDateTime(), range.end() );