Weird GeometryFixer behavior

[sheinbergon]

So, first of all, let me begin by acknowledging all of your hard work and effort in maintaining this library. Amazing stuff :clap:

To the point, it seems GeometryFixer algorithm can sometimes return... unexpected results.

Here are two examples:

Input	PostGIS ST_MakeValid Result	Geometry Fixer Result (keepCollapsed=false)	Geometry Fixer Result (keepCollapsed=true)
POLYGON((0 0, 1 1, 1 2, 1 1, 0 0))	MULTILINESTRING((0 0,1 1),(1 1,1 2))	POLYGON EMPTY	LINESTRING (0 0, 1 1, 1 2, 1 1, 0 0)
POLYGON((0 0, 10 0, 10 10, 0 10, 0 0), (2 2, 2 8, 8 8, 8 2, 2 2), (3 3, 3 7, 7 7, 7 3, 3 3))	MULTIPOLYGON(((10 10,10 0,0 0,0 10,10 10),(8 8,2 8,2 2,8 2,8 8)),((7 7,7 3,3 3,3 7,7 7)))	POLYGON ((0 10, 10 10, 10 0, 0 0, 0 10), (2 2, 8 2, 8 8, 2 8, 2 2)	POLYGON ((0 10, 10 10, 10 0, 0 0, 0 10), (2 2, 8 2, 8 8, 2 8, 2 2)

In the first row, it decides to return an empty polygon instead of a line/multiline. In the second case, it completely discards one of the interior rings.

I'm willing to dive in and provide PRs, assuming you'll confirm these are in fact, bugs

[dr-jts]

Thanks for the kudos, and for the nicely detailed issue report.

The semantics of GeometryFixer are slightly different to the PostGIS ST_MakeValid. Both of these cases are working as designed.

GeometryFixer tries to preserve the topology of the input, but ensuring the output is valid. So in the first case of a completely collapsed polygon, POLYGON EMPTY is returned. The keepCollapsed option allows recovering the collapsed linework if required. A LINESTRING seems more appropriate than the MULTILINESTRING returned by ST_MakeValid.

In the second case, the assumption is that holes are voids in the containing polygon. Two nested holes still form a single void, as constructed. The ST_MakeValid result is an alternate interpretation of the linework, but perhaps more suited as the result of a polygonize operation.

[sheinbergon]

So, for the first case, you confirmed my assumption. Agreed.

For the second case, while I understand your claim, I do feel PostGIS interpretation makes more sense, as it maintains more of the original structure, while returning a valid outcome. I think that at least having the ability to opt in such a "keep nested holes" feature would be good to have.

WDYT?

[dr-jts]

I think that at least having the ability to opt in such a "keep nested holes" feature would be good to have.

Maybe. Have to look and see how to implement that behaviour.

Another difference with the PostGIS semantic is the behaviour for overlapping holes. For consistency that might need to be addressed as well.

GeometryFixer treats all holes as actual holes