-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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.
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
Don't add unnecessary points on intersection when tracing #59055
Conversation
🪟 Windows buildsDownload Windows builds of this PR for testing. 🪟 Windows Qt6 buildsDownload Windows Qt6 builds of this PR for testing. |
23e9676
to
19483b2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good! 👍
Just a question: Do we really want an API call to keep the old (reported as error) behavior? And if yes, shall we not have a widget to be able to configure it?
My guts feeling here is that nobody wants the intersection vertex (but I'm no digitizing expert), no?
I believe so too. That's why I opted to not have that configurable in the gui. However, QgsTracer may be used as a general purpose graph solver in cases where the extra vertex does not really matter and it might not be worth the extra processing to have them removed, that's why I left it in the API. |
There is a certain logic (and simplicity) to honouring the 'Snap to Intersections' toggle as a mechanism to switch between the old and new behaviour. Thank you very much for implementing the change. |
I have considered this, but rejected the idea eventually. These are two distinct functionalities, as one may want to start/end his trace on intersections. In this case he would need to constantly toggle the Snap to intersections button to avoid having the extra points generated, which is arguably what pretty much everyone wants. |
I think it's likely that any tracing that requires a start or end on an intersection without a vertex would accept the original behaviour at intermediate intersections and vice versa, and so your scenario of repeated toggling of Snap to Intersections feels unlikely, however I'm not aware of specific use cases either way so can't back that up, sorry. |
Description
This fixes a longstanding issue with tracing, where points were being added where layers were intersecting.
This behavior only made sense on a graph theory context but not on a user digitizing context.
With this PR those intersection points are skipped by default. Anyone using the
QgsTracer
class can still enable the previous behavior by usingQgsTracer::setAddPointsOnIntersectionsEnabled( bool )
Fixes: #52935
Fixes: #59041