Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

render the side tag #10303

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 34 additions & 14 deletions modules/osm/node.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { osmEntity } from './entity';
import { geoAngle, geoExtent } from '../geo';
import { utilArrayUniq } from '../util';
import { utilArrayUniqBy } from '../util';

export const cardinal = {
north: 0, n: 0,
Expand All @@ -21,6 +21,8 @@ export const cardinal = {
northnorthwest: 337, nnw: 337
};

const SIDES = new Set(['left', 'right', 'both']);

export function osmNode() {
if (!(this instanceof osmNode)) {
return (new osmNode()).initialize(arguments);
Expand Down Expand Up @@ -65,16 +67,28 @@ Object.assign(osmNode.prototype, {

// Inspect tags and geometry to determine which direction(s) this node/vertex points
directions: function(resolver, projection) {
var val;
/** @type {{ type: 'side' | 'direction'; value: string }[]} */
const rawValues = [];
var i;

// which tag to use?
if (this.isHighwayIntersection(resolver) && (this.tags.stop || '').toLowerCase() === 'all') {
// all-way stop tag on a highway intersection
val = 'all';
rawValues.push({
type: 'direction',
value: 'all',
});
} else {
// generic side tag
if (SIDES.has(this.tags.side?.toLowerCase())) {
rawValues.push({
type: 'side',
value: this.tags.side.toLowerCase(),
});
}

// generic direction tag
val = (this.tags.direction || '').toLowerCase();
let val = (this.tags.direction || '').toLowerCase();

// better suffix-style direction tag
var re = /:direction$/i;
Expand All @@ -85,31 +99,36 @@ Object.assign(osmNode.prototype, {
break;
}
}
for (const value of val.split(';')) {
rawValues.push({ type: 'direction', value });
}
}

if (val === '') return [];
if (!rawValues.length) return [];


var values = val.split(';');
/** @type {{ type: 'side' | 'direction'; angle: number }[]} */
var results = [];

values.forEach(function(v) {
rawValues.forEach(({ type, value: v }) => {
// swap cardinal for numeric directions
if (cardinal[v] !== undefined) {
v = cardinal[v];
}

// numeric direction - just add to results
if (v !== '' && !isNaN(+v)) {
results.push(+v);
results.push({ type: 'direction', angle: +v });
return;
}

const isSide = type === 'side' && SIDES.has(v);

// string direction - inspect parent ways
var lookBackward =
(this.tags['traffic_sign:backward'] || v === 'backward' || v === 'both' || v === 'all');
(this.tags['traffic_sign:backward'] || v === (isSide ? 'left' : 'backward') || v === 'both' || v === 'all');
var lookForward =
(this.tags['traffic_sign:forward'] || v === 'forward' || v === 'both' || v === 'all');
(this.tags['traffic_sign:forward'] || v === (isSide ? 'right' : 'forward') || v === 'both' || v === 'all');

if (!lookForward && !lookBackward) return;

Expand All @@ -130,14 +149,15 @@ Object.assign(osmNode.prototype, {

Object.keys(nodeIds).forEach(function(nodeId) {
// +90 because geoAngle returns angle from X axis, not Y (north)
results.push(
(geoAngle(this, resolver.entity(nodeId), projection) * (180 / Math.PI)) + 90
);
results.push({
type: isSide ? 'side' : 'direction',
angle: (geoAngle(this, resolver.entity(nodeId), projection) * (180 / Math.PI)) + (isSide ? 0 : 90)
Copy link
Collaborator

@1ec5 1ec5 Jul 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, though I wonder if reusing the “cone” would risk confusing some users. Some mappers are also using side=* on traffic_signals=* vertices. This change will cause these vertices’ cones to point away from the street, which would be inconsistent with the cones for standalone traffic_signals=* nodes. Plus, it might be necessary to combine side=* with direction=* in some cases. In the screenshot above, this would imply four different “faces” rather than two. Maybe side=* could act as more of a mask, chopping the directional cone in half?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering about how to visualize the side as well.

The direction is more of a "looking direction" or "affected area".

But the side is more of "an anchor" or a "connection point" or a "this belongs to over there" indicator. However I don't see a way to visualize those well. Especially when the actual "anchor point" is unknown (we only now "it is to the right").

The other UI we use to show direction of elements is on barrier=retaining_wall and such with the small arrows next to the line. Maybe this is something we can build upon for this UI? Like one of those arrows next to the node?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like one of those arrows next to the node?

Something like … image?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, though I wonder if reusing the “cone” would risk confusing some users.
[...]
Maybe side=* could act as more of a mask, chopping the directional cone in half?

That's a cool idea, although it might be confusing if side+direction renders differently to side and direction on their own? Here's what it could look like:

:
:

Like one of those arrows next to the node?

Something like this maybe?

:
:
:
:
T:

Any other ideas would be greatly appreciated...

});
}, this);

}, this);

return utilArrayUniq(results);
return utilArrayUniqBy(results, item => item.type + item.angle);
},

isCrossing: function(){
Expand Down
38 changes: 38 additions & 0 deletions modules/svg/defs.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,44 @@ export function svgDefs(context) {
.attr('stroke-width', '0.5px')
.attr('stroke-opacity', '0.75');

_defsSelection
.append('marker')
.attr('id', 'ideditor-viewfield-marker-side')
.attr('viewBox', '0 0 16 16')
.attr('refX', 8)
.attr('refY', 16)
.attr('markerWidth', 4)
.attr('markerHeight', 4)
.attr('markerUnits', 'strokeWidth')
.attr('orient', 'auto')
.append('path')
.attr('class', 'viewfield-marker-path')
.attr('d', 'M 6 14 C 8 13.4 8 13.4 10 14 L 14 7 L 14 5 L 2 5 L 2 7 Z')
.attr('fill', '#333')
.attr('fill-opacity', '0.75')
.attr('stroke', '#fff')
.attr('stroke-width', '0.5px')
.attr('stroke-opacity', '0.75');

_defsSelection
.append('marker')
.attr('id', 'ideditor-viewfield-marker-side-wireframe')
.attr('viewBox', '0 0 16 16')
.attr('refX', 8)
.attr('refY', 16)
.attr('markerWidth', 4)
.attr('markerHeight', 4)
.attr('markerUnits', 'strokeWidth')
.attr('orient', 'auto')
.append('path')
.attr('class', 'viewfield-marker-path')
.attr('d', 'M 6 14 C 8 13.4 8 13.4 10 14 L 14 7 L 14 5 L 2 5 L 2 7 Z')
.attr('fill', 'none')
.attr('stroke', '#fff')
.attr('stroke-width', '0.5px')
.attr('stroke-opacity', '0.75');


// add patterns
var patterns = _defsSelection.selectAll('pattern')
.data([
Expand Down
4 changes: 2 additions & 2 deletions modules/svg/vertices.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,8 +194,8 @@ export function svgVertices(projection, context) {
.attr('class', 'viewfield')
.attr('d', 'M0,0H0')
.merge(viewfields)
.attr('marker-start', 'url(#ideditor-viewfield-marker' + (wireframe ? '-wireframe' : '') + ')')
.attr('transform', function(d) { return 'rotate(' + d + ')'; });
.attr('marker-start', d => 'url(#ideditor-viewfield-marker' + (d.type === 'side' ? '-side' : '') + (wireframe ? '-wireframe' : '') + ')')
.attr('transform', d => `rotate(${d.angle})`);
}


Expand Down
6 changes: 6 additions & 0 deletions modules/util/array.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,12 @@ export function utilArrayGroupBy(a, key) {
}


/**
* @template T
* @param {T[]} a
* @param {string | ((item: T) => string)} key
* @returns {T[]}
*/
// Returns an Array with all the duplicates removed
// where uniqueness determined by the given key
// `key` can be passed as a property or as a key function
Expand Down
Loading
Loading