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

remove point-feature #35

Open
jfirebaugh opened this issue Oct 9, 2015 · 4 comments
Open

remove point-feature #35

jfirebaugh opened this issue Oct 9, 2015 · 4 comments

Comments

@jfirebaugh
Copy link
Contributor

The use of point-feature and { x, y } objects seems out of place in such a low-level library. Can we switch to [x, y] coordinates for 2.0?

@mourner
Copy link
Member

mourner commented May 18, 2016

Two reasons to keep it:

  • {x, y} is more performant and takes less memory (apparently V8 reserves more than 2 items for each coordinate internally)
  • we need the point abstraction in GL JS, and returning [x, y] would force us to add a conversion step in GL JS, along with GC churn

If you view the library separately from GL JS though, it's indeed weird to have a point type. One thing I considered doing is accepting a point factory in the VT constructor (defaulting to (x, y) => [x, y]), which may be best of both worlds.

@jfirebaugh
Copy link
Contributor Author

I'm assuming we'd convert gl-js to use [x, y] internally, and use the Point type only for things like cursor coordinates (if even that).

@mourner
Copy link
Member

mourner commented May 18, 2016

My first point about performance is still valid, and we do a huge ton of coordinate math throughout the code, so rewriting everything for [x, y] and static point arithmetic functions would take a big effort.

@jfirebaugh
Copy link
Contributor Author

Profiling issues like mapbox/mapbox-gl-js#5208, the GC pressure from allocating lots of temporary arrays and tiny objects is substantial.

image

I would like to explore an alternate API for v2 based on either ES6 generators or passing in a function to be called for each geometry element (each ring and point). ES6 generators would be more ergonomic, but a callback approach would be more compatible with legacy browsers (IE11) and also avoid the overhead of Point allocations (you could pass x and y as individual parameters).

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

No branches or pull requests

2 participants