Skip to content

Commit

Permalink
fix #36585 where snapping on segments with a constraint snapped to ed…
Browse files Browse the repository at this point in the history
…ges instead of vertices
  • Loading branch information
olivierdalang committed May 21, 2020
1 parent 66ebe34 commit 2942312
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 15 deletions.
27 changes: 15 additions & 12 deletions src/core/qgscadutils.cpp
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -42,16 +42,19 @@ QgsCadUtils::AlignMapPointOutput QgsCadUtils::alignMapPoint( const QgsPointXY &o


// try to snap to anything // try to snap to anything
QgsPointLocator::Match snapMatch = ctx.snappingUtils->snapToMap( originalMapPoint, nullptr, true ); QgsPointLocator::Match snapMatch = ctx.snappingUtils->snapToMap( originalMapPoint, nullptr, true );
res.snapMatch = snapMatch;
QgsPointXY point = snapMatch.isValid() ? snapMatch.point() : originalMapPoint; QgsPointXY point = snapMatch.isValid() ? snapMatch.point() : originalMapPoint;

// try to snap explicitly to a segment - useful for some constraints
QgsPointXY edgePt0, edgePt1; QgsPointXY edgePt0, edgePt1;
EdgesOnlyFilter edgesOnlyFilter; if ( snapMatch.hasEdge() )
QgsPointLocator::Match edgeMatch = ctx.snappingUtils->snapToMap( originalMapPoint, &edgesOnlyFilter, true ); {
if ( edgeMatch.hasEdge() ) snapMatch.edgePoints( edgePt0, edgePt1 );
edgeMatch.edgePoints( edgePt0, edgePt1 ); // note : res.edgeMatch should be removed, as we can just check snapMatch.hasEdge()

res.edgeMatch = snapMatch;
res.edgeMatch = edgeMatch; }
else
{
res.edgeMatch = QgsPointLocator::Match();
}


QgsPointXY previousPt, penultimatePt; QgsPointXY previousPt, penultimatePt;
if ( ctx.cadPointList.count() >= 2 ) if ( ctx.cadPointList.count() >= 2 )
Expand All @@ -71,7 +74,7 @@ QgsCadUtils::AlignMapPointOutput QgsCadUtils::alignMapPoint( const QgsPointXY &o
{ {
point.setX( previousPt.x() + ctx.xConstraint.value ); point.setX( previousPt.x() + ctx.xConstraint.value );
} }
if ( edgeMatch.hasEdge() && !ctx.yConstraint.locked ) if ( snapMatch.hasEdge() && !ctx.yConstraint.locked )
{ {
// intersect with snapped segment line at X coordinate // intersect with snapped segment line at X coordinate
const double dx = edgePt1.x() - edgePt0.x(); const double dx = edgePt1.x() - edgePt0.x();
Expand Down Expand Up @@ -99,7 +102,7 @@ QgsCadUtils::AlignMapPointOutput QgsCadUtils::alignMapPoint( const QgsPointXY &o
{ {
point.setY( previousPt.y() + ctx.yConstraint.value ); point.setY( previousPt.y() + ctx.yConstraint.value );
} }
if ( edgeMatch.hasEdge() && !ctx.xConstraint.locked ) if ( snapMatch.hasEdge() && !ctx.xConstraint.locked )
{ {
// intersect with snapped segment line at Y coordinate // intersect with snapped segment line at Y coordinate
const double dy = edgePt1.y() - edgePt0.y(); const double dy = edgePt1.y() - edgePt0.y();
Expand Down Expand Up @@ -224,7 +227,7 @@ QgsCadUtils::AlignMapPointOutput QgsCadUtils::alignMapPoint( const QgsPointXY &o
point.setY( previousPt.y() + sina * v ); point.setY( previousPt.y() + sina * v );
} }


if ( edgeMatch.hasEdge() && !ctx.distanceConstraint.locked ) if ( snapMatch.hasEdge() && !ctx.distanceConstraint.locked )
{ {
// magnetize to the intersection of the snapped segment and the lockedAngle // magnetize to the intersection of the snapped segment and the lockedAngle


Expand Down Expand Up @@ -287,7 +290,7 @@ QgsCadUtils::AlignMapPointOutput QgsCadUtils::alignMapPoint( const QgsPointXY &o
previousPt.y() + ( point.y() - previousPt.y() ) * vP ); previousPt.y() + ( point.y() - previousPt.y() ) * vP );
} }


if ( edgeMatch.hasEdge() && !ctx.angleConstraint.locked ) if ( snapMatch.hasEdge() && !ctx.angleConstraint.locked )
{ {
// we will magnietize to the intersection of that segment and the lockedDistance ! // we will magnietize to the intersection of that segment and the lockedDistance !
res.valid &= QgsGeometryUtils::lineCircleIntersection( previousPt, ctx.distanceConstraint.value, edgePt0, edgePt1, point ); res.valid &= QgsGeometryUtils::lineCircleIntersection( previousPt, ctx.distanceConstraint.value, edgePt0, edgePt1, point );
Expand Down
8 changes: 7 additions & 1 deletion src/core/qgscadutils.h
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -92,7 +92,13 @@ class CORE_EXPORT QgsCadUtils
//! map point aligned according to the constraints //! map point aligned according to the constraints
QgsPointXY finalMapPoint; QgsPointXY finalMapPoint;


//! Snapped segment - only valid if actually used for something //! Snapped point - only valid if actually used for something
QgsPointLocator::Match snapMatch;

/**
* Snapped segment - only valid if actually used for something
* \deprecated will be removed in QGIS 4.0 - use snapMatch instead
*/
QgsPointLocator::Match edgeMatch; QgsPointLocator::Match edgeMatch;


//! Angle (in degrees) to which we have soft-locked ourselves (if not set it is -1) //! Angle (in degrees) to which we have soft-locked ourselves (if not set it is -1)
Expand Down
4 changes: 2 additions & 2 deletions src/gui/qgsadvanceddigitizingdockwidget.cpp
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -648,10 +648,10 @@ bool QgsAdvancedDigitizingDockWidget::applyConstraints( QgsMapMouseEvent *e )
bool res = output.valid; bool res = output.valid;
QgsPointXY point = output.finalMapPoint; QgsPointXY point = output.finalMapPoint;
mSnappedSegment.clear(); mSnappedSegment.clear();
if ( output.edgeMatch.hasEdge() ) if ( output.snapMatch.hasEdge() )
{ {
QgsPointXY edgePt0, edgePt1; QgsPointXY edgePt0, edgePt1;
output.edgeMatch.edgePoints( edgePt0, edgePt1 ); output.snapMatch.edgePoints( edgePt0, edgePt1 );
mSnappedSegment << edgePt0 << edgePt1; mSnappedSegment << edgePt0 << edgePt1;
} }
if ( mAngleConstraint->lockMode() != CadConstraint::HardLock ) if ( mAngleConstraint->lockMode() != CadConstraint::HardLock )
Expand Down

0 comments on commit 2942312

Please sign in to comment.