[stats] gather field/expression values in a thread to prevent long UI…

… freeze
qgis · Feb 7, 2018 · 8495e8b · 8495e8b
1 parent c12884c
commit 8495e8b
Show file tree

Hide file tree

Showing 4 changed files with 199 additions and 32 deletions.
diff --git a/src/app/qgsstatisticalsummarydockwidget.cpp b/src/app/qgsstatisticalsummarydockwidget.cpp
@@ -77,6 +77,9 @@ QgsStatisticalSummaryDockWidget::QgsStatisticalSummaryDockWidget( QWidget *paren
 {
   setupUi( this );
 
+  mCancelButton->hide();
+  mCalculatingProgressBar->hide();
+
   mFieldExpressionWidget->registerExpressionContextGenerator( this );
 
   mLayerComboBox->setFilters( QgsMapLayerProxyModel::VectorLayer );
@@ -101,6 +104,15 @@ QgsStatisticalSummaryDockWidget::QgsStatisticalSummaryDockWidget( QWidget *paren
   refreshStatisticsMenu();
 }
 
+QgsStatisticalSummaryDockWidget::~QgsStatisticalSummaryDockWidget()
+{
+  if ( mGatherer )
+  {
+    mGatherer->stop();
+    mGatherer->wait(); // mGatherer is deleted when wait completes
+  }
+}
+
 void QgsStatisticalSummaryDockWidget::fieldChanged()
 {
   if ( mFieldExpressionWidget->expression() != mExpression )
@@ -131,35 +143,73 @@ void QgsStatisticalSummaryDockWidget::refreshStatistics()
     mPreviousFieldType = mFieldType;
   }
 
+  QString sourceFieldExp = mFieldExpressionWidget->currentField();
   bool selectedOnly = mSelectedOnlyCheckBox->isChecked();
 
+  if ( mGatherer )
+  {
+    mGatherer->stop();
+    mGatherer->wait();
+  }
+
   switch ( mFieldType )
   {
     case DataType::Numeric:
-      updateNumericStatistics( selectedOnly );
+      mGatherer = new QgsStatisticsValueGatherer( mLayer, sourceFieldExp, selectedOnly );
+      connect( mGatherer, &QgsStatisticsValueGatherer::gatheredValues, this, &QgsStatisticalSummaryDockWidget::updateNumericStatistics );
       break;
     case DataType::String:
-      updateStringStatistics( selectedOnly );
+      mGatherer = new QgsStatisticsValueGatherer( mLayer, sourceFieldExp, selectedOnly );
+      connect( mGatherer, &QgsStatisticsValueGatherer::gatheredValues, this, &QgsStatisticalSummaryDockWidget::updateStringStatistics );
       break;
     case DataType::DateTime:
-      updateDateTimeStatistics( selectedOnly );
+      mGatherer = new QgsStatisticsValueGatherer( mLayer, sourceFieldExp, selectedOnly );
+      connect( mGatherer, &QgsStatisticsValueGatherer::gatheredValues, this, &QgsStatisticalSummaryDockWidget::updateDateTimeStatistics );
       break;
     default:
       break;
   }
+
+  connect( mGatherer, &QgsStatisticsValueGatherer::finished, this, &QgsStatisticalSummaryDockWidget::gathererThreadFinished );
+  connect( mGatherer, &QgsStatisticsValueGatherer::progressChanged, mCalculatingProgressBar, &QProgressBar::setValue );
+  connect( mCancelButton, &QPushButton::clicked, mGatherer, &QgsStatisticsValueGatherer::stop );
+  mCalculatingProgressBar->setMinimum( 0 );
+  mCalculatingProgressBar->setMaximum( 0 );
+  mCalculatingProgressBar->setValue( 0 );
+  mCancelButton->show();
+  mCalculatingProgressBar->show();
+
+  mGatherer->start();
 }
 
-void QgsStatisticalSummaryDockWidget::updateNumericStatistics( bool selectedOnly )
+void QgsStatisticalSummaryDockWidget::gathererThreadFinished()
 {
-  QString sourceFieldExp = mFieldExpressionWidget->currentField();
+  mGatherer->deleteLater();
+  mGatherer = nullptr;
+  mCalculatingProgressBar->setValue( -1 );
+  mCancelButton->hide();
+  mCalculatingProgressBar->hide();
+}
 
-  bool ok;
-  int missingValues = 0;
-  QList< double > values = mLayer->getDoubleValues( sourceFieldExp, ok, selectedOnly, &missingValues );
+void QgsStatisticalSummaryDockWidget::updateNumericStatistics()
+{
+  if ( !mGatherer || mGatherer->wasCanceled() )
+    return;
 
-  if ( ! ok )
+  QList< QVariant > variantValues = mGatherer->values();
+
+  QList<double> values;
+  bool convertOk;
+  int missingValues = 0;
+  Q_FOREACH ( const QVariant &value, variantValues )
   {
-    return;
+    double val = value.toDouble( &convertOk );
+    if ( convertOk )
+      values << val;
+    else if ( value.isNull() )
+    {
+      missingValues += 1;
+    }
   }
 
   QList< QgsStatisticalSummary::Statistic > statsToDisplay;
@@ -201,19 +251,17 @@ void QgsStatisticalSummaryDockWidget::updateNumericStatistics( bool selectedOnly
             stats.count() != 0 || missingValues != 0 );
     row++;
   }
+
+  mCancelButton->hide();
+  mCalculatingProgressBar->hide();
 }
 
-void QgsStatisticalSummaryDockWidget::updateStringStatistics( bool selectedOnly )
+void QgsStatisticalSummaryDockWidget::updateStringStatistics()
 {
-  QString field = mFieldExpressionWidget->currentField();
-
-  bool ok;
-  QVariantList values = mLayer->getValues( field, ok, selectedOnly );
-
-  if ( ! ok )
-  {
+  if ( !mGatherer || mGatherer->wasCanceled() )
     return;
-  }
+
+  QVariantList values = mGatherer->values();//mLayer->getValues( field, ok, selectedOnly );
 
   QList< QgsStringStatisticalSummary::Statistic > statsToDisplay;
   QgsStringStatisticalSummary::Statistics statsToCalc = nullptr;
@@ -241,6 +289,9 @@ void QgsStatisticalSummaryDockWidget::updateStringStatistics( bool selectedOnly
             stats.count() != 0 );
     row++;
   }
+
+  mCancelButton->hide();
+  mCalculatingProgressBar->hide();
 }
 
 void QgsStatisticalSummaryDockWidget::layerChanged( QgsMapLayer *layer )
@@ -260,6 +311,12 @@ void QgsStatisticalSummaryDockWidget::layerChanged( QgsMapLayer *layer )
 
   mFieldExpressionWidget->setLayer( mLayer );
 
+  if ( mGatherer )
+  {
+    mGatherer->stop();
+    mGatherer->wait();
+  }
+
   if ( mFieldExpressionWidget->currentField().isEmpty() )
   {
     mStatisticsTable->setRowCount( 0 );
@@ -319,17 +376,12 @@ void QgsStatisticalSummaryDockWidget::layerSelectionChanged()
     refreshStatistics();
 }
 
-void QgsStatisticalSummaryDockWidget::updateDateTimeStatistics( bool selectedOnly )
+void QgsStatisticalSummaryDockWidget::updateDateTimeStatistics()
 {
-  QString field = mFieldExpressionWidget->currentField();
-
-  bool ok;
-  QVariantList values = mLayer->getValues( field, ok, selectedOnly );
-
-  if ( ! ok )
-  {
+  if ( !mGatherer || mGatherer->wasCanceled() )
     return;
-  }
+
+  QVariantList values = mGatherer->values();
 
   QList< QgsDateTimeStatisticalSummary::Statistic > statsToDisplay;
   QgsDateTimeStatisticalSummary::Statistics statsToCalc = nullptr;
@@ -362,6 +414,9 @@ void QgsStatisticalSummaryDockWidget::updateDateTimeStatistics( bool selectedOnl
             stats.count() != 0 );
     row++;
   }
+
+  mCancelButton->hide();
+  mCalculatingProgressBar->hide();
 }
 
 void QgsStatisticalSummaryDockWidget::addRow( int row, const QString &name, const QString &value,

diff --git a/src/app/qgsstatisticalsummarydockwidget.h b/src/app/qgsstatisticalsummarydockwidget.h
@@ -22,6 +22,8 @@
 #include "qgsstringstatisticalsummary.h"
 #include "qgsdatetimestatisticalsummary.h"
 #include "qgsdockwidget.h"
+#include "qgsfeedback.h"
+#include "qgsvectorlayer.h"
 #include "qgis_app.h"
 
 class QMenu;
@@ -32,6 +34,89 @@ class QgsLayerItem;
 class QgsDataItem;
 class QgsBrowserTreeFilterProxyModel;
 
+
+/**
+ * \class QgsStatisticsValueGatherer
+* Calculated raster stats for paletted renderer in a thread
+*/
+class QgsStatisticsValueGatherer: public QThread
+{
+    Q_OBJECT
+
+  public:
+    QgsStatisticsValueGatherer( QgsVectorLayer *layer, QString sourceFieldExp, bool selectedOnly )
+      : mLayer( layer )
+      , mExpression( sourceFieldExp )
+      , mSelectedOnly( selectedOnly )
+      , mWasCanceled( false )
+    {}
+
+    void run() override
+    {
+      mWasCanceled = false;
+
+      // allow responsive cancelation
+      mFeedback = new QgsFeedback();
+      connect( mFeedback, &QgsFeedback::progressChanged, this, &QgsStatisticsValueGatherer::progressChanged );
+
+      bool ok;
+      mValues = mLayer->getValues( mExpression, ok, mSelectedOnly, mFeedback );
+      if ( !ok )
+      {
+        mWasCanceled = true;
+      }
+
+      // be overly cautious - it's *possible* stop() might be called between deleting mFeedback and nulling it
+      mFeedbackMutex.lock();
+      delete mFeedback;
+      mFeedback = nullptr;
+      mFeedbackMutex.unlock();
+
+      emit gatheredValues();
+    }
+
+    //! Informs the gatherer to immediately stop collecting values
+    void stop()
+    {
+      // be cautious, in case gatherer stops naturally just as we are canceling it and mFeedback gets deleted
+      mFeedbackMutex.lock();
+      if ( mFeedback )
+        mFeedback->cancel();
+      mFeedbackMutex.unlock();
+
+      mWasCanceled = true;
+    }
+
+    //! Returns true if collection was canceled before completion
+    bool wasCanceled() const { return mWasCanceled; }
+
+    QList<QVariant> values() const { return mValues; }
+
+  signals:
+
+    /**
+     * Emitted when values have been collected
+     */
+    void gatheredValues();
+
+  signals:
+    //! Internal routines can connect to this signal if they use event loop
+    void canceled();
+
+    void progressChanged( double progress );
+
+  private:
+
+    QgsVectorLayer *mLayer = nullptr;
+    QString mExpression;
+    bool mSelectedOnly = false;
+    QList<QVariant> mValues;
+    int mMissingValues;
+    QgsFeedback *mFeedback = nullptr;
+    QMutex mFeedbackMutex;
+    bool mWasCanceled;
+};
+
 /**
  * A dock widget which displays a statistical summary of the values in a field or expression
  */
@@ -41,6 +126,7 @@ class APP_EXPORT QgsStatisticalSummaryDockWidget : public QgsDockWidget, private
 
   public:
     QgsStatisticalSummaryDockWidget( QWidget *parent = nullptr );
+    ~QgsStatisticalSummaryDockWidget() override;
 
     /**
      * Returns the currently active layer for the widget
@@ -62,6 +148,7 @@ class APP_EXPORT QgsStatisticalSummaryDockWidget : public QgsDockWidget, private
     void statActionTriggered( bool checked );
     void layersRemoved( const QStringList &layers );
     void layerSelectionChanged();
+    void gathererThreadFinished();
 
   private:
 
@@ -80,9 +167,9 @@ class APP_EXPORT QgsStatisticalSummaryDockWidget : public QgsDockWidget, private
     static QList< QgsStringStatisticalSummary::Statistic > sDisplayStringStats;
     static QList< QgsDateTimeStatisticalSummary::Statistic > sDisplayDateTimeStats;
 
-    void updateNumericStatistics( bool selectedOnly );
-    void updateStringStatistics( bool selectedOnly );
-    void updateDateTimeStatistics( bool selectedOnly );
+    void updateNumericStatistics();
+    void updateStringStatistics();
+    void updateDateTimeStatistics();
     void addRow( int row, const QString &name, const QString &value, bool showValue );
 
     QgsExpressionContext createExpressionContext() const override;
@@ -95,6 +182,8 @@ class APP_EXPORT QgsStatisticalSummaryDockWidget : public QgsDockWidget, private
     DataType mPreviousFieldType;
 
     QString mExpression;
+
+    QgsStatisticsValueGatherer *mGatherer = nullptr;
 };
 
 #endif // QGSSTATISTICALSUMMARYDOCKWIDGET_H
diff --git a/src/core/qgsvectorlayer.cpp b/src/core/qgsvectorlayer.cpp
@@ -3730,7 +3730,7 @@ QList<double> QgsVectorLayer::getDoubleValues( const QString &fieldOrExpression,
   if ( nullCount )
     *nullCount = 0;
 
-  QList<QVariant> variantValues = getValues( fieldOrExpression, ok, selectedOnly );
+  QList<QVariant> variantValues = getValues( fieldOrExpression, ok, selectedOnly, feedback );
   if ( !ok )
     return values;
 

diff --git a/src/ui/qgsstatisticalsummarybase.ui b/src/ui/qgsstatisticalsummarybase.ui
@@ -48,6 +48,29 @@
        </size>
       </property>
      </widget>
+    </item>
+    <item>
+
+    <layout class="QHBoxLayout" name="horizontalLayout_3">
+     <property name="topMargin">
+      <number>0</number>
+     </property>
+     <item>
+      <widget class="QProgressBar" name="mCalculatingProgressBar">
+       <property name="value">
+        <number>0</number>
+       </property>
+      </widget>
+     </item>
+     <item>
+      <widget class="QPushButton" name="mCancelButton">
+       <property name="text">
+        <string>Cancel</string>
+       </property>
+      </widget>
+     </item>
+    </layout>
+
     </item>
     <item>
      <widget class="QTableWidget" name="mStatisticsTable">