From de041a542b72562c91040597c5842d62917b02e4 Mon Sep 17 00:00:00 2001 From: morb_au Date: Tue, 14 Jun 2005 21:09:48 +0000 Subject: [PATCH] Add better pointer hygiene in QgsFeature (less crashes hopefully, especially when "Copy"ing features). Some more code to support WKB <-> GEOS geometry in QgsGeometry (not there yet though). git-svn-id: http://svn.osgeo.org/qgis/trunk@3599 c8812cc2-4d05-0410-92ff-de0c093fc19c --- src/qgsfeature.cpp | 83 +++++++++++++++++++++----------- src/qgsgeometry.cpp | 112 ++++++++++++++++++++++++++++++++++++++++---- src/qgsgeometry.h | 19 +++++++- 3 files changed, 175 insertions(+), 39 deletions(-) diff --git a/src/qgsfeature.cpp b/src/qgsfeature.cpp index 57e370b7fc6..1418bd2abe1 100644 --- a/src/qgsfeature.cpp +++ b/src/qgsfeature.cpp @@ -98,8 +98,16 @@ QgsFeature::QgsFeature( QgsFeature const & rhs, if ( changedGeometries.find(mFid) == changedGeometries.end() ) { // copy geometry purely from rhs feature - mGeometry = rhs.mGeometry; - mOwnsGeometry = rhs.mOwnsGeometry; + if ( rhs.mGeometry ) + { + mGeometry = new QgsGeometry( *(rhs.mGeometry) ); + mOwnsGeometry = TRUE; + } + else + { + mGeometry = 0; + mOwnsGeometry = FALSE; + } } else { @@ -111,35 +119,55 @@ QgsFeature::QgsFeature( QgsFeature const & rhs, } -QgsFeature::QgsFeature( QgsFeature const & feature ) - : mFid( feature.mFid ), - mDirty( feature.mDirty ), - attributes( feature.attributes ), - fieldNames( feature.fieldNames ), - mValid( feature.mValid ), - mGeometry( feature.mGeometry ), - mOwnsGeometry( feature.mOwnsGeometry ), - mTypeName( feature.mTypeName ) +QgsFeature::QgsFeature( QgsFeature const & rhs ) + : mFid( rhs.mFid ), + mDirty( rhs.mDirty ), + attributes( rhs.attributes ), + fieldNames( rhs.fieldNames ), + mValid( rhs.mValid ), + mTypeName( rhs.mTypeName ) { - // NOOP + + // copy embedded geometry + if ( rhs.mGeometry ) + { + mGeometry = new QgsGeometry( *(rhs.mGeometry) ); + mOwnsGeometry = TRUE; + } + else + { + mGeometry = 0; + mOwnsGeometry = FALSE; + } + } -QgsFeature & QgsFeature::operator=( QgsFeature const & feature ) +QgsFeature & QgsFeature::operator=( QgsFeature const & rhs ) { - if ( &feature == this ) - { return *this; } + if ( &rhs == this ) + { return *this; } - mFid = feature.mFid ; - mDirty = feature.mDirty ; - attributes = feature.attributes ; - fieldNames = feature.fieldNames ; - mValid = feature.mValid ; - mGeometry = feature.mGeometry; - mOwnsGeometry = feature.mOwnsGeometry; - mTypeName = feature.mTypeName; + mFid = rhs.mFid ; + mDirty = rhs.mDirty ; + attributes = rhs.attributes ; + fieldNames = rhs.fieldNames ; + mValid = rhs.mValid ; + mTypeName = rhs.mTypeName; - return *this; + // copy embedded geometry + if ( rhs.mGeometry ) + { + mGeometry = new QgsGeometry( *(rhs.mGeometry) ); + mOwnsGeometry = TRUE; + } + else + { + mGeometry = 0; + mOwnsGeometry = FALSE; + } + + return *this; } // QgsFeature::operator=( QgsFeature const & rhs ) @@ -228,8 +256,9 @@ QgsGeometry * QgsFeature::geometry() QgsGeometry * QgsFeature::geometryAndOwnership() { - return mGeometry; mOwnsGeometry = FALSE; + + return mGeometry; } @@ -381,8 +410,8 @@ void QgsFeature::setGeometryAndOwnership(unsigned char *geom, size_t length) } mGeometry = new QgsGeometry(); - mGeometry->setFromWkb(geom, length); - // mOwnsGeometry does not change. + mGeometry->setWkbAndOwnership(geom, length); + mOwnsGeometry = TRUE; } diff --git a/src/qgsgeometry.cpp b/src/qgsgeometry.cpp index 31864977ef4..a6a7324dc71 100644 --- a/src/qgsgeometry.cpp +++ b/src/qgsgeometry.cpp @@ -1,5 +1,5 @@ /*************************************************************************** - qgsgeometry.h - Geometry (stored as Open Geospatial Consortium WKB) + qgsgeometry.cpp - Geometry (stored as Open Geospatial Consortium WKB) ------------------------------------------------------------------- Date : 02 May 2005 Copyright : (C) 2005 by Brendan Morley @@ -21,7 +21,13 @@ email : morb at ozemail dot com dot au QgsGeometry::QgsGeometry() : mGeometry(0), - mGeometrySize(0) + mGeometrySize(0), + mWkt(0), + mGeos(0), + + mDirtyGeos(FALSE), + mDirtyWkb(FALSE), + mDirtyWkt(FALSE) { // NOOP } @@ -29,13 +35,30 @@ QgsGeometry::QgsGeometry() QgsGeometry::QgsGeometry( QgsGeometry const & rhs ) : mGeometry(0), - mGeometrySize( rhs.mGeometrySize ) + mGeometrySize( rhs.mGeometrySize ), + mWkt( rhs.mWkt ), + + mDirtyGeos( rhs.mDirtyGeos ), + mDirtyWkb( rhs.mDirtyWkb ), + mDirtyWkt( rhs.mDirtyWkt ) { + + if ( mGeometrySize && rhs.mGeometry ) { mGeometry = new unsigned char[mGeometrySize]; memcpy( mGeometry, rhs.mGeometry, mGeometrySize ); } + + // deep-copy the GEOS Geometry if appropriate + if (rhs.mGeos) + { + mGeos = rhs.mGeos->clone(); + } + else + { + mGeos = 0; + } } @@ -53,7 +76,22 @@ QgsGeometry & QgsGeometry::operator=( QgsGeometry const & rhs ) mGeometry = 0; } - mGeometrySize = rhs.mGeometrySize; + mGeometrySize = rhs.mGeometrySize; + mWkt = rhs.mWkt; + + // deep-copy the GEOS Geometry if appropriate + if (rhs.mGeos) + { + mGeos = rhs.mGeos->clone(); + } + else + { + mGeos = 0; + } + + mDirtyGeos = rhs.mDirtyGeos; + mDirtyWkb = rhs.mDirtyWkb; + mDirtyWkt = rhs.mDirtyWkt; if ( mGeometrySize && rhs.mGeometry ) { @@ -70,12 +108,17 @@ QgsGeometry::~QgsGeometry() { if (mGeometry) { - delete [] mGeometry; + delete [] mGeometry; + } + + if (mGeos) + { + delete mGeos; } } -void QgsGeometry::setFromWkb(unsigned char * wkb, size_t length) +void QgsGeometry::setWkbAndOwnership(unsigned char * wkb, size_t length) { // delete any existing WKB geometry before assigning new one if ( mGeometry ) @@ -83,8 +126,15 @@ void QgsGeometry::setFromWkb(unsigned char * wkb, size_t length) delete [] mGeometry; } + // TODO: What about ownership? + mGeometry = wkb; mGeometrySize = length; + + mDirtyWkb = FALSE; + mDirtyGeos = TRUE; + mDirtyWkt = TRUE; + } unsigned char * QgsGeometry::wkbBuffer() const @@ -100,9 +150,19 @@ size_t QgsGeometry::wkbSize() const QString const& QgsGeometry::wkt() const { - if (!mWkt) + if (mDirtyWkt) { - exportToWkt(); + // see which geometry representation to convert from + if (mDirtyGeos) + { + // convert from WKB + exportToWkt(); + } + else + { + // TODO + } + } return mWkt; @@ -400,7 +460,7 @@ bool QgsGeometry::insertVertexBefore(double x, double y, #endif // set new geomtry to this object - setFromWkb(newWKB, mGeometrySize + 16); + setWkbAndOwnership(newWKB, mGeometrySize + 16); break; } @@ -1171,13 +1231,30 @@ bool QgsGeometry::exportToWkt(unsigned char * geom) const bool QgsGeometry::exportToWkt() const { - return exportToWkt( mGeometry ); + if (mDirtyWkt) + { + return (mDirtyWkt = exportToWkt( mGeometry )); + } + else + { + // Already have a fresh copy of Wkt available. + return TRUE; + } } geos::Geometry* QgsGeometry::geosGeometry() const { + + if (!mDirtyGeos) + { + // No need to convert again + return mGeos; + + // TODO: make mGeos useful - assign to it and clear mDirty before we return out of this function + } + if(!mGeometry) { return 0; @@ -1201,7 +1278,9 @@ geos::Geometry* QgsGeometry::geosGeometry() const QPointArray *pa; int wkbtype; + // TODO: Make this a static member - save generating for every geometry geos::GeometryFactory* geometryFactory = new geos::GeometryFactory(); + wkbtype=(int) mGeometry[1]; switch(wkbtype) { @@ -1209,6 +1288,8 @@ geos::Geometry* QgsGeometry::geosGeometry() const { x = (double *) (mGeometry + 5); y = (double *) (mGeometry + 5 + sizeof(double)); + + mDirtyGeos = FALSE; return geometryFactory->createPoint(geos::Coordinate(*x,*y)); } case QGis::WKBMultiPoint: @@ -1383,6 +1464,17 @@ geos::Geometry* QgsGeometry::geosGeometry() const } +void QgsGeometry::fromGeosGeometry() +// TODO: Make this work +{ + if(!mGeometry) + { +// return 0; + } + +} + + double QgsGeometry::distanceSquaredPointToSegment(QgsPoint& point, double *x1, double *y1, diff --git a/src/qgsgeometry.h b/src/qgsgeometry.h index 89b8832887a..540f57d4112 100644 --- a/src/qgsgeometry.h +++ b/src/qgsgeometry.h @@ -57,9 +57,9 @@ class QgsGeometry { /** Set the geometry, feeding in the buffer containing OGC Well-Known Binary and the buffer's length. - This class will take ownership of the buffer + This class will take ownership of the buffer. */ - void setFromWkb(unsigned char * wkb, size_t length); + void setWkbAndOwnership(unsigned char * wkb, size_t length); /** Returns the buffer containing this geometry in WKB format. @@ -115,6 +115,8 @@ class QgsGeometry { /**Creates a geos geometry from this features geometry. Note, that the returned object needs to be deleted*/ geos::Geometry* geosGeometry() const; + /** Converts from the GEOS geometry back to the native WKB geometry. (Experimental) */ + void fromGeosGeometry(); @@ -132,7 +134,20 @@ class QgsGeometry { /** cached WKT version of this geometry */ mutable QString mWkt; + + /** cached GEOS version of this geometry */ + mutable geos::Geometry* mGeos; + + /** If the geometry has been set since the last conversion to WKB **/ + mutable bool mDirtyWkb; + + /** If the geometry has been set since the last conversion to WKT **/ + mutable bool mDirtyWkt; + + /** If the geometry has been set since the last conversion to GEOS **/ + mutable bool mDirtyGeos; + /** Squared distance from point to the given line segment * TODO: Perhaps move this to QgsPoint