dfellis / h3-node

H3 binding to Node using N-API
MIT License
30 stars 5 forks source link

Add polyfill function #15

Closed dfellis closed 5 years ago

dfellis commented 5 years ago

This is not 100% done. I need to DRY up the code a bit, which currently consists of a lot of copy-pasted logic, and I need to figure out what's going on with the polyfill with holes test. It's not matching the h3-js results, but I suspect this may be a bug in h3-js and not h3-node?

dfellis commented 5 years ago

Should not have doubted the h3-js implementation. It is the correct one. Need to figure out what I've done wrong with the other.

H3-js

screenshot_20181125_101017

H3-node

screenshot_20181125_100814

dfellis commented 5 years ago

Fixed, now they match. There were three classes of issues -- overloading the i variable instead of using i and j, accidentally setting lng on the main geofence when setting the hole lng, and accidentally setting the hole data in a local copy of the hole instead of on the calloced hole record.

I think making the polyfill binding function easier to read can happen in another PR, along with general cleanup of the macros so they're more composable than they currently are.