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

don't treat all geometry collections as points/lines when noding #60694

Merged
merged 1 commit into from
Feb 21, 2025

Conversation

uclaros
Copy link
Contributor

@uclaros uclaros commented Feb 20, 2025

Description

When a GEOMETRYCOLLECTION is passed into QgsGeos::nodeGeometries() it was treated as a point or line geometry, potentially causing a crash later when splitting.
Now it will trigger a geos exception with an error message IllegalArgumentException: Operation not supported by GeometryCollection\n which is caught.
We should probably not allow splitting of geometry collections in the first place and inform the users about their messed up data, but at first let's avoid crashing.

Fixes #58379

@github-actions github-actions bot added this to the 3.42.0 milestone Feb 20, 2025
@nyalldawson
Copy link
Collaborator

Wouldn't it be better to iterate over the collection parts and node each in turn? The result should be usable -- if it's a valid geometry then none of the parts will overlap and potentially affect the noding solution.

Copy link

github-actions bot commented Feb 20, 2025

🪟 Windows builds

Download Windows builds of this PR for testing.
Debug symbols for this build are available here.
(Built from commit a3391fa)

🪟 Windows Qt6 builds

Download Windows Qt6 builds of this PR for testing.
(Built from commit a3391fa)

@uclaros
Copy link
Contributor Author

uclaros commented Feb 20, 2025

Wouldn't it be better to iterate over the collection parts and node each in turn? The result should be usable -- if it's a valid geometry then none of the parts will overlap and potentially affect the noding solution.

For that we would need to first make sure it is a homogeneous collection, ie not mixing lines and polygons, however in that case it would have been a multipolygon/multilinestring instead.
Properly splitting a mixed collection of lines and polygons would require non trivial changes to the splitting logic and will have several edge cases to handle, like what happens if line splitting and polygon splitting have different operation results?

@nyalldawson
Copy link
Collaborator

@uclaros ah sorry, I'd mistaken this method for one which just returns the GEOS noded geometry alone -- but that's QgsGeos::node. Please ignore!

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

Successfully merging this pull request may close these issues.

QGIS crashing when splitting features of certain countries after polygons have been added to them.
2 participants