Index: /trunk/src/org/openstreetmap/josm/actions/mapmode/ImproveWayAccuracyHelper.java =================================================================== --- /trunk/src/org/openstreetmap/josm/actions/mapmode/ImproveWayAccuracyHelper.java (revision 18962) +++ /trunk/src/org/openstreetmap/josm/actions/mapmode/ImproveWayAccuracyHelper.java (revision 18963) @@ -44,5 +44,5 @@ if (node != null) { - Optional candidate = node.referrers(Way.class).findFirst(); + Optional candidate = node.referrers(Way.class).filter(Way::isUsable).findFirst(); if (candidate.isPresent()) { return candidate.get(); @@ -70,11 +70,10 @@ EastNorth pEN = mv.getEastNorth(p.x, p.y); - Double bestDistance = Double.MAX_VALUE; - Double currentDistance; + double bestDistance = Double.MAX_VALUE; + double currentDistance; List> wpps = w.getNodePairs(false); Node result = null; - mainLoop: for (Node n : w.getNodes()) { EastNorth nEN = n.getEastNorth(); @@ -87,15 +86,5 @@ currentDistance = pEN.distance(nEN); - if (currentDistance < bestDistance) { - // Making sure this candidate is not behind any segment. - for (Pair wpp : wpps) { - if (!wpp.a.equals(n) - && !wpp.b.equals(n) - && Geometry.getSegmentSegmentIntersection( - wpp.a.getEastNorth(), wpp.b.getEastNorth(), - pEN, nEN) != null) { - continue mainLoop; - } - } + if (currentDistance < bestDistance && ensureCandidateIsNotBehindSegments(wpps, n, pEN, nEN)) { result = n; bestDistance = currentDistance; @@ -107,8 +96,31 @@ /** + * Check to see if a candidate node is underneath a way segment + * + * @param wpps The pairs of nodes to check for crossing way segments + * @param n The current node to check + * @param pEN The cursor east-north position + * @param nEN The node east-north position + * @return {@code true} if the candidate node is underneath a way segment + */ + private static boolean ensureCandidateIsNotBehindSegments(Iterable> wpps, Node n, EastNorth pEN, EastNorth nEN) { + // Making sure this candidate is not behind any segment. + for (Pair wpp : wpps) { + if (!wpp.a.equals(n) + && !wpp.b.equals(n) + && Geometry.getSegmentSegmentIntersection( + wpp.a.getEastNorth(), wpp.b.getEastNorth(), + pEN, nEN) != null) { + return false; + } + } + return true; + } + + /** * Returns the nearest way segment to cursor. The distance to segment ab is * the length of altitude from p to ab (say, c) or the minimum distance from * p to a or b if c is out of ab. - * + *

* The priority is given to segments where c is in ab. Otherwise, a segment * with the largest angle apb is chosen. @@ -126,8 +138,6 @@ EastNorth pEN = mv.getEastNorth(p.x, p.y); - Double currentDistance; - Double currentAngle; - Double bestDistance = Double.MAX_VALUE; - Double bestAngle = 0.0; + double bestDistance = Double.MAX_VALUE; + double bestAngle = 0.0; int candidate = -1; @@ -144,6 +154,7 @@ // Finding intersection of the segment with its altitude from p EastNorth altitudeIntersection = Geometry.closestPointToSegment(a, b, pEN); - currentDistance = pEN.distance(altitudeIntersection); + final double currentDistance = pEN.distance(altitudeIntersection); + final double currentAngle; if (!altitudeIntersection.equals(a) && !altitudeIntersection.equals(b)) { // If the segment intersects with the altitude from p, Index: /trunk/src/org/openstreetmap/josm/data/validation/ValidationTask.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/ValidationTask.java (revision 18962) +++ /trunk/src/org/openstreetmap/josm/data/validation/ValidationTask.java (revision 18963) @@ -112,4 +112,5 @@ } + protected ValidationTask(ProgressMonitor progressMonitor, Collection tests, Index: /trunk/src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java (revision 18962) +++ /trunk/src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java (revision 18963) @@ -354,4 +354,5 @@ return selection; } + static boolean isCoastline(OsmPrimitive w) { return w.hasTag("natural", "water", "coastline") || w.hasTag(LANDUSE, "reservoir"); Index: /trunk/test/unit/org/openstreetmap/josm/actions/mapmode/ImproveWayAccuracyActionTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/actions/mapmode/ImproveWayAccuracyActionTest.java (revision 18962) +++ /trunk/test/unit/org/openstreetmap/josm/actions/mapmode/ImproveWayAccuracyActionTest.java (revision 18963) @@ -2,23 +2,76 @@ package org.openstreetmap.josm.actions.mapmode; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.awt.Graphics2D; +import java.awt.Point; +import java.awt.event.MouseEvent; +import java.awt.image.BufferedImage; + import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; import org.openstreetmap.josm.TestUtils; import org.openstreetmap.josm.actions.mapmode.ImproveWayAccuracyAction.State; +import org.openstreetmap.josm.data.Bounds; +import org.openstreetmap.josm.data.coor.ILatLon; +import org.openstreetmap.josm.data.coor.LatLon; import org.openstreetmap.josm.data.osm.DataSet; +import org.openstreetmap.josm.data.osm.Node; +import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.gui.MainApplication; import org.openstreetmap.josm.gui.MapFrame; +import org.openstreetmap.josm.gui.MapView; import org.openstreetmap.josm.gui.layer.OsmDataLayer; +import org.openstreetmap.josm.gui.util.GuiHelper; import org.openstreetmap.josm.testutils.annotations.Main; import org.openstreetmap.josm.testutils.annotations.Projection; +import org.openstreetmap.josm.testutils.mockers.WindowlessMapViewStateMocker; +import org.openstreetmap.josm.testutils.mockers.WindowlessNavigatableComponentMocker; + +import mockit.Mock; /** * Unit tests for class {@link ImproveWayAccuracyAction}. */ -@Main @Projection class ImproveWayAccuracyActionTest { + @RegisterExtension + Main.MainExtension mainExtension = new Main.MainExtension().setMapViewMocker(SizedWindowlessMapViewStateMocker::new) + .setNavigableComponentMocker(SizedWindowlessNavigatableComponentMocker::new); + + private static final int WIDTH = 800; + private static final int HEIGHT = 600; + private static class SizedWindowlessMapViewStateMocker extends WindowlessMapViewStateMocker { + @Mock + public int getWidth() { + return WIDTH; + } + + @Mock + public int getHeight() { + return HEIGHT; + } + } + + private static class SizedWindowlessNavigatableComponentMocker extends WindowlessNavigatableComponentMocker { + @Mock + public int getWidth() { + return WIDTH; + } + + @Mock + public int getHeight() { + return HEIGHT; + } + } + + private static MouseEvent generateEvent(MapView mapView, ILatLon location) { + final Point p = mapView.getPoint(location); + return new MouseEvent(mapView, 0, 0, 0, p.x, p.y, p.x, p.y, 1, false, MouseEvent.BUTTON1); + } + /** * Unit test of {@link ImproveWayAccuracyAction#enterMode} and {@link ImproveWayAccuracyAction#exitMode}. @@ -45,5 +98,57 @@ @Test void testEnumState() { - TestUtils.superficialEnumCodeCoverage(State.class); + assertDoesNotThrow(() -> TestUtils.superficialEnumCodeCoverage(State.class)); + } + + @Test + void testNonRegression23444() { + final DataSet dataSet = new DataSet(); + final OsmDataLayer layer = new OsmDataLayer(dataSet, "ImproveWayAccuracyActionT", null); + MainApplication.getLayerManager().addLayer(layer); + final ImproveWayAccuracyAction mapMode = new ImproveWayAccuracyAction(); + final MapFrame map = MainApplication.getMap(); + assertTrue(map.selectMapMode(mapMode)); + assertEquals(mapMode, map.mapMode); + final Way testWay = TestUtils.newWay("", new Node(1, 1), new Node(2, 1), + new Node(3), new Node(4, 1), new Node(5, 1)); + testWay.firstNode().setCoor(new LatLon(0, 0)); + testWay.lastNode().setCoor(new LatLon(0.001, 0.001)); + testWay.getNode(1).setCoor(new LatLon(0.0001, 0.0001)); + testWay.getNode(3).setCoor(new LatLon(0.0009, 0.0009)); + dataSet.addPrimitiveRecursive(testWay); + assertFalse(testWay.getNode(2).isLatLonKnown(), "The second node should not have valid coordinates"); + final Graphics2D g2d = new BufferedImage(WIDTH, HEIGHT, BufferedImage.TYPE_INT_ARGB).createGraphics(); + g2d.setClip(0, 0, WIDTH, HEIGHT); + try { + // If this fails, something else is wrong + assertDoesNotThrow(() -> map.mapView.paint(g2d), "The mapview should be able to handle a null coordinate node"); + // Ensure that the test way is selected (and use the methods from the action to do so) + GuiHelper.runInEDTAndWaitWithException(() -> { + map.mapView.zoomTo(new Bounds(0, 0, 0.001, 0.001)); + // Get the target way selected (note: not selected in dataset -- use mapMode.mapReleased for that) + mapMode.mouseMoved(generateEvent(map.mapView, testWay.getNode(1))); + }); + // mouseMoved shouldn't cause the way to get selected + assertFalse(dataSet.getAllSelected().contains(testWay)); + + // Now check painting (where the problem should occur; the mapMode.paint call should be called as part of the map.mapView.paint call) + assertDoesNotThrow(() -> map.mapView.paint(g2d)); + assertDoesNotThrow(() -> mapMode.paint(g2d, map.mapView, new Bounds(0, 0, 0.001, 0.001))); + + // Finally, check painting during selection + GuiHelper.runInEDTAndWaitWithException(() -> { + // Set the way as selected + mapMode.mouseReleased(generateEvent(map.mapView, new LatLon(0.0001, 0.0001))); + // Set the mouse location (unset in mouseReleased call) + mapMode.mouseMoved(generateEvent(map.mapView, new LatLon(0.0001, 0.0001))); + }); + // The way shouldn't be selected, since it isn't usable for the improve way tool + assertFalse(dataSet.getAllSelected().contains(testWay)); + // Now check painting again (just in case) + assertDoesNotThrow(() -> map.paint(g2d)); + assertDoesNotThrow(() -> mapMode.paint(g2d, map.mapView, new Bounds(0, 0, 0.001, 0.001))); + } finally { + g2d.dispose(); + } } }