Index: applications/editors/josm/plugins/FastDraw/src/org/openstreetmap/josm/plugins/fastdraw/DrawnPolyLine.java =================================================================== --- applications/editors/josm/plugins/FastDraw/src/org/openstreetmap/josm/plugins/fastdraw/DrawnPolyLine.java (revision 36031) +++ applications/editors/josm/plugins/FastDraw/src/org/openstreetmap/josm/plugins/fastdraw/DrawnPolyLine.java (revision 36056) @@ -13,15 +13,22 @@ import org.openstreetmap.josm.data.coor.LatLon; import org.openstreetmap.josm.gui.MapView; - +import org.openstreetmap.josm.tools.Logging; + +/** + * A holder for how the polyline has been drawn + */ public class DrawnPolyLine { MapView mv; - private LinkedList points = new LinkedList<>(); + private final LinkedList points = new LinkedList<>(); private LinkedList simplePoints = new LinkedList<>(); private Set used; - private Set fixed = new HashSet<>(); + private final Set fixed = new HashSet<>(); private int lastIdx; private boolean closedFlag; + /** + * Create a new object + */ public DrawnPolyLine() { clear(); @@ -57,5 +64,5 @@ boolean wasSimplified() { - return (simplePoints != null && simplePoints.size() > 0); + return (simplePoints != null && !simplePoints.isEmpty()); } @@ -85,5 +92,4 @@ void undo() { - //if (points.size() > 0) points.removeLast(); if (lastIdx > 0 && lastIdx < points.size()) { points.remove(lastIdx); @@ -114,7 +120,7 @@ } } else { - // insert point into midlle of the line + // insert point into middle of the line if (points.isEmpty() || !coor.equals(points.get(lastIdx))) { - points.add(lastIdx+1, coor); + points.add(lastIdx + 1, coor); lastIdx++; } @@ -137,14 +143,19 @@ /** * Increase epsilon to fit points count in maxPKM point per 1 km + * @param initEpsilon min point-to line distance in pixels (tolerance, initial) + * @param ekf The multiplier against the epsilon for the next round, until the max points per km is reached + * @param k see {@link #getNodesPerKm(int)} + * @param maxPKM The maximum nodes per km + * @return The final epsilon value used */ double autoSimplify(double initEpsilon, double ekf, int k, double maxPKM) { double e = initEpsilon; if (e < 1e-3) e = 1e-3; - if (ekf < 1+1e-2) ekf = 1.01; + if (ekf < 1 + 1e-2) ekf = 1.01; simplify(e); while (getNodesPerKm(k) > maxPKM && e < 1e3) { - e = e*ekf; + e = e * ekf; simplify(e); - //System.out.printf("eps=%f n=%d\n", e,simplePoints.size()); + Logging.trace("DrawnPolyLine: eps={0} n={1}", e, simplePoints.size()); } return e; @@ -153,7 +164,8 @@ /** * Simplified drawn line, not touching the nodes includes in "fixed" set. + * @param epsilon min point-to line distance in pixels (tolerance) */ void simplify(double epsilon) { - //System.out.println("Simplify polyline..."); + Logging.trace("DrawnPolyLine: Simplify polyline..."); int n = points.size(); if (n < 3) return; @@ -173,5 +185,4 @@ simplePoints.addAll(points); simplePoints.retainAll(used); - //Main.map.mapView.repaint(); used = null; } @@ -179,5 +190,5 @@ /** * Simplification of the line specified by "points" field. - * Remainin points are included to "used" set. + * Remaining points are included to "used" set. * @param start - starting index * @param end - ending index @@ -190,6 +201,6 @@ LatLon first = points.get(start); LatLon last = points.get(end); - Point firstp = getPoint(first); - Point lastp = getPoint(last); + Point firstP = getPoint(first); + Point lastP = getPoint(last); used.add(first); used.add(last); @@ -197,25 +208,28 @@ if (end - start < 2) return; - int farthest_node = -1; - double farthest_dist = 0; - - double d = 0; + int farthestNode = -1; + double farthestDist = 0; for (int i = start + 1; i < end; i++) { - d = pointLineDistance(getPoint(points.get(i)), firstp, lastp); - if (d > farthest_dist) { - farthest_dist = d; - farthest_node = i; - } - } - - if (farthest_dist > epsilon) { - douglasPeucker(start, farthest_node, epsilon, depth + 1); - douglasPeucker(farthest_node, end, epsilon, depth + 1); - } - } - - /** Modfified funclion from LakeWalker + double d = pointLineDistance(getPoint(points.get(i)), firstP, lastP); + if (d > farthestDist) { + farthestDist = d; + farthestNode = i; + } + } + + if (farthestDist > epsilon) { + douglasPeucker(start, farthestNode, epsilon, depth + 1); + douglasPeucker(farthestNode, end, epsilon, depth + 1); + } + } + + /** + * Modified function from LakeWalker * Gets distance from point p1 to line p2-p3 + * @param p1 The originating point + * @param p2 An end of the line + * @param p3 The other end of the line + * @return The distance from point p1 to the line p2-p3 */ public double pointLineDistance(Point p1, Point p2, Point p3) { @@ -240,7 +254,16 @@ void deleteNode(int idx) { - if (idx <= lastIdx) lastIdx--; + final boolean isClosed = isClosed(); + final LatLon point = points.get(idx); + if (isClosed) { + if (idx == 0) idx = getPointCount() - 1; + if (idx == getPointCount() - 1) closedFlag = false; + } + if (idx <= lastIdx && lastIdx != 0) lastIdx--; fixed.remove(points.get(idx)); points.remove(idx); + if (isClosed && points.size() == 1 && points.get(0) == point) { + this.deleteNode(0); + } } @@ -280,6 +303,9 @@ } - /** find starting point of the polyline line fragment close to p + /** + * find starting point of the polyline line fragment close to p * line fragment = segments between two fixed (green) nodes + * @param p The point to find the segment for + * @return the LatLon of the start of the segment */ LatLon findBigSegment(Point p) { @@ -288,5 +314,5 @@ Iterator it2 = points.listIterator(1); Point p1, p2; - LatLon pp1, pp2, start = null; + LatLon pp1, pp2, start; start = points.getFirst(); do { @@ -306,16 +332,18 @@ } - private double pointSegmentDistance(Point p, Point p1, Point p2) { - double a, b, x, y, l, kt, kn, dist; - x = p.x-p1.x; - y = p.y-p1.y; - a = p2.x-p1.x; - b = p2.y-p1.y; - l = Math.hypot(a, b); - if (l == 0) return Math.hypot(x, y); // p1 = p2 - kt = (x*a+y*b)/l; - kn = Math.abs((-x*b+y*a)/l); + private static double pointSegmentDistance(Point p, Point p1, Point p2) { + final int x = p.x - p1.x; + final int y = p.y - p1.y; + final int a = p2.x - p1.x; + final int b = p2.y - p1.y; + final double l = Math.hypot(a, b); + if (l == 0) { + return Math.hypot(x, y); // p1 = p2 + } + final double kt = (x * a + y * b)/l; + final double kn = Math.abs((-x * b + y * a)/l); + double dist; if (kt >= 0 && kt < l) dist = kn; else { - dist = Math.min(Math.hypot(x, y), Math.hypot(x-a, y-b)); + dist = Math.min(Math.hypot(x, y), Math.hypot((double) x - a, (double) y - b)); } return dist; @@ -360,4 +388,5 @@ * max((k-1) / (L(i,i+1)+L(i+1,i+2)+...+L(i+k-1,i+k))) [ i=1..n-k ] * @param k - window size (number of points to average points per km + * @return The maximum number of simplified line points divided by the segment length */ public double getNodesPerKm(int k) { @@ -369,5 +398,5 @@ if (k > n) k = n; - ILatLon pp1, pp2 = null; + ILatLon pp1, pp2; Iterator it1, it2; it1 = pts.listIterator(0); @@ -376,12 +405,10 @@ for (int i = 0; i < n-1; i++) { pp1 = it1.next(); - //p1 = getPoint(pp1); pp2 = it2.next(); - //p2 =sa getPoint(pp2); lens[i] = pp1.greatCircleDistance(pp2); } double pkm = 0, maxpkm = 0; double len = 0; - int seg = 0; // averaged segments counts + int seg; // averaged segments counts for (int i = 1; i < n; i++) { len += lens[i-1]; // add next next point @@ -395,5 +422,5 @@ // len is length of points[i-windowSize] .. points[i] if (len > 0) pkm = seg / len * 1000; - //System.out.println("i="+i+" pkm="+len+" pkm="+pkm); + Logging.trace("DrawnPolyLine: i={0} len={1} pkm={2}", i, len, pkm); if (pkm > maxpkm) maxpkm = pkm; } Index: applications/editors/josm/plugins/FastDraw/test/unit/org/openstreetmap/josm/plugins/fastdraw/DrawnPolyLineTest.java =================================================================== --- applications/editors/josm/plugins/FastDraw/test/unit/org/openstreetmap/josm/plugins/fastdraw/DrawnPolyLineTest.java (revision 36056) +++ applications/editors/josm/plugins/FastDraw/test/unit/org/openstreetmap/josm/plugins/fastdraw/DrawnPolyLineTest.java (revision 36056) @@ -0,0 +1,74 @@ +package org.openstreetmap.josm.plugins.fastdraw; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.openstreetmap.josm.data.coor.LatLon; +import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.gui.MainApplication; +import org.openstreetmap.josm.gui.MapView; +import org.openstreetmap.josm.gui.layer.OsmDataLayer; +import org.openstreetmap.josm.testutils.annotations.BasicPreferences; +import org.openstreetmap.josm.testutils.annotations.Main; +import org.openstreetmap.josm.testutils.annotations.Projection; + +/** + * Test class for {@link DrawnPolyLine} + */ +@BasicPreferences +@Main +@Projection +class DrawnPolyLineTest { + private DrawnPolyLine drawnPolyLine; + private MapView mv; + + @BeforeEach + void setup() { + this.drawnPolyLine = new DrawnPolyLine(); + // We need to add a new layer to get the MapView set up + MainApplication.getLayerManager().addLayer(new OsmDataLayer(new DataSet(), "DrawnPolyLineTest", null)); + this.mv = MainApplication.getMap().mapView; + this.drawnPolyLine.setMv(this.mv); + } + + @Test + void testDeleteNode() { + for (int i = 0; i < 10; i++) { + this.drawnPolyLine.addLast(new LatLon(i, i)); + } + assertEquals(10, this.drawnPolyLine.getPoints().size()); + this.drawnPolyLine.deleteNode(0); + assertEquals(9, this.drawnPolyLine.getPoints().size()); + assertEquals(1, this.drawnPolyLine.getPoints().get(0).lat(), 1e-9); + assertEquals(1, this.drawnPolyLine.getPoints().get(0).lon(), 1e-9); + } + + @Test + void testDeleteSegment() { + for (int i = 0; i < 10; i++) { + this.drawnPolyLine.addLast(new LatLon(i, i)); + } + this.drawnPolyLine.tryToDeleteSegment(mv.getPoint(new LatLon(1, 1))); + assertEquals(1, this.drawnPolyLine.getPoints().size()); + assertEquals(0, this.drawnPolyLine.getPoints().get(0).lat(), 1e-9); + assertEquals(0, this.drawnPolyLine.getPoints().get(0).lon(), 1e-9); + this.drawnPolyLine.deleteNode(0); + assertTrue(this.drawnPolyLine.getPoints().isEmpty()); + } + + /** + * Non-regression test for #22317: IOOBE in {@link DrawnPolyLine#getLastPoint()} + */ + @Test + void testNonRegression22317() { + for (int i = 0; i < 10; i++) { + this.drawnPolyLine.addLast(new LatLon(i, i)); + } + this.drawnPolyLine.tryToDeleteSegment(mv.getPoint(new LatLon(1, 1))); + this.drawnPolyLine.deleteNode(0); + assertDoesNotThrow(() -> this.drawnPolyLine.getLastPoint()); + } +}