Skip to content
Permalink
Browse files
[convert to curve] cleanup comments
  • Loading branch information
olivierdalang authored and nyalldawson committed Jun 18, 2021
1 parent 8160ec2 commit 6aa7de8a51383740befffd46a62c97da3bf647eb
@@ -2576,34 +2576,41 @@ void QgsVertexTool::toggleVertexCurve()
}
else
{
// TODO support more than just 1 vertex
QgsMessageLog::logMessage( "Need exactly 1 selected/editted vertex", "DEBUG" );
// TODO support more than just 1 vertex
QgisApp::instance()->messageBar()->pushMessage(
tr( "Could not convert vertex" ),
tr( "Currently, conversion can only be done on exactly one vertex." ),
Qgis::Info );
return;
}

QgsVectorLayer *layer = toConvert.layer;

if ( ! QgsWkbTypes::isCurvedType( layer->wkbType() ) )
{
QgsMessageLog::logMessage( "Layer is not curved", "DEBUG" );
QgisApp::instance()->messageBar()->pushMessage(
tr( "Could not convert vertex" ),
tr( "Layer of type %1 does not support curved geometries." ).arg( layer->wkbType() ),
Qgis::Warning );
return;
}

layer->beginEditCommand( tr( "Converting vertex type" ) );
QgsGeometry geom = layer->getFeature( toConvert.fid ).geometry();
bool success = geom.convertVertex( toConvert.vertexId );
bool success = geom.convertVertex(toConvert.vertexId);

if ( success )
{
QgsMessageLog::logMessage( "Should be OK", "DEBUG" );
layer->changeGeometry( toConvert.fid, geom );
layer->endEditCommand();
layer->triggerRepaint();
}
else
{
QgsMessageLog::logMessage( "Has failed :-/", "DEBUG" );
layer->destroyEditCommand();
QgisApp::instance()->messageBar()->pushMessage(
tr( "Could not convert vertex" ),
tr( "Start/end of vertices of features and arcs can not be converted." ).arg( layer->wkbType() ),
Qgis::Warning );
}

if ( mVertexEditor && mLockedFeature )
@@ -921,46 +921,38 @@ bool QgsCompoundCurve::convertVertex( QgsVertexId position )
// First we find out the sub-curves that are contain that vertex.

// If there is more than one, it means the vertex was at the beginning or end
// of an arc, which we don't support. [TODO : could also happen if at the begging or end of
// two LineStrings, esp. after converting some other vertices... we may need to merge successive linestrings]
// of an arc, which we don't support.

// If there is exactly one, we may either be on a LineString, or on a CircularString.

// If on CircularString, we need to check if the vertex is a CurveVertex. I not, the vertex
// was at the beginning or end, which we don't support. If yes, we must split the CircularString
// at vertex -1 and +1, , drop the middle part and insert a LineString instead with the same points.
// [TODO : as said above, probably worth merging successive linestrings into one once this is done]

// If on a LineString, we need to split the LineString at vertex -1 and +1, and insert a CircularString
// instead.
// If on CircularString, we need to check if the vertex is a CurveVertex (odd index).
// If so, we split the subcurve at vertex -1 and +1, , drop the middle part and insert a LineString/CircularString
// instead with the same points.

// At the end, we call condenseCurves() to merge successible line/circular strings

QVector< QPair<int, QgsVertexId> > curveIds = curveVertexId( position );

// We cannot convert points at start/end of subcurves
if ( curveIds.length() != 1 )
{
QgsMessageLog::logMessage( "There is not exactly one subcurve at this position. " + QString::number( curveIds.length() ), "DEBUG" );
return false;
}

int curveId = curveIds[0].first;
QgsVertexId subVertexId = curveIds[0].second;
QgsCurve *curve = mCurves[curveId];

// We cannot convert first/last point of curve
if ( subVertexId.vertex == 0 || subVertexId.vertex == curve->numPoints() - 1 )
{
QgsMessageLog::logMessage( "Cannot convert first or last point of a sub-curve. Merge the subcurves first", "DEBUG" );
return false;
}

// TODO factorize circularString and lineString as logic is almost the same
if ( const QgsCircularString *circularString = dynamic_cast<const QgsCircularString *>( curve ) )
{
// If it's a circular string, we convert to LineString

// We cannot convert start/end points of arcs
if ( subVertexId.vertex % 2 == 0 ) // for some reason, subVertexId.type is always SegmentVertex...
{
QgsMessageLog::logMessage( "Cannot convert segment vertex of a circular string -> " + QString::number( subVertexId.type ), "DEBUG" );
return false;
}

QgsPointSequence points;
circularString->points( points );
@@ -571,34 +571,18 @@ bool QgsGeometry::convertVertex( int atVertex )
// If the ring is not a curve, we're probably on a point geometry
QgsCurve *curve = dynamic_cast<QgsCurve *>( ring ); // TODO dynamic_cast -> geom_cast
if ( curve == nullptr )
{
QgsMessageLog::logMessage( "Cannot execute convertVertex on " + geom->wktTypeStr(), "DEBUG" );
return false;
}

bool success = false;
QgsCompoundCurve *cpdCurve = dynamic_cast<QgsCompoundCurve *>( curve );
if ( cpdCurve != nullptr )
{
QgsMessageLog::logMessage( "Already compound", "DEBUG" );
// If the geom is a already compound curve, we convert inplace, and we're done
success = cpdCurve->convertVertex( id );

// // This doesn't work... Not sure how to get the geometry actuall update ? // <- REVIEW PLZ
// if ( success )
// if ( owningCollection != nullptr )
// reset( std::make_unique<QgsGeometryCollection>( *owningCollection ) ); // <- REVIEW PLZ
// else if ( owningPolygon != nullptr )
// reset( std::make_unique<QgsCurvePolygon>( *owningPolygon ) ); // <- REVIEW PLZ
// else
// reset( std::make_unique<QgsCompoundCurve>( *cpdCurve ) ); // <- REVIEW PLZ

}
else
{
// TODO : move this block before the above, so we call convertVertex only in one place

QgsMessageLog::logMessage( "Convert to compound", "DEBUG" );
// If the geom is a linestring or cirularstring, we create a compound curve
QgsCompoundCurve *cpdCurve = new QgsCompoundCurve();
cpdCurve->addCurve( curve->clone() );
@@ -607,19 +591,16 @@ bool QgsGeometry::convertVertex( int atVertex )
// In that case, we must also reassign the instances
if ( success )
{
QgsMessageLog::logMessage( "Success", "DEBUG" );

if ( owningPolygon == nullptr && owningCollection == nullptr )
{
// Standalone linestring
QgsMessageLog::logMessage( "case A", "DEBUG" );
reset( std::make_unique<QgsCompoundCurve>( *cpdCurve ) ); // <- REVIEW PLZ
}

if ( owningPolygon != nullptr )
{
// Replace the ring in the owning polygon
QgsMessageLog::logMessage( "case B", "DEBUG" );
if ( id.ring == 0 )
{
owningPolygon->setExteriorRing( cpdCurve );
@@ -633,16 +614,10 @@ bool QgsGeometry::convertVertex( int atVertex )
else if ( owningCollection != nullptr )
{
// Replace the curve in the owning collection
QgsMessageLog::logMessage( "case C", "DEBUG" );
owningCollection->removeGeometry( id.part );
owningCollection->insertGeometry( cpdCurve, id.part );
}
}
else
{
QgsMessageLog::logMessage( "failure ?!", "DEBUG" );

}
}

return success;
@@ -1325,31 +1325,22 @@ void TestQgsVertexTool::testSelectVerticesByPolygon()

void TestQgsVertexTool::testConvertVertex()
{
// convert vertex in linestring

// convert vertex in compoundCurve
QCOMPARE( mLayerCompoundCurve->getFeature( mFidCompoundCurveF1 ).geometry(), QgsGeometry::fromWkt( "CompoundCurve ( CircularString (14 14, 10 10, 17 10))" ) );
mouseClick( 1, 1, Qt::LeftButton );
keyClick( Qt::Key_C );
QCOMPARE( mLayerCompoundCurve->undoStack()->index(), 2 );
QCOMPARE( mLayerCompoundCurve->getFeature( mFidCompoundCurveF1 ).geometry(), QgsGeometry::fromWkt( "CompoundCurve ((14 14, 10 10, 17 10))" ) );
mLayerCompoundCurve->undoStack()->undo();
QCOMPARE( mLayerCompoundCurve->undoStack()->index(), 1 );

QCOMPARE( mLayerLine->undoStack()->index(), 2 );
QCOMPARE( mLayerLine->getFeature( mFidLineF1 ).geometry(), QgsGeometry::fromWkt( "COMPOUNDCURVE(CIRCULARSTRING(2 1, 1 1, 1 3))" ) );

mLayerLine->undoStack()->undo();

// convert vertex in linestring
QCOMPARE( mLayerLine->getFeature( mFidLineF1 ).geometry(), QgsGeometry::fromWkt( "LINESTRING(2 1, 0.5 0.5, 1 3)" ) );
mouseClick( 1, 1, Qt::LeftButton );
keyClick( Qt::Key_C ); // DOES NOTHING AS IT'S A LINE LAYER
QCOMPARE( mLayerLine->undoStack()->index(), 1 );
QCOMPARE( mLayerLine->getFeature( mFidLineF1 ).geometry(), QgsGeometry::fromWkt( "LINESTRING(2 1, 1 1, 1 3)" ) );


// convert vertex in polygon

mouseClick( 7, 4, Qt::LeftButton );
keyClick( Qt::Key_C );

QCOMPARE( mLayerPolygon->undoStack()->index(), 2 );
QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "CURVEPOLYGON(COMPOUNDCURVE((4 1, 7 1), CIRCULARSTRING(7 1, 7 4, 4 4), (4 4, 4 1)))" ) );

mLayerPolygon->undoStack()->undo();
QCOMPARE( mLayerLine->getFeature( mFidLineF1 ).geometry(), QgsGeometry::fromWkt( "LINESTRING(2 1, 0.5 0.5, 1 3)" ) );

QCOMPARE( mLayerPolygon->getFeature( mFidPolygonF1 ).geometry(), QgsGeometry::fromWkt( "POLYGON((4 1, 7 1, 7 4, 4 4, 4 1))" ) );
}

QGSTEST_MAIN( TestQgsVertexTool )
@@ -4580,12 +4580,8 @@ def testConvertVertex(self):

for wkt_before, wkt_expected in test_setup.items():
geom = QgsGeometry.fromWkt(wkt_before)
# Ensure convert has the expected result
geom.convertVertex(geom.closestVertex(QgsPointXY(10, 10))[1])
self.assertTrue(QgsGeometry.equals(geom, QgsGeometry.fromWkt(wkt_expected)), f'convertVertex() did not create expected geometry.\nconverted wkt : {geom.asWkt()}\nexpected wkt : {wkt_expected}\ninput wkt : {wkt_before}).')
# Ensure converting again returns back to initial
# geom.convertVertex(geom.closestVertex(QgsPointXY(10, 10))[1])
# self.assertEqual(geom.asWkt(), QgsGeometry.fromWkt(wkt_before).asWkt(), 'Second call to convertVertex did not properly revert changes.')

def testSingleSidedBuffer(self):

0 comments on commit 6aa7de8

Please sign in to comment.