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

Add extent support. Refs #1227 #1234

Merged
merged 4 commits into from
Jun 2, 2015
Merged

Add extent support. Refs #1227 #1234

merged 4 commits into from
Jun 2, 2015

Conversation

tmcw
Copy link
Contributor

@tmcw tmcw commented Jun 1, 2015

This works on my example style.

  • Adds a cache for the typed arrays that set extents
  • Fixes drawing ratio of lines to match different extents

Main point of uncertainty:

  • This adds support for a single extent value per vector tile. While
    multiple values are technically permitted there aren't any tiles that
    create them yet.

Could use a review /cc @ansis @jfirebaugh @kkaefer

This works on my example style.

* Adds a cache for the typed arrays that set extents
* Fixes drawing ratio of lines to match different extents

Main point of uncertainty:

* This adds support for **a single extent value per vector tile**. While
  multiple values are technically permitted there aren't any tiles that
  create them yet.

Could use a review /cc @ansis @jfirebaugh @kkaefer
@tmcw tmcw mentioned this pull request Jun 1, 2015
@jfirebaugh
Copy link
Contributor

For terminology, we should reserve the word "buffer" for the GL concept and call the backing arrays "arrays" or "buffer arrays". We probably don't do this elsewhere but we should.

I don't think it's worth caching the array creation -- it'll be heavily dominated by gl.createBuffer / gl.bufferData. If you want to cache something, cache the buffer.

@tmcw
Copy link
Contributor Author

tmcw commented Jun 1, 2015

Okay - given that caching the buffer seems pretty tricky and this seems to perform pretty well, i'll remove the cache.

@tmcw
Copy link
Contributor Author

tmcw commented Jun 1, 2015

Removed cache in 598891b

@kkaefer
Copy link
Contributor

kkaefer commented Jun 2, 2015

This adds support for a single extent value per vector tile. While multiple values are technically permitted there aren't any tiles that create them yet.

Is there major effort associated with doing it per-layer? While there currently may not be vector tiles that specify multiple extents, compositing vector tiles may easily produce such tiles.

@tmcw
Copy link
Contributor Author

tmcw commented Jun 2, 2015

@kkaefer mostly it's just extremely hard to follow how a "source layer" becomes what we actually paint and where i would attach this data to make that possible. If you can give a pointer or two in that direction it's probably not terribly hard to implement.

@tmcw tmcw changed the title Add extent support. Fixes #1227 Add extent support. Refs #1227 Jun 2, 2015
@tmcw
Copy link
Contributor Author

tmcw commented Jun 2, 2015

@ansis @kkaefer Dove into multi-extent-per-tile in #1236 and figured out that it entails a much larger refactor, since we'd have to split Tile into two classes, one for the tile id and the other for buffers & transforms.

I think we should land this PR now, refactor & document gl-js, and then implement support for multiple extents in a single tile. Changed this one to Refs #1227

tmcw added a commit that referenced this pull request Jun 2, 2015
@tmcw tmcw merged commit 293809b into master Jun 2, 2015
@tmcw tmcw deleted the extent-support branch June 2, 2015 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants