[Processing] init first algorithms from geometry_checker

[lbartoletti]

Description

This Pull Request presents three new processing examples for integration within the scope of QEP 236.

The main goal is to adapt the initial algorithms from Geometry Checker to function within Processing. Each process will follow a consistent logic: one input layer and two distinct outputs for erroneous geometries and identified errors through "error->location". These outputs will be based on QgsGeometryCheckError fields, providing users with additional manipulation possibilities. Other Geometry Checker processes will be added following this review, and we plan to include additional correction/manipulation processes.

The aim is to adapt QGIS verification and correction tools (topology and geometry checker plugins) to make them compatible with Processing. Our foundation lies in some verification and data correction scripts we have encountered or developed during various projects.

To ensure a consistent user experience, each process will operate similarly:

• An input layer, with the ability to select a single layer for each algorithm (using batch mode for multiple layers if needed).
• A default tolerance parameter set at 8 (for 1e-8) in the advanced settings.
• Specific configurations for algorithms (QVariantMap configurationCheck).
• Outputs will display erroneous geometries and their associated points without modification, empowering users to decide on subsequent actions. While direct modifications to the layer could be considered (see native:truncate), we've chosen to separate these actions to allow users to make their own decisions. The idea of copying the input layer before applying changes to the input layer is also under consideration for future steps. Correction algorithms based on these outputs will be developed later to address various use cases. Moreover, even processes with a "No action" mode will be offered for in-place editing for consistency purposes.

Regarding the code:

• During development, I encountered several crashes with Processing, influencing certain parts of the code.
• It's essential to note that the algorithms are not thread-safe, hence the use of the NoThreading flag.
• To use QgsFeaturePool, it's necessary to convert QgsProcessingFeatureSource into a layer (where "materialize" proved helpful for this). It's also worth mentioning that the input layer cannot be a QgsProcessingParameterVectorLayer; cloning is necessary to avoid crashes when a layer is already loaded, especially with the editinplace option.
• I refrained from altering the geometry_checker code in this PR. However, QgsGeometry*Check::resolutionMethod is deprecated and not static, just like availableResolutionmethod. We require a QStringList here, thus a copy/paste from resolutionMethod() was performed. It would be beneficial to reconsider these methods/enums to avoid repetition.
• The new processes are grouped under a new category named "Check geometry". Suggestions for improved naming are welcome. The aim is to unify verification and correction processes from topology and geometry_checker plugins within this category.
• The current displayName, "Check Geometry (algorithm name)", is provisional. Any suggestions for enhancing this designation are welcome.
• To avoid overwhelming process options, I've opted to enforce the "gc_" prefix for field names (see outputFields). It's worth considering whether to make this parameter configurable.
• The creation of QgsGeometryCheckContext requires a QgsProject. Currently, I utilize the one from the input layer if available, otherwise QgsProject(). However, the optimal method for this is yet to be determined.

Lastly, this initial proposal introduces three algorithms to initiate the discussion. For ease of review, we'll open a PR for each process.

Funded by QGIS (Grant OpenSource 2023) and Oslandia

Part of qgis/QGIS-Enhancement-Proposals#236

cc @Djedouas @manisandro

[github-actions]

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit 6f6231c)

[nyalldawson]

Can we avoid accessing project here? It's likely a cause of the multithreading issues

[lbartoletti]

I've tried:

  std::unique_ptr<QgsGeometryCheckContext> checkContext = std::make_unique<QgsGeometryCheckContext>( mTolerance, mInputLayer->sourceCrs(), QgsCoordinateTransformContext(), nullptr );

But, I got these thread errors:

CRITICAL    Qt : setValid (/home/lbartoletti/QGIS_fix_polygon/src/core/qgsmaplayer.cpp:2518) is run from a different thread than the object lives in [0xd237d0 vs 0x20e6540]
CRITICAL    Qt : project (/home/lbartoletti/QGIS_fix_polygon/src/core/qgsmaplayer.cpp:2749) is run from a different thread than the object lives in [0xd237d0 vs 0x20e6540]

[nyalldawson]

Backing up a step, why do the geometry checker classes need access to a project at all?

[lbartoletti]

"Project can be used to resolve additional layers on algorithms". Since, it's the layerId that is passed and one has to retrieve the mapLayer from this ID.
AFAIK, it's used in Gap Algorithm Check. Maybe @m-kuhn has a better idea than me on how and where it's used?

[github-actions]

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

• that all unit tests are passing

• that all comments by reviewers have been addressed

• that there is enough information for reviewers, in particular

  • link to any issues which this pull request fixes

  • add a description of workflows which this pull request fixes

  • add screenshots if applicable

• that you have written unit tests where possible
  In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
  If there is no further activity on this pull request, it will be closed in a week.

[nyalldawson]

@lbartoletti

Sorry, it's taken a long time to look into this one! (Christmas break!)

I'm honestly a little confused by the approach here. I get the benefit of moving these tools to processing algorithms, but I'm unsure how this will permit removal of the interactive geometry checker tools. Is that in scope for this work?

(If not, then I'm concerned that we'll just end up with ANOTHER set of geometry checker tools which don't exactly overlap the functionality of the others 🤣 )

[lbartoletti]

@lbartoletti

Sorry, it's taken a long time to look into this one! (Christmas break!)

I'm honestly a little confused by the approach here. I get the benefit of moving these tools to processing algorithms, but I'm unsure how this will permit removal of the interactive geometry checker tools. Is that in scope for this work?

(If not, then I'm concerned that we'll just end up with ANOTHER set of geometry checker tools which don't exactly overlap the functionality of the others 🤣 )

@nyalldawson
Our primary objective here is to generate error reports from geometry and topology tools. To achieve this, we will use the improved/corrected/amended code from the geometry_checker, as you suggested in the QEP. The first step, therefore, is to present error feedback in tables, similar to the plugin.

Regarding correction, we are exploring several approaches that @Djedouas can detail better than I can. In this Pull Request, I attempted to propose an in-place solution, but it doesn't work well (if at all); a false good solution. Ultimately, we will opt for two combined approaches, addressing various identified cases:

• "Generic" tools to finely manage correction methods (e.g., removing a vertex, a hole based on parameters).
• "Integrated" tools that will approach the interactive logic of the plugin; however, we cannot mimic the plugin's interface as we are in the processing context, not the plugin.
  Unfortunately, we are only at the beginning of the work, in my humble opinion. In the end, we will cover the other tools and even more, but it will take time. Things will go faster once our first iteration has been tested.

For your information, I will let Jacky continue on this subject, but I will still be present, albeit more in the background.

[github-actions]

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

• that all unit tests are passing

• that all comments by reviewers have been addressed

• that there is enough information for reviewers, in particular

  • link to any issues which this pull request fixes

  • add a description of workflows which this pull request fixes

  • add screenshots if applicable

• that you have written unit tests where possible
  In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
  If there is no further activity on this pull request, it will be closed in a week.

[github-actions]

While we hate to see this happen, this PR has been automatically closed because it has not had any activity in the last 21 days. If this pull request should be reconsidered, please follow the guidelines in the previous comment and reopen this pull request. Or, if you have any further questions, just ask! We love to help, and if there's anything the QGIS project can do to help push this PR forward please let us know how we can assist.