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

Vector tile data rendered incorrectly if extent != 4096 #1093

Closed
Tracked by #229
paulmach opened this issue Mar 15, 2015 · 7 comments · Fixed by #2010
Closed
Tracked by #229

Vector tile data rendered incorrectly if extent != 4096 #1093

paulmach opened this issue Mar 15, 2015 · 7 comments · Fixed by #2010
Assignees
Labels

Comments

@paulmach
Copy link

It looks like all vector tiles get rendered the same regardless of the extent value in the pbf file. By default this value is 4096 so usually everything is fine. However, tippecanoe generates tiles with extents of 1024 and 2048 and those display incorrectly in mapbox-gl-js

I created an example gist (sorry no live version) that renders 3 vector tiles, each with the same short linestring data. The only difference is the extent values in the file (no other values changed or scaled). All are rendered on top of each other, when that is not correct. https://gist.github.com/paulmach/14fcf55bc67e8abdff62 Here is what it looks like:
screen shot 2015-03-15 at 12 04 02 pm

FYI @ericfischer

@incanus
Copy link
Contributor

incanus commented Mar 15, 2015

Native side: mapbox/mapbox-gl-native#250

@tmcw
Copy link
Contributor

tmcw commented Jun 1, 2015

https://github.com/mapbox/mapbox-gl-js/blob/master/js/render/painter.js#L137

the magic number is used for raster and video rendering. Rendering raster and video is just copying a rectangular texture to part of the screen. Each vertex of this rectangle needs to have a position (where on the screen it is), and which part of the texture it is. For some reason that number is used to represent the right/bottom edge of the texture


setup() doesn't need to be called again because it contains a lot of stuff that only runs once. I think instead of having one tileExtentBuffer there needs to be one for every extent. An object that indexes the buffers by extent maybe.
Or this could be handled on the matrix side by accounting for the difference there. But I think that might be even worse.
The buffers created in setup can be created sometime later, outside of setup()
maybe lazily, with painter.getTileExtentBuffer(extent) that creates it if it's missing

@tmcw tmcw self-assigned this Jun 1, 2015
@tmcw
Copy link
Contributor

tmcw commented Jun 1, 2015

#1227 was my first shot at this and I'm restarting it with a new branch.

@tmcw
Copy link
Contributor

tmcw commented Jun 2, 2015

With #1234 merged:

We now have support for vector tiles that have an extent that isn't 4096.

Still TODO is supporting vector tiles with multiple extents. In order to support that, we'll need to split the Tile class so that calculateMatrices doesn't have to constantly recalculate, and split the extent that is output by the WorkerTile to an object of extents, indexed by id || ref.

@e-n-f
Copy link
Contributor

e-n-f commented Aug 19, 2015

@ansis, here's an example of the problem persisting: http://trafficways.org/gltest/gl.html

(copied from https://www.mapbox.com/mapbox-gl-js/examples/ except for adding my layer)

It looks reasonable at low zooms, but z12 and beyond are garbage. The vector data is enf.fwimgqfr, and I believe has an extent of 1024 at low zooms and 16384 at z12.

Made with

./tippecanoe -f -z12 -o ala12.mbtiles tl_2014_06001_roads.json

from TIGER 2014 data.

It looks like an overflow problem, where coordinates that are supposed to be a little beyond the tile boundaries to the right or bottom become massively negative instead.

@e-n-f
Copy link
Contributor

e-n-f commented Aug 19, 2015

Must be the Int16Array that will overflow when something goes beyond 32767?

@jfirebaugh
Copy link
Contributor

91d2e0e added another hard-coded 4096.

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

Successfully merging a pull request may close this issue.

6 participants