[processing] Proper progress reports during model execution

Instead of showing the progress reports for each child algorithm individually, which leads to repeated 0->100% progress for every step of a model, we proxy the progress reports and account for the overall progress through a model as well. This means that the progress accounts for both the progress within the current model step AND the total number of steps left to execute.
qgis · Sep 22, 2017 · 6afe25e · 6afe25e
1 parent 9caa722
commit 6afe25e
Show file tree

Hide file tree

Showing 6 changed files with 138 additions and 16 deletions.
diff --git a/python/core/processing/models/qgsprocessingmodelalgorithm.sip b/python/core/processing/models/qgsprocessingmodelalgorithm.sip
@@ -374,6 +374,8 @@ Translated description of variable
 };
 
 
+
+
 /************************************************************************
  * This file has been generated automatically from                      *
  *                                                                      *

diff --git a/python/core/qgsfeedback.sip b/python/core/qgsfeedback.sip
@@ -38,11 +38,6 @@ class QgsFeedback : QObject
     QgsFeedback( QObject *parent /TransferThis/ = 0 );
 %Docstring
 Construct a feedback object
-%End
-
-    void cancel();
-%Docstring
-Tells the internal routines that the current operation should be canceled. This should be run by the main thread
 %End
 
     bool isCanceled() const;
@@ -71,6 +66,13 @@ Tells whether the operation has been canceled already
  :rtype: float
 %End
 
+  public slots:
+
+    void cancel();
+%Docstring
+Tells the internal routines that the current operation should be canceled. This should be run by the main thread
+%End
+
   signals:
     void canceled();
 %Docstring

diff --git a/src/core/CMakeLists.txt b/src/core/CMakeLists.txt
@@ -682,6 +682,7 @@ SET(QGIS_CORE_MOC_HDRS
   processing/qgsprocessingfeedback.h
   processing/qgsprocessingprovider.h
   processing/qgsprocessingregistry.h
+  processing/models/qgsprocessingmodelalgorithm.h
 
   providers/memory/qgsmemoryprovider.h
 
@@ -978,7 +979,6 @@ SET(QGIS_CORE_HDRS
   processing/qgsprocessingoutputs.h
   processing/qgsprocessingparameters.h
   processing/qgsprocessingutils.h
-  processing/models/qgsprocessingmodelalgorithm.h
   processing/models/qgsprocessingmodelchildalgorithm.h
   processing/models/qgsprocessingmodelchildparametersource.h
   processing/models/qgsprocessingmodelcomponent.h

diff --git a/src/core/processing/models/qgsprocessingmodelalgorithm.cpp b/src/core/processing/models/qgsprocessingmodelalgorithm.cpp
@@ -214,6 +214,7 @@ QVariantMap QgsProcessingModelAlgorithm::processAlgorithm( const QVariantMap &pa
   QTime totalTime;
   totalTime.start();
 
+  QgsProcessingModelFeedback modelFeedback( toExecute.count(), feedback );
   QgsExpressionContext baseContext = createExpressionContext( parameters, context );
 
   QVariantMap childResults;
@@ -225,6 +226,9 @@ QVariantMap QgsProcessingModelAlgorithm::processAlgorithm( const QVariantMap &pa
     executedAlg = false;
     Q_FOREACH ( const QString &childId, toExecute )
     {
+      if ( feedback && feedback->isCanceled() )
+        break;
+
       if ( executed.contains( childId ) )
         continue;
 
@@ -260,7 +264,7 @@ QVariantMap QgsProcessingModelAlgorithm::processAlgorithm( const QVariantMap &pa
 
       bool ok = false;
       std::unique_ptr< QgsProcessingAlgorithm > childAlg( child.algorithm()->create( child.configuration() ) );
-      QVariantMap results = childAlg->run( childParams, context, feedback, &ok );
+      QVariantMap results = childAlg->run( childParams, context, &modelFeedback, &ok );
       childAlg.reset( nullptr );
       if ( !ok )
       {
@@ -280,8 +284,12 @@ QVariantMap QgsProcessingModelAlgorithm::processAlgorithm( const QVariantMap &pa
       }
 
       executed.insert( childId );
+      modelFeedback.setCurrentStep( executed.count() );
       feedback->pushDebugInfo( QObject::tr( "OK. Execution took %1 s (%2 outputs)." ).arg( childTime.elapsed() / 1000.0 ).arg( results.count() ) );
     }
+
+    if ( feedback && feedback->isCanceled() )
+      break;
   }
   feedback->pushDebugInfo( QObject::tr( "Model processed OK. Executed %1 algorithms total in %2 s." ).arg( executed.count() ).arg( totalTime.elapsed() / 1000.0 ) );
 
@@ -1123,4 +1131,65 @@ QgsProcessingAlgorithm *QgsProcessingModelAlgorithm::createInstance() const
   return alg;
 }
 
+
+
+
+//
+// QgsProcessingModelFeedback
+//
+
+QgsProcessingModelFeedback::QgsProcessingModelFeedback( int childAlgorithmCount, QgsProcessingFeedback *feedback )
+  : mChildSteps( childAlgorithmCount )
+  , mFeedback( feedback )
+{
+  connect( mFeedback, &QgsFeedback::canceled, this, &QgsFeedback::cancel, Qt::DirectConnection );
+  connect( this, &QgsFeedback::progressChanged, this, &QgsProcessingModelFeedback::updateOverallProgress );
+}
+
+void QgsProcessingModelFeedback::setCurrentStep( int step )
+{
+  mCurrentStep = step;
+  mFeedback->setProgress( 100.0 * static_cast< double >( mCurrentStep ) / mChildSteps );
+}
+
+void QgsProcessingModelFeedback::setProgressText( const QString &text )
+{
+  mFeedback->setProgressText( text );
+}
+
+void QgsProcessingModelFeedback::reportError( const QString &error )
+{
+  mFeedback->reportError( error );
+}
+
+void QgsProcessingModelFeedback::pushInfo( const QString &info )
+{
+  mFeedback->pushInfo( info );
+}
+
+void QgsProcessingModelFeedback::pushCommandInfo( const QString &info )
+{
+  mFeedback->pushCommandInfo( info );
+}
+
+void QgsProcessingModelFeedback::pushDebugInfo( const QString &info )
+{
+  mFeedback->pushDebugInfo( info );
+}
+
+void QgsProcessingModelFeedback::pushConsoleInfo( const QString &info )
+{
+  mFeedback->pushConsoleInfo( info );
+}
+
+void QgsProcessingModelFeedback::updateOverallProgress( double progress )
+{
+  double baseProgress = 100.0 * static_cast< double >( mCurrentStep ) / mChildSteps;
+  double currentAlgorithmProgress = progress / mChildSteps;
+  mFeedback->setProgress( baseProgress + currentAlgorithmProgress );
+}
+
+
+
 ///@endcond
+
diff --git a/src/core/processing/models/qgsprocessingmodelalgorithm.h b/src/core/processing/models/qgsprocessingmodelalgorithm.h
@@ -403,6 +403,53 @@ class CORE_EXPORT QgsProcessingModelAlgorithm : public QgsProcessingAlgorithm
     friend class TestQgsProcessing;
 };
 
+
+#ifndef SIP_RUN
+
+/**
+ * Model algorithm feedback which proxies its calls to an underlying
+ * feedback object, but scales overall progress reports to account
+ * for the number of child steps in a model.
+ */
+class QgsProcessingModelFeedback : public QgsProcessingFeedback
+{
+    Q_OBJECT
+
+  public:
+
+    /**
+     * Constructor for QgsProcessingModelFeedback, for a model with the specified
+     * number of active child algorithms. This feedback object will proxy calls
+     * to the specified \a feedback object.
+     */
+    QgsProcessingModelFeedback( int childAlgorithmCount, QgsProcessingFeedback *feedback );
+
+    /**
+     * Sets the current child algorithm \a step which is being executed. This is used
+     * to scale the overall progress to account for progress through the overall model.
+     */
+    void setCurrentStep( int step );
+
+    void setProgressText( const QString &text ) override;
+    void reportError( const QString &error ) override;
+    void pushInfo( const QString &info ) override;
+    void pushCommandInfo( const QString &info ) override;
+    void pushDebugInfo( const QString &info ) override;
+    void pushConsoleInfo( const QString &info ) override;
+
+  private slots:
+
+    void updateOverallProgress( double progress );
+
+  private:
+
+    int mChildSteps = 0;
+    int mCurrentStep = 0;
+    QgsProcessingFeedback *mFeedback = nullptr;
+};
+
+#endif
+
 ///@endcond
 
 #endif // QGSPROCESSINGMODELALGORITHM_H
diff --git a/src/core/qgsfeedback.h b/src/core/qgsfeedback.h
@@ -50,15 +50,6 @@ class CORE_EXPORT QgsFeedback : public QObject
       , mCanceled( false )
     {}
 
-    //! Tells the internal routines that the current operation should be canceled. This should be run by the main thread
-    void cancel()
-    {
-      if ( mCanceled )
-        return;  // only emit the signal once
-      mCanceled = true;
-      emit canceled();
-    }
-
     //! Tells whether the operation has been canceled already
     bool isCanceled() const { return mCanceled; }
 
@@ -88,6 +79,17 @@ class CORE_EXPORT QgsFeedback : public QObject
      */
     double progress() const { return mProgress; }
 
+  public slots:
+
+    //! Tells the internal routines that the current operation should be canceled. This should be run by the main thread
+    void cancel()
+    {
+      if ( mCanceled )
+        return;  // only emit the signal once
+      mCanceled = true;
+      emit canceled();
+    }
+
   signals:
     //! Internal routines can connect to this signal if they use event loop
     void canceled();