validation/geometry/threading fixes:

- fTools/polygon centroids: catch missing centroid with invalid geometry (fixes #4963) - fTools/validate geometry: * fix final position of progress bar * don't use isGeosValid() * zoom to feature on locationless errors (including OTFR support) * store/restore dialog position - QgsGeometry(Validator): * log GEOS exceptions * (closed) rings need at least 4 points not 3 * log error when geometry can't be exported to GEOS * don't produce "geometry is valid" error - improve logging from threads * keep message output from main thread as popup * log message from threads to message log (fixes crash on python error in thread)
2025-04-14 00:07:35 -04:00 · 2012-05-25 13:46:18 +02:00 · 2012-05-25 13:46:18 +02:00 · d751036823
commit d751036823
parent d208b29c22
8 changed files with 129 additions and 83 deletions
--- a/python/plugins/fTools/tools/doGeometry.py
+++ b/python/plugins/fTools/tools/doGeometry.py
@ -620,11 +620,11 @@ class geometryThread( QThread ):
      self.emit( SIGNAL( "runStatus( PyQt_PyObject )" ),  nElement )
      inGeom = inFeat.geometry()
      atMap = inFeat.attributeMap()
-      outGeom = QgsGeometry( inGeom.centroid() )
+      outGeom = inGeom.centroid()
      if outGeom is None:
        return "math_error"
      outFeat.setAttributeMap( atMap )
-      outFeat.setGeometry( outGeom )
+      outFeat.setGeometry( QgsGeometry( outGeom ) )
      writer.addFeature( outFeat )
    del writer
    return True
--- a/python/plugins/fTools/tools/doValidate.py
+++ b/python/plugins/fTools/tools/doValidate.py
@ -93,6 +93,15 @@ class ValidateDialog( QDialog, Ui_Dialog ):
    self.storedScale = self.iface.mapCanvas().scale()
    # create marker for error
    self.marker = MarkerErrorGeometry(self.iface.mapCanvas())
+
+    settings = QSettings()
+    self.restoreGeometry( settings.value("/fTools/ValidateDialog/geometry").toByteArray() )
+
+  def closeEvent(self, e):
+    settings = QSettings()
+    settings.setValue( "/fTools/ValidateDialog/geometry", QVariant(self.saveGeometry()) )
+    QMainWindow.closeEvent(self, e)
+    del self.marker
    
  def keyPressEvent( self, e ):
    if ( e.modifiers() == Qt.ControlModifier or \
@ -115,7 +124,8 @@ class ValidateDialog( QDialog, Ui_Dialog ):
    elif self.cmbField.isVisible() and self.cmbField.currentText() == "":
      QMessageBox.information( self, self.tr("Error!"), self.tr( "Please specify input field" ) )
    else:
-      self.validate( self.inShape.currentText(), self.useSelected.checkState() )
+      self.vlayer = ftools_utils.getVectorLayerByName( self.inShape.currentText() )
+      self.validate( self.useSelected.checkState() )
      
  def zoomToError(self, curr, prev):
      if curr is None:
@ -123,30 +133,47 @@ class ValidateDialog( QDialog, Ui_Dialog ):
      row = curr.row() # if we clicked in the first column, we want the second
      item = self.tblUnique.item(row, 1)

-      e = item.data(Qt.UserRole).toPyObject()
-      if e is None:
-	return
-
      mc = self.iface.mapCanvas()
-      x = e.x()
-      y = e.y()
-      self.marker.setGeom(x, y) # Set Marker
      mc.zoomToPreviousExtent()
-      scale = mc.scale()
-      rect = QgsRectangle(float(x)-(4.0/scale),float(y)-(4.0/scale),
-                          float(x)+(4.0/scale),float(y)+(4.0/scale))
+
+      if mc.mapRenderer().hasCrsTransformEnabled():
+        ct = QgsCoordinateTransform( self.vlayer.crs(), mc.mapRenderer().destinationCrs() )
+
+      e = item.data(Qt.UserRole).toPyObject()
+
+      if type(e)==QgsPoint:
+        if not ct is None:
+          e = ct.transform( e )
+
+        self.marker.setGeom(e.x(), e.y())
+
+        rect = mc.extent()
+        rect.expand( 0.25, e )
+
+      else:
+        self.marker.reset()
+
+        ft = QgsFeature()
+        (fid,ok) = self.tblUnique.item(row, 0).text().toInt()
+        if not ok or not self.vlayer.featureAtId( fid, ft, True):
+          return
+
+	rect = ft.geometry().boundingBox()
+	if not ct is None:
+          rect = ct.transformBoundingBox( rect )
+        rect.expand(1.05)
+
      # Set the extent to our new rectangle
      mc.setExtent(rect)
      # Refresh the map
      mc.refresh()

-  def validate( self,  myLayer, mySelection ):
-    vlayer = ftools_utils.getVectorLayerByName( myLayer )
+  def validate( self, mySelection ):
    self.tblUnique.clearContents()
    self.tblUnique.setRowCount( 0 )
    self.lstCount.clear()
    self.buttonOk.setEnabled( False )
-    self.testThread = validateThread( self.iface.mainWindow(), self, vlayer, mySelection )
+    self.testThread = validateThread( self.iface.mainWindow(), self, self.vlayer, mySelection )
    QObject.connect( self.testThread, SIGNAL( "runFinished(PyQt_PyObject)" ), self.runFinishedFromThread )
    QObject.connect( self.testThread, SIGNAL( "runStatus(PyQt_PyObject)" ), self.runStatusFromThread )
    QObject.connect( self.testThread, SIGNAL( "runRange(PyQt_PyObject)" ), self.runRangeFromThread )
@ -214,7 +241,6 @@ class validateThread( QThread ):
    self.running = True
    output = self.check_geometry( self.vlayer )
    self.emit( SIGNAL( "runFinished(PyQt_PyObject)" ), output )
-    self.emit( SIGNAL( "runStatus(PyQt_PyObject)" ), 100 )

  def stop(self):
    self.running = False
@ -243,7 +269,7 @@ class validateThread( QThread ):
      self.emit(SIGNAL("runStatus(PyQt_PyObject)"), nElement)
      nElement += 1
      # Check Add error
-      if not (geom.isGeosEmpty() or geom.isGeosValid() ) :
+      if not geom.isGeosEmpty():
        lstErrors.append((feat.id(), list(geom.validateGeometry())))
    self.emit( SIGNAL( "runStatus(PyQt_PyObject)" ), nFeat )
    return lstErrors
--- a/src/app/qgisapp.cpp
+++ b/src/app/qgisapp.cpp
@ -65,6 +65,7 @@
 #include <QToolButton>
 #include <QVBoxLayout>
 #include <QWhatsThis>
+#include <QThread>

 #include <qgsnetworkaccessmanager.h>

@ -318,7 +319,10 @@ static void setTitleBarText_( QWidget & qgisApp )
 */
 static QgsMessageOutput *messageOutputViewer_()
 {
-  return new QgsMessageViewer( QgisApp::instance() );
+  if ( QThread::currentThread() == QApplication::instance()->thread() )
+    return new QgsMessageViewer( QgisApp::instance() );
+  else
+    return new QgsMessageOutputConsole();
 }

 static void customSrsValidation_( QgsCoordinateReferenceSystem* srs )
--- a/src/app/qgscustomization.cpp
+++ b/src/app/qgscustomization.cpp
@ -35,7 +35,6 @@
 #include <QToolButton>
 #include <QStatusBar>
 #include <QMetaObject>
-#include <QThread>

 #ifdef Q_OS_MACX
 QgsCustomizationDialog::QgsCustomizationDialog( QWidget *parent )
--- a/src/core/qgsgeometry.cpp
+++ b/src/core/qgsgeometry.cpp
@ -22,6 +22,7 @@ email                : morb at ozemail dot com dot au
 #include "qgsgeometry.h"
 #include "qgsapplication.h"
 #include "qgslogger.h"
+#include "qgsmessagelog.h"
 #include "qgspoint.h"
 #include "qgsrectangle.h"

@ -36,8 +37,7 @@ email                : morb at ozemail dot com dot au
 #define CATCH_GEOS(r) \
  catch (GEOSException &e) \
  { \
-    Q_UNUSED(e); \
-    QgsDebugMsg("GEOS: " + QString( e.what() ) ); \
+    QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr("GEOS") ); \
    return r; \
  }

@ -90,7 +90,7 @@ static void throwGEOSException( const char *fmt, ... )
  vsnprintf( buffer, sizeof buffer, fmt, ap );
  va_end( ap );

-  QgsDebugMsg( QString( "GEOS exception encountered: %1" ).arg( buffer ) );
+  QgsDebugMsg( QString( "GEOS exception: %1" ).arg( buffer ) );

  throw GEOSException( QString::fromUtf8( buffer ) );
 }
@ -166,7 +166,7 @@ static GEOSGeometry *cloneGeosGeom( const GEOSGeometry *geom )
    }
    catch ( GEOSException &e )
    {
-      Q_UNUSED( e );
+      QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
      for ( int i = 0; i < geoms.count(); i++ )
        GEOSGeom_destroy( geoms[i] );

@ -261,7 +261,7 @@ static GEOSCoordSequence *createGeosCoordSequence( const QgsPolyline& points )
  }
  catch ( GEOSException &e )
  {
-    Q_UNUSED( e );
+    QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
    /*if ( coord )
      GEOSCoordSeq_destroy( coord );*/
    throw;
@ -285,7 +285,7 @@ static GEOSGeometry *createGeosCollection( int typeId, QVector<GEOSGeometry*> ge
  }
  catch ( GEOSException &e )
  {
-    Q_UNUSED( e );
+    QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
  }

  delete [] geomarr;
@ -304,7 +304,7 @@ static GEOSGeometry *createGeosLineString( const QgsPolyline& polyline )
  }
  catch ( GEOSException &e )
  {
-    Q_UNUSED( e );
+    QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
    //MH: for strange reasons, geos3 crashes when removing the coordinate sequence
    //if ( coord )
    //GEOSCoordSeq_destroy( coord );
@ -337,7 +337,7 @@ static GEOSGeometry *createGeosLinearRing( const QgsPolyline& polyline )
  }
  catch ( GEOSException &e )
  {
-    Q_UNUSED( e );
+    QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
    /* as MH has noticed ^, this crashes geos
    if ( coord )
      GEOSCoordSeq_destroy( coord );*/
@ -389,7 +389,7 @@ static GEOSGeometry *createGeosPolygon( const QgsPolygon& polygon )
  }
  catch ( GEOSException &e )
  {
-    Q_UNUSED( e );
+    QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
    for ( int i = 0; i < geoms.count(); i++ )
      GEOSGeom_destroy( geoms[i] );
    return 0;
@ -421,7 +421,7 @@ QgsGeometry* QgsGeometry::fromWkt( QString wkt )
  }
  catch ( GEOSException &e )
  {
-    Q_UNUSED( e );
+    QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
    return 0;
  }
 }
@ -456,7 +456,8 @@ QgsGeometry* QgsGeometry::fromMultiPoint( const QgsMultiPoint& multipoint )
  }
  catch ( GEOSException &e )
  {
-    Q_UNUSED( e );
+    QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
+
    for ( int i = 0; i < geoms.size(); ++i )
      GEOSGeom_destroy( geoms[i] );

@ -477,7 +478,8 @@ QgsGeometry* QgsGeometry::fromMultiPolyline( const QgsMultiPolyline& multiline )
  }
  catch ( GEOSException &e )
  {
-    Q_UNUSED( e );
+    QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
+
    for ( int i = 0; i < geoms.count(); i++ )
      GEOSGeom_destroy( geoms[i] );

@ -501,7 +503,8 @@ QgsGeometry* QgsGeometry::fromMultiPolygon( const QgsMultiPolygon& multipoly )
  }
  catch ( GEOSException &e )
  {
-    Q_UNUSED( e );
+    QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
+
    for ( int i = 0; i < geoms.count(); i++ )
      GEOSGeom_destroy( geoms[i] );

@ -2394,7 +2397,7 @@ double QgsGeometry::closestVertexWithContext( const QgsPoint& point, int& atVert
  }
  catch ( GEOSException &e )
  {
-    Q_UNUSED( e );
+    QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
    return -1;
  }

@ -2706,7 +2709,8 @@ int QgsGeometry::addRing( const QList<QgsPoint>& ring )
  }
  catch ( GEOSException &e )
  {
-    Q_UNUSED( e );
+    QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
+
    if ( newRingPolygon )
      GEOSGeom_destroy( newRingPolygon );
    else if ( newRing )
@ -2739,7 +2743,8 @@ int QgsGeometry::addRing( const QList<QgsPoint>& ring )
    }
    catch ( GEOSException &e )
    {
-      Q_UNUSED( e );
+      QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
+
      if ( shell )
        GEOSGeom_destroy( shell );
      else if ( shellRing )
@ -2776,7 +2781,8 @@ int QgsGeometry::addRing( const QList<QgsPoint>& ring )
      }
      catch ( GEOSException &e )
      {
-        Q_UNUSED( e );
+        QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
+
        if ( hole )
          GEOSGeom_destroy( hole );
        else if ( holeRing )
@ -2924,7 +2930,8 @@ int QgsGeometry::addPart( const QList<QgsPoint> &points )
      }
      catch ( GEOSException &e )
      {
-        Q_UNUSED( e );
+        QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
+
        if ( newRing )
          GEOSGeom_destroy( newRing );

@ -2947,7 +2954,7 @@ int QgsGeometry::addPart( const QList<QgsPoint> &points )
  }
  catch ( GEOSException &e )
  {
-    Q_UNUSED( e );
+    QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );

    if ( newPart )
      GEOSGeom_destroy( newPart );
@ -3839,7 +3846,7 @@ bool QgsGeometry::contains( QgsPoint* p )
  }
  catch ( GEOSException &e )
  {
-    Q_UNUSED( e );
+    QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
    returnval = false;
  }

@ -5458,7 +5465,7 @@ GEOSGeometry* QgsGeometry::reshapePolygon( const GEOSGeometry* polygon, const GE
  }
  catch ( GEOSException &e )
  {
-    Q_UNUSED( e );
+    QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
    nIntersections = 0;
  }

@ -5569,7 +5576,7 @@ GEOSGeometry* QgsGeometry::reshapeLine( const GEOSGeometry* line, const GEOSGeom
  }
  catch ( GEOSException &e )
  {
-    Q_UNUSED( e );
+    QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
    atLeastTwoIntersections = false;
  }

@ -6724,9 +6731,7 @@ bool QgsGeometry::isGeosValid()
  }
  catch ( GEOSException &e )
  {
-    // looks like geometry is fubar
-    Q_UNUSED( e );
-    QgsDebugMsg( QString( "GEOS exception caught: %1" ).arg( e.what() ) );
+    QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
    return false;
  }
 }
@ -6749,9 +6754,7 @@ bool QgsGeometry::isGeosEmpty()
  }
  catch ( GEOSException &e )
  {
-    // looks like geometry is fubar
-    Q_UNUSED( e );
-    QgsDebugMsg( QString( "GEOS exception caught: %1" ).arg( e.what() ) );
+    QgsMessageLog::logMessage( QObject::tr( "Exception: %1" ).arg( e.what() ), QObject::tr( "GEOS" ) );
    return false;
  }
 }
--- a/src/core/qgsgeometryvalidator.cpp
+++ b/src/core/qgsgeometryvalidator.cpp
@ -82,9 +82,9 @@ void QgsGeometryValidator::validatePolyline( int i, QgsPolyline line, bool ring
 {
  if ( ring )
  {
-    if ( line.size() < 3 )
+    if ( line.size() < 4 )
    {
-      QString msg = QObject::tr( "ring %1 with less than three points" ).arg( i );
+      QString msg = QObject::tr( "ring %1 with less than four points" ).arg( i );
      QgsDebugMsg( msg );
      emit errorFound( QgsGeometry::Error( msg ) );
      mErrorCount++;
@ -193,8 +193,6 @@ void QgsGeometryValidator::validatePolygon( int idx, const QgsPolygon &polygon )

 void QgsGeometryValidator::run()
 {
-  QgsDebugMsg( "validation thread started." );
-
  mErrorCount = 0;
 #if defined(GEOS_VERSION_MAJOR) && defined(GEOS_VERSION_MINOR) && \
    ( (GEOS_VERSION_MAJOR==3 && GEOS_VERSION_MINOR>=3) || GEOS_VERSION_MAJOR>3)
@ -202,38 +200,48 @@ void QgsGeometryValidator::run()
  if ( settings.value( "/qgis/digitizing/validate_geometries", 1 ).toInt() == 2 )
  {
    char *r = 0;
-    GEOSGeometry *g = 0;
-    if ( GEOSisValidDetail( mG.asGeos(), GEOSVALID_ALLOW_SELFTOUCHING_RING_FORMING_HOLE, &r, &g ) != 1 )
+    GEOSGeometry *g0 = mG.asGeos();
+    if ( !g0 )
    {
-      if ( g )
+      emit errorFound( QgsGeometry::Error( QObject::tr( "GEOS error:could not produce geometry for GEOS (check log window)" ) ) );
+    }
+    else
+    {
+      GEOSGeometry *g1 = 0;
+      if ( GEOSisValidDetail( g0, GEOSVALID_ALLOW_SELFTOUCHING_RING_FORMING_HOLE, &r, &g1 ) != 1 )
      {
-        const GEOSCoordSequence *cs = GEOSGeom_getCoordSeq( g );
-
-        unsigned int n;
-        if ( GEOSCoordSeq_getSize( cs, &n ) && n == 1 )
+        if ( g1 )
        {
-          double x, y;
-          GEOSCoordSeq_getX( cs, 0, &x );
-          GEOSCoordSeq_getY( cs, 0, &y );
-          emit errorFound( QgsGeometry::Error( QObject::tr( "GEOS error:%1" ).arg( r ), QgsPoint( x, y ) ) );
+          const GEOSCoordSequence *cs = GEOSGeom_getCoordSeq( g1 );
+
+          unsigned int n;
+          if ( GEOSCoordSeq_getSize( cs, &n ) && n == 1 )
+          {
+            double x, y;
+            GEOSCoordSeq_getX( cs, 0, &x );
+            GEOSCoordSeq_getY( cs, 0, &y );
+            emit errorFound( QgsGeometry::Error( QObject::tr( "GEOS error:%1" ).arg( r ), QgsPoint( x, y ) ) );
+            mErrorCount++;
+          }
+
+          GEOSGeom_destroy( g1 );
+        }
+        else
+        {
+          emit errorFound( QgsGeometry::Error( QObject::tr( "GEOS error:%1" ).arg( r ) ) );
          mErrorCount++;
        }

-        GEOSGeom_destroy( g );
+        GEOSFree( r );
      }
-      else
-      {
-        emit errorFound( QgsGeometry::Error( QObject::tr( "GEOS error:%1" ).arg( r ) ) );
-        mErrorCount++;
-      }
-
-      GEOSFree( r );
    }

    return;
  }
 #endif

+  QgsDebugMsg( "validation thread started." );
+
  switch ( mG.wkbType() )
  {
    case QGis::WKBPoint:
@ -309,14 +317,16 @@ void QgsGeometryValidator::run()
  {
    emit errorFound( QObject::tr( "Geometry validation was aborted." ) );
  }
-  else if ( mErrorCount == 0 )
-  {
-    emit errorFound( QObject::tr( "Geometry is valid." ) );
-  }
-  else
+  else if ( mErrorCount > 0 )
  {
    emit errorFound( QObject::tr( "Geometry has %1 errors." ).arg( mErrorCount ) );
  }
+#if 0
+  else
+  {
+    emit errorFound( QObject::tr( "Geometry is valid." ) );
+  }
+#endif
 }

 void QgsGeometryValidator::addError( QgsGeometry::Error e )
--- a/src/core/qgsmessageoutput.cpp
+++ b/src/core/qgsmessageoutput.cpp
@ -15,6 +15,9 @@

 #include "qgsmessageoutput.h"
 #include "qgslogger.h"
+#include "qgsmessagelog.h"
+
+#include <QRegExp>

 static QgsMessageOutput* messageOutputConsole_()
 {
@ -47,9 +50,10 @@ QgsMessageOutputConsole::QgsMessageOutputConsole()
 {
 }

-void QgsMessageOutputConsole::setMessage( const QString& message, MessageType )
+void QgsMessageOutputConsole::setMessage( const QString& message, MessageType msgType )
 {
  mMessage = message;
+  mMsgType = msgType;
 }

 void QgsMessageOutputConsole::appendMessage( const QString& message )
@ -59,14 +63,13 @@ void QgsMessageOutputConsole::appendMessage( const QString& message )

 void QgsMessageOutputConsole::showMessage( bool )
 {
-  // show title if provided
-  if ( !mTitle.isNull() )
+  if ( mMsgType == MessageHtml )
  {
-    QgsDebugMsg( QString( "%1:" ).arg( mTitle ) );
+    mMessage.replace( "<br>", "\n" );
+    mMessage.replace( "&nbsp;", " " );
+    mMessage.replace( QRegExp("<\/?[^>]+>"), "" );
  }
-
-  // show the message
-  QgsDebugMsg( mMessage );
+  QgsMessageLog::logMessage( mMessage, mTitle.isNull() ? QObject::tr( "Console" ) : mTitle );
  emit destroyed();
  delete this;
 }
--- a/src/core/qgsmessageoutput.h
+++ b/src/core/qgsmessageoutput.h
@ -107,6 +107,7 @@ class CORE_EXPORT QgsMessageOutputConsole : public QObject, public QgsMessageOut
    //! stores current title
    QString mTitle;

+    MessageType mMsgType;
 };

 #endif