biigle / core

:large_blue_circle: Application core of BIIGLE
https://biigle.de
GNU General Public License v3.0
12 stars 16 forks source link

Reject/clean non-simple polygons #407

Closed mzur closed 7 months ago

mzur commented 2 years ago

It is possible to create nonsensical annotations, e.g. a polygon annotation with four identical vertices (by clicking four times on the same spot). The line string shape is probably also affected by this. Also check the other shapes.

Implement a client-side check that disallows annotations with all identical coordinates (except point, circle). Just remove such an annotation without sending it to the backend.

Edit: "pinch points" and "dangling lines" (see below) could be removed automatically before the annotation is created. The backend should reject these but the frontend could clean these up before sending. "bow-ties" are more problematic.

Summary:

GMDuncan commented 2 years ago

It is also possible within the user interface to create non-simple polygon features within the BIIGLE UI.

For example self-intersection "bow-tie" style self-intersecting polygons using the polygon tool: image

And the magic wand tool can also produce self-intersection in terms of polygon edges overlapping (creating the effect of a dangling line) and single vertices overlapping (producing a pinch point). image

These errors are usually avoided in geometric operations (certainly in GIS) as they can have impact on some analysis functions (e.g. area calculations can commonly be affected by "bowties"). I'm not sure if there

If possible, as a future extension could it be worthwhile wrapping this issue in a general functionality to check for non-simple geometries created during the annotation process?

mzur commented 2 years ago

In the case of your first example, I'd say that the users are responsible if they produce "incorrect" annotations. But you have a point with the magic wand tool (and possibly the polygon brush?). These tools should definitely avoid this issue. We should add a check that "cleans" the automatically generated polygons.

GMDuncan commented 2 years ago

Just as a note on relative impact here - in summary whilst this does affect area value calculations in BIIGLE, at a technical level, it's less likely to have a large scale real world impact.

As far as I can see (which is consistent with how this issue interacts with other geometry applications), with self intersecting polygons, one part of the self-intersecting polygon becomes an "negative-area" in effect that deducts its own area from the overall calculations. Generally it is the smaller area that is deducted, though I'm not sure if this would be consistently the case in BIIGLE.

In the case of the large bowtie in the top image in the initial comment, this has a large impact as both parts of the polygon are relatively similar in area and so when one is deducted from the other the "total" area is small.

However, in the real world, I'd expect such self-intersections to be relatively small (at the edges of the polygon as in the magic wand tool), and thus contribute a relatively low area to deduct from the much larger "correct" polygon. Thus impact on area calculations should hopefully be pretty low!

dlangenk commented 1 year ago

I also experienced some of these problems. In python this can be achieved using the shapely library. However, this is unfeasible to do. In JS we could try to use jsts (https://github.com/bjornharrtell/jsts), which does the same thing (I guess). There is also an example using OpenLayers (https://openlayers.org/en/latest/examples/jsts.html).

In shapely I implemented a heuristic: 1) check if polygon is valid 2) if not use .buffer(0) to make it possibly valid 3) if it is a multipolygon take the larger part (heurstically the one with more coordinates).

GMDuncan commented 1 year ago

JTS recently implemented a GeometryFixer class I believe (https://github.com/locationtech/jts/pull/704) , but it doesn't look like it has (yet) been ported into jsts.

mzur commented 1 year ago

There are also reported cases of identical subsequent coordinates (for line strings), possible because of double clicks. Maybe we can clean these up as well. I'll add a summary to the first post.

dlangenk commented 1 year ago

@mzur These would be automatically cleaned up by jsts if used.

mzur commented 1 year ago

The following error occurred on an annotation created with the magic wand when the polygon eraser tool should be applied to it: "Each LinearRing of a Polygon must have 4 or more Positions."

This is probably related to this issue. I'll classify this as a bug.

GMDuncan commented 1 year ago

The following error occurred on an annotation created with the magic wand when the polygon eraser tool should be applied to it: "Each LinearRing of a Polygon must have 4 or more Positions."

This is probably related to this issue. I'll classify this as a bug.

I'd assume the eraser tool somehow reduced the original annotation down to, in effect a line (3 vertex polygon)? Which would indeed create an invalid polygon.

Could be caused by something like erasing the red area on the image below, leaving the small remainder? image

(wild guess)