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

Polygonizer::getInvalidRingLines returns duplicate lines #947

Closed
dbaston opened this issue Jan 5, 2023 · 3 comments · Fixed by #949
Closed

Polygonizer::getInvalidRingLines returns duplicate lines #947

dbaston opened this issue Jan 5, 2023 · 3 comments · Fixed by #949

Comments

@dbaston
Copy link
Contributor

dbaston commented Jan 5, 2023

As reported in libgeos/geos#782

After polygonizing the following geometry, Polygonizer::getInvalidRingLines returns duplicates of the invalid ring.

MULTILINESTRING ((0 0, 1 0, 1 1, 0 1, 0 0), (0 0, 0.5 0.5), (1 1, 2 2, 1 2, 2 1, 1 1))

@dr-jts
Copy link
Contributor

dr-jts commented Jan 12, 2023

I have realized that the documented semantics for the Polygonizer.getInvalidRingLines are ambiguous. The method currently returns a set of closed LineStrings which are the linework for invalid rings found. However, the documentation says:

a collection of the input LineStrings which form invalid rings

In other words, this is documented as being consistent with the semantics of the other methods getting invalid input edges (i.e. returning the original input lines, rather than net new linear rings not present in the input).

Returning the original input edges makes uniquifying easier, but will change the return values. I'm not sure how critical this is for unit tests and usage.

@dbaston any thoughts on which is better?

@dbaston
Copy link
Contributor Author

dbaston commented Jan 12, 2023

I'm having trouble following the comment above, but I would expect the Polygonizer to be length-conserving: the total length of generated polygons, cut edges, dangles, and invalid ring lines should equal the length of the inputs.

I don't have any particular use for the non-polygon outputs, other than to check that they are absent. I just noticed this behavior when writing a GEOS test and found it surprising.

@dr-jts
Copy link
Contributor

dr-jts commented Jan 12, 2023

I would expect the Polygonizer to be length-conserving: the total length of generated polygons, cut edges, dangles, and invalid ring lines should equal the length of the inputs.

It's not possible to do this in general, because the generated polygons which are adjacent will unavoidably contain duplicate linework.

Perhaps the closest approach to this is for the linework for the various errors to contain only unique edges. In the case of invalid rings, this could mean that only the linework which does not also appear in other rings (valid or invalid) is included.

The downside to this is that it makes it harder to use simple tools to detect exactly where invalidities occur (which are generally where linework has self-intersectios). Returning full closed linear rings makes this easier. But not sure how useful this really is.

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

Successfully merging a pull request may close this issue.

2 participants