Z-interpolation by default regression

[bjornharrtell]

When OverlayNG was introduced in 1.18, missing Z-values became interpolated both in original Overlay and OverlayNG (and potentially elsewhere).

Past logic could rely on missing Z-values to be able to supply those values by other means as post processing so it can be seen as a breaking change for the original Overlay and potentially an undesirable feature of OverlayNG.

[dr-jts]

Past logic could rely on missing Z-values to be able to supply those values by other means

Is this theoretical, or is there actually code broken because of this change?

[bjornharrtell]

We have a large suite of custom / data specific geometry validation rules that breaks because of this.

[FObermaier]

I wondered why ElevationModel doesn't implement some interface that defines the contract for interpolating/getting z-ordinate values. Then it would be possible to implement and use some DEM instead of ElevationModel or create an implementation that behaves like in pre v1.18 versions

[bjornharrtell]

@FObermaier the ElevationModel is only part of the problem and only applies to OverlayNG. The interpolation that affects much more is in RobustLineIntersector. I agree it could make sense to make it possible to customise ElevationModel for OverlayNG but that's another issue.

[FObermaier]

But RobustLineIntersector could rely on ElevationModel or whatever that interface were to be named.

[bjornharrtell]

@FObermaier agree that could make sense but requires a more involved effort and is perhaps not the way to tackle the regression.

[dr-jts]

We have a large suite of custom / data specific geometry validation rules that breaks because of this.

Can you provide more detail about what semantics these tests are checking? E.g. Do they use the absence of Z as a way of determining vertices which have been added?

I am curious about whether your use case involves your own custom way of computing Z, or you just don't want Z interpolated at all.

[dr-jts]

The solutions for setting Z-interpolation on/off in order of increasing complexity seem to be:

1. a public static flag on RobustLineIntersector
2. a System property used by RobustLineIntersector
3. a public static flag on GeometryFactory
4. a JTSOptions singleton containing the flag
5. a field on GeometryFactory
6. a field flag on RobustLineIntersector, with options down through the call chain from functions which expose it for setting

The next level of complexity is to replace the flag with a Strategy object to provide custom Z interpolation, with built-ins for linear interpolation and no interpolation.

[bjornharrtell]

@dr-jts it's not entirely easy to summarize but I'll try. In our case we have -999 to signify a "missing" Z-value and leave it up to the user to supply the Z-value with three different methods (manual, custom interpolation or derived from height model) but in most/best cases the data is produced "externally" from stereographic images and as such have a measured correct height. We then have lots of different rules that checks objects (and suggested changes to objects) and relations between objects and for some checks we track overlay results as the "error". It's an important feature that the "errors" we produce are reasonably stable.

PS. In most cases we replace NaN with -999 late in the logic involved.

[bjornharrtell]

I note that NTS has since way back introduced NtsGeometryServices singleton which is probably the place for a global flag like this in that port, similar to the JTSOptions suggestion I guess. Currently in #715 I've made a class ZInterpolating that corresponds to the same thing. It probably makes sense to me to name it JTSOptions instead and place it higher in the hierarchy.

[dr-jts]

Time to get back to this after the summer break!

@bjornharrtell so I assume that you would like to see the ability to disable Z-ordinate calculation as a permanent part of the JTS Overlay API?

If so, I might spend some time thinking about how to provide this as an option to the Overlay API, rather than a "magic switch". This will require add this as a flag to RobustLineIntersector, of course, and thus require it being a parameter to all of the noders. So a fairly substantial change.

[bjornharrtell]

@dr-jts I could make an attempt to rework into an option for the overlay API, is that still your preferred choice on how to approach this?

[bjornharrtell]

@dr-jts I've done a spike on making it a flag to RobustLineIntersector. As you suspected it's a substantial change. I'm doubting if it's the right approach, considering what it will mean for downstream. What is the rationale of introducing optional zinterpolating at this fine grained level? I would think you either want it or not, i.e global level makes sense?