qgis / QGIS

QGIS is a free, open source, cross platform (lin/win/mac) geographical information system (GIS)
https://qgis.org
GNU General Public License v2.0
10.35k stars 2.98k forks source link

[processing] Auto geometry fixer #35183

Open roya0045 opened 4 years ago

roya0045 commented 4 years ago

Feature description. Sometimes when you try to process some data you get the invalid geometry, you never know which layer and you end up running fix geometry on the two inputs just to be safe.

Though this is a hassle.

We have an option to skip invalid geometry but you end up missing element.

The best option would be something that intercept the invalid geometry and make it valid by fixing it.

Though with the current codebase this is hard to do as the validation steps is far remote from the part that output the feature. Here is what I can think that might work:

uclaros commented 4 years ago

Keep in mind that fix geometries does not always provide the fix the user wants to apply to the data so it should not be applied automatically and silently. For example fixing a self intersection might generate a new feature, while the user would prefer to delete a couple of vertices, and that may cause unexpected results in the processing algorithm.

roya0045 commented 4 years ago

@uclaros I don't think there can be a magic solution to geometry errors, I simply feel that since fix geometry would be the function that is applied anyway to a geometry when the user receive this error, then doing it within the process is not that bad. And yes a message/popup would be beneficial. Its simply that to implement this efficiently its not that easy with the current flow.

uclaros commented 4 years ago

@roya0045 For me, Check validity is the next step when I receive this error on data that I want to have control over, not an automatic fix, so more info on the geometry error would me welcome!

The current setting Invalid features filtering has the following options:

Maybe there could be a fourth option to do as you propose like fix invalid geometries and retry.

roya0045 commented 4 years ago

@uclaros yes this is what I'm proposing.

nyalldawson commented 4 years ago

@roya0045 some rough notes on how you'd do this:

You'd then need to handle the new enum value in various places in processing. You'll get build warnings everywhere you need to handle that.

New tests would need to be added to TestQgsProcessing::features() covering the functionality.

One warning: makeValid is a lossy operation. You'll silently drop z and m values from the geometry by calling it, so you want to make sure all the new documentation states this warning in strong terms!

roya0045 commented 4 years ago

@nyalldawson Thanks Nyall, I had already noted this and done a part of what you suggested in my prototype. But I rapidly found out that there was no way to replace a feature from the iterator as they are not an output of any function. If features were provided on request there could be a way to replace it with a fixed one.

If I do anything I want to make sure that the original dataset stays intact. Hence why I was opting to replace a provided feature ( would need to make a feature handler that fetches the current feature from the iterator and fixes it) or fix the whole input and use the temporary result as the new input.

Thanks for telling me where the test is!