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

Street name labeling obscuring node features #4271

Closed
simonpoole opened this issue Aug 25, 2017 · 7 comments
Closed

Street name labeling obscuring node features #4271

simonpoole opened this issue Aug 25, 2017 · 7 comments
Labels
usability An issue with ease-of-use or design

Comments

@simonpoole
Copy link
Contributor

See http://www.openstreetmap.org/edit?editor=id#map=21/47.40755/8.54858 there is actually a pedestrian crossing already there, but the node can't be seen (a slightly more prominent icon would address the issue too).

@bhousel
Copy link
Member

bhousel commented Aug 25, 2017

I agree, the label placement should avoid tagged nodes..

@bhousel bhousel added the usability An issue with ease-of-use or design label Aug 25, 2017
@pnorman
Copy link
Contributor

pnorman commented Sep 9, 2017

image

image

@bhousel bhousel added the good first issue Best for first-time contributors. No experience necessary! label Sep 18, 2017
@bhousel
Copy link
Member

bhousel commented Sep 28, 2017

Relevant code is here for anyone who wants to work on this..

@Nitiquita
Copy link

Hi @simonpoole, can you clarify what is being obscured by the street name? Thanks!

@bhousel
Copy link
Member

bhousel commented Oct 7, 2017

Hi @Nitiquita this code is a bit advanced for a get started issue, but I'll try to break down what's happening.

This code in map.js draws all the map stuff - drawLabels is a render function that gets passed all the map data in an array called entities:

iD/modules/renderer/map.js

Lines 296 to 302 in 7e98d7f

surface.selectAll('.data-layer-osm')
.call(drawVertices, graph, data, filter, map.extent(), map.zoom())
.call(drawLines, graph, data, filter)
.call(drawAreas, graph, data, filter)
.call(drawMidpoints, graph, data, filter, map.trimmedExtent())
.call(drawLabels, graph, data, filter, dimensions, !difference && !extent)
.call(drawPoints, graph, data, filter);

Inside drawLabels, we have a spatial index called rdrawn (see the rbush repo for details) - it will store bounding boxes around where the text labels can go. There are lots of calls to tryInsert() scattered throughout this file - this is how we add labels to the rdrawn index as long as they don't collide with anything else.

iD/modules/svg/labels.js

Lines 275 to 278 in 7e98d7f

if (fullRedraw) {
rdrawn.clear();
rskipped.clear();
entitybboxes = {};

One of the first things the drawLabels code does is it loops through all the entities to decide which ones get labels:

iD/modules/svg/labels.js

Lines 293 to 316 in 7e98d7f

// Split entities into groups specified by labelStack
for (i = 0; i < entities.length; i++) {
entity = entities[i];
geometry = entity.geometry(graph);
if (geometry === 'vertex') { geometry = 'point'; } // treat vertex like point
var preset = geometry === 'area' && context.presets().match(entity, graph),
icon = preset && !blacklisted(preset) && preset.icon;
if (!icon && !utilDisplayName(entity))
continue;
for (k = 0; k < labelStack.length; k++) {
var matchGeom = labelStack[k][0],
matchKey = labelStack[k][1],
matchVal = labelStack[k][2],
hasVal = entity.tags[matchKey];
if (geometry === matchGeom && hasVal && (matchVal === '*' || matchVal === hasVal)) {
labelable[k].push(entity);
break;
}
}
}

☝️ My thought is that inside the if (geometry === vertex) { ... } block, you could add code like this to block out some space around "interesting" vertices, so that no labels will get drawn on top of there:

if (entity.hasInterestingTags()) { 
  var coord = projection(entity.loc);
  var pad = 5;    // pixels of padding around the vertex
  var bbox = { 
     minX: coord[0] - pad, 
     minY: coord[1] - pad, 
     maxX: coord[0] + pad, 
     maxY: coord[1] + pad 
  };
  rdrawn.insert(bbox);
}

To get more insight into what the label placement code is doing, you can open a developer console and type id.setDebug('collision') - this will turn on debugging that draws the label placement boxes.

@bhousel bhousel added help wanted For intermediate contributors, requires investigation or knowledge of iD code and removed good first issue Best for first-time contributors. No experience necessary! labels Oct 12, 2017
@bhousel bhousel added wip Work in progress and removed help wanted For intermediate contributors, requires investigation or knowledge of iD code labels Dec 13, 2017
bhousel added a commit that referenced this issue Dec 13, 2017
(re: #4271, #3636)

- better classification of "interesting" vertices
  (include tagged, selected, or child of selected)
- now we can draw labels on selected lines again (revert #3636)
  because the labels will avoid the vertices
- if debugging is on, draw a collision box for the mouse
@bhousel
Copy link
Member

bhousel commented Dec 13, 2017

I did some work on this today and it's a lot better..

006ee69 Adds label avoidance boxes for all points
d9e3367 Adds label avoidance boxes for interesting vertices (tagged, selected, child of selected)
This also reverts #3636, because we can draw labels on selected ways now that the labels will avoid the vertices.

This work will land with #4602, which changes a bit how vertices are rendered.

labels avoid vertices

@bhousel
Copy link
Member

bhousel commented Jan 2, 2018

done in #4602

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
usability An issue with ease-of-use or design
Projects
None yet
Development

No branches or pull requests

4 participants