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

Respect text-offset in line-placed symbol collision handling #4798

Open
anandthakker opened this issue Jun 7, 2017 · 4 comments
Open

Respect text-offset in line-placed symbol collision handling #4798

anandthakker opened this issue Jun 7, 2017 · 4 comments
Assignees
Labels

Comments

@anandthakker
Copy link
Contributor

We currently do not respect the value of text-offset when determining whether two symbol instances collide.

Example -- in this style:

{
  "version": 8,
  "metadata": {
    "test": {
      "height": 64,
      "collisionDebug": true
    }
  },
  "center": [ 0, 0 ],
  "zoom": 0,
  "sources": {
    "geojson": {
      "type": "geojson",
      "data": {
        "type": "Feature",
        "properties": {},
        "geometry": {
          "type": "LineString",
          "coordinates": [
            [-20, -20],
            [20, -20],
            [20, 20],
            [-20, 20]
          ]
        }
      }
    }
  },
  "glyphs": "mapbox://fonts/mapbox/{fontstack}/{range}.pbf",
  "layers": [
    {
      "id": "guid",
      "type": "line",
      "source": "geojson"
    },
    {
      "id": "text",
      "type": "symbol",
      "source": "geojson",
      "layout": {
          "symbol-placement": "line",
          "symbol-spacing": 115,
          "text-size": 10,
          "text-allow-overlap": false,
          "text-ignore-placement": false,
          "text-field": "A",
          "text-font": [
              "Open Sans Semibold",
              "Arial Unicode MS Bold"
          ],
          "text-keep-upright": false,
          "text-offset": [0, -3]
      }
    }
  ]
}

We'd expect only one of the 'A' instances to be rendered, but both are:

screen shot 2017-06-07 at 8 28 18 am

@anandthakker
Copy link
Contributor Author

The above JSON should work verbatim as an integration test -- I'm holding off on adding it because I'm not sure whether it's well defined which of those two labels should be shown.

cc @ansis @ChrisLoer

@ChrisLoer
Copy link
Contributor

ChrisLoer commented Jun 7, 2017

It's not totally ignored: it feeds into the "top" and "bottom" values of the shaped text, which feeds into CollisionFeature as the shaped parameter. For point labels, those values are fed into the single collision box, but they're essentially ignored for line labels (they feed into "height" and "width" but the offset won't make a difference).

Empirically, it's still partially broken for point labels:

screenshot 2017-06-07 09 09 31

Maybe something to do with boxScale being in different units then we're using at shaping time... that stuff is hard to keep track of. (Looks like it's pretty much accurate at zoom levels <n>.99, so that's a pretty good hint)

@ChrisLoer
Copy link
Contributor

I've started looking at this in conjunction with #6075 (combing text-offset with text-rotate). Point placement is looking pretty straightforward, line placement may still be a bit tricky.

@ChrisLoer ChrisLoer changed the title Respect text-offset in symbol collision handling Respect text-offset in line-placed symbol collision handling May 7, 2018
@ChrisLoer
Copy link
Contributor

I'm going to address point-placed symbols in #6075, and leave this as the line-placed version of the bug. I started exploring an implementation, and it's tractable but a bit more than I'm ready to take on right now. Because we don't know what the offset will be until we actually project the line, we have to modify collision circles on the fly at the time we're doing projection in placeCollisionCircles. To make the collision debug shaders accurate, we'd have to implement logic similar to what the symbol shaders currently do for calculating offsets.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants