search

locationtech / jts

The JTS Topology Suite is a Java library for creating and manipulating vector geometry.
Other
1.99k stars 443 forks source link

Fix TriangleEdgesListVisitor #945

Closed SonarSonic closed 1 year ago

SonarSonic commented 1 year ago

Currently the TriangleEdgesListVisitor, simply adds the provided QuadEdge[] triEdges array too the list, however this array is always the same instance, as fetchTriangleToVisit just updates the same array each time, see below.

https://github.com/locationtech/jts/blob/1c0902d47f96f383be3723cfc2c5c24f60cbe0dc/modules/core/src/main/java/org/locationtech/jts/triangulate/quadedge/QuadEdgeSubdivision.java#L693

So currently a call to getTriangleEdges will provide a list of identical arrays, this change fixes that by creating a new instance of the array passed too the TriangleEdgesListVisitor so the getTriangleEdges method now performs as expected.

SonarSonic commented 1 year ago

@dr-jts Sorry for the duplicate Pull Request, it had the wrong email attached to the commit so I couldn't verify the ECA, this commit has the correct one.

dr-jts commented 1 year ago

One reason this bug still exists is that the QuadEdgeSubdivision.getTriangleEdges method is never used in JTS itself. I'm curious to know what your use case is for calling this?

QuadEdgeSubdivision really needs some dedicated unit tests, starting with this method (since it is not tested indirectly currently.

SonarSonic commented 1 year ago

It was a very specific use case, where I was linking the resulting edges of the Delaunay Triangulation to create one continuous line which visited all triangle edges at least once.