Improve how zero-length linestring are handled by relate

[mukoki]

Adding the point to the graph instead of doing nothing fix the problem with LineString.intersects in case of 0-length linestring.
LineString.touches with a 0-length geometry still does not return the same result as if it was a point, because touches consider the dimension of input geometry (1 instead of 0). Maybe the dimension returned by LineString should be 0 rather than 1 in this case.
I tested with
LineString#getDimension() {return getLength() == 0 ? 0 : 1}
but it breaks some tests asserting that empty linestring dimension must be 1.

[dr-jts]

Perhaps we could use a getDimension implementation which checks for all coordinates in a LineString being identical? Would that pass tests?

[mukoki]

I thing the first change (Adding the point to the graph) is worthwhile even if touches still does not return an intuitive result (may also be the case for other predicates which definition depends on the geometry dimension)
For touches, I tried a double test for linestring#getDimension() (return dimension=0 if not empty and length=0) and all tests pass and touches behave the same way as for points. I can test if all coordinates are identical instead if you think it is more efficient, but I wanted to be sure that we can safely change the dimension returned by a collapsed geometry. I'm not 100% sure that changing the returned dimension is safe even if it passes all tests, and it sounds a bit strange to me to return 0 for linestrings containing a single point and dimension 1 if it's completely empty.

[dr-jts]

Agreed that changing the semantics of getDimension is risky. Best not to do that.

A slightly less intrusive option would be to have relate compute an "effective dimension", and use that in the IM predicates.

It's still expensive to test for a polygon with effective dimension 1, however, so probably should not do that. And that means that intersects still produces an unexpected result for "flat" polygons.

[dr-jts]

There should be a BIG comment here indicating that this is an enhancement to standard OGC semantics, and is potentially a source of (as yet unknown) logic bugs.

[mukoki]

I will add a comment.
I've tried to change the returned dimension of LineString and Polygon when they collapse to a single point, but I've also checked the whole test suite and discover some side effects on getInteriorPoint which I'll try to fix.
About the semantic, I'm also wondering why empty geometry dimension don't always return Dimension.FALSE. One consequence is that in a GeometryCollection, where the highest dimension is used, a geometry made of, e.g. an empty LineString and a collapsed LineString will take the dimension of the empty LineString rather than the dimension of the collapsed one (0). I did not try to change that though as there are a few test checking explicitly that dimension of empty geometry is 0, 1 or 2.

[mukoki]

I had to go a little further to keep all tests positives. Beside the change in GeometryGraph, I changed

• the returned dimension of collapsed geometries (0 in case of linestrings or polygons made of a single point, but I did not change the returned dimension for empty geometries nor for flat 1-dimension polygons)
• interior point now use dimension rather than geometry type to return consistent results
• I changed the semantic of 2 tests in the tests suite : I think a point and a collapsed linestring located at the same coordinate should be considered topologically equal, and that the interior point of a GeometryCollection should be in the geometry of highest dimension in case of collapsed geometries

[dr-jts]

This is starting to feel like a major change to semantics.

I'm feeling uncomfortable about changing the dimension value based on geometry coordinates, because of both semantics changes and also cost of computation.

I'd prefer to see operation semantics changed where needed to be extended to handle collapsed geometry. I.e in the minimal case allow intersects to handle collapsed geometry. (As mentioned, a better algorithm will handle this fairly easily).

[mukoki]

I wanted to explore your proposition to change the dimension during operations instead of changing the semantic of getDimension, but in IntersectionMatrix, all public methods where dimension has an impact on the result take the dimension as a parameter like
IM.isTouches(dimA, dimB)
How will the user know the "actual" dimension if getDimension() returns only the "theoretical" dimension ?
Would it be useful to benchmark the changes ?
As geometries are almost immutable, maybe it would be cheaper to compute dimension only once and keep it in a cache.

[dr-jts]

One way to extend the existing predicates (which use full IM computation) is to provide a getEffectiveDimension method used only for predicate calculation. And perhaps any other algorithm which needs to be sensitive to geometry collapse.

Actually perhaps that's promising too much, since it can't realistically check for polygons collapsing to a line. So it could just check for collapse to a point, and perhaps be called isEffectivePoint. That would be very precise about what is being computed, and where it is being used.

[dr-jts]

Really this comes down to what the use cases are. I can see the utility of extending some of the predicates to handle all possible geometries. And enhancing centroid makes sense too. But doing this does not require a major change to semantics and existing test cases.

[dr-jts]

Is there a PostGIS issue which reported the limitation in ST_Intersects?

[mukoki]

About polygon collapsing : maybe we can handle the simple cases by traversing all segments of the outer ring from the starting point in cw and in ccw and check that all pairs of segments are equal and opposite. Cases where a point lies in the interior of another segment are not processed, but these case are subject to precision issues and it sounds reasonable to let them out.

[mukoki]

Where would you implement getEffectiveDimension so that it can be used by different algorithm like IM or interiorPoint ?
I understand that you hesitate to change the semantic of getDimension and it would be useful to have more use case or more feedback to decide.
Here are some points in favour of returning effectiveDimension in getDimension (just thinking aloud)

• getDimension would not be redundant with geometryType, it would give unique, useful information
• getDimension would not be consistent with the geometry type, but it would be consistant with the geometry itself, which would make it at least as consistent/intuitive as with the current definition
• the only cases where dimension would not be consistent with the type are cases of collapsed geometries which are invalid geometries
• for the only two test cases which were returning false after my change, I think that changing the unit test is more consistent than trying to conform to it.

And some points against

• it may add overhead in many operations like relate or interiorPoint
• it is difficult to say if we are more close or less close to the SFS and to other libraries by returning the effectiveDimension, and as JTS is a reference library, so that the status quo may be the best option
• it is difficult to measure the effect in all algorithms (even if results of the test suite is reassuring)

[mukoki]

I did not write an issue in the PostGIS list yet because I did not have the opportunity to test the very last version (I use PostGIS 2.4). I'll try to install it and test it ASAP.

[dr-jts]

Good summary of the issues around changing the semantics of getDimension, thanks. (Perhaps the title of this PR should be changed to reflect this).

[dr-jts]

Good question about where to implement getEffectiveDimension (or maybe: getTrueDimension, getTopoDimension). I'm reluctant to enlarge the Geometry class API, but that would be the obvious place for it. Alternatively it could be a static method on a new class in the geom package.

[dr-jts]

As for the proposed algorithm for detecting polygons collapsed to a line, is it really worth doing this for such a small subset of "linear" polygons? Is this really a use case?

This touches on a deeper issue about this whole proposal. It seems to me that it's not just extending the domain of (say) the intersects function, but also that a system can determine whether it can trust the result of calling that function on a geometry. At the moment the contract is that valid geometries will produce valid answers, and otherwise there are no guarantees. If intersects is extended to handle some (technically) invalid geometry, does that really help anything? The valid domain can be documented, of course, and perhaps that useful for important operations like intersects. But it's still most useful to have an automated check - otherwise it's risky to embed the operation deep inside an process.

This makes me even more reluctant to cope with this by changing getDimension, since it's a deeper question of overall semantics.

More understanding of use cases might help here.

[mukoki]

Polygons collapsed to a line : seems to me that the proposition only excludes the case where the endpoint of a segment lies in the middle of another segment. A case which is sensible to precision problems (ex. dimension 1 in a projection may become dimension 2 after reprojection). To handle a larger set of cases, I can only imagine something based on area or using some sort of tolerance. But perhaps I may have missed something.
Anyway. I have no use case for that. I only tried to extend the proposition to make it as consistent as possible. And I have no problem with excluding the case of flat polygons for now.

[dr-jts]

It seems like there would be many more cases where a collapsed polygon had segment endpoints which do not line up exactly. So my question is whether it's really worth trying to handle linear collapses given the cost of checking the general case.

Actually note that it's (theoretically) easy to test intersects for polygons no matter what kind of shape their linework is in. And it seems like a worthwhile goal to extend intersects to handle this. Really the issue is that the current intersects algorithm requires a hack to get it to work. But noted before, there are other ways of computing this which don't require that hack.

[mukoki]

If intersects is extended to handle some (technically) invalid geometry, does that really help anything?

• IMHO, what would help is to not return a counter-intuitive result (like line.intersects(buffer(line,10)) = false) , even with cases out of the library scope. My original proposition was to return either an exception or a consistent answer in this case.

This is the only use case I have for now. In this case just adding a point in the GeometryGraph seems to solve the problem. Maybe it is enough. The dimension question arise when we try to get consistent results for all predicates (predicates sensible to dimension) and for all kinds of collapsed geometries.

I agree that adding another method to Geometry is not very exciting. Adding a helper class/method to return trueDimension would probably be enough, but in both cases, my feeling is that having two kinds of dimension may make the library more difficult to understand and to use correctly.

[dr-jts]

There are many theoretically true geometric statements which are false when computed by JTS! This does seem like one of the easier ones to fix, so I suppose it doesn't hurt to implement it (although as you say it doesn't fix other predicates). My concern is how to document this and maintain the semantics going forward. A unit test will help of course, but it's still a bit obscure.

[dr-jts]

@mukoki are you still interested in this PR?

I would rather see a PR that addresses just the issue of handling zero-length lines in relate. There's a lot of other code in this one.