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.

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

[ReshapeMapTool] Fix snapping matches with tracing #58779

Merged
merged 3 commits into from
Sep 18, 2024

Conversation

troopa81
Copy link
Contributor

addCurve() method already update the mSnappingMatches variable so we need to update pointBefore variable after the addCurve() call

Fixes #57225

@troopa81 troopa81 added Map Tools Related to non-digitizing map tools Bug Either a bug report, or a bug fix. Let's hope for the latter! labels Sep 17, 2024
@github-actions github-actions bot added this to the 3.40.0 milestone Sep 17, 2024
@troopa81 troopa81 added the backport queued_ltr_backports Queued Backports label Sep 17, 2024
Copy link

github-actions bot commented Sep 17, 2024

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit 5480379)

addCurve( new QgsLineString( mapPoints ) );
int pointBefore = mCaptureCurve.numPoints();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to make it clearer what this is actually doing, could we move all the logic relating to pointBefore/pointAfter and the sync snapping matches into the block which starts with if ( QgsSettingsRegistryCore::settingsDigitizingConvertToCurve->value() ) ? As far as I can see this logic ONLY applies now if the curve approximation is happening.

And on that note... if the unsegmentized line ends up with LESS vertices then the segmentized one, won't mSnappingMatches have too many entries and need to be trimmed? Maybe this whole logic could be simplified down to just:

    // sync the snapping matches list
    mSnappingMatches.resize( mCaptureCurve.numPoints() )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Done c3c3ccc

When we need curve de-approximation
So we keep the existing snapping matches
@nyalldawson nyalldawson merged commit a4bc5fc into qgis:master Sep 18, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport release-3_34 Bug Either a bug report, or a bug fix. Let's hope for the latter! Crash/Data Corruption Map Tools Related to non-digitizing map tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Repeated Crash when using Reshape Feature Tool
2 participants