Skip to content

Commit

Permalink
Feature Filtering: don't match multipolygon lines as 'others'
Browse files Browse the repository at this point in the history
(fixes openstreetmap#2548)

If the entity is a way that is a member of a parent relation,
use that parent relation's matches instead of matching 'other'
  • Loading branch information
bhousel authored and paulmach committed Jun 8, 2015
1 parent c95eb80 commit f3bd8b7
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 20 deletions.
48 changes: 30 additions & 18 deletions js/id/renderer/features.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,20 +154,11 @@ iD.Features = function(context) {
return false;
});

// lines or areas that don't match another feature filter.
// Lines or areas that don't match another feature filter.
// IMPORTANT: The 'others' feature must be the last one defined,
// so that code in getMatches can skip this test if `hasMatch = true`
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)
);
return (geometry === 'line' || geometry === 'area');
});


Expand Down Expand Up @@ -237,6 +228,8 @@ iD.Features = function(context) {

features.gatherStats = function(d, resolver, dimensions) {
var needsRedraw = false,
type = _.groupBy(d, function(ent) { return ent.type; }),
entities = [].concat(type.relation || [], type.way || [], type.node || []),
currHidden, geometry, matches;

_.each(_features, function(f) { f.count = 0; });
Expand All @@ -245,10 +238,10 @@ iD.Features = function(context) {
// a _cullFactor of 1 corresponds to a 1000x1000px viewport..
_cullFactor = dimensions[0] * dimensions[1] / 1000000;

for (var i = 0; i < d.length; i++) {
geometry = d[i].geometry(resolver);
for (var i = 0; i < entities.length; i++) {
geometry = entities[i].geometry(resolver);
if (!(geometry === 'vertex' || geometry === 'relation')) {
matches = Object.keys(features.getMatches(d[i], resolver, geometry));
matches = Object.keys(features.getMatches(entities[i], resolver, geometry));
for (var j = 0; j < matches.length; j++) {
_features[matches[j]].count++;
}
Expand Down Expand Up @@ -296,9 +289,28 @@ iD.Features = function(context) {

if (!(geometry === 'vertex' || geometry === 'relation')) {
for (var i = 0; i < _keys.length; i++) {
if (hasMatch && _keys[i] === 'others') {
continue;

if (_keys[i] === 'others') {
if (hasMatch) continue;

// If the entity is a way that has not matched any other
// feature type, see if it has a parent relation, and if so,
// match whatever feature types the parent has matched.
// (The way is a member of a multipolygon.)
//
// IMPORTANT:
// For this to work, getMatches must be called on relations before ways.
//
if (entity.type === 'way') {
var parents = features.getParents(entity, resolver, geometry);
if (parents.length === 1) {
var pkey = iD.Entity.key(parents[0]);
matches = _.clone(_cache[pkey].matches);
continue;
}
}
}

if (_features[_keys[i]].filter(entity, resolver, geometry)) {
matches[_keys[i]] = hasMatch = true;
}
Expand Down
13 changes: 11 additions & 2 deletions test/spec/renderer/features.js
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,15 @@ describe('iD.Features', function() {
iD.Way({id: 'scrub', tags: {area: 'yes', natural: 'scrub'}, version: 1}),
iD.Way({id: 'industrial', tags: {area: 'yes', landuse: 'industrial'}, version: 1}),
iD.Way({id: 'parkinglot', tags: {area: 'yes', amenity: 'parking', parking: 'surface'}, version: 1}),
iD.Way({id: 'inner', version: 1}),
iD.Way({id: 'outer', version: 1}),
iD.Relation({id: 'retail', tags: {landuse: 'retail', type: 'multipolygon'},
members: [
{id: 'outer', role: 'outer', type: 'way'},
{id: 'inner', role: 'inner', type: 'way'}
],
version: 1
}),

// Boundaries
iD.Way({id: 'boundary', tags: {boundary: 'administrative'}, version: 1}),
Expand Down Expand Up @@ -279,7 +288,7 @@ describe('iD.Features', function() {

doMatch([
'forest', 'scrub', 'industrial', 'parkinglot', 'building_no',
'rail_landuse', 'landuse_construction'
'rail_landuse', 'landuse_construction', 'retail', 'inner', 'outer'
]);

dontMatch([
Expand Down Expand Up @@ -385,7 +394,7 @@ describe('iD.Features', function() {
dontMatch([
'point_bar', 'motorway', 'service', 'path', 'building_yes',
'forest', 'boundary', 'water', 'railway', 'power_line',
'motorway_construction',
'motorway_construction', 'retail', 'inner', 'outer'
]);
});
});
Expand Down

0 comments on commit f3bd8b7

Please sign in to comment.