Ensure GeometryUtil.toJts() returns valid geometry #4668
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
These cover some tricky cases that have been tested manually so far. Test inputs are specified in full precision, but will be truncated according to the current precision model. The tests make sure that a specific topology is returned for each case.
Partly this is just a matter of picking a more reasonable precision. But we also now make `GeometryUtil.toJtsPolygons()` the authoritative source for geometry, with `GeometryUtil.toJts()` merely packaging that as a `MultiPolygon`. A defensive change to flatten any curved areas also guards against future usage that might run afoul of the current assumption that that input is only polygonal areas.
This avoids precision loss in VBL (and other shapes) when sent to other clients. This increase precision applies to `Mapper.map(Area)`, as well as `Mapper.map(AreaDto)`. The latter has also be simplified by directly building a `Path2D` instead of going through a customer `PathIterator` intermediary.
kwvanderlinde
force-pushed
the
bugfix/4506-precision-sensitivity
branch
from
1b57040 to
ae2d229
Compare
cwisniew
approved these changes
Feb 1, 2024
cwisniew
approved these changes
Feb 1, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Identify the Bug or Feature request
Improves on #4506
Description of the Change
With the recent changes to how we build
AreaTree, certain edge case geometries started being treated differently and would stop vision and pathfinding from working properly. These edge cases include ultra-precise artifacts that the user cannot see but are enough to confuse JTS about what counts as the interior of the VBL.This change attempts to avoid these problems by adding unit tests for
Geometry.toJtsPolygons()andGeometry.toJts()that make sure we get topologically correct results. The precision was adjusted to make sure all these cases result in geometry that agrees with what the user sees, making sure vision and pathfinding to behave as the user expects.Geometry.toJts()to now based onGeometry.toJtsPolygons()so that we can guarantee a validMultiPolygonis returned, saving the callers from having to check it themselves.Also included is a change to the DTO mapping for
Areaso that we keep full double precision instead of truncating to single precision. This ensures that clients each receive the same topology (and otherArea) as the original.Possible Drawbacks
I guarantee someone else's weird VBL will break now. But at least we can add any new cases to the test suite.
Documentation Notes
N/A
Release Notes
N/A
This change is