Skip to content

Commit

Permalink
geometry: allow removing parts and rings or geometries by removing al…
Browse files Browse the repository at this point in the history
…l vertices

(fixes #10684)
  • Loading branch information
jef-n committed Jun 25, 2014
1 parent d7bd9c7 commit c686c4f
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 13 deletions.
2 changes: 1 addition & 1 deletion src/app/nodetool/qgsselectedfeature.cpp
Expand Up @@ -258,7 +258,7 @@ void QgsSelectedFeature::deleteSelectedVertexes()
beginGeometryChange();

int count = 0;
for ( int i = mVertexMap.size() - 1; i > -1; i-- )
for ( int i = mVertexMap.size() - 1; i > -1 && nSelected > 0; i-- )
{
if ( mVertexMap[i]->isSelected() )
{
Expand Down
97 changes: 87 additions & 10 deletions src/core/qgsgeometry.cpp
Expand Up @@ -1363,18 +1363,31 @@ bool QgsGeometry::moveVertex( double x, double y, int atVertex )
}
}

bool QgsGeometry::deleteVertex( QgsConstWkbPtr &srcPtr, QgsWkbPtr &dstPtr, int atVertex, bool hasZValue, int &pointIndex, bool isRing, bool lastItem )
// copy vertices from srcPtr to dstPtr and skip/delete one vertex
// @param srcPtr ring/part starting with number of points (adjusted in each call)
// @param dstPtr ring/part to copy to (adjusted in each call)
// @param atVertex index of vertex to skip
// @param hasZValue points have 3 elements
// @param pointIndex reference to index of first ring/part vertex in overall object (adjusted in each call)
// @param isRing srcPtr points to a ring
// @param lastItem last ring/part, atVertex after this one must be wrong
// @return
// 0 no delete was done
// 1 "normal" delete was done
// 2 last element of the ring/part was deleted
int QgsGeometry::deleteVertex( QgsConstWkbPtr &srcPtr, QgsWkbPtr &dstPtr, int atVertex, bool hasZValue, int &pointIndex, bool isRing, bool lastItem )
{
QgsDebugMsg( QString( "atVertex:%1 hasZValue:%2 pointIndex:%3 isRing:%4" ).arg( atVertex ).arg( hasZValue ).arg( pointIndex ).arg( isRing ) );
const int ps = ( hasZValue ? 3 : 2 ) * sizeof( double );
int nPoints;
srcPtr >> nPoints;

// copy complete ring/part if vertex is in a following one
if ( atVertex < pointIndex || atVertex >= pointIndex + nPoints )
{
// atVertex does not exist
if ( lastItem && atVertex >= pointIndex + nPoints )
return false;
return 0;

dstPtr << nPoints;

Expand All @@ -1383,14 +1396,25 @@ bool QgsGeometry::deleteVertex( QgsConstWkbPtr &srcPtr, QgsWkbPtr &dstPtr, int a
dstPtr += len;
srcPtr += len;
pointIndex += nPoints;
return false;
return 0;
}

// delete the first vertex of a ring instead of the last
if ( isRing && atVertex == pointIndex + nPoints - 1 )
atVertex = pointIndex;

if ( nPoints == ( isRing ? 2 : 1 ) )
{
// last point of the part/ring is deleted
// skip the whole part/ring
srcPtr += nPoints * ps;
pointIndex += nPoints;
return 2;
}

dstPtr << nPoints - 1;

// copy ring before vertex
int len = ( atVertex - pointIndex ) * ps;
if ( len > 0 )
{
Expand All @@ -1399,9 +1423,13 @@ bool QgsGeometry::deleteVertex( QgsConstWkbPtr &srcPtr, QgsWkbPtr &dstPtr, int a
srcPtr += len;
}

// skip deleted vertex
srcPtr += ps;

// copy reset of ring
len = ( pointIndex + nPoints - atVertex - 1 ) * ps;

// save position of vertex, if we delete the first vertex of a ring
const unsigned char *first = 0;
if ( isRing && atVertex == pointIndex )
{
Expand All @@ -1416,20 +1444,22 @@ bool QgsGeometry::deleteVertex( QgsConstWkbPtr &srcPtr, QgsWkbPtr &dstPtr, int a
srcPtr += len;
}

// copy new first vertex instead of the old last, if we deleted the original first vertex
if ( first )
{
memcpy( dstPtr, first, ps );
dstPtr += ps;
srcPtr += ps;
}

pointIndex += nPoints - 1;
pointIndex += nPoints;

return true;
return 1;
}

bool QgsGeometry::deleteVertex( int atVertex )
{
QgsDebugMsg( QString( "atVertex:%1" ).arg( atVertex ) );
if ( atVertex < 0 )
return false;

Expand Down Expand Up @@ -1468,7 +1498,14 @@ bool QgsGeometry::deleteVertex( int atVertex )
case QGis::WKBLineString:
{
int pointIndex = 0;
deleted = deleteVertex( srcPtr, dstPtr, atVertex, hasZValue, pointIndex, false, true );
int res = deleteVertex( srcPtr, dstPtr, atVertex, hasZValue, pointIndex, false, true );
if ( res == 2 )
{
// Linestring with 0 points
dstPtr << 0;
}

deleted = res != 0;
break;
}

Expand All @@ -1477,10 +1514,17 @@ bool QgsGeometry::deleteVertex( int atVertex )
{
int nRings;
srcPtr >> nRings;
QgsWkbPtr ptrN( dstPtr );
dstPtr << nRings;

for ( int ringnr = 0, pointIndex = 0; ringnr < nRings; ++ringnr )
deleted |= deleteVertex( srcPtr, dstPtr, atVertex, hasZValue, pointIndex, true, ringnr == nRings - 1 );
{
int res = deleteVertex( srcPtr, dstPtr, atVertex, hasZValue, pointIndex, true, ringnr == nRings - 1 );
if ( res == 2 )
ptrN << nRings - 1;

deleted |= res != 0;
}

break;
}
Expand Down Expand Up @@ -1524,13 +1568,24 @@ bool QgsGeometry::deleteVertex( int atVertex )
{
int nLines;
srcPtr >> nLines;
QgsWkbPtr ptrN( dstPtr );
dstPtr << nLines;

for ( int linenr = 0, pointIndex = 0; linenr < nLines; ++linenr )
{
QgsWkbPtr saveDstPtr( dstPtr );
srcPtr >> endianness >> wkbType;
dstPtr << endianness << wkbType;
deleted |= deleteVertex( srcPtr, dstPtr, atVertex, hasZValue, pointIndex, false, linenr == nLines - 1 );

int res = deleteVertex( srcPtr, dstPtr, atVertex, hasZValue, pointIndex, false, linenr == nLines - 1 );
if ( res == 2 )
{
// line string was completely removed
ptrN << nLines - 1;
dstPtr = saveDstPtr;
}

deleted |= res != 0;
}

break;
Expand All @@ -1541,16 +1596,38 @@ bool QgsGeometry::deleteVertex( int atVertex )
{
int nPolys;
srcPtr >> nPolys;
QgsWkbPtr ptrNPolys( dstPtr );
dstPtr << nPolys;

for ( int polynr = 0, pointIndex = 0; polynr < nPolys; ++polynr )
{
int nRings;
srcPtr >> endianness >> wkbType >> nRings;
dstPtr << endianness << wkbType << nRings;
QgsWkbPtr saveDstPolyPtr( dstPtr );
dstPtr << endianness << wkbType;
QgsWkbPtr ptrNRings( dstPtr );
dstPtr << nRings;

for ( int ringnr = 0; ringnr < nRings; ++ringnr )
deleted |= deleteVertex( srcPtr, dstPtr, atVertex, hasZValue, pointIndex, true, polynr == nPolys - 1 && ringnr == nRings - 1 );
{
int res = deleteVertex( srcPtr, dstPtr, atVertex, hasZValue, pointIndex, true, polynr == nPolys - 1 && ringnr == nRings - 1 );
if ( res == 2 )
{
// ring was completely removed
if ( nRings == 1 )
{
// last ring => remove polygon
ptrNPolys << nPolys - 1;
dstPtr = saveDstPolyPtr;
}
else
{
ptrNRings << nRings - 1;
}
}

deleted |= res != 0;
}
}
break;
}
Expand Down
2 changes: 1 addition & 1 deletion src/core/qgsgeometry.h
Expand Up @@ -648,7 +648,7 @@ class CORE_EXPORT QgsGeometry
double leftOf( double x, double y, double& x1, double& y1, double& x2, double& y2 );

static inline bool moveVertex( QgsWkbPtr &wkbPtr, const double &x, const double &y, int atVertex, bool hasZValue, int &pointIndex, bool isRing );
static inline bool deleteVertex( QgsConstWkbPtr &srcPtr, QgsWkbPtr &dstPtr, int atVertex, bool hasZValue, int &pointIndex, bool isRing, bool lastItem );
static inline int deleteVertex( QgsConstWkbPtr &srcPtr, QgsWkbPtr &dstPtr, int atVertex, bool hasZValue, int &pointIndex, bool isRing, bool lastItem );
static inline bool insertVertex( QgsConstWkbPtr &srcPtr, QgsWkbPtr &dstPtr, int beforeVertex, const double &x, const double &y, bool hasZValue, int &pointIndex, bool isRing );

/** try to convert the geometry to a point */
Expand Down
2 changes: 1 addition & 1 deletion tests/src/python/CMakeLists.txt
Expand Up @@ -4,7 +4,7 @@ ADD_PYTHON_TEST(PyQgsLocalServer test_qgis_local_server.py)
ADD_PYTHON_TEST(PyQgsFontUtils test_qgsfontutils.py)
ADD_PYTHON_TEST(PyQgsFeature test_qgsfeature.py)
ADD_PYTHON_TEST(PyQgsFeatureIterator test_qgsfeatureiterator.py)
ADD_PYTHON_TEST(PyQgsGeometry test_qgsgeometry.py)
ADD_PYTHON_TEST(PyQgsGeometryTest test_qgsgeometry.py)
ADD_PYTHON_TEST(PyQgsGeometryAvoidIntersections test_qgsgeometry_avoid_intersections.py)
ADD_PYTHON_TEST(PyQgsVectorLayer test_qgsvectorlayer.py)
ADD_PYTHON_TEST(PyQgsRasterLayer test_qgsrasterlayer.py)
Expand Down
25 changes: 25 additions & 0 deletions tests/src/python/test_qgsgeometry.py
Expand Up @@ -950,6 +950,31 @@ def testDeleteVertex(self):
wkt = polygon.exportToWkt()
assert compareWkt( expwkt, wkt ), "Expected:\n%s\nGot:\n%s\n" % (expwkt, wkt )

# 3-+-+-+-+-+-+-+-+-2
# | |
# + 8-7 3-2 8-7 3-2 +
# | | | | | | | | | |
# + 5-6 0-1 5-6 0-1 +
# | |
# 0-+-+-+-+---+-+-+-1
polygon = QgsGeometry.fromWkt( "POLYGON((0 0,9 0,9 3,0 3,0 0),(1 1,2 1,2 2,1 2,1 1),(3 1,4 1,4 2,3 2,3 1),(5 1,6 1,6 2,5 2,5 1),(7 1,8 1,8 2,7 2,7 1))" )
# 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24

for i in range(4):
assert polygon.deleteVertex(16), "Delete vertex 16 failed" % i

expwkt = "POLYGON((0 0,9 0,9 3,0 3,0 0),(1 1,2 1,2 2,1 2,1 1),(3 1,4 1,4 2,3 2,3 1),(7 1,8 1,8 2,7 2,7 1))"
wkt = polygon.exportToWkt()
assert compareWkt( expwkt, wkt ), "Expected:\n%s\nGot:\n%s\n" % (expwkt, wkt )

for i in range(3):
for j in range(4):
assert polygon.deleteVertex(5), "Delete vertex 5 failed" % i

expwkt = "POLYGON((0 0,9 0,9 3,0 3,0 0))"
wkt = polygon.exportToWkt()
assert compareWkt( expwkt, wkt ), "Expected:\n%s\nGot:\n%s\n" % (expwkt, wkt )

def testInsertVertex(self):
linestring = QgsGeometry.fromWkt( "LINESTRING(1 0,2 0)" )

Expand Down

1 comment on commit c686c4f

@nirvn
Copy link
Contributor

@nirvn nirvn commented on c686c4f Jun 25, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jef-n there should be some safeguards against invalid inner rings (i.e. inner rings with less than 3 nodes). Right now, you can delete all but one or two nodes, and that breaks the geometry (can't be edited, can't be deleted).

Maybe the logic should be if an inner ring node count is less than 3, it should be removed.

Please sign in to comment.