preserve step_count/… while joining ways (#10926)

* preserve the sum of certain tags (`step_count`, `parking:*:capacity`) during _join_ operation
* preserve total value of `parking:*:capacity` tags during _split_ operation by distributing it proportionally to the resulting ways
* the abstract osm entity now accepts a list of tags to override during the merging, but otherwise is agnostic about how tags can be merged: the concrete merging resolution might depend on the concrete action that was performed
This commit is contained in:
Chaitanya Kadu
2025-04-17 17:43:53 +05:30
committed by GitHub
parent 666583d516
commit 552c6b6148
8 changed files with 116 additions and 21 deletions
+5 -1
View File
@@ -38,8 +38,10 @@ _Breaking developer changes, which may affect downstream projects or sites that
# unreleased (v2.34.0-dev)
#### :sparkles: Usability & Accessibility
* Show full relation label as hover-text in _membership editor_, disambiguate relations with duplicate labels by appending the relation id ([#10942])
* Show full relation label as hover-text in _membership editor_, disambiguate relations with duplicate labels by appending the relation id ([#10492])
#### :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])
#### :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])
@@ -57,11 +59,13 @@ _Breaking developer changes, which may affect downstream projects or sites that
[#9873]: https://github.com/openstreetmap/iD/issues/9873
[#10104]: https://github.com/openstreetmap/iD/issues/10104
[#10492]: https://github.com/openstreetmap/iD/issues/10492
[#10942]: https://github.com/openstreetmap/iD/pull/10942
[#10943]: https://github.com/openstreetmap/iD/pull/10943
[#10946]: https://github.com/openstreetmap/iD/issues/10946
[#10959]: https://github.com/openstreetmap/iD/issues/10959
[#10966]: https://github.com/openstreetmap/iD/issues/10966
[@ChaitanyaKadu03]: https://github.com/ChaitanyaKadu03
# v2.33.0
+15 -2
View File
@@ -1,6 +1,6 @@
import { actionDeleteRelation } from './delete_relation';
import { actionDeleteWay } from './delete_way';
import { osmIsInterestingTag } from '../osm/tags';
import { osmIsInterestingTag, osmSummableTags } from '../osm/tags';
import { osmJoinWays } from '../osm/multipolygon';
import { geoPathIntersections } from '../geo';
import { utilArrayGroupBy, utilArrayIdentical, utilArrayIntersection, utilOldestID } from '../util';
@@ -61,7 +61,12 @@ export function actionJoin(ids) {
graph = graph.replace(parent.replaceMember(way, survivor));
});
survivor = survivor.mergeTags(way.tags);
const summedTags = {};
for (const key in way.tags) {
if (!canSumTags(key, way.tags, survivor.tags)) continue;
summedTags[key] = (+way.tags[key] + +survivor.tags[key]).toString();
}
survivor = survivor.mergeTags(way.tags, summedTags);
graph = graph.replace(survivor);
graph = actionDeleteWay(way.id)(graph);
@@ -181,6 +186,8 @@ export function actionJoin(ids) {
for (var k in way.tags) {
if (!(k in tags)) {
tags[k] = way.tags[k];
} else if (canSumTags(k, tags, way.tags)) {
tags[k] = (+tags[k] + +way.tags[k]).toString();
} else if (tags[k] && osmIsInterestingTag(k) && tags[k] !== way.tags[k]) {
conflicting = true;
}
@@ -196,6 +203,12 @@ export function actionJoin(ids) {
}
};
function canSumTags(key, tagsA, tagsB) {
return osmSummableTags.has(key) &&
isFinite(tagsA[key] &&
isFinite(tagsB[key]));
}
return action;
}
+12 -12
View File
@@ -2,6 +2,7 @@ import { geoSphericalDistance } from '../geo/geo';
import { osmRelation } from '../osm/relation';
import { osmWay } from '../osm/way';
import { utilArrayIntersection, utilWrap, utilArrayUniq } from '../util';
import { osmSummableTags } from '../osm/tags';
// Split a way at the given node.
@@ -136,32 +137,31 @@ export function actionSplit(nodeIds, newWayIds) {
wayB = wayB.update({ nodes: nodesB });
}
if (wayA.tags.step_count) {
// divide up the the step count proportionally between the two ways
for (const key in wayA.tags) {
if (!osmSummableTags.has(key)) continue;
var stepCount = Number(wayA.tags.step_count);
if (stepCount &&
// divide up the the e.g. step count proportionally between the two ways
var count = Number(wayA.tags[key]);
if (count &&
// ensure a number
isFinite(stepCount) &&
isFinite(count) &&
// ensure positive
stepCount > 0 &&
count > 0 &&
// ensure integer
Math.round(stepCount) === stepCount) {
Math.round(count) === count) {
var tagsA = Object.assign({}, wayA.tags);
var tagsB = Object.assign({}, wayB.tags);
var ratioA = lengthA / (lengthA + lengthB);
var countA = Math.round(stepCount * ratioA);
tagsA.step_count = countA.toString();
tagsB.step_count = (stepCount - countA).toString();
var countA = Math.round(count * ratioA);
tagsA[key] = countA.toString();
tagsB[key] = (count - countA).toString();
wayA = wayA.update({ tags: tagsA });
wayB = wayB.update({ tags: tagsB });
}
}
graph = graph.replace(wayA);
graph = graph.replace(wayB);
+21 -6
View File
@@ -123,12 +123,20 @@ osmEntity.prototype = {
},
mergeTags: function(tags) {
var merged = Object.assign({}, this.tags); // shallow copy
var changed = false;
for (var k in tags) {
var t1 = merged[k];
var t2 = tags[k];
/**
*
* @param {Tags} tags tags to merge into this entity's tags
* @param {Tags} setTags (optional) a set of tags to overwrite in this entity's tags
* @returns {iD.OsmEntity}
*/
mergeTags: function(tags, setTags = {}) {
const merged = Object.assign({}, this.tags); // shallow copy
let changed = false;
for (const k in tags) {
if (setTags.hasOwnProperty(k)) continue;
const t1 = this.tags[k];
const t2 = tags[k];
if (!t1) {
changed = true;
merged[k] = t2;
@@ -140,6 +148,13 @@ osmEntity.prototype = {
);
}
}
for (const k in setTags) {
if (this.tags[k] !== setTags[k]) {
changed = true;
merged[k] = setTags[k];
}
}
return changed ? this.update({ tags: merged }) : this;
},
+7
View File
@@ -322,3 +322,10 @@ export function osmShouldRenderDirection(vertexTags, wayTags) {
if (vertexTags.cycleway === 'asl') return !!wayTags.highway;
return true;
}
export var osmSummableTags = new Set([
'step_count',
'parking:both:capacity',
'parking:left:capacity',
'parking:left:capacity'
]);
+13
View File
@@ -598,6 +598,19 @@ describe('iD.actionJoin', function () {
expect(graph.entity('-').tags).to.eql({ natural: 'cliff' });
});
it('merges the number of steps', function () {
let graph = iD.coreGraph([
iD.osmNode({ id: 'a', loc: [0, 0] }),
iD.osmNode({ id: 'b', loc: [1, 0] }),
iD.osmNode({ id: 'c', loc: [4, 0] }),
iD.osmWay({ id: '-', nodes: ['a', 'b'], tags: { highway: 'steps', step_count: '12' } }),
iD.osmWay({ id: '=', nodes: ['b', 'c'], tags: { highway: 'steps', step_count: '30' } })
]);
graph = iD.actionJoin(['-', '='])(graph);
// step count should be merged
expect(graph.entity('-').tags.step_count).to.equal('42');
});
it('merges relations', function () {
var graph = iD.coreGraph([
+34
View File
@@ -531,6 +531,40 @@ describe('iD.actionSplit', function () {
expect(g1.entity('-').nodes).to.eql(['b', 'c', 'd', 'a']);
expect(g1.entity('=').nodes).to.eql(['a', 'b']);
});
it('distributes the number of steps proportionally', () => {
const tags = { highway: 'steps', step_count: '40' };
let graph = iD.coreGraph([
iD.osmNode({ id: 'a', loc: [0, 0] }),
iD.osmNode({ id: 'b', loc: [1, 0] }),
iD.osmNode({ id: 'c', loc: [4, 0] }),
iD.osmWay({ id: '-', nodes: ['a', 'b', 'c'], tags: tags })
]);
graph = iD.actionSplit('b', ['='])(graph);
// step count should be distributed according the the resulting ways'
// segment lengths
expect(graph.entity('=').tags.step_count).to.equal('10');
expect(graph.entity('-').tags.step_count).to.equal('30');
});
it('preserves the total number of steps', () => {
const tags = { highway: 'steps', step_count: '42' };
let graph = iD.coreGraph([
iD.osmNode({ id: 'a', loc: [0, 0] }),
iD.osmNode({ id: 'b', loc: [1, 0] }),
iD.osmNode({ id: 'c', loc: [4, 0] }),
iD.osmWay({ id: '-', nodes: ['a', 'b', 'c'], tags: tags })
]);
graph = iD.actionSplit('b', ['='])(graph);
// the sum of the resulting step count should be preserved
// even when the intermediate values are rounded
expect(+graph.entity('=').tags.step_count +
+graph.entity('-').tags.step_count).to.equal(42);
});
});
+9
View File
@@ -168,6 +168,15 @@ describe('iD.osmEntity', function () {
expect(a.mergeTags(b.tags).tags).to.eql({a: 'a;b'});
expect(b.mergeTags(a.tags).tags).to.eql({a: 'b;a'});
});
it('accepts override tags', function () {
const a = iD.osmEntity({tags: {a: 'a', c: '1'}});
const b = iD.osmEntity({tags: {b: 'b', c: '2'}});
const merged = a.mergeTags(b.tags, { c: '3' });
expect(merged.tags.c).to.eql('3');
});
});
describe('#osmId', function () {