h3 icon indicating copy to clipboard operation
h3 copied to clipboard

Refactor `bbox.c` code to use radians instead of kilometers

Open ajfriend opened this issue 5 years ago • 3 comments

For example:

https://github.com/uber/h3/blob/cd34251c87cae84998b06053fe9230ab1c939339/src/h3lib/lib/bbox.c#L100-L111

Radians would be more natural, especially when considering spheres that aren't the Earth.

Also, we might want to use the new area/length functions for things like pentagonAreaKm2 and pentagonRadiusKm in the code?

ajfriend avatar Aug 25 '20 07:08 ajfriend

This is an estimator function to make sure we allocate enough memory to fill a bounding box for polyfill. It's intentionally undersized (the 0.8) to make sure that it never underestimates the memory needs. I wanted to just have this baked into the polyfill algorithm as that's the only thing using it, but it needed access to functions (like _hexRadiusKm) that weren't available outside of the bbox.c file.

dfellis avatar Aug 25 '20 14:08 dfellis

So I would argue refactoring this wouldn't actually get us anything, and it's best to leave it alone.

dfellis avatar Aug 25 '20 14:08 dfellis

I'm just wondering if radians is a more natural unit for a general sphere, and that the area functions would simplify some of the math that you're doing manually in the code.

Not trying to get a tighter estimate, just trying to simplify the code for readability, and avoid a question like "But hey, does this work on Mars?".

ajfriend avatar Aug 25 '20 18:08 ajfriend