fix split operation from creating 1-node ways when 2+ nodes selected

fixes #10997

Also change the split operation to only split the ways which contain all selected nodes (when thare are more than one node selected). This is more likely what the person performing the splitting intends to do
This commit is contained in:
Martin Raifer
2025-04-28 12:05:44 +02:00
parent e0d6a0a116
commit 4dc4e7dbdc
2 changed files with 60 additions and 57 deletions
+3
View File
@@ -43,6 +43,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
#### :scissors: Operations
* Preserve the sum of certain tags (`step_count`, `parking:*:capacity`) during _join_ operation ([#10492], thanks [@ChaitanyaKadu03])
* Preserve total value of `parking:*:capacity` tags during _split_ operation by distributing it proportionally to the resulting ways ([#10492])
* A _split_ operation will now only split the ways that contain all selected nodes when more than one node is selected ([#10997])
#### :camera: Street-Level
* Keep photo viewer open when disabling Panoramax overlay ([#10966])
* Don't de-select map feature when clicking on a street level photo ([#10959])
@@ -52,6 +53,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
* Fix removed tooltips from re-appearing when using keyboard navigation ([#9873])
* Only consider feature with proper lifecycle tags in "past/futures" layer ([#10943])
* Fix zoom level from resetting to the starting value when switching background imagery layer during the zoom transition
* Fix invalid single-noded ways from being created by a _split_ operation under certain conditions when multiple nodes are selected ([#10997])
#### :earth_asia: Localization
#### :hourglass: Performance
#### :mortar_board: Walkthrough / Help
@@ -67,6 +69,7 @@ _Breaking developer changes, which may affect downstream projects or sites that
[#10946]: https://github.com/openstreetmap/iD/issues/10946
[#10959]: https://github.com/openstreetmap/iD/issues/10959
[#10966]: https://github.com/openstreetmap/iD/issues/10966
[#10997]: https://github.com/openstreetmap/iD/issues/10997
[@ChaitanyaKadu03]: https://github.com/ChaitanyaKadu03
+57 -57
View File
@@ -1,7 +1,7 @@
import { geoSphericalDistance } from '../geo/geo';
import { osmRelation } from '../osm/relation';
import { osmWay } from '../osm/way';
import { utilArrayIntersection, utilWrap, utilArrayUniq } from '../util';
import { utilArrayIntersection, utilWrap } from '../util';
import { osmSummableTags } from '../osm/tags';
@@ -394,14 +394,14 @@ export function actionSplit(nodeIds, newWayIds) {
return graph;
}
var action = function(graph) {
const action = function(graph) {
_createdWayIDs = [];
var newWayIndex = 0;
for (var i = 0; i < nodeIds.length; i++) {
var nodeId = nodeIds[i];
var candidates = action.waysForNode(nodeId, graph);
for (var j = 0; j < candidates.length; j++) {
graph = split(graph, nodeId, candidates[j], newWayIds && newWayIds[newWayIndex], nodeIds.slice(j + 1));
let newWayIndex = 0;
for (const i in nodeIds) {
const nodeId = nodeIds[i];
const candidates = waysForNodes(nodeIds.slice(i), graph);
for (const candidate of candidates) {
graph = split(graph, nodeId, candidate, newWayIds && newWayIds[newWayIndex], nodeIds.slice(i + 1));
newWayIndex += 1;
}
}
@@ -412,81 +412,81 @@ export function actionSplit(nodeIds, newWayIds) {
return _createdWayIDs;
};
action.waysForNode = function(nodeId, graph) {
var node = graph.entity(nodeId);
var splittableParents = graph.parentWays(node).filter(isSplittable);
function waysForNodes(nodeIds, graph) {
const splittableWays = nodeIds
.map(nodeId => waysForNode(nodeId, graph))
.reduce((cur, acc) => utilArrayIntersection(cur, acc));
if (!_wayIDs) {
// If the ways to split aren't specified, only split the lines.
// If there are no lines to split, split the areas.
var hasLine = splittableParents.some(function(parent) {
return parent.geometry(graph) === 'line';
});
const hasLine = splittableWays.some(way => way.geometry(graph) === 'line');
if (hasLine) {
return splittableParents.filter(function(parent) {
return parent.geometry(graph) === 'line';
});
return splittableWays.filter(way => way.geometry(graph) === 'line');
}
}
return splittableParents;
function isSplittable(parent) {
return splittableWays;
}
function waysForNode(nodeId, graph) {
const node = graph.entity(nodeId);
return graph.parentWays(node).filter(isSplittable);
function isSplittable(way) {
// If the ways to split are specified, ignore everything else.
if (_wayIDs && _wayIDs.indexOf(parent.id) === -1) return false;
if (_wayIDs && _wayIDs.indexOf(way.id) === -1) return false;
// We can fake splitting closed ways at their endpoints...
if (parent.isClosed()) return true;
if (way.isClosed()) return true;
// otherwise, we can't split nodes at their endpoints.
for (var i = 1; i < parent.nodes.length - 1; i++) {
if (parent.nodes[i] === nodeId) return true;
for (let i = 1; i < way.nodes.length - 1; i++) {
if (way.nodes[i] === nodeId) return true;
}
return false;
}
};
action.ways = function(graph) {
return utilArrayUniq([].concat.apply([], nodeIds.map(function(nodeId) {
return action.waysForNode(nodeId, graph);
})));
return waysForNodes(nodeIds, graph);
};
action.disabled = function(graph) {
for (const nodeId of nodeIds) {
const candidates = action.waysForNode(nodeId, graph);
if (candidates.length === 0 || (_wayIDs && _wayIDs.length !== candidates.length)) {
return 'not_eligible';
}
for (const way of candidates) {
const parentRelations = graph.parentRelations(way);
for (const parentRelation of parentRelations) {
if (parentRelation.hasFromViaTo()) {
// turn restrictions: via members must be loaded
const vias = [
...parentRelation.membersByRole('via'),
...parentRelation.membersByRole('intersection'),
];
if (!vias.every(via => graph.hasEntity(via.id))) {
return 'parent_incomplete';
}
} else {
// other relations (e.g. route relations): at least one members before or after way must be present
for (let i = 0; i < parentRelation.members.length; i++) {
if (parentRelation.members[i].id === way.id) {
const memberBeforePresent = i > 0 && graph.hasEntity(parentRelation.members[i - 1].id);
const memberAfterPresent = i < parentRelation.members.length - 1 && graph.hasEntity(parentRelation.members[i + 1].id);
if (!memberBeforePresent && !memberAfterPresent && parentRelation.members.length > 1) {
return 'parent_incomplete';
}
const candidates = waysForNodes(nodeIds, graph);
if (candidates.length === 0 || (_wayIDs && _wayIDs.length !== candidates.length)) {
return 'not_eligible';
}
for (const way of candidates) {
const parentRelations = graph.parentRelations(way);
for (const parentRelation of parentRelations) {
if (parentRelation.hasFromViaTo()) {
// turn restrictions: via members must be loaded
const vias = [
...parentRelation.membersByRole('via'),
...parentRelation.membersByRole('intersection'),
];
if (!vias.every(via => graph.hasEntity(via.id))) {
return 'parent_incomplete';
}
} else {
// other relations (e.g. route relations): at least one members before or after way must be present
for (let i = 0; i < parentRelation.members.length; i++) {
if (parentRelation.members[i].id === way.id) {
const memberBeforePresent = i > 0 && graph.hasEntity(parentRelation.members[i - 1].id);
const memberAfterPresent = i < parentRelation.members.length - 1 && graph.hasEntity(parentRelation.members[i + 1].id);
if (!memberBeforePresent && !memberAfterPresent && parentRelation.members.length > 1) {
return 'parent_incomplete';
}
}
}
const relTypesExceptions = ['junction', 'enforcement']; // some relation types should not prehibit a member from being split
if (circularJunctions.includes(way.tags.junction) && way.isClosed() && !relTypesExceptions.includes(parentRelation.tags.type)) {
return 'simple_roundabout';
}
}
const relTypesExceptions = ['junction', 'enforcement']; // some relation types should not prehibit a member from being split
if (circularJunctions.includes(way.tags.junction) &&
way.isClosed() &&
!relTypesExceptions.includes(parentRelation.tags.type)) {
return 'simple_roundabout';
}
}
}