Merge branch 'master' into validation_and_change_perf

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": {

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}));
     };

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) {

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);

modules/validations/almost_junction.js

@@ -49,49 +49,27 @@ 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 !== null) {
-                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 !== null) {
-                testNodes = graph.childNodes(way).slice();   // shallow copy
-                index = testNodes.length - 1;  // last
-                testNodes[index] = testNodes[index].move(connNearFirst.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(connectionInfo);
+        });
 
-                // 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;
     }
 
@@ -126,8 +104,9 @@ 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 {
+                        node: tipNode,
                         wid: way2.id,
                         edge: [nA.id, nB.id],
                         cross_loc: crossLoc

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) {