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

Geometry checker into processing #59637

Merged
merged 11 commits into from
Jan 6, 2025

Conversation

Djedouas
Copy link
Member

@Djedouas Djedouas commented Nov 28, 2024

Description

Supersedes #55939

This Pull Request aims to integrate the geometry checker in the processing toolbox within the scope of QEP 236.

Each process will follow a consistent logic: one input layer and two distinct outputs.

For check processing outputs are

  • a layer of the same type as the input layer with erroneous geometries only
  • a point layer with the error locations and information (feature id, part and vertex number, etc.)

For fix processing outputs are

  • a fixed layer of the same type as the input layer with corrected features according the the chosen method
  • a point layer with the error locations and report about the fix (fixed or not, message about the processed feature)

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

A default tolerance parameter is set at 8 (for 1e-8) in the advanced settings of every processing.

Demo video

https://vimeo.com/991495790

Screenshots

1. Check errors

Check geometry: errors will be created as a point layer showing places where the angle is < min angle

Launch processing

image

Errors layer attribute table

image

Source and errors layer

image

2. Fix with appropriate processing

Launch the delete vertex processing With the errors layer (output from the previous check) as input

image

Source and fixed layer (neon red)

image

Report layer showing what was done

image

Remarks - feedbacks welcome

The new processes are grouped under new categories named "Check geometry" and "Fix geometry". Suggestions for improved naming are welcome. The aim is to unify verification and correction processes from topology and geometry_checker plugins within these categories.

The current displayName, "Check/FIX Geometry (algorithm name)", is provisional. Any suggestions for enhancing this 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.

Next

Other Geometry Checker processes will be added following this review, and we plan to include additional correction/manipulation processes.


Funded by QGIS (Grant OpenSource 2023) and Oslandia

Part of qgis/QGIS-Enhancement-Proposals#236

cc @lbartoletti

@github-actions github-actions bot added this to the 3.42.0 milestone Nov 28, 2024
@Djedouas Djedouas force-pushed the geometry_checker_into_processing branch from 54438c3 to 2bd6a42 Compare November 28, 2024 17:02
Copy link

github-actions bot commented Nov 28, 2024

🪟 Windows builds

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

🪟 Windows Qt6 builds

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

@Djedouas Djedouas force-pushed the geometry_checker_into_processing branch 3 times, most recently from 2f69603 to 07d1492 Compare December 5, 2024 16:33
@Djedouas Djedouas closed this Dec 5, 2024
@Djedouas Djedouas reopened this Dec 5, 2024
@Djedouas Djedouas force-pushed the geometry_checker_into_processing branch from 03f2a75 to 41d2499 Compare December 5, 2024 17:13
@Djedouas Djedouas force-pushed the geometry_checker_into_processing branch from 41d2499 to d16face Compare December 6, 2024 10:24
Djedouas and others added 10 commits December 9, 2024 16:16
Porting angle check from geometry checker to processings
Porting angle fix from geometry checker to processings
Porting area check from geometry checker to processings
Porting area fix from geometry checker to processing
Porting hole check from geometry checker to processings
Porting hole fix from geometry checker to processings
Porting missing vertex check from geometry checker to processings
Porting missing vertex fix from geometry checker to processings
@Djedouas Djedouas force-pushed the geometry_checker_into_processing branch from d16face to cc42df1 Compare December 9, 2024 15:19
@lbartoletti
Copy link
Member

Great job @Djedouas and well documented!

Copy link

github-actions bot commented Jan 4, 2025

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 github-actions bot added the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jan 4, 2025
@lbartoletti lbartoletti merged commit a11c18f into qgis:master Jan 6, 2025
30 checks passed
@lbartoletti
Copy link
Member

@Djedouas we go like this, it's tested and used locally. You can carry on, and if we need to improve parts of it, we can always do so.

@lbartoletti lbartoletti self-assigned this Jan 6, 2025
@github-actions github-actions bot removed the stale Uh oh! Seems this work is abandoned, and the PR is about to close. label Jan 6, 2025
@pigreco
Copy link
Contributor

pigreco commented Jan 7, 2025

@lbartoletti I think it would be great to have an advanced option to define the area of ​​the holes to check

@lbartoletti
Copy link
Member

@lbartoletti I think it would be great to have an advanced option to define the area of ​​the holes to check

Please, can you open a dedicated Issue/Feature request? Thanks

@pigreco
Copy link
Contributor

pigreco commented Jan 8, 2025

@lbartoletti

Please, can you open a dedicated Issue/Feature request? Thanks

#60079

@agiudiceandrea agiudiceandrea added Feature Processing Relating to QGIS Processing framework or individual Processing algorithms Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo. labels Jan 8, 2025
@qgis-bot
Copy link
Collaborator

qgis-bot commented Jan 8, 2025

@Djedouas
This pull request has been tagged as requiring documentation.

A documentation ticket will be opened at https://github.com/qgis/QGIS-Documentation when this PR is merged.

Please update the description (not the comments) with helpful description and screenshot to help the work from documentors.
Also, any commit having [needs-doc] or [Needs Documentation] in will see its message pushed to the issue, so please be as verbose as you can.

Thank you!

@qgis-bot
Copy link
Collaborator

qgis-bot commented Jan 8, 2025

@Djedouas
A documentation ticket has been opened at qgis/QGIS-Documentation#9530
It is your responsibility to visit this ticket and add as much detail as possible for the documentation team to correctly document this change.
Thank you!

@agiudiceandrea
Copy link
Contributor

@Djedouas, as well as adding the proper documentation in the docs or providing enough info for those who will do it, I would also suggest to extend and improve the shortHelp string of each processing algorithm being a little bit more descriptive and, e.g., clarify the meaning of the "Tolerance" advanced parameter.

@Djedouas
Copy link
Member Author

Djedouas commented Jan 8, 2025

@agiudiceandrea thanks for reviewing :)

I will create a PR in the coming weeks to add all the remaining checks and fixes in processings, so all feedback is welcome :)

I will in the same time add (or help) the documentation creation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Needs Documentation When merging a labeled PR, an issue will be created in the Doc repo. Processing Relating to QGIS Processing framework or individual Processing algorithms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants