Address review

nirvn · nirvn · commit d0060fd89a27 · 2020-05-10T14:02:11.000+07:00
diff --git 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
@@ -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
@@ -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
@@ -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;

Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,7 @@ Base class for temporal property.`
`24`	`24`	`#include "qgstemporalproperty.h"`
`25`	`25`	`%End`
`26`	`26`	`public:`
	`27`	`+`
`27`	`28`	`enum Flag`
`28`	`29`	`{`
`29`	`30`	`NoFlags,`
Original file line number	Diff line number	Diff line change
`@@ -698,12 +698,12 @@ void QgsMapCanvas::rendererJobFinished()`
`698`	`698`	`{`
`699`	`699`	`mLastLayerRenderTime.insert( it.key()->id(), it.value() );`
`700`	`700`	`}`
`701`		`- if ( mUsePreviewJobs && !mTemporalRefreshAfterJob )`
	`701`	`+ if ( mUsePreviewJobs && !mRefreshAfterJob )`
`702`	`702`	`startPreviewJobs();`
`703`	`703`	`}`
`704`	`704`	`else`
`705`	`705`	`{`
`706`		`- mTemporalRefreshAfterJob = false;`
	`706`	`+ mRefreshAfterJob = false;`
`707`	`707`	`}`
`708`	`708`
`709`	`709`	`// now we are in a slot called from mJob - do not delete it immediately`
`@@ -713,9 +713,9 @@ void QgsMapCanvas::rendererJobFinished()`
`713`	`713`
`714`	`714`	`emit mapCanvasRefreshed();`
`715`	`715`
`716`		`- if ( mTemporalRefreshAfterJob )`
	`716`	`+ if ( mRefreshAfterJob )`
`717`	`717`	`{`
`718`		`- mTemporalRefreshAfterJob = false;`
	`718`	`+ mRefreshAfterJob = false;`
`719`	`719`	`clearTemporalCache();`
`720`	`720`	`refresh();`
`721`	`721`	`}`
`@@ -801,17 +801,12 @@ void QgsMapCanvas::setTemporalRange( const QgsDateTimeRange &dateTimeRange )`
`801`	`801`
`802`	`802`	`emit temporalRangeChanged();`
`803`	`803`
	`804`	`+ // we need to discard any previously cached images which have temporal properties enabled, so that these will be updated when`
	`805`	`+ // the canvas is redrawn`
`804`	`806`	`if ( !mJob )`
`805`		`- {`
`806`		`- // we need to discard any previously cached images which have temporal properties enabled, so that these will be updated when`
`807`		`- // the canvas is redrawn`
`808`	`807`	`clearTemporalCache();`
`809`		`- autoRefreshTriggered();`
`810`		`- }`
`811`		`- else`
`812`		`- {`
`813`		`- mTemporalRefreshAfterJob = true;`
`814`		`- }`
	`808`	`+`
	`809`	`+ autoRefreshTriggered();`
`815`	`810`	`}`
`816`	`811`
`817`	`812`	`const QgsDateTimeRange &QgsMapCanvas::temporalRange() const`
`@@ -2059,9 +2054,9 @@ void QgsMapCanvas::autoRefreshTriggered()`
`2059`	`2054`	`{`
`2060`	`2055`	`if ( mJob )`
`2061`	`2056`	`{`
`2062`		`- // canvas is currently being redrawn, so we skip this auto refresh`
`2063`		`- // otherwise we could get stuck in the situation where an auto refresh is triggered`
`2064`		`- // too often to allow the canvas to ever finish rendering`
	`2057`	`+ // canvas is currently being redrawn, so we defer the last requested`
	`2058`	`+ // auto refresh until current rendering job finishes`
	`2059`	`+ mRefreshAfterJob = true;`
`2065`	`2060`	`return;`
`2066`	`2061`	`}`
`2067`	`2062`