From d65298f383f1ad991fe280dc9df813d6d1891b83 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Wed, 3 Apr 2019 09:29:14 -0700 Subject: [PATCH 1/6] Fix issue where generic vertex icon wouldn't show (close #6138) --- modules/ui/preset_icon.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/ui/preset_icon.js b/modules/ui/preset_icon.js index fe639e5f0..87150053d 100644 --- a/modules/ui/preset_icon.js +++ b/modules/ui/preset_icon.js @@ -151,7 +151,7 @@ export function uiPresetIcon() { var isiDIcon = picon && !(isMaki || isTemaki || isFa); var isCategory = !p.setTags; var drawPoint = picon && geom === 'point' && isSmall() && !isFallback; - var drawVertex = picon && geom === 'vertex' && !isFallback; + var drawVertex = picon !== null && geom === 'vertex' && (!isSmall() || !isFallback); var drawLine = picon && geom === 'line' && !isFallback && !isCategory; var drawArea = picon && geom === 'area' && !isFallback; var isFramed = (drawVertex || drawArea || drawLine); From 34792d7fbf818b9eefcce8c92e432328ea1200a9 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Wed, 3 Apr 2019 10:57:42 -0700 Subject: [PATCH 2/6] Don't add field defaults when upgrading tags to a specific replacement --- modules/actions/change_preset.js | 4 ++-- modules/presets/preset.js | 4 ++-- modules/validations/outdated_tags.js | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/actions/change_preset.js b/modules/actions/change_preset.js index d39a62836..1a193b878 100644 --- a/modules/actions/change_preset.js +++ b/modules/actions/change_preset.js @@ -1,11 +1,11 @@ -export function actionChangePreset(entityID, oldPreset, newPreset) { +export function actionChangePreset(entityID, oldPreset, newPreset, skipFieldDefaults) { return function action(graph) { var entity = graph.entity(entityID); var geometry = entity.geometry(graph); var tags = entity.tags; if (oldPreset) tags = oldPreset.unsetTags(tags, geometry); - if (newPreset) tags = newPreset.setTags(tags, geometry); + if (newPreset) tags = newPreset.setTags(tags, geometry, skipFieldDefaults); return graph.replace(entity.update({tags: tags})); }; diff --git a/modules/presets/preset.js b/modules/presets/preset.js index 0778813a8..121483873 100644 --- a/modules/presets/preset.js +++ b/modules/presets/preset.js @@ -218,7 +218,7 @@ export function presetPreset(id, preset, fields, visible, rawPresets) { preset.addTags = preset.addTags || preset.tags || {}; - preset.setTags = function(tags, geometry) { + preset.setTags = function(tags, geometry, skipFieldDefaults) { var addTags = preset.addTags; var k; @@ -253,7 +253,7 @@ export function presetPreset(id, preset, fields, visible, rawPresets) { } } } - if (geometry) { + if (geometry && !skipFieldDefaults) { for (var f in preset.fields) { var field = preset.fields[f]; if (field.matchGeometry(geometry) && field.key && !tags[field.key] && field.default) { diff --git a/modules/validations/outdated_tags.js b/modules/validations/outdated_tags.js index 43805b741..bf2d66f53 100644 --- a/modules/validations/outdated_tags.js +++ b/modules/validations/outdated_tags.js @@ -54,7 +54,7 @@ export function validationOutdatedTags() { function(graph) { if (replacementPreset) { var oldPreset = context.presets().match(graph.entity(entityID), context.graph()); - graph = actionChangePreset(entityID, oldPreset, replacementPreset)(graph); + graph = actionChangePreset(entityID, oldPreset, replacementPreset, true /* skip field defaults */)(graph); deprecatedTagsArray = graph.entity(entityID).deprecatedTags(); } deprecatedTagsArray.forEach(function(deprecatedTags) { From cbae090f0899a8820c7cc7a073c4fd44ee9b6334 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Wed, 3 Apr 2019 14:23:21 -0700 Subject: [PATCH 3/6] Fix crash in almost junction validation --- modules/validations/almost_junction.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/validations/almost_junction.js b/modules/validations/almost_junction.js index cf7ab8ecb..bdd06e52c 100644 --- a/modules/validations/almost_junction.js +++ b/modules/validations/almost_junction.js @@ -57,7 +57,7 @@ export function validationAlmostJunction() { if (isExtendableCandidate(nodeFirst, way, graph)) { var connNearFirst = canConnectByExtend(way, 0, graph, tree); - if (connNearFirst !== null) { + if (connNearFirst) { testNodes = graph.childNodes(way).slice(); // shallow copy index = 0; // first testNodes[index] = testNodes[index].move(connNearFirst.cross_loc); @@ -76,10 +76,10 @@ export function validationAlmostJunction() { if (isExtendableCandidate(nodeLast, way, graph)) { var connNearLast = canConnectByExtend(way, way.nodes.length - 1, graph, tree); - if (connNearLast !== null) { + if (connNearLast) { testNodes = graph.childNodes(way).slice(); // shallow copy index = testNodes.length - 1; // last - testNodes[index] = testNodes[index].move(connNearFirst.cross_loc); + testNodes[index] = testNodes[index].move(connNearLast.cross_loc); // don't flag issue if connecting the ways would cause self-intersection if (!geoHasSelfIntersections(testNodes, nodeLast.id)) { @@ -126,7 +126,7 @@ export function validationAlmostJunction() { var nA = graph.entity(way2.nodes[j]); var nB = graph.entity(way2.nodes[j + 1]); var crossLoc = geoLineIntersection([tipNode.loc, extTipLoc], [nA.loc, nB.loc]); - if (crossLoc !== null) { + if (crossLoc) { return { wid: way2.id, edge: [nA.id, nB.id], From 6641f62685e248e9f55561708dc2447c910a1227 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Wed, 3 Apr 2019 15:44:45 -0700 Subject: [PATCH 4/6] Reduce duplicate code in almost junction validation --- modules/validations/almost_junction.js | 61 ++++++++++---------------- 1 file changed, 22 insertions(+), 39 deletions(-) diff --git a/modules/validations/almost_junction.js b/modules/validations/almost_junction.js index bdd06e52c..0b8c059a3 100644 --- a/modules/validations/almost_junction.js +++ b/modules/validations/almost_junction.js @@ -49,49 +49,32 @@ export function validationAlmostJunction() { var results = []; if (way.isClosed()) return results; - var nidFirst = way.nodes[0]; - var nidLast = way.nodes[way.nodes.length - 1]; - var nodeFirst = graph.entity(nidFirst); - var nodeLast = graph.entity(nidLast); - var testNodes, index; + var testNodes; + var endpointIndicies = [0, way.nodes.length - 1]; + endpointIndicies.forEach(function(nodeIndex) { - if (isExtendableCandidate(nodeFirst, way, graph)) { - var connNearFirst = canConnectByExtend(way, 0, graph, tree); - if (connNearFirst) { - testNodes = graph.childNodes(way).slice(); // shallow copy - index = 0; // first - testNodes[index] = testNodes[index].move(connNearFirst.cross_loc); + var nodeID = way.nodes[nodeIndex]; + var node = graph.entity(nodeID); - // don't flag issue if connecting the ways would cause self-intersection - if (!geoHasSelfIntersections(testNodes, nodeFirst.id)) { - results.push({ - node: nodeFirst, - wid: connNearFirst.wid, - edge: connNearFirst.edge, - cross_loc: connNearFirst.cross_loc - }); - } - } - } + if (!isExtendableCandidate(node, way, graph)) return; - if (isExtendableCandidate(nodeLast, way, graph)) { - var connNearLast = canConnectByExtend(way, way.nodes.length - 1, graph, tree); - if (connNearLast) { - testNodes = graph.childNodes(way).slice(); // shallow copy - index = testNodes.length - 1; // last - testNodes[index] = testNodes[index].move(connNearLast.cross_loc); + var connectionInfo = canConnectByExtend(way, nodeIndex, graph, tree); + if (!connectionInfo) return; + + testNodes = graph.childNodes(way).slice(); // shallow copy + testNodes[nodeIndex] = testNodes[nodeIndex].move(connectionInfo.cross_loc); + + // don't flag issue if connecting the ways would cause self-intersection + if (geoHasSelfIntersections(testNodes, nodeID)) return; + + results.push({ + node: node, + wid: connectionInfo.wid, + edge: connectionInfo.edge, + cross_loc: connectionInfo.cross_loc + }); + }); - // don't flag issue if connecting the ways would cause self-intersection - if (!geoHasSelfIntersections(testNodes, nodeLast.id)) { - results.push({ - node: nodeLast, - wid: connNearLast.wid, - edge: connNearLast.edge, - cross_loc: connNearLast.cross_loc - }); - } - } - } return results; } From e71b1391717977cdf9eb20c779223b3fe7ad1336 Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Wed, 3 Apr 2019 16:41:26 -0700 Subject: [PATCH 5/6] Simplify almost junction validation --- modules/validations/almost_junction.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/modules/validations/almost_junction.js b/modules/validations/almost_junction.js index 0b8c059a3..82443528e 100644 --- a/modules/validations/almost_junction.js +++ b/modules/validations/almost_junction.js @@ -67,12 +67,7 @@ export function validationAlmostJunction() { // don't flag issue if connecting the ways would cause self-intersection if (geoHasSelfIntersections(testNodes, nodeID)) return; - results.push({ - node: node, - wid: connectionInfo.wid, - edge: connectionInfo.edge, - cross_loc: connectionInfo.cross_loc - }); + results.push(connectionInfo); }); return results; @@ -111,6 +106,7 @@ export function validationAlmostJunction() { var crossLoc = geoLineIntersection([tipNode.loc, extTipLoc], [nA.loc, nB.loc]); if (crossLoc) { return { + node: tipNode, wid: way2.id, edge: [nA.id, nB.id], cross_loc: crossLoc From b52d8c1790d20fbb0abb3520f7d464ab43ce12ec Mon Sep 17 00:00:00 2001 From: Quincy Morgan Date: Wed, 3 Apr 2019 18:06:13 -0700 Subject: [PATCH 6/6] Add documentation for the `geometry`, `tags`, `addTags`, `removeTags`, and `matchScore` preset properties --- data/presets/README.md | 62 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 55 insertions(+), 7 deletions(-) diff --git a/data/presets/README.md b/data/presets/README.md index 306fcd3df..543113be7 100644 --- a/data/presets/README.md +++ b/data/presets/README.md @@ -97,6 +97,50 @@ fields as `shop`. In both explicit and implicit inheritance, fields for keys that define the preset are not inherited. E.g. the `shop` field is not inherited by `shop/…` presets. +##### `geometry` + +An array of possible geometry types that a feature can have in order to match this preset. + +* `point`: an OSM node that is not a member of any way +* `vertex`: an OSM node that is a member of one or more ways +* `line`: an OSM way that is not an area +* `area`: an OSM way that is closed/circular (the first and last nodes are the same) or a `type=multipolygon` relation +* `relation`: an OSM relation + +Closed ways can be treated as both `line` or `area` geometry. If a preset allows both, iD will add an additional `area=yes` tag when choosing the preset for an area feature. + +This property is required. There is no default. + +##### `tags` + +An object with the `"key": "value"` tags a feature must have to match this preset. A `"*"` wildcard value can be set to have this preset match any value for that key. + +A features can only match one preset even if its tags and geometry could technically match more than one. iD will pick the best match based on `matchScore`, the number of tags, and the use of wildcard values. + +This property is required. There is no default. + +##### `addTags`/`removeTags` + +Objects with the tags that are added to or removed from the feature when selected or deselecting this preset. These both default to the value of `tags`. + +Generally, these properties will be equivalent and should be supersets of `tags`. + +iD's validator will recommend that users add missing tags from `addTags` to matching features. + +For example, the Bridge preset has these properties: + +``` + "tags": { + "man_made": "bridge" + }, + "addTags": { + "man_made": "bridge", + "layer": "1" + }, +``` + +When adding a feature with this preset, it will be given the tags `man_made=bridge` and `layer=1`. The user could then change `layer` to `3`, for instance, and the feature would still match the preset because it still has `man_made=bridge`. If the user removes the `layer` tag altogether, iD will recommend adding it back with a value of `1`. + ##### `icon` The name of a local SVG icon file. You can use icons from any of the following open source icon sets. @@ -118,12 +162,22 @@ For example, `imageURL` is used to specify the logos of brand presets from the [ Bitmap images should be at least 100x100px to look good at 50x50pt on high-resolution screens. +##### `matchScore` + +A number that ranks this preset against others that match the feature. + +For example, a feature with `amenity=cafe` and `building=commercial` will match the Cafe preset instead of the Commercial Building preset because Commercial Building has a lower `matchScore`. + +The default is `1.0`. + ##### `name` The primary name of the feature type in American English. Upon merging with `master`, this is sent to Transifex for translating to other localizations. Changing the name of an existing preset will require it to be re-translated to all localizations. +This property is required. There is no default. + ##### `replacement` The ID of a preset that is preferable to this one. iD's validator will flag features matching this preset and recommend that the user upgrade the tags. @@ -382,13 +436,7 @@ For example: "point": { "name": "Point", "tags": {}, - "geometry": ["point"], - "matchScore": 0.1 -}, -"vertex": { - "name": "Other", - "tags": {}, - "geometry": ["vertex"], + "geometry": ["point", "vertex"], "matchScore": 0.1 }, "relation": {