addressed review comments

python/core/auto_generated/qgstemporalnavigationobject.sip.in

@@ -125,46 +125,6 @@ Sets the frame ``duration``, which dictates the temporal length of each frame in
 Returns the current set frame duration, which dictates the temporal length of each frame in the animation.
 
 .. seealso:: :py:func:`setFrameDuration`
-%End
-
-    void setFrameTimeStep( double timeStep );
-%Docstring
-Sets the frame ``timestep``, which together with the frame timestep unit dictates
-the temporal length of each frame in the animation.
-
-.. note::
-
-   Calling this will reset the :py:func:`~QgsTemporalNavigationObject.currentFrameNumber` to the closest temporal match for the previous temporal range.
-
-.. seealso:: :py:func:`frameTimeStep`
-%End
-
-    void setFrameTimeStepUnit( QgsUnitTypes::TemporalUnit timeStepUnit );
-%Docstring
-Sets the frame ``timestep`` unit, which together with the frame time step dictates
-the temporal length of each frame in the animation.
-
-.. note::
-
-   Calling this will reset the :py:func:`~QgsTemporalNavigationObject.currentFrameNumber` to the first frame.
-
-.. seealso:: :py:func:`frameTimeStepUnit`
-%End
-
-    double frameTimeStep() const;
-%Docstring
-Returns the current set frame timestep, which together with frame timestep unit
-dictates the temporal length of each frame in the animation.
-
-.. seealso:: :py:func:`setFrameTimeStep`
-%End
-
-    QgsUnitTypes::TemporalUnit frameTimeStepUnit() const;
-%Docstring
-Returns the current set frame timestep unit, which together with frame timestep
-dictates the temporal length of each frame in the animation.
-
-.. seealso:: :py:func:`setFrameTimeStepUnit`
 %End
 
     QgsDateTimeRange dateTimeRangeForFrameNumber( long long frame ) const;

python/core/auto_generated/qgstemporalutils.sip.in

@@ -66,23 +66,28 @@ support.
          - 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 );
+    static QDateTime calculateFrameTime( const QDateTime &start, const long long frame, const QgsInterval interval );
 %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.
+If the interval original duration is fractional or interval original unit is
+unknown (QgsUnitTypes.TemporalUnit.TemporalUnknownUnit), 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
+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
+:param start: time of the animation
+:param frame: number
+:param interval: duration of the animation
 
 :return: The calculated datetime for the frame.
+
+
+.. versionadded:: 3.18
 %End
 };
 

src/core/qgstemporalnavigationobject.cpp

@@ -75,8 +75,8 @@ QgsExpressionContextScope *QgsTemporalNavigationObject::createExpressionContextS
   std::unique_ptr< QgsExpressionContextScope > scope = qgis::make_unique< QgsExpressionContextScope >( QStringLiteral( "temporal" ) );
   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_timestep_unit" ), mFrameTimeStepUnit, true );
+  scope->setVariable( QStringLiteral( "frame_timestep" ), mFrameDuration.originalDuration(), true );
+  scope->setVariable( QStringLiteral( "frame_timestep_unit" ), mFrameDuration.originalUnit(), 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 );
@@ -92,8 +92,8 @@ QgsDateTimeRange QgsTemporalNavigationObject::dateTimeRangeForFrameNumber( long
 
   const long long nextFrame = frame + 1;
 
-  QDateTime begin = QgsTemporalUtils::calculateFrameTime( start, frame, mFrameTimeStep, mFrameTimeStepUnit );
-  QDateTime end = QgsTemporalUtils::calculateFrameTime( start, nextFrame, mFrameTimeStep, mFrameTimeStepUnit );
+  QDateTime begin = QgsTemporalUtils::calculateFrameTime( start, frame, mFrameDuration );
+  QDateTime end = QgsTemporalUtils::calculateFrameTime( start, nextFrame, mFrameDuration );
 
   QDateTime frameStart = begin;
 
@@ -194,10 +194,6 @@ void QgsTemporalNavigationObject::setFrameDuration( QgsInterval frameDuration )
   QgsDateTimeRange oldFrame = dateTimeRangeForFrameNumber( currentFrameNumber() );
   mFrameDuration = frameDuration;
 
-  // sync frame duration with time step and time step unit.
-  mFrameTimeStep = mFrameDuration.originalDuration();
-  mFrameTimeStepUnit = mFrameDuration.originalUnit();
-
   mCurrentFrameNumber = findBestFrameNumberForFrameStart( oldFrame.begin() );
   emit temporalFrameDurationChanged( mFrameDuration );
 
@@ -215,30 +211,6 @@ QgsInterval QgsTemporalNavigationObject::frameDuration() const
   return mFrameDuration;
 }
 
-void QgsTemporalNavigationObject::setFrameTimeStep( double timeStep )
-{
-  mFrameTimeStep = timeStep;
-  setFrameDuration( QgsInterval( mFrameTimeStep, mFrameTimeStepUnit ) );
-  setCurrentFrameNumber( 0 );
-}
-
-void QgsTemporalNavigationObject::setFrameTimeStepUnit( QgsUnitTypes::TemporalUnit timeStepUnit )
-{
-  mFrameTimeStepUnit = timeStepUnit;
-  setFrameDuration( QgsInterval( mFrameTimeStep, mFrameTimeStepUnit ) );
-  setCurrentFrameNumber( 0 );
-}
-
-double QgsTemporalNavigationObject::frameTimeStep() const
-{
-  return mFrameTimeStep;
-}
-
-QgsUnitTypes::TemporalUnit QgsTemporalNavigationObject::frameTimeStepUnit() const
-{
-  return mFrameTimeStepUnit;
-}
-
 void QgsTemporalNavigationObject::setFramesPerSecond( double framesPerSeconds )
 {
   if ( framesPerSeconds > 0 )

src/core/qgstemporalnavigationobject.h

@@ -142,42 +142,6 @@ class CORE_EXPORT QgsTemporalNavigationObject : public QgsTemporalController, pu
      */
     QgsInterval frameDuration() const;
 
-    /**
-     * Sets the frame \a timestep, which together with the frame timestep unit dictates
-     * the temporal length of each frame in the animation.
-     *
-     * \note Calling this will reset the currentFrameNumber() to the closest temporal match for the previous temporal range.
-     *
-     * \see frameTimeStep()
-     */
-    void setFrameTimeStep( double timeStep );
-
-    /**
-     * Sets the frame \a timestep unit, which together with the frame time step dictates
-     * the temporal length of each frame in the animation.
-     *
-     * \note Calling this will reset the currentFrameNumber() to the first frame.
-     *
-     * \see frameTimeStepUnit()
-     */
-    void setFrameTimeStepUnit( QgsUnitTypes::TemporalUnit timeStepUnit );
-
-    /**
-     * Returns the current set frame timestep, which together with frame timestep unit
-     * dictates the temporal length of each frame in the animation.
-     *
-     * \see setFrameTimeStep()
-     */
-    double frameTimeStep() const;
-
-    /**
-     * Returns the current set frame timestep unit, which together with frame timestep
-     * dictates the temporal length of each frame in the animation.
-     *
-     * \see setFrameTimeStepUnit()
-     */
-    QgsUnitTypes::TemporalUnit frameTimeStepUnit() const;
-
     /**
      * Calculates the temporal range associated with a particular animation \a frame.
      *
@@ -340,10 +304,7 @@ class CORE_EXPORT QgsTemporalNavigationObject : public QgsTemporalController, pu
     long long mCurrentFrameNumber = 0;
 
     //! Frame duration
-    QgsInterval mFrameDuration;
-
-    double mFrameTimeStep = 1.0;
-    QgsUnitTypes::TemporalUnit mFrameTimeStepUnit = QgsUnitTypes::TemporalUnit::TemporalHours;
+    QgsInterval mFrameDuration = QgsInterval( 1.0, QgsUnitTypes::TemporalUnit::TemporalHours );
 
     //! Member for frame rate
     double mFramesPerSecond = 1;

src/core/qgstemporalutils.cpp

@@ -80,8 +80,7 @@ bool QgsTemporalUtils::exportAnimation( const QgsMapSettings &mapSettings, const
 
   QgsTemporalNavigationObject navigator;
   navigator.setTemporalExtents( settings.animationRange );
-  navigator.setFrameTimeStep( settings.frameTimeStep );
-  navigator.setFrameTimeStepUnit( settings.frameTimeStepUnit );
+  navigator.setFrameDuration( QgsInterval( settings.frameTimeStep, settings.frameTimeStepUnit ) );
   QgsMapSettings ms = mapSettings;
   const QgsExpressionContext context = ms.expressionContext();
 
@@ -144,54 +143,49 @@ bool QgsTemporalUtils::exportAnimation( const QgsMapSettings &mapSettings, const
 }
 
 
-QDateTime QgsTemporalUtils::calculateFrameTime( const QDateTime &start, const long long frame, const double timeStep, const QgsUnitTypes::TemporalUnit timeStepUnit )
+QDateTime QgsTemporalUtils::calculateFrameTime( const QDateTime &start, const long long frame, const QgsInterval interval )
 {
 
   double unused;
-  bool isFractional = fabs( modf( timeStep, &unused ) ) > 0.00001;
+  const bool isFractional = !qgsDoubleNear( fabs( modf( interval.originalDuration(), &unused ) ), 0.0 );
 
-  if ( isFractional )
+  if ( isFractional || interval.originalUnit() == QgsUnitTypes::TemporalUnit::TemporalUnknownUnit )
   {
-    double duration = QgsInterval( timeStep, timeStepUnit ).seconds();
-    return start.addSecs( frame * duration );
+    return start + interval;
   }
   else
   {
-    switch ( timeStepUnit )
+    switch ( interval.originalUnit() )
     {
       case QgsUnitTypes::TemporalUnit::TemporalMilliseconds:
-        return start.addMSecs( frame * timeStep );
+        return start.addMSecs( frame * interval.originalDuration() );
         break;
       case QgsUnitTypes::TemporalUnit::TemporalSeconds:
-        return start.addSecs( frame * timeStep );
+        return start.addSecs( frame * interval.originalDuration() );
         break;
       case QgsUnitTypes::TemporalUnit::TemporalMinutes:
-        return start.addSecs( 60 * frame * timeStep );
+        return start.addSecs( 60 * frame * interval.originalDuration() );
         break;
       case QgsUnitTypes::TemporalUnit::TemporalHours:
-        return start.addSecs( 3600 * frame * timeStep );
+        return start.addSecs( 3600 * frame * interval.originalDuration() );
         break;
       case QgsUnitTypes::TemporalUnit::TemporalDays:
-        return start.addDays( frame * timeStep );
+        return start.addDays( frame * interval.originalDuration() );
         break;
       case QgsUnitTypes::TemporalUnit::TemporalWeeks:
-        return start.addDays( 7 * frame * timeStep );
+        return start.addDays( 7 * frame * interval.originalDuration() );
         break;
       case QgsUnitTypes::TemporalUnit::TemporalMonths:
-        return start.addMonths( frame * timeStep );
+        return start.addMonths( frame * interval.originalDuration() );
         break;
       case QgsUnitTypes::TemporalUnit::TemporalYears:
-        return start.addYears( frame * timeStep );
+        return start.addYears( frame * interval.originalDuration() );
         break;
       case QgsUnitTypes::TemporalUnit::TemporalDecades:
-        return start.addYears( 10 * frame * timeStep );
+        return start.addYears( 10 * frame * interval.originalDuration() );
         break;
       case QgsUnitTypes::TemporalUnit::TemporalCenturies:
-        return start.addYears( 100 * frame * timeStep );
-        break;
-      case QgsUnitTypes::TemporalUnit::TemporalUnknownUnit:
-        Q_ASSERT( false );
-        return start;
+        return start.addYears( 100 * frame * interval.originalDuration() );
         break;
     }
   }

src/core/qgstemporalutils.h

@@ -96,21 +96,25 @@ class CORE_EXPORT QgsTemporalUtils
     /**
      * 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.
+     * If the interval original duration is fractional or interval original unit is
+     * unknown (QgsUnitTypes::TemporalUnit::TemporalUnknownUnit), 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
+     * 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
+     * \param start time of the animation
+     * \param frame number
+     * \param interval duration of the animation
      *
      * \returns The calculated datetime for the frame.
+     *
+     * \since QGIS 3.18
      */
-    static QDateTime calculateFrameTime( const QDateTime &start, const long long frame, double timeStep, QgsUnitTypes::TemporalUnit timeStepUnit );
+    static QDateTime calculateFrameTime( const QDateTime &start, const long long frame, const QgsInterval interval );
 };
 
 

src/gui/qgstemporalcontrollerwidget.cpp

@@ -297,8 +297,9 @@ void QgsTemporalControllerWidget::updateFrameDuration()
 
   if ( !mBlockFrameDurationUpdates )
   {
-    mNavigationObject->setFrameTimeStep( QgsProject::instance()->timeSettings()->timeStep() );
-    mNavigationObject->setFrameTimeStepUnit( QgsProject::instance()->timeSettings()->timeStepUnit() );
+    mNavigationObject->setFrameDuration(
+      QgsInterval( QgsProject::instance()->timeSettings()->timeStep(),
+                   QgsProject::instance()->timeSettings()->timeStepUnit() ) );
     mSlider->setValue( mNavigationObject->currentFrameNumber() );
   }
   mSlider->setRange( 0, mNavigationObject->totalFrameCount() - 1 );

tests/src/core/testqgstemporalnavigationobject.cpp

@@ -86,8 +86,7 @@ void TestQgsTemporalNavigationObject::animationState()
                            );
   navigationObject->setTemporalExtents( range );
 
-  navigationObject->setFrameTimeStep( 1 );
-  navigationObject->setFrameTimeStepUnit( QgsUnitTypes::TemporalMonths );
+  navigationObject->setFrameDuration( QgsInterval( 1, QgsUnitTypes::TemporalMonths ) );
 
   qRegisterMetaType<QgsTemporalNavigationObject::AnimationState>( "AnimationState" );
   QSignalSpy stateSignal( navigationObject, &QgsTemporalNavigationObject::stateChanged );
@@ -196,10 +195,9 @@ void TestQgsTemporalNavigationObject::frameSettings()
   navigationObject->setTemporalExtents( range );
   QCOMPARE( temporalRangeSignal.count(), 1 );
 
-  navigationObject->setFrameTimeStep( 1 );
-  navigationObject->setFrameTimeStepUnit( QgsUnitTypes::TemporalHours );
-  QCOMPARE( navigationObject->frameTimeStep(), 1.0 );
-  QCOMPARE( navigationObject->frameTimeStepUnit(), QgsUnitTypes::TemporalHours );
+  navigationObject->setFrameDuration( QgsInterval( 1, QgsUnitTypes::TemporalHours ) );
+  QCOMPARE( navigationObject->frameDuration().originalDuration(), 1.0 );
+  QCOMPARE( navigationObject->frameDuration().originalUnit(), QgsUnitTypes::TemporalHours );
 
   QCOMPARE( navigationObject->currentFrameNumber(), 0 );
   QCOMPARE( navigationObject->totalFrameCount(), 5 );
@@ -244,8 +242,7 @@ void TestQgsTemporalNavigationObject::expressionContext()
                              QDateTime( QDate( 2020, 1, 1 ), QTime( 12, 0, 0 ) )
                            );
   object.setTemporalExtents( range );
-  object.setFrameTimeStep( 1 );
-  object.setFrameTimeStepUnit( QgsUnitTypes::TemporalHours );
+  object.setFrameDuration( QgsInterval( 1, QgsUnitTypes::TemporalHours ) );
   object.setCurrentFrameNumber( 1 );
   object.setFramesPerSecond( 30 );