Skip to content

Commit

Permalink
Implement undo logic for streamed digitizing mode
Browse files Browse the repository at this point in the history 
When the backspace key is held down, points will be repeatedly
removed from streamed digitized sections, right up till the start
of the streaming section. Then the point removal will temporarily
pause until the user releases and re-holds down the backspace key.

This allows users to selectively remove portions of the geometry
captured with the streaming mode by holding down the undo key,
without risking accidental undo of non-streamed portions.
  • Loading branch information
@nyalldawson
nyalldawson committed Mar 11, 2021
1 parent 135dd71 commit bffa42a
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 9 deletions.
7 changes: 5 additions & 2 deletions python/gui/auto_generated/qgsmaptoolcapture.sip.in
View file
Expand Up @@ -232,9 +232,12 @@ Variant to supply more information in the case of snapping
.. versionadded:: 2.14
%End

void undo();
void undo( bool isAutoRepeat = false );
%Docstring
Removes the last vertex from mRubberBand and mCaptureList
Removes the last vertex from mRubberBand and mCaptureList.

Since QGIS 3.20, if ``isAutoRepeat`` is set to ``True`` then the undo operation will be treated
as a auto repeated undo as if the user has held down the undo key for an extended period of time.
%End

void startCapturing();
Expand Down
18 changes: 16 additions & 2 deletions src/gui/qgsmaptoolcapture.cpp
View file
Expand Up @@ -710,7 +710,7 @@ QList<QgsPointLocator::Match> QgsMapToolCapture::snappingMatches() const
return mSnappingMatches;
}

void QgsMapToolCapture::undo()
void QgsMapToolCapture::undo( bool isAutoRepeat )
{
mTracingStartPoint = QgsPointXY();

Expand All @@ -719,6 +719,10 @@ void QgsMapToolCapture::undo()
if ( size() <= 1 && mTempRubberBand->pointsCount() != 0 )
return;

if ( isAutoRepeat && mIgnoreSubsequentAutoRepeatUndo )
return;
mIgnoreSubsequentAutoRepeatUndo = false;

QgsPoint lastPoint = mTempRubberBand->lastPoint();

if ( mTempRubberBand->stringType() == QgsWkbTypes::CircularString && mTempRubberBand->pointsCount() > 2 )
Expand All @@ -742,12 +746,22 @@ void QgsMapToolCapture::undo()
}
else
{
const int curvesBefore = mCaptureCurve.nCurves();
const bool lastCurveIsLineString = qgsgeometry_cast< QgsLineString * >( mCaptureCurve.curveAt( curvesBefore - 1 ) );

int pointsCountBefore = mCaptureCurve.numPoints();
mCaptureCurve.deleteVertex( vertexToRemove );
int pointsCountAfter = mCaptureCurve.numPoints();
for ( ; pointsCountAfter < pointsCountBefore; pointsCountAfter++ )
if ( !mSnappingMatches.empty() )
mSnappingMatches.removeLast();

// if we have removed the last point in a linestring curve, then we "stick" here and ignore subsequent
// autorepeat undo actions until the user releases the undo key and holds it down again. This allows
// users to selectively remove portions of the geometry captured with the streaming mode by holding down
// the undo key, without risking accidental undo of non-streamed portions.
if ( mCaptureCurve.nCurves() < curvesBefore && lastCurveIsLineString )
mIgnoreSubsequentAutoRepeatUndo = true;
}

updateExtraSnapLayer();
Expand All @@ -773,7 +787,7 @@ void QgsMapToolCapture::keyPressEvent( QKeyEvent *e )
{
if ( e->key() == Qt::Key_Backspace || e->key() == Qt::Key_Delete )
{
undo();
undo( e->isAutoRepeat() );

// Override default shortcut management in MapCanvas
e->ignore();
Expand Down
10 changes: 8 additions & 2 deletions src/gui/qgsmaptoolcapture.h
View file
Expand Up @@ -326,8 +326,13 @@ class GUI_EXPORT QgsMapToolCapture : public QgsMapToolAdvancedDigitizing
*/
int addVertex( const QgsPointXY &mapPoint, const QgsPointLocator::Match &match );

//! Removes the last vertex from mRubberBand and mCaptureList
void undo();
/**
* Removes the last vertex from mRubberBand and mCaptureList.
*
* Since QGIS 3.20, if \a isAutoRepeat is set to TRUE then the undo operation will be treated
* as a auto repeated undo as if the user has held down the undo key for an extended period of time.
*/
void undo( bool isAutoRepeat = false );

/**
* Start capturing
Expand Down Expand Up @@ -466,6 +471,7 @@ class GUI_EXPORT QgsMapToolCapture : public QgsMapToolAdvancedDigitizing
bool mAllowAddingStreamingPoints = false;
bool mStartNewCurve = false;

bool mIgnoreSubsequentAutoRepeatUndo = false;

};

Expand Down
86 changes: 86 additions & 0 deletions tests/src/app/testqgsmaptooladdfeatureline.cpp
View file
Expand Up @@ -76,6 +76,7 @@ class TestQgsMapToolAddFeatureLine : public QObject
void testLineString();
void testCompoundCurve();
void testStream();
void testUndo();

private:
QgisApp *mQgisApp = nullptr;
Expand Down Expand Up @@ -903,5 +904,90 @@ void TestQgsMapToolAddFeatureLine::testStream()
mLayerLine->undoStack()->undo();
}

void TestQgsMapToolAddFeatureLine::testUndo()
{
// test undo logic
TestQgsMapToolAdvancedDigitizingUtils utils( mCaptureTool );

mCanvas->setCurrentLayer( mLayerLine );

QSet<QgsFeatureId> oldFids = utils.existingFeatureIds();

QgsSnappingConfig cfg = mCanvas->snappingUtils()->config();
cfg.setEnabled( false );
mCanvas->snappingUtils()->setConfig( cfg );

oldFids = utils.existingFeatureIds();
utils.mouseClick( 5, 6.5, Qt::LeftButton );
utils.mouseClick( 6.25, 6.5, Qt::LeftButton );
utils.mouseClick( 6.75, 6.5, Qt::LeftButton );

mCaptureTool->setStreamDigitizingEnabled( true );
utils.mouseMove( 7.0, 6.6 );
utils.mouseMove( 7.1, 6.7 );
utils.mouseMove( 7.2, 6.6 );
utils.mouseMove( 7.3, 6.5 );
utils.mouseMove( 7.5, 6.9 );
utils.mouseMove( 7.6, 6.3 );
utils.mouseClick( 7.75, 6.5, Qt::LeftButton );
mCaptureTool->setStreamDigitizingEnabled( false );
utils.mouseClick( 7.5, 5.0, Qt::LeftButton );
mCaptureTool->setStreamDigitizingEnabled( true );
utils.mouseMove( 7.4, 5.0 );
utils.mouseMove( 7.3, 5.1 );
utils.mouseMove( 7.2, 5.0 );
utils.mouseMove( 7.1, 4.9 );

// check capture curve initially -- the streamed sections MUST become their own curve in the geometry, and not be compined with the straight line segments!
QCOMPARE( mCaptureTool->captureCurve()->asWkt( 2 ), QStringLiteral( "CompoundCurve ((5 6.5, 6.25 6.5, 6.75 6.5),(6.75 6.5, 7 6.59, 7.09 6.7, 7.2 6.59, 7.3 6.5, 7.5 6.91, 7.59 6.3),(7.59 6.3, 7.5 5),(7.5 5, 7.41 5, 7.3 5.09, 7.2 5, 7.09 4.91))" ) );

// now lets try undoing...
utils.keyClick( Qt::Key_Backspace );
QCOMPARE( mCaptureTool->captureCurve()->asWkt( 2 ), QStringLiteral( "CompoundCurve ((5 6.5, 6.25 6.5, 6.75 6.5),(6.75 6.5, 7 6.59, 7.09 6.7, 7.2 6.59, 7.3 6.5, 7.5 6.91, 7.59 6.3),(7.59 6.3, 7.5 5),(7.5 5, 7.41 5, 7.3 5.09, 7.2 5))" ) );
// simulate auto repeating undo from a held-down backspace key
utils.keyClick( Qt::Key_Backspace, Qt::KeyboardModifiers(), true );
QCOMPARE( mCaptureTool->captureCurve()->asWkt( 2 ), QStringLiteral( "CompoundCurve ((5 6.5, 6.25 6.5, 6.75 6.5),(6.75 6.5, 7 6.59, 7.09 6.7, 7.2 6.59, 7.3 6.5, 7.5 6.91, 7.59 6.3),(7.59 6.3, 7.5 5),(7.5 5, 7.41 5, 7.3 5.09))" ) );
utils.keyClick( Qt::Key_Backspace, Qt::KeyboardModifiers(), true );
QCOMPARE( mCaptureTool->captureCurve()->asWkt( 2 ), QStringLiteral( "CompoundCurve ((5 6.5, 6.25 6.5, 6.75 6.5),(6.75 6.5, 7 6.59, 7.09 6.7, 7.2 6.59, 7.3 6.5, 7.5 6.91, 7.59 6.3),(7.59 6.3, 7.5 5),(7.5 5, 7.41 5))" ) );
utils.keyClick( Qt::Key_Backspace, Qt::KeyboardModifiers(), true );
QCOMPARE( mCaptureTool->captureCurve()->asWkt( 2 ), QStringLiteral( "CompoundCurve ((5 6.5, 6.25 6.5, 6.75 6.5),(6.75 6.5, 7 6.59, 7.09 6.7, 7.2 6.59, 7.3 6.5, 7.5 6.91, 7.59 6.3),(7.59 6.3, 7.5 5))" ) );
utils.keyClick( Qt::Key_Backspace, Qt::KeyboardModifiers(), true );
// we've now finished undoing the streamed digitizing section, so undo should pause until the user releases the backspace key and then re-presses it
QCOMPARE( mCaptureTool->captureCurve()->asWkt( 2 ), QStringLiteral( "CompoundCurve ((5 6.5, 6.25 6.5, 6.75 6.5),(6.75 6.5, 7 6.59, 7.09 6.7, 7.2 6.59, 7.3 6.5, 7.5 6.91, 7.59 6.3),(7.59 6.3, 7.5 5))" ) );
utils.keyClick( Qt::Key_Backspace, Qt::KeyboardModifiers(), true );
QCOMPARE( mCaptureTool->captureCurve()->asWkt( 2 ), QStringLiteral( "CompoundCurve ((5 6.5, 6.25 6.5, 6.75 6.5),(6.75 6.5, 7 6.59, 7.09 6.7, 7.2 6.59, 7.3 6.5, 7.5 6.91, 7.59 6.3),(7.59 6.3, 7.5 5))" ) );
utils.keyClick( Qt::Key_Backspace, Qt::KeyboardModifiers(), false );
QCOMPARE( mCaptureTool->captureCurve()->asWkt( 2 ), QStringLiteral( "CompoundCurve ((5 6.5, 6.25 6.5, 6.75 6.5),(6.75 6.5, 7 6.59, 7.09 6.7, 7.2 6.59, 7.3 6.5, 7.5 6.91, 7.59 6.3))" ) );
utils.keyClick( Qt::Key_Backspace, Qt::KeyboardModifiers(), false );
QCOMPARE( mCaptureTool->captureCurve()->asWkt( 2 ), QStringLiteral( "CompoundCurve ((5 6.5, 6.25 6.5, 6.75 6.5),(6.75 6.5, 7 6.59, 7.09 6.7, 7.2 6.59, 7.3 6.5, 7.5 6.91))" ) );
// simulate holding down another key
utils.keyClick( Qt::Key_Backspace, Qt::KeyboardModifiers(), true );
QCOMPARE( mCaptureTool->captureCurve()->asWkt( 2 ), QStringLiteral( "CompoundCurve ((5 6.5, 6.25 6.5, 6.75 6.5),(6.75 6.5, 7 6.59, 7.09 6.7, 7.2 6.59, 7.3 6.5))" ) );
utils.keyClick( Qt::Key_Backspace, Qt::KeyboardModifiers(), true );
QCOMPARE( mCaptureTool->captureCurve()->asWkt( 2 ), QStringLiteral( "CompoundCurve ((5 6.5, 6.25 6.5, 6.75 6.5),(6.75 6.5, 7 6.59, 7.09 6.7, 7.2 6.59))" ) );
utils.keyClick( Qt::Key_Backspace, Qt::KeyboardModifiers(), true );
QCOMPARE( mCaptureTool->captureCurve()->asWkt( 2 ), QStringLiteral( "CompoundCurve ((5 6.5, 6.25 6.5, 6.75 6.5),(6.75 6.5, 7 6.59, 7.09 6.7))" ) );
utils.keyClick( Qt::Key_Backspace, Qt::KeyboardModifiers(), true );
QCOMPARE( mCaptureTool->captureCurve()->asWkt( 2 ), QStringLiteral( "CompoundCurve ((5 6.5, 6.25 6.5, 6.75 6.5),(6.75 6.5, 7 6.59))" ) );
utils.keyClick( Qt::Key_Backspace, Qt::KeyboardModifiers(), true );
QCOMPARE( mCaptureTool->captureCurve()->asWkt( 2 ), QStringLiteral( "CompoundCurve ((5 6.5, 6.25 6.5, 6.75 6.5))" ) );
utils.keyClick( Qt::Key_Backspace, Qt::KeyboardModifiers(), true );
// should get "stuck" here again
QCOMPARE( mCaptureTool->captureCurve()->asWkt( 2 ), QStringLiteral( "CompoundCurve ((5 6.5, 6.25 6.5, 6.75 6.5))" ) );
utils.keyClick( Qt::Key_Backspace, Qt::KeyboardModifiers(), true );
QCOMPARE( mCaptureTool->captureCurve()->asWkt( 2 ), QStringLiteral( "CompoundCurve ((5 6.5, 6.25 6.5, 6.75 6.5))" ) );
// release and repress
utils.keyClick( Qt::Key_Backspace, Qt::KeyboardModifiers(), false );
QCOMPARE( mCaptureTool->captureCurve()->asWkt( 2 ), QStringLiteral( "CompoundCurve ((5 6.5, 6.25 6.5))" ) );
utils.keyClick( Qt::Key_Backspace, Qt::KeyboardModifiers(), true );
QCOMPARE( mCaptureTool->captureCurve()->asWkt( 2 ), QStringLiteral( "CompoundCurve ((5 6.5))" ) );
utils.keyClick( Qt::Key_Backspace, Qt::KeyboardModifiers(), true );
QCOMPARE( mCaptureTool->captureCurve()->asWkt( 2 ), QStringLiteral( "CompoundCurve ((5 6.5))" ) );
utils.keyClick( Qt::Key_Backspace, Qt::KeyboardModifiers(), true );

utils.mouseClick( 7.0, 5.0, Qt::RightButton );
mCaptureTool->setStreamDigitizingEnabled( false );
}

QGSTEST_MAIN( TestQgsMapToolAddFeatureLine )
#include "testqgsmaptooladdfeatureline.moc"
6 changes: 3 additions & 3 deletions tests/src/app/testqgsmaptoolutils.h
View file
Expand Up @@ -97,12 +97,12 @@ class TestQgsMapToolAdvancedDigitizingUtils
mouseRelease( mapX, mapY, button, stateKey, snap );
}

void keyClick( int key, Qt::KeyboardModifiers stateKey = Qt::KeyboardModifiers() )
void keyClick( int key, Qt::KeyboardModifiers stateKey = Qt::KeyboardModifiers(), bool autoRepeat = false )
{
QKeyEvent e1( QEvent::KeyPress, key, stateKey );
QKeyEvent e1( QEvent::KeyPress, key, stateKey, QString(), autoRepeat );
mMapTool->keyPressEvent( &e1 );

QKeyEvent e2( QEvent::KeyRelease, key, stateKey );
QKeyEvent e2( QEvent::KeyRelease, key, stateKey, QString(), autoRepeat );
mMapTool->keyReleaseEvent( &e2 );
}

Expand Down

0 comments on commit bffa42a

Please sign in to comment.