From dd0be84da47585169ae468653a64c412b08ff6e5 Mon Sep 17 00:00:00 2001 From: Huon Wilson Date: Wed, 28 Nov 2018 01:25:14 +1100 Subject: [PATCH] Add one-sided triangular markers to ways with sides (e.g. natural=cliff). This generalizes the oneway arrow logic for adding SVG markers along a line. Using that functionality, certain tags get arrows on their right-hand side, indicating which side is "inside", e.g. the right-side of a cliff is the lower side. The list of tags considered to be sided (unless there's a two_sided=yes tag) is: - natural=cliff - natural=coastline - barrier=retaining_wall - barrier=kerb - barrier=guard_rail - barrier=city_wall - man_made=embankment The triangles attempt to be reminiscent of the triangles used for rendering cliffs on OSM (and elsewhere). The different tags get different renderings (e.g. colors that match the main way, and different spacings). In addition, natural=coastline is special-cased to have blue markers (despite having a green way), to emphasise that the "inside" of a coastline is the water. Fixes https://github.com/openstreetmap/iD/issues/1475. --- css/20_map.css | 3 +- modules/osm/tags.js | 16 ++++++++ modules/osm/way.js | 30 +++++++++++++- modules/svg/defs.js | 32 +++++++++++++++ modules/svg/helpers.js | 10 ++--- modules/svg/index.js | 2 +- modules/svg/lines.js | 90 ++++++++++++++++++++++++++---------------- test/spec/osm/way.js | 65 ++++++++++++++++++++++++++++++ test/spec/svg/lines.js | 37 +++++++++++++++++ 9 files changed, 243 insertions(+), 42 deletions(-) diff --git a/css/20_map.css b/css/20_map.css index a36d9237b..67d9f154e 100644 --- a/css/20_map.css +++ b/css/20_map.css @@ -205,7 +205,8 @@ text { } .onewaygroup path.oneway, -.viewfieldgroup path.viewfield { +.viewfieldgroup path.viewfield, +.sidedgroup path.sided { stroke-width: 6px; } diff --git a/modules/osm/tags.js b/modules/osm/tags.js index 10d71ec4a..1d2b80ed6 100644 --- a/modules/osm/tags.js +++ b/modules/osm/tags.js @@ -56,3 +56,19 @@ export var osmPavedTags = { 'grade1': true } }; + +export var osmRightSideIsInsideTags = { + 'natural': { + 'cliff': true, + 'coastline': 'coastline', + }, + 'barrier': { + 'retaining_wall': true, + 'kerb': true, + 'guard_rail': true, + 'city_wall': true, + }, + 'man_made': { + 'embankment': true + } +}; diff --git a/modules/osm/way.js b/modules/osm/way.js index c4da27c25..3c98c8833 100644 --- a/modules/osm/way.js +++ b/modules/osm/way.js @@ -7,7 +7,7 @@ import { geoArea as d3_geoArea } from 'd3-geo'; import { geoExtent, geoVecCross } from '../geo'; import { osmEntity } from './entity'; import { osmLanes } from './lanes'; -import { osmOneWayTags } from './tags'; +import { osmOneWayTags, osmRightSideIsInsideTags } from './tags'; import { areaKeys } from '../core/context'; @@ -129,6 +129,34 @@ _extend(osmWay.prototype, { return false; }, + // Some identifier for tag that implies that this way is "sided", + // i.e. the right side is the 'inside' (e.g. the right side of a + // natural=cliff is lower). + sidednessIdentifier: function() { + for (var key in this.tags) { + var value = this.tags[key]; + if (key in osmRightSideIsInsideTags && (value in osmRightSideIsInsideTags[key])) { + if (osmRightSideIsInsideTags[key][value] === true) { + return key; + } else { + // if the map's value is something other than a + // literal true, we should use it so we can + // special case some keys (e.g. natural=coastline + // is handled differently to other naturals). + return osmRightSideIsInsideTags[key][value]; + } + } + } + + return null; + }, + isSided: function() { + if (this.tags.two_sided === 'yes') { + return false; + } + + return this.sidednessIdentifier() != null; + }, lanes: function() { return osmLanes(this); diff --git a/modules/svg/defs.js b/modules/svg/defs.js index 52ec2f8c8..1d9ef8b5d 100644 --- a/modules/svg/defs.js +++ b/modules/svg/defs.js @@ -31,6 +31,38 @@ export function svgDefs(context) { .attr('fill', '#000') .attr('opacity', '0.75'); + // SVG markers have to be given a colour where they're defined + // (they can't inherit it from the line they're attached to), + // so we need to manually define markers for each color of tag + // (also, it's slightly nicer if we can control the + // positioning for different tags) + function addSidedMarker(name, color, offset) { + defs + .append('marker') + .attr('id', 'sided-marker-' + name) + .attr('viewBox', '0 0 2 2') + .attr('refX', 1) + .attr('refY', -offset) + .attr('markerWidth', 1.5) + .attr('markerHeight', 1.5) + .attr('markerUnits', 'strokeWidth') + .attr('orient', 'auto') + .append('path') + .attr('class', 'sided-marker-path sided-marker-' + name + '-path') + .attr('d', 'M 0,0 L 1,2 L 2,0 z') + .attr('stroke', 'none') + .attr('fill', color); + } + addSidedMarker('natural', 'rgb(140, 208, 95)', 0); + // for a coastline, the arrows are (somewhat unintuitively) on + // the water side, so let's color them blue (with a gap) to + // give a stronger indication + addSidedMarker('coastline', '#77dede', 1); + // barriers have a dashed line, and separating the triangle + // from the line visually suits that + addSidedMarker('barrier', '#ddd', 1); + addSidedMarker('man_made', '#fff', 0); + defs .append('marker') .attr('id', 'viewfield-marker') diff --git a/modules/svg/helpers.js b/modules/svg/helpers.js index 7449a6518..48095f322 100644 --- a/modules/svg/helpers.js +++ b/modules/svg/helpers.js @@ -63,7 +63,9 @@ export function svgPassiveVertex(node, graph, activeID) { } -export function svgOneWaySegments(projection, graph, dt) { +export function svgMarkerSegments(projection, graph, dt, + shouldReverse, + bothDirections) { return function(entity) { var i = 0; var offset = dt; @@ -72,12 +74,10 @@ export function svgOneWaySegments(projection, graph, dt) { var coordinates = graph.childNodes(entity).map(function(n) { return n.loc; }); var a, b; - if (entity.tags.oneway === '-1') { + if (shouldReverse(entity)) { coordinates.reverse(); } - var isReversible = (entity.tags.oneway === 'reversible' || entity.tags.oneway === 'alternating'); - d3_geoStream({ type: 'LineString', coordinates: coordinates @@ -116,7 +116,7 @@ export function svgOneWaySegments(projection, graph, dt) { } segments.push({ id: entity.id, index: i++, d: segment }); - if (isReversible) { + if (bothDirections(entity)) { segment = ''; for (j = coord.length - 1; j >= 0; j--) { segment += (j === coord.length - 1 ? 'M' : 'L') + coord[j][0] + ',' + coord[j][1]; diff --git a/modules/svg/index.js b/modules/svg/index.js index 099d18fca..310acc058 100644 --- a/modules/svg/index.js +++ b/modules/svg/index.js @@ -10,7 +10,7 @@ export { svgMapillaryImages } from './mapillary_images.js'; export { svgMapillarySigns } from './mapillary_signs.js'; export { svgMidpoints } from './midpoints.js'; export { svgNotes } from './notes.js'; -export { svgOneWaySegments } from './helpers.js'; +export { svgMarkerSegments } from './helpers.js'; export { svgOpenstreetcamImages } from './openstreetcam_images.js'; export { svgOsm } from './osm.js'; export { svgPassiveVertex } from './helpers.js'; diff --git a/modules/svg/lines.js b/modules/svg/lines.js index ed6762de8..27806e291 100644 --- a/modules/svg/lines.js +++ b/modules/svg/lines.js @@ -7,7 +7,7 @@ import _map from 'lodash-es/map'; import { range as d3_range } from 'd3-array'; import { - svgOneWaySegments, + svgMarkerSegments, svgPath, svgRelationMemberTags, svgSegmentWay, @@ -148,11 +148,45 @@ export function svgLines(projection, context) { }; } + function addMarkers(layergroup, pathclass, groupclass, groupdata, marker) { + var markergroup = layergroup + .selectAll('g.' + groupclass) + .data([pathclass]); + + markergroup = markergroup.enter() + .append('g') + .attr('class', groupclass) + .merge(markergroup); + + var markers = markergroup + .selectAll('path') + .filter(filter) + .data( + function data() { return groupdata[this.parentNode.__data__] || []; }, + function key(d) { return [d.id, d.index]; } + ); + + markers.exit() + .remove(); + + markers = markers.enter() + .append('path') + .attr('class', pathclass) + .attr('marker-mid', marker) + .merge(markers) + .attr('d', function(d) { return d.d; }); + + if (detected.ie) { + markers.each(function() { this.parentNode.insertBefore(this, this); }); + } + } + var getPath = svgPath(projection, graph); var ways = []; var pathdata = {}; var onewaydata = {}; + var sideddata = {}; var oldMultiPolygonOuters = {}; for (var i = 0; i < entities.length; i++) { @@ -170,8 +204,21 @@ export function svgLines(projection, context) { pathdata = _groupBy(ways, function(way) { return way.layer(); }); _forOwn(pathdata, function(v, k) { - var arr = _filter(v, function(d) { return d.isOneWay(); }); - onewaydata[k] = _flatten(_map(arr, svgOneWaySegments(projection, graph, 35))); + var onewayArr = _filter(v, function(d) { return d.isOneWay(); }); + var onewaySegments = svgMarkerSegments( + projection, graph, 35, + function shouldReverse(entity) { return entity.tags.oneway === '-1'; }, + function bothDirections(entity) { + return entity.tags.oneway === 'reversible' || entity.tags.oneway === 'alternating'; + }); + onewaydata[k] = _flatten(_map(onewayArr, onewaySegments)); + + var sidedArr = _filter(v, function(d) { return d.isSided(); }); + var sidedSegments = svgMarkerSegments( + projection, graph, 30, + function shouldReverse() { return false; }, + function bothDirections() { return false; }); + sideddata[k] = _flatten(_map(sidedArr, sidedSegments)); }); @@ -212,37 +259,12 @@ export function svgLines(projection, context) { layergroup.selectAll('g.line-stroke-highlighted') .call(drawLineGroup, 'stroke', true); - - var onewaygroup = layergroup - .selectAll('g.onewaygroup') - .data(['oneway']); - - onewaygroup = onewaygroup.enter() - .append('g') - .attr('class', 'onewaygroup') - .merge(onewaygroup); - - var oneways = onewaygroup - .selectAll('path') - .filter(filter) - .data( - function data() { return onewaydata[this.parentNode.__data__] || []; }, - function key(d) { return [d.id, d.index]; } - ); - - oneways.exit() - .remove(); - - oneways = oneways.enter() - .append('path') - .attr('class', 'oneway') - .attr('marker-mid', 'url(#oneway-marker)') - .merge(oneways) - .attr('d', function(d) { return d.d; }); - - if (detected.ie) { - oneways.each(function() { this.parentNode.insertBefore(this, this); }); - } + addMarkers(layergroup, 'oneway', 'onewaygroup', onewaydata, 'url(#oneway-marker)'); + addMarkers(layergroup, 'sided', 'sidedgroup', sideddata, + function marker(d) { + var category = graph.entity(d.id).sidednessIdentifier(); + return 'url(#sided-marker-' + category + ')'; + }); }); // Draw touch targets.. diff --git a/test/spec/osm/way.js b/test/spec/osm/way.js index e847da830..e52446d19 100644 --- a/test/spec/osm/way.js +++ b/test/spec/osm/way.js @@ -338,6 +338,71 @@ describe('iD.osmWay', function() { }); }); + describe('#sidednessIdentifier', function() { + it('returns tag when the tag has implied sidedness', function() { + expect(iD.Way({tags: { natural: 'cliff' }}).sidednessIdentifier()).to.eql('natural'); + expect(iD.Way({tags: { natural: 'coastline' }}).sidednessIdentifier()).to.eql('coastline'); + expect(iD.Way({tags: { barrier: 'retaining_wall' }}).sidednessIdentifier()).to.eql('barrier'); + expect(iD.Way({tags: { barrier: 'kerb' }}).sidednessIdentifier()).to.eql('barrier'); + expect(iD.Way({tags: { barrier: 'guard_rail' }}).sidednessIdentifier()).to.eql('barrier'); + expect(iD.Way({tags: { barrier: 'city_wall' }}).sidednessIdentifier()).to.eql('barrier'); + expect(iD.Way({tags: { man_made: 'embankment' }}).sidednessIdentifier()).to.eql('man_made'); + }); + + it('returns null when tag does not have implied sidedness', function() { + expect(iD.Way({tags: { natural: 'ridge' }}).sidednessIdentifier()).to.be.null; + expect(iD.Way({tags: { barrier: 'fence' }}).sidednessIdentifier()).to.be.null; + expect(iD.Way({tags: { man_made: 'dyke' }}).sidednessIdentifier()).to.be.null; + expect(iD.Way({tags: { highway: 'motorway' }}).sidednessIdentifier()).to.be.null; + }); + }); + describe('#isSided', function() { + it('returns false when the way has no tags', function() { + expect(iD.Way().isSided()).to.be.false; + }); + + it('returns false when the way has two_sided=yes', function() { + expect(iD.Way({tags: { two_sided: 'yes' }}).isSided()).to.be.false; + }); + + it('returns true when the tag has implied sidedness', function() { + expect(iD.Way({tags: { natural: 'cliff' }}).isSided()).to.be.true; + expect(iD.Way({tags: { natural: 'coastline' }}).isSided()).to.be.true; + expect(iD.Way({tags: { barrier: 'retaining_wall' }}).isSided()).to.be.true; + expect(iD.Way({tags: { barrier: 'kerb' }}).isSided()).to.be.true; + expect(iD.Way({tags: { barrier: 'guard_rail' }}).isSided()).to.be.true; + expect(iD.Way({tags: { barrier: 'city_wall' }}).isSided()).to.be.true; + expect(iD.Way({tags: { man_made: 'embankment' }}).isSided()).to.be.true; + }); + + it('returns false when two_sided=yes overrides tag with implied sidedness', function() { + expect(iD.Way({tags: { natural: 'cliff', two_sided: 'yes' }}).isSided()).to.be.false; + expect(iD.Way({tags: { natural: 'coastline', two_sided: 'yes' }}).isSided()).to.be.false; + expect(iD.Way({tags: { barrier: 'retaining_wall', two_sided: 'yes' }}).isSided()).to.be.false; + expect(iD.Way({tags: { barrier: 'kerb', two_sided: 'yes' }}).isSided()).to.be.false; + expect(iD.Way({tags: { barrier: 'guard_rail', two_sided: 'yes' }}).isSided()).to.be.false; + expect(iD.Way({tags: { barrier: 'city_wall', two_sided: 'yes' }}).isSided()).to.be.false; + expect(iD.Way({tags: { man_made: 'embankment', two_sided: 'yes' }}).isSided()).to.be.false; + }); + + it('returns true when two_sided=no is on tag with implied sidedness', function() { + expect(iD.Way({tags: { natural: 'cliff', two_sided: 'no' }}).isSided()).to.be.true; + expect(iD.Way({tags: { natural: 'coastline', two_sided: 'no' }}).isSided()).to.be.true; + expect(iD.Way({tags: { barrier: 'retaining_wall', two_sided: 'no' }}).isSided()).to.be.true; + expect(iD.Way({tags: { barrier: 'kerb', two_sided: 'no' }}).isSided()).to.be.true; + expect(iD.Way({tags: { barrier: 'guard_rail', two_sided: 'no' }}).isSided()).to.be.true; + expect(iD.Way({tags: { barrier: 'city_wall', two_sided: 'no' }}).isSided()).to.be.true; + expect(iD.Way({tags: { man_made: 'embankment', two_sided: 'no' }}).isSided()).to.be.true; + }); + + it('returns false when the tag does not have implied sidedness', function() { + expect(iD.Way({tags: { natural: 'ridge' }}).isSided()).to.be.false; + expect(iD.Way({tags: { barrier: 'fence' }}).isSided()).to.be.false; + expect(iD.Way({tags: { man_made: 'dyke' }}).isSided()).to.be.false; + expect(iD.Way({tags: { highway: 'motorway' }}).isSided()).to.be.false; + }); + }); + describe('#isArea', function() { before(function() { iD.Context(); diff --git a/test/spec/svg/lines.js b/test/spec/svg/lines.js index 0a6762a3c..66efcd5aa 100644 --- a/test/spec/svg/lines.js +++ b/test/spec/svg/lines.js @@ -185,4 +185,41 @@ describe('iD.svgLines', function () { expect(selection.empty()).to.be.true; }); }); + + describe('sided-markers', function() { + it('has marker layer for sided way', function() { + var a = iD.osmNode({id: 'a', loc: [0, 0]}); + var b = iD.osmNode({id: 'b', loc: [1e-2, 0]}); + var c = iD.osmNode({id: 'c', loc: [0, 1e-2]}); + var d = iD.osmNode({id: 'd', loc: [1e-2, 1e-2]}); + + var i_n = iD.osmWay({id: 'implied-natural', tags: {natural: 'cliff'}, nodes: [a.id, b.id]}); + var i_nc = iD.osmWay({id: 'implied-coastline', tags: {natural: 'coastline'}, nodes: [a.id, c.id]}); + var i_b = iD.osmWay({id: 'implied-barrier', tags: {barrier: 'city_wall'}, nodes: [a.id, d.id]}); + var i_mm = iD.osmWay({id: 'implied-man_made', tags: {man_made: 'embankment'}, nodes: [b.id, c.id]}); + + var graph = iD.coreGraph([a, b, c, d, i_n, i_nc, i_b, i_mm]); + + surface.call(iD.svgLines(projection, context), graph, [i_n, i_nc, i_b, i_mm], all); + var selection = surface.selectAll('g.sidedgroup > path'); + expect(selection.size()).to.eql(4); + expect(selection.nodes()[0].attributes['marker-mid'].nodeValue).to.eql('url(#sided-marker-natural)'); + expect(selection.nodes()[1].attributes['marker-mid'].nodeValue).to.eql('url(#sided-marker-coastline)'); + expect(selection.nodes()[2].attributes['marker-mid'].nodeValue).to.eql('url(#sided-marker-barrier)'); + expect(selection.nodes()[3].attributes['marker-mid'].nodeValue).to.eql('url(#sided-marker-man_made)'); + }); + + it('has no marker layer for two_sided way', function() { + var a = iD.osmNode({id: 'a', loc: [0, 0]}); + var b = iD.osmNode({id: 'b', loc: [1e-2, 0]}); + + var e_ts = iD.osmWay({id: 'explicit-two-sided', tags: {barrier: 'city_wall', two_sided: 'yes'}, nodes: [a.id, b.id]}); + + var graph = iD.coreGraph([a, b, e_ts]); + + surface.call(iD.svgLines(projection, context), graph, [e_ts], all); + var selection = surface.selectAll('g.sidedgroup > path'); + expect(selection.empty()).to.be.true; + }); + }); });