[GRASS] fixed race condition in reading import progress

qgis · Sep 26, 2015 · 9df1a08 · 9df1a08
1 parent e6e79c1
commit 9df1a08
Show file tree

Hide file tree

Showing 7 changed files with 222 additions and 83 deletions.
diff --git a/src/plugins/grass/qgsgrassmodule.cpp b/src/plugins/grass/qgsgrassmodule.cpp
@@ -897,34 +897,16 @@ void QgsGrassModule::readStderr()
     line = QString::fromLocal8Bit( ba ).replace( '\n', "" );
     //QgsDebugMsg(QString("line: '%1'").arg(line));
 
-    if ( rxpercent.indexIn( line ) != -1 )
-    {
-      int progress = rxpercent.cap( 1 ).toInt();
-      mProgressBar->setValue( progress );
-    }
-    else if ( rxmessage.indexIn( line ) != -1 )
-    {
-      mOutputTextBrowser->append( "<pre>" + rxmessage.cap( 1 ) + "</pre>" );
-    }
-    else if ( rxwarning.indexIn( line ) != -1 )
+    QString text, html;
+    int percent;
+    QgsGrass::ModuleOutput type =  QgsGrass::parseModuleOutput( line, text, html, percent );
+    if ( type == QgsGrass::OutputPercent )
     {
-      QString warn = rxwarning.cap( 1 );
-      QString img = QgsApplication::pkgDataPath() + "/themes/default/grass/grass_module_warning.png";
-      mOutputTextBrowser->append( "<img src=\"" + img + "\">" + warn );
+      mProgressBar->setValue( percent );
     }
-    else if ( rxerror.indexIn( line ) != -1 )
+    else if ( type == QgsGrass::OutputMessage || type == QgsGrass::OutputWarning || type == QgsGrass::OutputError )
     {
-      QString error = rxerror.cap( 1 );
-      QString img = QgsApplication::pkgDataPath() + "/themes/default/grass/grass_module_error.png";
-      mOutputTextBrowser->append( "<img src=\"" + img + "\">" + error );
-    }
-    else if ( rxend.indexIn( line ) != -1 )
-    {
-      // Do nothing
-    }
-    else
-    {
-      mOutputTextBrowser->append( "<pre>" + line + "</pre>" );
+      mOutputTextBrowser->append( html );
     }
   }
 }

diff --git a/src/providers/grass/qgsgrass.cpp b/src/providers/grass/qgsgrass.cpp
@@ -1752,6 +1752,7 @@ QProcess *QgsGrass::startModule( const QString& gisdbase, const QString&  locati
   gisrcFile.close();
   QStringList environment = QProcess::systemEnvironment();
   environment.append( "GISRC=" + gisrcFile.fileName() );
+  environment.append( "GRASS_MESSAGE_FORMAT=gui" );
 
   process->setEnvironment( environment );
 
@@ -2646,3 +2647,53 @@ void QgsGrass::sleep( int ms )
   nanosleep( &ts, NULL );
 #endif
 }
+
+QgsGrass::ModuleOutput QgsGrass::parseModuleOutput( const QString & input, QString &text, QString &html, int &percent )
+{
+  QRegExp rxpercent( "GRASS_INFO_PERCENT: (\\d+)" );
+  QRegExp rxmessage( "GRASS_INFO_MESSAGE\\(\\d+,\\d+\\): (.*)" );
+  QRegExp rxwarning( "GRASS_INFO_WARNING\\(\\d+,\\d+\\): (.*)" );
+  QRegExp rxerror( "GRASS_INFO_ERROR\\(\\d+,\\d+\\): (.*)" );
+  QRegExp rxend( "GRASS_INFO_END\\(\\d+,\\d+\\)" );
+
+
+  if ( input.trimmed().isEmpty() )
+  {
+    return OutputNone;
+  }
+  else if ( rxpercent.indexIn( input ) != -1 )
+  {
+    percent = rxpercent.cap( 1 ).toInt();
+    return OutputPercent;
+  }
+  else if ( rxmessage.indexIn( input ) != -1 )
+  {
+    text = rxmessage.cap( 1 );
+    html = "<pre>" + text + "</pre>"  ;
+    return OutputMessage;
+  }
+  else if ( rxwarning.indexIn( input ) != -1 )
+  {
+    text = rxwarning.cap( 1 );
+    QString img = QgsApplication::pkgDataPath() + "/themes/default/grass/grass_module_warning.png";
+    html = "<img src=\"" + img + "\">" + text;
+    return OutputWarning;
+  }
+  else if ( rxerror.indexIn( input ) != -1 )
+  {
+    text = rxerror.cap( 1 );
+    QString img = QgsApplication::pkgDataPath() + "/themes/default/grass/grass_module_error.png";
+    html =  "<img src=\"" + img + "\">" + text;
+    return OutputError;
+  }
+  else if ( rxend.indexIn( input ) != -1 )
+  {
+    return OutputNone;
+  }
+  else // some plain text which cannot be parsed
+  {
+    text = input;
+    html = "<pre>" + text + "</pre>";
+    return OutputMessage;
+  }
+}
diff --git a/src/providers/grass/qgsgrass.h b/src/providers/grass/qgsgrass.h
@@ -140,6 +140,16 @@ class GRASS_LIB_EXPORT QgsGrass : public QObject
   public:
     static jmp_buf jumper; // used to get back from fatal error
 
+    // Parsed module output
+    enum ModuleOutput
+    {
+      OutputNone,
+      OutputPercent,
+      OutputMessage,
+      OutputWarning,
+      OutputError
+    };
+
     // This does not work (gcc/Linux), such exception cannot be caught
     // so I have enabled the old version, if you are able to fix it, please
     // check first if it realy works, i.e. can be caught!
@@ -558,11 +568,18 @@ class GRASS_LIB_EXPORT QgsGrass : public QObject
     // Free struct Map_info
     static void vectDestroyMapStruct( struct Map_info *map );
 
-    // Sleep miliseconds (for debugging)
+    // Sleep miliseconds (for debugging), does not work on threads(?)
     static void sleep( int ms );
 
     void emitNewLayer( QString uri, QString name ) { emit newLayer( uri, name ); }
 
+    /** Parse single line of output from GRASS modules run with GRASS_MESSAGE_FORMAT=gui
+     * @param input input string read from module stderr
+     * @param text parsed text
+     * @param html html formated parsed text, e.g. + icons
+     * @param percent progress 0-100 */
+    static ModuleOutput parseModuleOutput( const QString & input, QString &text, QString &html, int &percent );
+
   public slots:
     /** Close mapset and show warning if closing failed */
     bool closeMapsetWarn();

diff --git a/src/providers/grass/qgsgrassimport.cpp b/src/providers/grass/qgsgrassimport.cpp
@@ -48,16 +48,81 @@ QgsGrassImportIcon::QgsGrassImportIcon()
 {
 }
 
+//------------------------------ QgsGrassImportProcess ------------------------------------
+QgsGrassImportProgress::QgsGrassImportProgress( QProcess *process, QObject *parent )
+    : QObject( parent )
+    , mProcess( process )
+    , mProgressMin( 0 )
+    , mProgressMax( 0 )
+    , mProgressValue( 0 )
+{
+  connect( mProcess, SIGNAL( readyReadStandardError() ), SLOT( onReadyReadStandardError() ) );
+}
+
+void QgsGrassImportProgress::setProcess( QProcess *process )
+{
+  mProcess = process;
+  connect( mProcess, SIGNAL( readyReadStandardError() ), SLOT( onReadyReadStandardError() ) );
+}
+
+void QgsGrassImportProgress::onReadyReadStandardError()
+{
+  QgsDebugMsg( "entered" );
+  if ( mProcess )
+  {
+    // TODO: parse better progress output
+    QString output = QString::fromLocal8Bit( mProcess->readAllStandardError() );
+    Q_FOREACH ( QString line, output.split( "\n" ) )
+    {
+      QgsDebugMsg( "line = '" + line + "'" );
+      QString text, html;
+      int percent;
+      QgsGrass::ModuleOutput type =  QgsGrass::parseModuleOutput( line, text, html, percent );
+      if ( type == QgsGrass::OutputPercent )
+      {
+        mProgressMin = 0;
+        mProgressMax = 100;
+        mProgressValue = percent;
+        emit progressChanged( html, mProgressHtml, mProgressMin, mProgressMax, mProgressValue );
+      }
+      else if ( type == QgsGrass::OutputMessage || type == QgsGrass::OutputWarning || type == QgsGrass::OutputError )
+      {
+        mProgressHtml += html;
+        QgsDebugMsg( "text = " + text );
+        emit progressChanged( html, mProgressHtml, mProgressMin, mProgressMax, mProgressValue );
+      }
+    }
+  }
+}
+
+void QgsGrassImportProgress::append( const QString & html )
+{
+  mProgressHtml += html;
+  emit progressChanged( html, mProgressHtml, mProgressMin, mProgressMax, mProgressValue );
+}
+
+void QgsGrassImportProgress::setRange( int min, int max )
+{
+  mProgressMin = min;
+  mProgressMax = max;
+  mProgressValue = min;
+  emit progressChanged( "", mProgressHtml, mProgressMin, mProgressMax, mProgressValue );
+}
+
+void QgsGrassImportProgress::setValue( int value )
+{
+  mProgressValue = value;
+  emit progressChanged( "", mProgressHtml, mProgressMin, mProgressMax, mProgressValue );
+}
+
 //------------------------------ QgsGrassImport ------------------------------------
 QgsGrassImport::QgsGrassImport( QgsGrassObject grassObject )
     : QObject()
     , mGrassObject( grassObject )
     , mCanceled( false )
     , mProcess( 0 )
+    , mProgress( 0 )
     , mFutureWatcher( 0 )
-    , mProgressMin( 0 )
-    , mProgressMax( 0 )
-    , mProgressValue( 0 )
 {
   // QMovie used by QgsAnimatedIcon is using QTimer which cannot be start from another thread
   // (it works on Linux however) so we cannot start it connecting from QgsGrassImportItem and
@@ -125,28 +190,6 @@ void QgsGrassImport::cancel()
   mCanceled = true;
 }
 
-void QgsGrassImport::emitProgressChanged()
-{
-  emit progressChanged( mProgressHtml + mProgressTmpHtml, mProgressMin, mProgressMax, mProgressValue );
-
-}
-
-void QgsGrassImport::onReadyReadStandardError()
-{
-  if ( mProcess )
-  {
-    // TODO: should be locked? Lock does not help anyway.
-    // TODO: parse better progress output
-    mProgressHtml += QString( mProcess->readAllStandardError() ).replace( "\n", "<br>" );
-    emitProgressChanged();
-  }
-}
-
-void QgsGrassImport::addProgressRow( QString html )
-{
-  mProgressHtml += html + "<br>";
-}
-
 //------------------------------ QgsGrassRasterImport ------------------------------------
 QgsGrassRasterImport::QgsGrassRasterImport( QgsRasterPipe* pipe, const QgsGrassObject& grassObject,
     const QgsRectangle &extent, int xSize, int ySize )
@@ -196,8 +239,6 @@ bool QgsGrassRasterImport::import()
   for ( int band = 1; band <= provider->bandCount(); band++ )
   {
     QgsDebugMsg( QString( "band = %1" ).arg( band ) );
-    addProgressRow( tr( "Writing band %1/%2" ).arg( band ).arg( provider->bandCount() ) );
-    emitProgressChanged();
     int colorInterpretation = provider->colorInterpretation( band );
     if ( colorInterpretation ==  QgsRaster::RedBand )
     {
@@ -265,6 +306,15 @@ bool QgsGrassRasterImport::import()
       setError( e.what() );
       return false;
     }
+    if ( !mProgress )
+    {
+      mProgress = new QgsGrassImportProgress( mProcess, this );
+    }
+    else
+    {
+      mProgress->setProcess( mProcess );
+    }
+    mProgress->append( tr( "Writing band %1/%2" ).arg( band ).arg( provider->bandCount() ) );
 
     QDataStream outStream( mProcess );
 
@@ -292,13 +342,13 @@ bool QgsGrassRasterImport::import()
     int iterRows = 0;
     QgsRasterBlock* block = 0;
     mProcess->setReadChannel( QProcess::StandardOutput );
-    mProgressMax = mYSize;
+    mProgress->setRange( 0, mYSize - 1 );
     while ( iter.readNextRasterPart( band, iterCols, iterRows, &block, iterLeft, iterTop ) )
     {
       for ( int row = 0; row < iterRows; row++ )
       {
-        mProgressValue = iterTop + row;
-        emitProgressChanged();
+        mProgress->setValue( iterTop + row );
+
         if ( !block->convert( qgis_out_type ) )
         {
           setError( tr( "Cannot convert block (%1) to data type %2" ).arg( block->toString() ).arg( qgis_out_type ) );
@@ -385,7 +435,6 @@ bool QgsGrassRasterImport::import()
     if ( mProcess->exitStatus() != QProcess::NormalExit )
     {
       setError( mProcess->errorString() );
-      delete mProcess;
       mProcess = 0;
       return false;
     }
@@ -528,8 +577,7 @@ bool QgsGrassVectorImport::import()
     setError( e.what() );
     return false;
   }
-  // TODO: connecting readyReadStandardError() is causing hangs or crashes
-  //connect(mProcess, SIGNAL(readyReadStandardError()), this, SLOT(onReadyReadStandardError()));
+  mProgress = new QgsGrassImportProgress( mProcess, this );
 
   QDataStream outStream( mProcess );
   mProcess->setReadChannel( QProcess::StandardOutput );
@@ -542,10 +590,10 @@ bool QgsGrassVectorImport::import()
 
   QgsFeatureIterator iterator = mProvider->getFeatures();
   QgsFeature feature;
-  mProgressMax = mProvider->featureCount();
+  mProgress->setRange( 1, mProvider->featureCount() );
+  mProgress->append( tr( "Writing features" ) );
   for ( int i = 0; i < ( isPolygon ? 2 : 1 ); i++ ) // two cycles with polygons
   {
-    addProgressRow( tr( "Writing features" ) );
     if ( i > 0 ) // second run for polygons
     {
       //iterator.rewind(); // rewind does not work
@@ -555,9 +603,7 @@ bool QgsGrassVectorImport::import()
     int count = 0;
     while ( iterator.nextFeature( feature ) )
     {
-      mProgressTmpHtml = tr( "Feature %1/%2" ).arg( count + 1 ).arg( mProgressMax );
-      mProgressValue = count + 1;
-      emitProgressChanged();
+      mProgress->setValue( count + 1 );
       if ( !feature.isValid() )
       {
         continue;
@@ -603,7 +649,6 @@ bool QgsGrassVectorImport::import()
     outStream >> result;
 #endif
   }
-
   iterator.close();
 
   // Close write channel before waiting for response to avoid stdin buffer problem on Windows