'Straighten' on long ways can separate junction points

[skylerbunny]

In a case where you have a long way, and are on one end of it (but iD knows that way is straight 'enough' to allow 'straighten' as a choice), it's possible to straighten a way from one end, and have it break junction points on the other end (presumably where loading ways has fallen out of scope for the editor, so it doesn't know to carry the junction points along).

Example: Consider this way: http://www.openstreetmap.org/way/277201243 Then, zoom to the north end, and enter the iD editor. Straighten will be given as an option for the way. However, if you use it, as you scroll south, you'll eventually notice that junctions on crossing ways haven't been carried along, leaving the way orphaned.

This is sort of a data loss bug, in the sense that junctions and navigation can be unintentionally broken by a user who doesn't realize this can happen. This would especially be true in cases where one is dealing with a large city grid, and just wants to 'straighten the roads', again, not realizing that ways further out from the work area are being broken apart.

[jfirebaugh]

Great report, thank you!

What's happening is that the Straighten action has some logic for deleting nodes that are in a direct line with the nodes on either side. This logic omits nodes that are shared with other ways or relations, or have tags on them. But as you note, this fails when iD has only partially loaded a way, and cannot reliably determine that its nodes are not shared. In that case, it still tries to delete them, but sends a flag with the deletion request that says "just ignore this deletion if it turns out the node is still in use". The end result is that the node doesn't get deleted, but it does get removed from the straightened way.

It looks like this can also happen with "Make square" (the equivalent for areas).

The easy fix would be to remove the logic from these actions that deletes nodes. The hard fix would be to keep track of which nodes are potentially incomplete, and not delete them, or make additional requests to the OSM API in order to determine if they can be safely deleted. (This last one is probably not feasible because it would require the straighten action to work asynchronously.)

[skylerbunny]

Thanks!

Hmm. How about if, when you click a way and bring up the straighten option, if a check is made based on your zoom level and how far away the far end of the way is - and if it's merely possible intersecting ways wouldn't be loaded, just gray it out and do the 'You can't straighten this because not enough of the way is visible'? Like is done for say, the circular-ize function.

This way you don't necessarily have to care if affected nodes exist; just that they COULD, and be safe about it.

[bhousel]

Possibly related to this bug: https://lists.openstreetmap.org/pipermail/talk-us/2014-September/013585.html

Orthogonalize is already disabled on areas that exceed the viewport (bounding extent check), so it is probably safe from this.

How about if I restrict Straighten to operate on only the visible portion of the way (nodes on screen)? I'm a big fan of letting people change only what they can see.

[brycenesbitt]

This may be a live example:
http://www.openstreetmap.org/changeset/25206929#map=18/39.75833/-105.03600&layers=D
Where 29th Avenue became disconnected from all the side streets.

[bhousel]

I realized this morning that I can trigger this bug pretty easily just by straightening a way while the map is still loading. So, what I said before about viewport checking is not really correct, and we probably do need the "hard" fix of keeping track of the load status of all the entities and disabling the operations in some situations.

[bhousel]

Planning to use iD.restrictions to fix this too.
WIP on this: 8467433

[verdy-p]

Why not just straightening ways only on the part which is fully in the fully loaded area ?

This means that you could create one or two extra nodes where they cross that area, but still allow the fully loaded section between these nodes to be straightened.

The same logic could be used for orthogonalization (or circularization), to limit the scope of the action only to the viewport, not touching whay is not visible: it would be a convenient way to limit the action: we just need to zoom into the interesting area where we want to "regularize" the drawings of the selected object.

In my opinion it is always unsafe to edits geometries of objects that are not fully visible, but it is safe to do it on partial objects. As well it would be still very useful for giant objects (such as forests or rivers): having a bounding box or circle limiting the area where regularization is made is still very useful

[verdy-p]

Note: this logic works as long as the visible area is fully loaded. We need a visible indication for this, and as long as this is not the case, these regularization options should be disabled.

Note however that the server API frequently has issues (returns HTTP 500 errors due to timeout/resources) when querying parent objects of an object selected by id (this generally happens when there are several parent relations, as it seems that the server needlessly need to load all their children and fails with an "internal out of memory", when we are just interested in just getting the reference of parents, for exampe when selecting nodes on important highways where there are multiple bus routes including long distance ones, or admin boundaries for low admin_levels and multiple boundaries sharing this way).

Such server-side API issues will most often not occur if we query the API by area (that area can be extremely small, a few centimeters, around a single node in the selection): we still have the full list of parent objects (all parent relations for any object, plus the parent ways only for nodes) and the server replies immediately without failing !

[bhousel]

Why not just straightening ways only on the part which is fully in the fully loaded area ?

Yes, that's the idea for fixing this, but right now the only part of iD that knows what the "fully loaded area" is, is an internal data structure in the service code that fetches tiles from OSM, and we haven't exposed that datastructure publicly to other parts of iD. Once that's done we can restrict users editing to just the parts of features contained in fully loaded tiles.

[verdy-p]

You don't need that structure if you jsut limit the action to the viewport (and then add a few nodes where the objects are getting outside the viewport).

Users can then pan the map if needed to regularize other parts (including the few nodes that were just added and my be deleted by this new regularization).

One note: when regularizing, if some nodes have tags, they must be preserved and just moved to the nearest position on the regularized form. The regularization could as well merge ways that have identical tags (ignoring only tags known to be ignored such as "created_by") Otherwise they can be safely deleted if they are not part of any other object.

In all cases, before deleting object, or merging ways, it is essential to know they they are not referenced by other objects: it is where you need to track if they are fully loaded or not (but normally not needed at all to track if the full area is fully loaded, unless this creates posibly undesired intersections with objects that are still not loaded, but as these intersections have no nodes in common, it is simple to fix later and separately on these other objects, or to merge some common segments if needed or create real points for crossings)

Adding "fully loaded areas" adds severe constraints (including on the server by adding volumes, and adding editing delays for users, if the server is a bit longer than expected due to the workload). We can be selective to reduce the necessary workload on the server, but the need to have fully loaded areas (including in large zones that have high density of data) and is probably bad, when all that is needed is to have some all parent objects loaded in smaller areas where edits are being performed by users.

In addition, "fully loaded areas" adds memory constraints in browsers (notably those that don't support the "local storage" feature of HTML5, or that have very limited storage volumes such as tablets: these suers won't be able to work on these zones, except by zooming in and panning the map in order to work on smaller successive areas, keeping in local storage only the edited data but not necessarily all background tiles or referenced objects that were not edited: their browser could purge tags, or members, keeping only the minimum as undiscardable if they lack local storage and some data must be flushed, just like what JOSM does using "incomplete objects").

[bhousel]

You don't need that structure if you jsut limit the action to the viewport (and then add a few nodes where the objects are getting outside the viewport).

I thought this too originally, but turns out we do need the structure because the viewport is not guaranteed to be fully loaded. see #2248 (comment).

[verdy-p]

I thought this too originally, but turns out we do need the structure because the viewport is not guaranteed to be fully loaded. see #2248 (comment).

I did not say that: It's NOT needed to have the full view port loaded, only the dependant objects (parent relations for the selected way(s) before splitting/merging it. Loading the rest can be delayed.

However as I said, getting all parent relations from a way sometimes fails on the server: instead you can download a tiny area around one of the existing nodes of the way, probably the node that is the nearest where you want to split it, but certainly the node connecting two ways to merge. That query on the server extremely rarely fails (unless the query API is down of course).

And fully loading an area is not possible due to memory constraints or server constraints on data size: we need to be a bit more selective when working with ways that may be part of one or serveral complex relations with many members, even if this relation fully fits within the viewport.

For other objects nearby, as they are still not connected to the object we intend to modify, this won't break existing objects. If this ever creates superpositions with nearby objects, this won't happen on common nodes at it's easy to fix by zoomin in (where the viewport can easily have all the data loaded): at this point a validator process may signal those extra undesired intersections in a list of warnings (e.g. roads near a river: the intersection requires either a tunnel or bridge at distinct layers, or, for the same layer a ford or for vehicles/pedestrians or slipway for boats).

So drop that "fully loaded" guarantee from the requirements. It is needessly more restrictive than what is really needed to avoid breaking existing objects when merging/splitting ways/nodes or deleting/moving nodes by geometric simplifications or regularizations.

[slhh]

@bhousel

I thought this too originally, but turns out we do need the structure because the viewport is not guaranteed to be fully loaded. see #2248 (comment).

Then we seem to have a much more general problem, which is not limited to the straighten operation.
All operations seem to be checked wheather they need to be disabled when entering mode select. If the structure around the selected elements isn't completely loaded at the time of selecting the element, the relation protecting checks will likely fail. Right?

[bhousel]

Then we seem to have a much more general problem, which is not limited to the straighten operation.
All operations seem to be checked whether they need to be disabled when entering mode select. If the structure around the selected elements isn't completely loaded at the time of selecting the element, the relation protecting checks will likely fail. Right?

Kind of, but I'm not sure whether it would happen in practice... For example the "Disconnect" operation checks whether a vertex has multiple parent ways that belong to a shared relation. If the multiple parent ways have not all been loaded from OSM, then iD wouldn't think that Disconnecting is allowed at that vertex anyway.

[slhh]

Kind of, but I'm not sure whether it would happen in practice... For example the "Disconnect" operation checks whether a vertex has multiple parent ways that belong to a shared relation. If the multiple parent ways have not all been loaded from OSM, then iD wouldn't think that Disconnecting is allowed at that vertex anyway.

Could the ways be loaded, but not the relation?

Or lets assume we have 3 ways a,b, and c connected at the node, b and c are members of a relation. If way c has not been loaded from OSM, iD would likely think that "Disconnect" can be enabled to disconnect ways a and b, but the action would likely disconnect way c too, and break the relation.

We do also have open issues of breaking a relation by continuing a way (#2989) or deleting a node (#3117), the second one is much more general than it title does indicate. Btw, @bhousel, could you have a look at my comments there please?
Any logic to fix these issues would likely be impaired by unloaded elements.

Even simple operations like dragging a node should be disabled in case of a hidden connection. In case the hidden connection has not been loaded, such protection would be impaired.

I think we need to make sure that the selection including all connected ways and relations have been loaded, before the menu can be generated. For dragging of nodes we might need to wait until the full viewport has been loaded, before we can enable dragging.

[bhousel]

Could the ways be loaded, but not the relation?

I don't believe so - the API is supposed to return all relations that reference the ways.

Or lets assume we have 3 ways a,b, and c connected at the node, b and c are members of a relation. If way c has not been loaded from OSM, iD would likely think that "Disconnect" can be enabled to disconnect ways a and b, but the action would likely disconnect way c too, and break the relation.

Yes, it could happen.. Disconnect action would only disconnect the ways that it thinks are connected, so it would not consider c in this case. When it disconnects a and b, one of them might keep the original vertex, and one of them might get a new vertex. If a happens to get the new vertex, then b and c would probably stay connected and the relation would be ok. But if b happens to get the new vertex, c might stay connected to a and the relation with b and c would break.

We do also have open issues of breaking a relation by continuing a way (#2989) or deleting a node (#3117), the second one is much more general than it title does indicate. Btw, @bhousel, could you have a look at my comments there please?

Yes, I agree with your comments on those issues..

Even simple operations like dragging a node should be disabled in case of a hidden connection. In case the hidden connection has not been loaded, such protection would be impaired.

Yes, in order to enforce the "no edits to things that are connected to hidden features" rule, that's another reason to prevent edits in tiles not yet fully loaded from the API.

For dragging of nodes we might need to wait until the full viewport has been loaded, before we can enable dragging.

I think it's safe to drag nodes in tiles that are completely loaded (meaning we don't have to wait for the full viewport, just the tile containing the node).

[slhh]

To solve the more general issue we seem to have some options:

1. Keep track of nodes with a potentially incompletely parent list. When entering mode select, make sure that all selected elements are loaded, and that all nodes including nodes of selected ways have their complete parents. This would need several fine grained API calls. Most of the code of mode.enter needs to be postponed to the last callback of these API calls as I already did in Add a parent ways section to the inspector for vertexes, and improve vertex navigation #3631 (but limited to loading unloaded selected elements). In the worst case, when any selected ID is not loaded, we will have to wait for two consecutive stages of API calls because there isn't any suitable API call including parents of returned nodes, the map call can't be used due to unknown position.
2. Same as above, but load complete parents of node where required to decide on enabled operations only. This results in less API calls, but not in less stages of them, which have to be waited for before operations can be enabled. Execution of some critical actions (like straighten) has to be postponed until all tiles containing the selection have been loaded, these tiles might have to be loaded for rendering anyway.
3. Keep track of loaded tiles only. Prevent any operation on nodes which are out of these tiles. Especially we need to prevent any kind of moving a node, which has an incomplete parent list, into any loaded tile. Otherwise we can't assume that a node within a loaded tile has also all of its parents been loaded.
   On entering mode select we have to load unloaded selected nodes and ways (including their nodes) first, this is to know which tiles they are located in. Then we need to load all missing tiles containing any selected nodes or nodes of selected ways. Operations can be enabled on the last callback of these tile loads. This option needs the most data to be loaded, but likely the least number API calls, before operations can be enabled. In addition most of these calls are map calls, which might be more reliable due to the cgimap implementation.

For all of these options requiring two stages of API calls is a worst case scenario, the normal case would be zero or one stage to wait for.

Or lets assume we have 3 ways a,b, and c connected at the node, b and c are members of a relation. If way c has not been loaded from OSM, iD would likely think that "Disconnect" can be enabled to disconnect ways a and b, but the action would likely disconnect way c too, and break the relation.

Yes, it could happen.. Disconnect action would only disconnect the ways that it thinks are connected, so it would not consider c in this case. When it disconnects a and b, one of them might keep the original vertex, and one of them might get a new vertex. If a happens to get the new vertex, then b and c would probably stay connected and the relation would be ok. But if b happens to get the new vertex, c might stay connected to a and the relation with b and c would break.

I think there is also another failing case when c gets loaded before the action is performed.

[bhousel]

Update: this is working now in #6140
The operations will now check whether they should be disabled because they operate on nodes which are on unloaded tiles (to recap: these nodes might be connected to things iD doesn't know about yet).

If the operation is disabled for this reason, iD will schedule download of the needed tiles in the background.

In this screenshot, downloaded tiles are purple. iD actually downloaded the one that it needed - the offscreen tile north of this one - before I snapped the picture. But for consistency, the menu will still display the operation as disabled until the user clicks on that way again.

[verdy-p]

Note: straightening lines should not be done on overlong distances: segments should have enough points (at most 10 kilometers), as is it necessary to avoid rendering artefacts (notably for maps not using a cylindric projection) or very incorrect "smoothing" of angles along broadly curves: in JOSM it is easy to add such nodes on overlong straight lines and not only keep them aligned but also correctly distributed:

• 1st split the line containing the overlong segment if needed.
• select that segment : JOSM displays its length in km
• divide this length by 8 km, this gives a good estimate of the number of intermediate nodes that should be added
• add enough nodes anywhere along the line without caring about the alignment (do not add any tag to these intermediate nodes, instead tag the way or its two end nodes)
• select the segment again, press Shift+B to realign and distribute the intermediate nodes.
• join again that segment to the previous or next ways if you had to split that segment in the first step.

Note also that some nodes must NOT be deleted/moved: in some cases (notably international boundaries) they are survey points with legal coordinates and references. For such cases, it may be often best to separate segments (or curves) between legal nodes, so that these nodes are the first or last on in each way. This efficiently protects them from being "realigned" or moved as these nodes must be kept unchanged (long with their tags).

In any case, nodes that have tags like "ref" ("source" may be ignored but should be kept if the node is kept because of the "ref";), should never be moved or deleted by automatic straightening. Such moves or deletions should be done manually individually on each node.

If you "straighten" a way which has such intermediate nodes with important tags (like "ref"), limit the straightening only to separate sections of the way between these nodes that must be preserved.

[verdy-p]

Also I think that ways that have intermediate nodes shared by other ways intersecting with it, should have these nodes tagged explicitly (notably crossroads), especially if these ways are long (more than about 1km). If there are many intersections (e.g. with small access ways or paths), they should be split (but before splitting them, make sure you've loaded all relations referencing it).

The maximum distance to consider for long "straight" ways is caused by the fact that we can't load all data in common editors over a large area. And it is very unlikely that long straight ways are real (they can only be very rough approximations, except possibly for artificial admin boundaries, but here also you should add enough untagged intermediate nodes).

Note that JOSM signals you all overlong segments (more than 10 km) in its validator. Adding nodes to ways below that threshold will have almost no cost in the database (including for the many very straight roads in the United States, or arbitrary "cut" lines, in maritime boundaries or for large landareas like forests/crops/riverbanks that have complex shapes in multipolygons): these ways are really a very tiny minority of cases, even if you put the threshold to just 1 km: above this threshold, you've certainly forgotten many details (including speed limits and other restrictions or equipments, such as poles on electric lines, or derivations on railways).

Limiting the overall sizes of objects in OSM by splitting them also offers a good protection against big accidental damages (which are hard to repair on very large areas, where it is difficult to search the missing items that were accidentially dropped): fitting them in squares not larger that 1 km² should be favored.

[bhousel]

Thanks for your comments @verdy-p ...
iD has handled all of stuff you mentioned for many years. You should give it a try!

[verdy-p]

Well, if you think so... It's not been "so many years" and the fact it was once again correct a few hours ago contradicts what you say. Now we still need to test all these. I'm not convinced that "all of stuff" is accurate.