From dfe8fd1e906354f19ab2ef49597d54c453bd4235 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Tue, 18 Nov 2014 15:14:33 -0500 Subject: [PATCH] features performance enhancements 1. reusue entity.geometry where possible 2. cache entity parents --- js/id/renderer/features.js | 113 +++++++++++++++++---------------- js/id/svg/vertices.js | 2 +- test/spec/renderer/features.js | 24 ++++--- 3 files changed, 77 insertions(+), 62 deletions(-) diff --git a/js/id/renderer/features.js b/js/id/renderer/features.js index 16375e035..5c31305a4 100644 --- a/js/id/renderer/features.js +++ b/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); } } diff --git a/js/id/svg/vertices.js b/js/id/svg/vertices.js index d70d751a8..75589199d 100644 --- a/js/id/svg/vertices.js +++ b/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++) { diff --git a/test/spec/renderer/features.js b/test/spec/renderer/features.js index 0727a7d9d..3b82a070f 100644 --- a/test/spec/renderer/features.js +++ b/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() {