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

is_valid for all geometry types. #1130

Open
andriyDev opened this issue Dec 29, 2023 · 7 comments
Open

is_valid for all geometry types. #1130

andriyDev opened this issue Dec 29, 2023 · 7 comments

Comments

@andriyDev
Copy link
Contributor

andriyDev commented Dec 29, 2023

As stated in the description of the geometry types (e.g., Polygon, LineString), validity is not enforced, but it is expected. While the validity requirements are clearly documented, there is no "canonical" way to check if your geometry adheres to the validity requirements.

An example of when this would be desirable is loading polygons from a file. There is no guarantee that the file contained valid geometry, but once we check this, the algorithms should produce valid polygons.

Therefore, if checking validity is expensive, that is ok, since this should be a one-time cost.

Ideally this is supported by each geometry type separately, and also on Geometry.

related: #123

@andriyDev
Copy link
Contributor Author

For LineString, its requirements are either empty, or 2+ coordinates. This is trivial to check. In addition, if the LineString is closed, it must also not self-intersect. We can check this by using Intersections (inserting all the edges in the line string) and checking that all intersections are vertices of the original line string (it's not clear to me if a line string is allowed to use the same vertex twice, but if it's not then I think we can just check if there are any intersections).

@mthh
Copy link
Member

mthh commented Dec 29, 2023

Some time ago I tried to implement various validity checks for geo-types geometries : https://github.com/mthh/geo-validity-check

I think it implements most of the validity checks of GEOS as well as some other checks.

The non-validity diagnostic messages could greatly be improved, and there is no functionality to attempt to repair the geometries.
Anyway, if that would be useful, feel free to reuse code from this crate to implement an is_valid function directly in geo.

@dabreegster
Copy link
Contributor

Somewhat related to #1120

@michaelkirk
Copy link
Member

I'm currently studying the JTS IsValid implementation and considering porting it.

It's quite a bit of code, but includes some machinery which would also be useful for buffering (maybe some day)!

We already have monotone sweep functionality - but I'm having a hard time squaring it's API with the functionality needed by JTS's IsValid.

@michaelkirk
Copy link
Member

Some time ago I tried to implement various validity checks for geo-types geometries : https://github.com/mthh/geo-validity-check

I haven't looked over the implementation very closely, but I wired it up to the JTS test suite for their IsValid method. They're mostly passing!

The failures are:

5 failures / 750 successes in JTS test suite:
failed TestValid2.xml case "Test 740" with error: expected true, actual: false
failed TestValid2.xml case "Test 749" with error: expected true, actual: false
failed TestValid2.xml case "Test 752" with error: expected true, actual: false
failed TestValid2.xml case "Test 753" with error: expected true, actual: false
failed TestValid2.xml case "Test 754" with error: expected true, actual: false

Interestingly, it seems like all the failures have to do with considering empty geometries (and empty rings) as invalid, while JTS considers them valid, which might be a relatively simple to fix.

@michaelkirk
Copy link
Member

Starting from @mthh's code, I was able to easily work through the few failing JTS tests.

I'm still making some changes before opening a PR, but it's looking pretty promising. I'm not sure performance wise how it will compare with the more battle hardened JTS implementation, but so long as it's correct, I'm happy to use it as a start.

@michaelkirk
Copy link
Member

Here's my WIP: https://github.com/georust/geo/compare/mkirk/mthh_is_valid_2?expand=1

Note it's not currently compiling - I broke it recently while DRY'ing up the implementation, but I think I'll be able to work through it in the next day or so.

To see an older version, where the build is working and the tests are passing you can check out ea171e8

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

No branches or pull requests

4 participants