From d0060fd89a27eb143713f88f764c0b7abe9b7bea Mon Sep 17 00:00:00 2001
From: nirvn <nirvn.asia@gmail.com>
Date: Sat, 9 May 2020 11:17:05 +0700
Subject: [PATCH] Address review

---
 .../auto_generated/qgstemporalproperty.sip.in |  1 +
 src/core/qgstemporalproperty.h                |  6 ++++-
 src/gui/qgsmapcanvas.cpp                      | 27 ++++++++-----------
 src/gui/qgsmapcanvas.h                        |  4 +--
 4 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/python/core/auto_generated/qgstemporalproperty.sip.in b/python/core/auto_generated/qgstemporalproperty.sip.in
index 9e4204a73d2..95ea547c606 100644
--- a/python/core/auto_generated/qgstemporalproperty.sip.in
+++ b/python/core/auto_generated/qgstemporalproperty.sip.in
@@ -24,6 +24,7 @@ Base class for temporal property.
 #include "qgstemporalproperty.h"
 %End
   public:
+
     enum Flag
     {
       NoFlags,
diff --git a/src/core/qgstemporalproperty.h b/src/core/qgstemporalproperty.h
index dbb3edf70b8..0b6f11656fd 100644
--- a/src/core/qgstemporalproperty.h
+++ b/src/core/qgstemporalproperty.h
@@ -37,10 +37,14 @@ class CORE_EXPORT QgsTemporalProperty : public QObject
     Q_OBJECT
 
   public:
+
+    /**
+     * Flags attached to the temporal property.
+     */
     enum Flag
     {
       NoFlags                                         = 0,
-      FlagDontInvalidateCachedRendersWhenRangeChanges = 1  //!< Any cached rendering will not be invalidated when temporal range context is modified
+      FlagDontInvalidateCachedRendersWhenRangeChanges = 1  //!< Any cached rendering will not be invalidated when temporal range context is modified.
     };
     Q_DECLARE_FLAGS( Flags, Flag )
 
diff --git a/src/gui/qgsmapcanvas.cpp b/src/gui/qgsmapcanvas.cpp
index 4217f29da23..22935724997 100644
--- a/src/gui/qgsmapcanvas.cpp
+++ b/src/gui/qgsmapcanvas.cpp
@@ -698,12 +698,12 @@ void QgsMapCanvas::rendererJobFinished()
     {
       mLastLayerRenderTime.insert( it.key()->id(), it.value() );
     }
-    if ( mUsePreviewJobs && !mTemporalRefreshAfterJob )
+    if ( mUsePreviewJobs && !mRefreshAfterJob )
       startPreviewJobs();
   }
   else
   {
-    mTemporalRefreshAfterJob = false;
+    mRefreshAfterJob = false;
   }
 
   // now we are in a slot called from mJob - do not delete it immediately
@@ -713,9 +713,9 @@ void QgsMapCanvas::rendererJobFinished()
 
   emit mapCanvasRefreshed();
 
-  if ( mTemporalRefreshAfterJob )
+  if ( mRefreshAfterJob )
   {
-    mTemporalRefreshAfterJob = false;
+    mRefreshAfterJob = false;
     clearTemporalCache();
     refresh();
   }
@@ -801,17 +801,12 @@ void QgsMapCanvas::setTemporalRange( const QgsDateTimeRange &dateTimeRange )
 
   emit temporalRangeChanged();
 
+  // we need to discard any previously cached images which have temporal properties enabled, so that these will be updated when
+  // the canvas is redrawn
   if ( !mJob )
-  {
-    // we need to discard any previously cached images which have temporal properties enabled, so that these will be updated when
-    // the canvas is redrawn
     clearTemporalCache();
-    autoRefreshTriggered();
-  }
-  else
-  {
-    mTemporalRefreshAfterJob = true;
-  }
+
+  autoRefreshTriggered();
 }
 
 const QgsDateTimeRange &QgsMapCanvas::temporalRange() const
@@ -2059,9 +2054,9 @@ void QgsMapCanvas::autoRefreshTriggered()
 {
   if ( mJob )
   {
-    // canvas is currently being redrawn, so we skip this auto refresh
-    // otherwise we could get stuck in the situation where an auto refresh is triggered
-    // too often to allow the canvas to ever finish rendering
+    // canvas is currently being redrawn, so we defer the last requested
+    // auto refresh until current rendering job finishes
+    mRefreshAfterJob = true;
     return;
   }
 
diff --git a/src/gui/qgsmapcanvas.h b/src/gui/qgsmapcanvas.h
index 50d3072301c..9b0ab7bbcbe 100644
--- a/src/gui/qgsmapcanvas.h
+++ b/src/gui/qgsmapcanvas.h
@@ -1063,8 +1063,8 @@ class GUI_EXPORT QgsMapCanvas : public QGraphicsView
     //! Flag that allows squashing multiple refresh() calls into just one delayed rendering job
     bool mRefreshScheduled = false;
 
-    //! Flag that triggers a refresh after an ongoing rendering job finishes and clear cache for temporal-enabled layers
-    bool mTemporalRefreshAfterJob = false;
+    //! Flag that triggers a refresh after an ongoing rendering job triggered by autoRefresh
+    bool mRefreshAfterJob = false;
 
     //! determines whether user has requested to suppress rendering
     bool mRenderFlag = true;