QGIS icon indicating copy to clipboard operation
QGIS copied to clipboard

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

Open 3nids opened this issue 10 months ago • 1 comments

fixes #57255

cherry-picked from #55371 and #55810

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

3nids avatar Apr 26 '24 15:04 3nids

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 avatar May 15 '24 15:05 Djedouas

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.

Djedouas avatar May 20 '24 16:05 Djedouas

follow-up: @Djedouas has solved this by implementing addPart with WkbTypes rather than GeometryType. For the sake of safety, it was decided to add a specific method for the LTR rather than backporting the whole V2 and bigger code changes.

other PR is #57636

3nids avatar Jun 03 '24 11:06 3nids