Opt-out Z interpolation #715
Conversation
|
Using a global option set via System property seems like the simplest way to allow a choice of Z-interpolation behaviour (although it's the least flexible). The unit test issues can be handled by either making Z-interpolation the default, or by changing the option setting in the unit test. The latter approach is already used in the |
dr-jts
changed the title
|
@dr-jts I've opted in the interpolation in the test cases where it is the assumption. |
bjornharrtell
changed the title
|
I've now reworked this so that the option to Z-interpolate is standalone and affects both RobustLineIntersector and ElevationModel. |
modules/core/src/main/java/org/locationtech/jts/geom/ZInterpolating.java
Show resolved
Hide resolved
| * @author bjornharrtell | ||
| * | ||
| */ | ||
| public class ZInterpolating { |
There was a problem hiding this comment.
This seems a lot of boilerplate to hold a boolean value decision, can this class take on more responsibility and swap between interpolation approaches as a stratagy object?
There was a problem hiding this comment.
I agree it feel like boilerplate. I have difficulties properly understanding/interpreting what you mean with "take on more responsibility and swap between interpolation approaches as a stratagy object". In either case it's intentionally done this way because it is modeled after the other existing system property option jts.overlay and broken out into a self contained class because it is affecting two very separate things (RobustLineIntersector and ElevationModel), and I wanted to do it in a way that is as non intrusive as possible on the existing source.
I suppose system wide settings could be done with smarter and nicer desig but I don't think this is the place to do such general improvement and restructuring.
bjornharrtell
force-pushed
the
issue-714
branch
from
b50694d
to
1fc16a8
Compare
|
Since Z interpolation has now been enabled for so long time it's no longer sensible to regard it as a regression. So I've changed it to be default on in this PR. This means this PR now do not change any behaviour and only allows the interpolation to be opt out. |
|
@jodygarnett and @dr-jts please consider accepting this. |
ref #714
Wanted to see what effect it would have to turn interpolation off by default and it causes a number of expected test failures (OverlayNGZTest, RobustLineIntersectorZTest and GeometryFixerTest).
If we agree that #714 is indeed a regression then I would have no better current idea than doing something like this, unfortunately.