features performance enhancements

1. reusue entity.geometry where possible
2. cache entity parents

js/id/renderer/features.js

@@ -71,8 +71,8 @@ iD.Features = function(context) {
     }
 
 
-    defineFeature('points', function isPoint(entity, resolver) {
-        return entity.geometry(resolver) === 'point';
+    defineFeature('points', function isPoint(entity, resolver, geometry) {
+        return geometry === 'point';
     }, 200);
 
     defineFeature('major_roads', function isMajorRoad(entity) {
@@ -99,8 +99,8 @@ iD.Features = function(context) {
         );
     }, 250);
 
-    defineFeature('landuse', function isLanduse(entity, resolver) {
-        return entity.geometry(resolver) === 'area' &&
+    defineFeature('landuse', function isLanduse(entity, resolver, geometry) {
+        return geometry === 'area' &&
             !_features.buildings.filter(entity) &&
             !_features.water.filter(entity);
     });
@@ -155,19 +155,18 @@ iD.Features = function(context) {
     });
 
     // lines or areas that don't match another feature filter.
-    defineFeature('others', function isOther(entity, resolver) {
-        var geom = entity.geometry(resolver);
-        return (geom === 'line' || geom === 'area') && !(
-            _features.major_roads.filter(entity, resolver) ||
-            _features.minor_roads.filter(entity, resolver) ||
-            _features.paths.filter(entity, resolver) ||
-            _features.buildings.filter(entity, resolver) ||
-            _features.landuse.filter(entity, resolver) ||
-            _features.boundaries.filter(entity, resolver) ||
-            _features.water.filter(entity, resolver) ||
-            _features.rail.filter(entity, resolver) ||
-            _features.power.filter(entity, resolver) ||
-            _features.past_future.filter(entity, resolver)
+    defineFeature('others', function isOther(entity, resolver, geometry) {
+        return (geometry === 'line' || geometry === 'area') && !(
+            _features.major_roads.filter(entity, resolver, geometry) ||
+            _features.minor_roads.filter(entity, resolver, geometry) ||
+            _features.paths.filter(entity, resolver, geometry) ||
+            _features.buildings.filter(entity, resolver, geometry) ||
+            _features.landuse.filter(entity, resolver, geometry) ||
+            _features.boundaries.filter(entity, resolver, geometry) ||
+            _features.water.filter(entity, resolver, geometry) ||
+            _features.rail.filter(entity, resolver, geometry) ||
+            _features.power.filter(entity, resolver, geometry) ||
+            _features.past_future.filter(entity, resolver, geometry)
         );
     });
 
@@ -234,7 +233,7 @@ iD.Features = function(context) {
 
     features.gatherStats = function(d, resolver, dimensions) {
         var needsRedraw = false,
-            currHidden, geometry, feats;
+            currHidden, geometry, matches;
 
         _.each(_features, function(f) { f.count = 0; });
 
@@ -245,9 +244,9 @@ iD.Features = function(context) {
         for (var i = 0, imax = d.length; i !== imax; i++) {
             geometry = d[i].geometry(resolver);
             if (!(geometry === 'vertex' || geometry === 'relation')) {
-                feats = Object.keys(features.matchEntity(d[i], resolver));
-                for (var j = 0, jmax = feats.length; j !== jmax; j++) {
-                    _features[feats[j]].count++;
+                matches = Object.keys(features.getMatches(d[i], resolver, geometry));
+                for (var j = 0, jmax = matches.length; j !== jmax; j++) {
+                    _features[matches[j]].count++;
                 }
             }
         }
@@ -281,18 +280,14 @@ iD.Features = function(context) {
         _cache = {};
     };
 
-    features.match = function(d) {
-        for (var i = 0, imax = d.length; i !== imax; i++) {
-            features.matchEntity(d[i]);
-        }
-    };
-
-    features.matchEntity = function(entity, resolver) {
+    features.getMatches = function(entity, resolver, geometry) {
         var ent = iD.Entity.key(entity);
 
         if (!_cache[ent]) {
-            var geometry = entity.geometry(resolver),
-                matches = {},
+            _cache[ent] = {};
+        }
+        if (!_cache[ent].matches) {
+            var matches = {},
                 hasMatch = false;
 
             if (!(geometry === 'vertex' || geometry === 'relation')) {
@@ -300,43 +295,57 @@ iD.Features = function(context) {
                     if (hasMatch && _keys[i] === 'others') {
                         continue;
                     }
-                    if (_features[_keys[i]].filter(entity, resolver)) {
+                    if (_features[_keys[i]].filter(entity, resolver, geometry)) {
                         matches[_keys[i]] = hasMatch = true;
                     }
                 }
             }
-            _cache[ent] = matches;
+            _cache[ent].matches = matches;
         }
-        return _cache[ent];
+        return _cache[ent].matches;
     };
 
-    features.isHiddenFeature = function(entity, resolver) {
-        var matches = features.matchEntity(entity, resolver);
+    features.getParents = function(entity, resolver, geometry) {
+        var ent = iD.Entity.key(entity);
 
+        if (!_cache[ent]) {
+            _cache[ent] = {};
+        }
+        if (!_cache[ent].parents) {
+            var parents = [];
+
+            if (geometry !== 'point') {
+                if (geometry === 'vertex') {
+                    parents = resolver.parentWays(entity);
+                } else {   // 'line', 'area', 'relation'
+                    parents = resolver.parentRelations(entity);
+                }
+            }
+            _cache[ent].parents = parents;
+        }
+        return _cache[ent].parents;
+    };
+
+    features.isHiddenFeature = function(entity, resolver, geometry) {
         if (!entity.version) return false;
 
+        var matches = features.getMatches(entity, resolver, geometry);
+
         for (var i = 0, imax = _hidden.length; i !== imax; i++) {
             if (matches[_hidden[i]]) { return true; }
         }
         return false;
     };
 
-    features.isHiddenChild = function(entity, resolver, geom) {
-        var geometry = geom || entity.geometry(resolver),
-            parents;
-
+    features.isHiddenChild = function(entity, resolver, geometry) {
         if (!entity.version || geometry === 'point') { return false; }
 
-        if (geometry === 'vertex') {
-            parents = resolver.parentWays(entity);
-        } else {   // 'line', 'area', 'relation'
-            parents = resolver.parentRelations(entity);
-        }
+        var parents = features.getParents(entity, resolver, geometry);
 
         if (!parents.length) { return false; }
 
         for (var i = 0, imax = parents.length; i !== imax; i++) {
-            if (!features.isHidden(parents[i], resolver)) {
+            if (!features.isHidden(parents[i], resolver, parents[i].geometry(resolver))) {
                 return false;
             }
         }
@@ -353,28 +362,26 @@ iD.Features = function(context) {
         }
 
         // gather parents..
-        connections = _.union(resolver.parentWays(entity), resolver.parentRelations(entity));
+        connections = features.getParents(entity, resolver, entity.geometry(resolver));
         // gather ways connected to child nodes..
         connections = _.reduce(childNodes, function(result, e) {
             return resolver.isShared(e) ? _.union(result, resolver.parentWays(e)) : result;
         }, connections);
 
         return connections.length ? _.any(connections, function(e) {
-            return features.isHidden(e, resolver);
+            return features.isHidden(e, resolver, e.geometry(resolver));
         }) : false;
     };
 
-    features.isHidden = function(entity, resolver) {
+    features.isHidden = function(entity, resolver, geometry) {
         if (!entity.version) return false;
 
-        var geometry = entity.geometry(resolver);
-
         if (geometry === 'vertex')
             return features.isHiddenChild(entity, resolver, geometry);
         if (geometry === 'point')
-            return features.isHiddenFeature(entity, resolver);
+            return features.isHiddenFeature(entity, resolver, geometry);
 
-        return features.isHiddenFeature(entity, resolver) ||
+        return features.isHiddenFeature(entity, resolver, geometry) ||
                features.isHiddenChild(entity, resolver, geometry);
     };
 
@@ -385,7 +392,7 @@ iD.Features = function(context) {
         var result = [];
         for (var i = 0, imax = d.length; i !== imax; i++) {
             var entity = d[i];
-            if (!features.isHidden(entity, resolver)) {
+            if (!features.isHidden(entity, resolver, entity.geometry(resolver))) {
                 result.push(entity);
             }
         }

js/id/svg/vertices.js

@@ -12,7 +12,7 @@ iD.svg.Vertices = function(projection, context) {
         var vertices = {};
 
         function addChildVertices(entity) {
-            if (!context.features().isHiddenFeature(entity, graph)) {
+            if (!context.features().isHiddenFeature(entity, graph, entity.geometry(graph))) {
                 var i;
                 if (entity.type === 'way') {
                     for (i = 0; i < entity.nodes.length; i++) {

test/spec/renderer/features.js

@@ -173,13 +173,17 @@ describe('iD.Features', function() {
 
         function doMatch(ids) {
             _.each(ids, function(id) {
-                expect(features.isHidden(graph.entity(id), graph), 'doMatch: ' + id).to.be.true;
+                var entity = graph.entity(id),
+                    geometry = entity.geometry(graph);
+                expect(features.isHidden(entity, graph, geometry), 'doMatch: ' + id).to.be.true;
             });
         }
 
         function dontMatch(ids) {
             _.each(ids, function(id) {
-                expect(features.isHidden(graph.entity(id), graph), 'dontMatch: ' + id).to.be.false;
+                var entity = graph.entity(id),
+                    geometry = entity.geometry(graph);
+                expect(features.isHidden(entity, graph, geometry), 'dontMatch: ' + id).to.be.false;
             });
         }
 
@@ -393,13 +397,14 @@ describe('iD.Features', function() {
                 b = iD.Node({id: 'b', version: 1}),
                 w = iD.Way({id: 'w', nodes: [a.id, b.id], tags: {highway: 'path'}, version: 1}),
                 graph = iD.Graph([a, b, w]),
+                geometry = a.geometry(graph),
                 all = _.values(graph.base().entities);
 
             features.disable('paths');
             features.gatherStats(all, graph, dimensions);
 
-            expect(features.isHiddenChild(a, graph)).to.be.true;
-            expect(features.isHidden(a, graph)).to.be.true;
+            expect(features.isHiddenChild(a, graph, geometry)).to.be.true;
+            expect(features.isHidden(a, graph, geometry)).to.be.true;
         });
 
         it('hides child ways on a hidden multipolygon relation', function() {
@@ -421,26 +426,29 @@ describe('iD.Features', function() {
                     version: 1
                 }),
                 graph = iD.Graph([a, b, c, d, e, f, outer, inner, r]),
+                geometry = inner.geometry(graph),
                 all = _.values(graph.base().entities);
 
             features.disable('landuse');
             features.gatherStats(all, graph, dimensions);
 
-            expect(features.isHiddenChild(inner, graph)).to.be.true;
-            expect(features.isHidden(inner, graph)).to.be.true;
+            expect(features.isHiddenChild(inner, graph, geometry)).to.be.true;
+            expect(features.isHidden(inner, graph, geometry)).to.be.true;
         });
 
         it('hides only versioned entities', function() {
             var a = iD.Node({id: 'a', version: 1}),
                 b = iD.Node({id: 'b'}),
                 graph = iD.Graph([a, b]),
+                ageo = a.geometry(graph),
+                bgeo = b.geometry(graph),
                 all = _.values(graph.base().entities);
 
             features.disable('points');
             features.gatherStats(all, graph, dimensions);
 
-            expect(features.isHidden(a, graph)).to.be.true;
-            expect(features.isHidden(b, graph)).to.be.false;
+            expect(features.isHidden(a, graph, ageo)).to.be.true;
+            expect(features.isHidden(b, graph, bgeo)).to.be.false;
         });
 
         it('auto-hides features', function() {