Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix move and rotation map tools preview when layer crs != canvas crs #51445

Merged
merged 2 commits into from
Feb 21, 2023
Merged

Fix move and rotation map tools preview when layer crs != canvas crs #51445

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5ec0da8
Fix move and rotation map tools preview when layer crs != canvas crs
YoannQDQ Jan 12, 2023
da9df38
Apply suggestions from code review
YoannQDQ Jan 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 54 additions & 31 deletions src/app/qgsmaptoolmovefeature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,34 +42,55 @@ QgsMapToolMoveFeature::QgsMapToolMoveFeature( QgsMapCanvas *canvas, MoveMode mod

QgsMapToolMoveFeature::~QgsMapToolMoveFeature()
{
delete mRubberBand;
deleteRubberband();
}

void QgsMapToolMoveFeature::cadCanvasMoveEvent( QgsMapMouseEvent *e )
{
if ( mRubberBand )
{
const QgsPointXY pointCanvasCoords = e->mapPoint();
const double offsetX = pointCanvasCoords.x() - mStartPointMapCoords.x();
const double offsetY = pointCanvasCoords.y() - mStartPointMapCoords.y();
mRubberBand->setTranslationOffset( offsetX, offsetY );
mRubberBand->updatePosition();
mRubberBand->update();
Comment on lines -56 to -57
Copy link
Member

Choose a reason for hiding this comment

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

why are these 2 lines dropped?

Copy link
Contributor Author

@YoannQDQ YoannQDQ Jan 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

If I'm not mistaken, setTranslationOffset already updates the rubberband, making the call to updatePosition and update redundant.

mSnapIndicator->setMatch( e->mapPointMatch() );
}
else
{
mSnapIndicator->setMatch( e->mapPointMatch() );
QgsVectorLayer *vlayer = currentVectorLayer();

// When MapCanvas crs == layer crs, fast rubberband translation
if ( vlayer->crs() == canvas()->mapSettings().destinationCrs() )
{
const QgsPointXY pointCanvasCoords = e->mapPoint();
const double offsetX = pointCanvasCoords.x() - mStartPointMapCoords.x();
const double offsetY = pointCanvasCoords.y() - mStartPointMapCoords.y();
mRubberBand->setTranslationOffset( offsetX, offsetY );
}

// Else, recreate the rubber band from the translated geometries
else
{
const QgsPointXY startPointLayerCoords = toLayerCoordinates( ( QgsMapLayer * )vlayer, mStartPointMapCoords );
const QgsPointXY stopPointLayerCoords = toLayerCoordinates( ( QgsMapLayer * )vlayer, e->mapPoint() );

const double dx = stopPointLayerCoords.x() - startPointLayerCoords.x();
const double dy = stopPointLayerCoords.y() - startPointLayerCoords.y();

QgsGeometry geom = mGeom;

if ( geom.translate( dx, dy ) == Qgis::GeometryOperationResult::Success )
{
mRubberBand->setToGeometry( geom, vlayer );
}
else
{
mRubberBand->reset( vlayer->geometryType() );
}
}
}

mSnapIndicator->setMatch( e->mapPointMatch() );
}

void QgsMapToolMoveFeature::cadCanvasReleaseEvent( QgsMapMouseEvent *e )
{
QgsVectorLayer *vlayer = currentVectorLayer();
if ( !vlayer || !vlayer->isEditable() )
{
delete mRubberBand;
mRubberBand = nullptr;
deleteRubberband();
mSnapIndicator->setMatch( QgsPointLocator::Match() );
cadDockWidget()->clear();
notifyNotEditableLayer();
Expand Down Expand Up @@ -123,7 +144,8 @@ void QgsMapToolMoveFeature::cadCanvasReleaseEvent( QgsMapMouseEvent *e )
mMovedFeatures << cf.id(); //todo: take the closest feature, not the first one...

mRubberBand = createRubberBand( vlayer->geometryType() );
mRubberBand->setToGeometry( cf.geometry(), vlayer );
mGeom = cf.geometry();
mRubberBand->setToGeometry( mGeom, vlayer );
}
else
{
Expand All @@ -136,15 +158,16 @@ void QgsMapToolMoveFeature::cadCanvasReleaseEvent( QgsMapMouseEvent *e )
bool allFeaturesInView = true;
const QgsRectangle viewRect = mCanvas->mapSettings().mapToLayerCoordinates( vlayer, mCanvas->extent() );

QVector <QgsGeometry> selectedGeometries;
while ( it.nextFeature( feat ) )
{
mRubberBand->addGeometry( feat.geometry(), vlayer, false );
selectedGeometries << feat.geometry();

if ( allFeaturesInView && !viewRect.intersects( feat.geometry().boundingBox() ) )
allFeaturesInView = false;
}
mRubberBand->updatePosition();
mRubberBand->update();
mGeom = QgsGeometry::collectGeometry( selectedGeometries );
mRubberBand->setToGeometry( mGeom, vlayer );

if ( !allFeaturesInView )
{
Expand All @@ -156,8 +179,7 @@ void QgsMapToolMoveFeature::cadCanvasReleaseEvent( QgsMapMouseEvent *e )
if ( res != QMessageBox::Yes )
{
mMovedFeatures.clear();
delete mRubberBand;
mRubberBand = nullptr;
deleteRubberband();
mSnapIndicator->setMatch( QgsPointLocator::Match() );
return;
}
Expand All @@ -173,8 +195,7 @@ void QgsMapToolMoveFeature::cadCanvasReleaseEvent( QgsMapMouseEvent *e )
if ( e->button() != Qt::LeftButton )
{
cadDockWidget()->clear();
delete mRubberBand;
mRubberBand = nullptr;
deleteRubberband();
mSnapIndicator->setMatch( QgsPointLocator::Match() );
return;
}
Expand Down Expand Up @@ -216,8 +237,7 @@ void QgsMapToolMoveFeature::cadCanvasReleaseEvent( QgsMapMouseEvent *e )
vlayer->addTopologicalPoints( vlayer->getGeometry( id ) );
}
}
delete mRubberBand;
mRubberBand = nullptr;
deleteRubberband();
mSnapIndicator->setMatch( QgsPointLocator::Match() );
cadDockWidget()->clear();
break;
Expand All @@ -229,8 +249,7 @@ void QgsMapToolMoveFeature::cadCanvasReleaseEvent( QgsMapMouseEvent *e )
if ( !QgisApp::instance()->vectorLayerTools()->copyMoveFeatures( vlayer, request, dx, dy, errorMsg, QgsProject::instance()->topologicalEditing(), mSnapIndicator->match().layer() ) )
{
emit messageEmitted( *errorMsg, Qgis::MessageLevel::Critical );
delete mRubberBand;
mRubberBand = nullptr;
deleteRubberband();
mSnapIndicator->setMatch( QgsPointLocator::Match() );
}
break;
Expand All @@ -243,9 +262,7 @@ void QgsMapToolMoveFeature::cadCanvasReleaseEvent( QgsMapMouseEvent *e )

void QgsMapToolMoveFeature::deactivate()
{
//delete rubber band
delete mRubberBand;
mRubberBand = nullptr;
deleteRubberband();
mSnapIndicator->setMatch( QgsPointLocator::Match() );

QgsMapToolAdvancedDigitizing::deactivate();
Expand All @@ -256,8 +273,14 @@ void QgsMapToolMoveFeature::keyReleaseEvent( QKeyEvent *e )
if ( mRubberBand && e->key() == Qt::Key_Escape )
{
cadDockWidget()->clear();
delete mRubberBand;
mRubberBand = nullptr;
deleteRubberband();
mSnapIndicator->setMatch( QgsPointLocator::Match() );
}
}

void QgsMapToolMoveFeature::deleteRubberband()
{
delete mRubberBand;
mRubberBand = nullptr;
mGeom = QgsGeometry();
}
8 changes: 7 additions & 1 deletion src/app/qgsmaptoolmovefeature.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ class APP_EXPORT QgsMapToolMoveFeature: public QgsMapToolAdvancedDigitizing
void keyReleaseEvent( QKeyEvent *e ) override;

private:

void deleteRubberband();

//! Start point of the move in map coordinates
QgsPointXY mStartPointMapCoords;

Expand All @@ -57,13 +60,16 @@ class APP_EXPORT QgsMapToolMoveFeature: public QgsMapToolAdvancedDigitizing
//! Snapping indicators
std::unique_ptr<QgsSnapIndicator> mSnapIndicator;

//! Id of moved feature
//! Id of moved features
QgsFeatureIds mMovedFeatures;

QPoint mPressPos;

MoveMode mMode;

// MultiGeometry of the moved features
QgsGeometry mGeom;

};

#endif
34 changes: 25 additions & 9 deletions src/app/qgsmaptoolrotatefeature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,8 @@ void QgsMapToolRotateFeature::cadCanvasReleaseEvent( QgsMapMouseEvent *e )
mRotatedFeatures << cf.id(); //todo: take the closest feature, not the first one...

mRubberBand = createRubberBand( vlayer->geometryType() );
mRubberBand->setToGeometry( cf.geometry(), vlayer );
mGeom = cf.geometry();
mRubberBand->setToGeometry( mGeom, vlayer );
}
else
{
Expand All @@ -313,12 +314,13 @@ void QgsMapToolRotateFeature::cadCanvasReleaseEvent( QgsMapMouseEvent *e )

QgsFeature feat;
QgsFeatureIterator it = vlayer->getSelectedFeatures();
QVector <QgsGeometry> selectedGeometries;
while ( it.nextFeature( feat ) )
{
mRubberBand->addGeometry( feat.geometry(), vlayer, false );
selectedGeometries << feat.geometry();
}
mRubberBand->updatePosition();
mRubberBand->update();
mGeom = QgsGeometry::collectGeometry( selectedGeometries );
mRubberBand->setToGeometry( mGeom, vlayer );
}

mRubberBand->show();
Expand Down Expand Up @@ -363,15 +365,28 @@ void QgsMapToolRotateFeature::updateRubberband( double rotation )
if ( mRotationActive )
{
mRotation = rotation;

mStPoint = toCanvasCoordinates( mStartPointMapCoords );
const double offsetX = mStPoint.x() - mRubberBand->x();
const double offsetY = mStPoint.y() - mRubberBand->y();

if ( mRubberBand )
{
mRubberBand->setTransform( QTransform().translate( offsetX, offsetY ).rotate( mRotation ).translate( -1 * offsetX, -1 * offsetY ) );
mRubberBand->update();
QgsVectorLayer *vlayer = currentVectorLayer();

// When MapCanvas crs == layer crs, fast rubberband rotation
if ( vlayer->crs() == canvas()->mapSettings().destinationCrs() )
{
const double offsetX = mStPoint.x() - mRubberBand->x();
const double offsetY = mStPoint.y() - mRubberBand->y();
mRubberBand->setTransform( QTransform().translate( offsetX, offsetY ).rotate( mRotation ).translate( -1 * offsetX, -1 * offsetY ) );
mRubberBand->update();
}
// Else, recreate the rubber band from the rotated geometries
else
{
const QgsPointXY anchorPoint = toLayerCoordinates( vlayer, mStartPointMapCoords );
QgsGeometry geom = mGeom;
geom.rotate( mRotation, anchorPoint );
mRubberBand->setToGeometry( geom, vlayer );
}
}
}
}
Expand Down Expand Up @@ -460,6 +475,7 @@ void QgsMapToolRotateFeature::deleteRubberband()
{
delete mRubberBand;
mRubberBand = nullptr;
mGeom = QgsGeometry();
}

void QgsMapToolRotateFeature::deactivate()
Expand Down
3 changes: 3 additions & 0 deletions src/app/qgsmaptoolrotatefeature.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ class APP_EXPORT QgsMapToolRotateFeature: public QgsMapToolAdvancedDigitizing

//! Shows current angle value and allows numerical editing
QgsAngleMagnetWidget *mRotationWidget = nullptr;

// MultiGeometry of the features being rotated
QgsGeometry mGeom;
};

#endif