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

add test for add part to geometry less features on single geometry type layers #57261

Open
wants to merge 6 commits into
base: release-3_34
Choose a base branch
from

Conversation

3nids
Copy link
Member

@3nids 3nids commented Apr 26, 2024

fixes #57255

cherry-picked from #55371 and #55810

I'm a bit lost in here, would be great to have your eyes @lbartoletti

@Djedouas
Copy link
Member

Hi @3nids,

It seems that the problem comes from the step where the geometryless feature is created. It is created with an incorrect geometry type.

I found that when debugging at this level.

Below are some information on where I investigated, and misbehavior I detected.

Scenario 1: Line Curve layer - add a part with the segment digitizing tool

When the add-part map tool tries to add the geometry, the empty geometry is of type MultiLineString and the drawn part is of type CompoundCurve.

We fall here in the code and then the QgsMultiLineString::addGeometry(g) fails because the part can't be cast to a QgsLineString.

Scenario 2: Line Curve layer - add a part with the curve digitizing tool

Still same type for the empty feature (MultiLineString) and the drawn part (CompoundCurve).

But with the curve digitizing tool the drawn part hasCurvedSegments() is true, the line is segmentized and transformed into a LineString, which is ok to be added to a MultiLineString.

--> IMO this behavior, which results in a feature being added, is still wrong, because we are in a curve layer, which supports curves, and the good behavior should be to have a curve, not a segmentized linestring.

Scenario 3: Polygon Curve layer - add a part with the curve digitizing tool

The empty geometry is of type MultiPolygon and the drawn part is of type CompoundCurve, which can't be cast into a QgsPolygon and is not added.

Scenario 4: Polygon Curve layer - add a part with the segment digitizing tool

The empty geometry is of type MultiPolygon and the drawn part is of type LineString, which can be cast into a QgsPolygon and is added.

--> IMO this behavior is what we want.

My conclusion

I think that one must look into why the geometryless feature is added with the wrong geometry type according to the layer, and everything should fall into place.

Don't hesitate to ask if something is not clear 😉

@Djedouas
Copy link
Member

Djedouas commented May 20, 2024

Some more investigations lead me to this point

https://github.com/qgis/QGIS/blob/master/src/core/vector/qgsvectorlayereditutils.cpp#L295

calling

https://github.com/qgis/QGIS/blob/master/src/core/geometry/qgsgeometry.cpp#L980

where it is assumed that an empty geometry on which we want to add a part can't be curved. The logic must be changed.

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

3 participants