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

BUG: fix add vertex from end point for CompoundCurveZ in gpkg, fix #3… #34055

Merged
merged 3 commits into from
Feb 4, 2020

Conversation

SebastienPeillet
Copy link
Contributor

@SebastienPeillet SebastienPeillet commented Jan 27, 2020

Description

#32080

As described in the issue, using a compound curve layer (in gpkg) it's not possible to extend existing entity from the end point, while it's possible from start point.

This was not possible because compound curve is composed of several part and user shouldn't be able to add an extra vertex in a part that is followed by another part. Extent from start point was available because the vertex index is 0, so we are sure there is no part before.

I fixed it by allowing vertex insert if it is for the last past (i == mCurves.size() - 1) of the curve and the vertex index is equal to the existing vertex count + 1

Checklist

  • Commit messages are descriptive and explain the rationale for changes
  • Commits which fix bugs include Fixes #11111 at the bottom of the commit message
  • I have read the QGIS Coding Standards and this PR complies with them
  • New unit tests have been added for core changes
  • I have run the scripts/prepare-commit.sh script before each commit
  • I have evaluated whether it is appropriate for this PR to be backported, backport requests are left as label or comment

@nyalldawson
Copy link
Collaborator

Can you please add a unit test for this?

@SebastienPeillet
Copy link
Contributor Author

Sorry for the delay, I added the tests for compound curve in the vertex tools, especially the ones about adding a extra vertex to a linestring in a compound curve layer and verifying that this is not available for circular string !

Copy link
Collaborator

@nyalldawson nyalldawson left a comment

Choose a reason for hiding this comment

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

The new test is much appreciated, but can we get a specific test in testqgsgeometry.cpp testing the compound curved insertVertex method in this circumstance?

@SebastienPeillet
Copy link
Contributor Author

SebastienPeillet commented Feb 3, 2020

Ok, I added a new test in compoundCurve function of testqgsgeometry.cpp. It verifies that a vertex can be added to a Linestring in a compoundCurve geometry.

It's already tested that no extra vertex can be added to a CircularString (line 10595):

  //insert vertex at end
  QVERIFY( !c24.insertVertex( QgsVertexId( 0, 0, 9 ), QgsPoint( 31.0, 32.0 ) ) );

@SebastienPeillet SebastienPeillet force-pushed the fix_compoundCurve branch 2 times, most recently from a1cb8fc to f541fa3 Compare February 3, 2020 17:09
@nyalldawson nyalldawson merged commit 859f741 into qgis:master Feb 4, 2020
@nyalldawson
Copy link
Collaborator

Thanks @SebastienPeillet !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants