After squaring a building and uploading, the downloaded building is not square enough for the extrude tool to avoid creating unnecessary nodes

[bo.goiny@…]

i created another ticket before, wich was not specific enough. here is the link to the discussion there
https://josm.openstreetmap.de/ticket/22454

i also did some more test with some features i created near equator with the buildingstool. after up- and downloading again i used extrusion tool. the smaller the buildings are, i think the more frequently extrusion tool should fail because angles getting bigger due to the shift of nodes.

the non orthogonality of the smallest feature after up and download is even visible with the bare eye

extrusion fails already with bigger buildings at realistic building sizes.

[bo.goiny@…]

features with 11 digits for decimal portion directly saved from JOSM

[anonymous]

the same features up and downloaded from OSM Server with only 7 digits for decimal portion of coordinates

[bo.goiny@…]

use of extrusion tool fails on features because they are not square enough

[taylor.smock]

I've been debugging this, and it looks like it is due to (a) using EastNorth and (b) not correcting for ILatLon.MAX_SERVER_PRECISION. There is a third issue where the math to check for a zero angle does not account for cases where the first and third locations could be switched and the angle could be < 1e-5.

The fix I'm looking at for this is probably going to have to wait for #22453 (for the Geometry#segmentsParallel fix) to avoid accidentally redoing the same work multiple times.

Anyway, minimal patch for the zero angle check fix:

src/org/openstreetmap/josm/actions/mapmode/ExtrudeAction.java

@@ -38 +38 @@
 import org.openstreetmap.josm.data.UndoRedoHandler;
 import org.openstreetmap.josm.data.coor.EastNorth;
 import org.openstreetmap.josm.data.coor.ILatLon;
+import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.data.osm.DataIntegrityProblemException;
 import org.openstreetmap.josm.data.osm.DataSet;
+import org.openstreetmap.josm.data.osm.INode;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.Way;
@@ -651 +653 @@
         boolean nodeOverlapsSegment = prevNode != null && Geometry.segmentsParallel(initialN1en, prevNode.getEastNorth(), initialN1en, newN1en);
         // segmentAngleZero marks subset of nodeOverlapsSegment.
         // nodeOverlapsSegment is true if angle between segments is 0 or PI, segmentAngleZero only if angle is 0
-        boolean segmentAngleZero = prevNode != null && Math.abs(Geometry.getCornerAngle(prevNode.getEastNorth(), initialN1en, newN1en)) < 1e-5;
+        boolean segmentAngleZero = checkAngleIsZero(prevNode, initialN1en, newN1en);
         boolean hasOtherWays = hasNodeOtherWays(selectedSegment.getFirstNode(), selectedSegment.getWay());
         List<Node> changedNodes = new ArrayList<>();
         if (nodeOverlapsSegment && !alwaysCreateNodes && !hasOtherWays) {
@@ -681 +683 @@
         //find if the new points overlap existing segments (in case of 90 degree angles)
         Node nextNode = getNextNode(selectedSegment.getUpperIndex());
         nodeOverlapsSegment = nextNode != null && Geometry.segmentsParallel(initialN2en, nextNode.getEastNorth(), initialN2en, newN2en);
-        segmentAngleZero = nextNode != null && Math.abs(Geometry.getCornerAngle(nextNode.getEastNorth(), initialN2en, newN2en)) < 1e-5;
+        segmentAngleZero = checkAngleIsZero(nextNode, initialN2en, newN2en);
         hasOtherWays = hasNodeOtherWays(selectedSegment.getSecondNode(), selectedSegment.getWay());
 
         if (nodeOverlapsSegment && !alwaysCreateNodes && !hasOtherWays) {
@@ -722 +724 @@
         joinNodesIfCollapsed(changedNodes);
     }
 
+    /**
+     * Check if an angle is almost zero, taking into account server precision
+     * @param node The node to check
+     * @param segmentNode1 A node of the segment to check
+     * @param segmentNode2 A node of the segment to check
+     * @return {@code true} if it is likely that the angle is supposed to be zero.
+     */
+    private boolean checkAngleIsZero(INode node, EastNorth segmentNode1, EastNorth segmentNode2) {
+        if (node != null) {
+            // Convert into LatLon -- we need distance from the line in degrees to compare against server precision
+            // This bit should probably be in Geometry.
+            LatLon ll1 = ProjectionRegistry.getProjection().eastNorth2latlon(segmentNode1);
+            LatLon ll2 = ProjectionRegistry.getProjection().eastNorth2latlon(segmentNode2);
+            double x0 = node.lon(); double x1 = ll1.lon(); double x2 = ll2.lon();
+            double y0 = node.lat(); double y1 = ll1.lat(); double y2 = ll2.lat();
+            double distanceToNewSegment = Math.abs((x2 - x1) * (y1 - y0) - (x1 - x0) * (y2 - y1)) /
+                    Math.sqrt(Math.pow(x2 - x1, 2) + Math.pow(y2 - y1, 2));
+            double angle = Math.abs(Geometry.getCornerAngle(node.getEastNorth(), segmentNode1, segmentNode2));
+            // Account for cases where the first/third points should have been reversed
+            if (angle > Math.PI / 2) {
+                angle = Math.abs(angle - Math.PI);
+            }
+            // 1e-5 radians == 0.0057 degrees
+            // 1e-2 radians == 0.57 degrees
+            return angle < 1e-5 ||
+                    (angle < 1e-2 && distanceToNewSegment < ILatLon.MAX_SERVER_PRECISION);
+        }
+        return false;
+    }
+
     private void joinNodesIfCollapsed(List<Node> changedNodes) {
         if (!dualAlignActive || newN1en == null || newN2en == null) return;
         if (newN1en.distance(newN2en) > 1e-6) return;

[bo.goiny@…]

thank you very much for your answer!

is it possible to say at wich angle difference to 90° extrusion tool starts to fail?
sorry, i only know a little bit of python, im not 100% understanding the java code. do i understand it right, that extrude works savely for all angles +- 90.0057 degrees?

[taylor.smock]

It currently starts to fail at 0.0057 degrees. Rather unfortunately, the calculation for that angle (Geometry.getCornerAngle) treats the calculation as if the right side of the line formed by a, b, c (node.getEastNorth(), segmentNode1, segmentNode2) as the "interior" of the corner, and returns that. This is the correct behavior for the specification, but for the purposes of this calculation, I believe we want to have the smaller of the two angles.

Anyway, keep in mind that I'm reading the code for the extrude tool for the first time, so I'm having to figure out what it is supposed to do and why. I'm much more familiar with other parts of the code base.