Merge pull request #5235 from nyalldawson/model_progress2

[processing] Proper progress reports during model execution

python/core/core_auto.sip

@@ -176,7 +176,6 @@
 %Include processing/qgsprocessingoutputs.sip
 %Include processing/qgsprocessingparameters.sip
 %Include processing/qgsprocessingutils.sip
-%Include processing/models/qgsprocessingmodelalgorithm.sip
 %Include processing/models/qgsprocessingmodelchildalgorithm.sip
 %Include processing/models/qgsprocessingmodelchildparametersource.sip
 %Include processing/models/qgsprocessingmodelcomponent.sip
@@ -384,6 +383,7 @@
 %Include processing/qgsprocessingfeedback.sip
 %Include processing/qgsprocessingprovider.sip
 %Include processing/qgsprocessingregistry.sip
+%Include processing/models/qgsprocessingmodelalgorithm.sip
 %Include raster/qgsrasterfilewritertask.sip
 %Include raster/qgsrasterlayer.sip
 %Include raster/qgsrasterdataprovider.sip

python/core/processing/models/qgsprocessingmodelalgorithm.sip

@@ -374,6 +374,8 @@ Translated description of variable
 };
 
 
+
+
 /************************************************************************
  * This file has been generated automatically from                      *
  *                                                                      *

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

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

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;
 
@@ -252,15 +256,23 @@ QVariantMap QgsProcessingModelAlgorithm::processAlgorithm( const QVariantMap &pa
 
       QVariantMap childParams = parametersForChildAlgorithm( child, parameters, childResults, expContext );
       feedback->setProgressText( QObject::tr( "Running %1 [%2/%3]" ).arg( child.description() ).arg( executed.count() + 1 ).arg( toExecute.count() ) );
-      //feedback->pushDebugInfo( "Parameters: " + ', '.join( [str( p ).strip() +
-      //           '=' + str( p.value ) for p in alg.algorithm.parameters] ) )
+
+      QStringList params;
+      for ( auto childParamIt = childParams.constBegin(); childParamIt != childParams.constEnd(); ++childParamIt )
+      {
+        params << QStringLiteral( "%1: %2" ).arg( childParamIt.key(),
+               child.algorithm()->parameterDefinition( childParamIt.key() )->valueAsPythonString( childParamIt.value(), context ) );
+      }
+
+      feedback->pushInfo( QObject::tr( "Input Parameters:" ) );
+      feedback->pushCommandInfo( QStringLiteral( "{ %1 }" ).arg( params.join( QStringLiteral( ", " ) ) ) );
 
       QTime childTime;
       childTime.start();
 
       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 +292,12 @@ QVariantMap QgsProcessingModelAlgorithm::processAlgorithm( const QVariantMap &pa
       }
 
       executed.insert( childId );
-      feedback->pushDebugInfo( QObject::tr( "OK. Execution took %1 s (%2 outputs)." ).arg( childTime.elapsed() / 1000.0 ).arg( results.count() ) );
+      modelFeedback.setCurrentStep( executed.count() );
+      feedback->pushInfo( 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 +1139,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
+

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

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();