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

Switch to sint32 for coords #24

Closed
mourner opened this issue Dec 10, 2014 · 6 comments
Closed

Switch to sint32 for coords #24

mourner opened this issue Dec 10, 2014 · 6 comments

Comments

@mourner
Copy link
Member

mourner commented Dec 10, 2014

Since we use 1e6 encoding, it might make sense to switch coords from int64 to int32 for better geometry compression.

int32 range is -2147,483,648 through 2147,483,647, which is plenty of headroom for the usual -180..180 plus a handful of repeating worlds. We probably should not care about other CRS because it's going to be ditched out of the GeoJSON spec.

@mourner
Copy link
Member Author

mourner commented Dec 10, 2014

Actually, if we use delta encoding, we need sint32 instead of int32 to use zigzag encoding: https://developers.google.com/protocol-buffers/docs/encoding

@mourner mourner changed the title Switch to int32 for coords Switch to sint32 for coords Dec 10, 2014
@brendan-ward
Copy link

From my testing, using sint64 only had an effect on resultant file size using protocol-buffers not the current implementation on ProtobufJS (it appeared to be silently ignored there). Not sure about the effect of sint32, I didn't test that.

I'm in favor of this change.

@mourner
Copy link
Member Author

mourner commented Dec 10, 2014

@brendan-ward interesting! one more reason to switch to protocol-buffers. The size different will be huge once we do #23.

@kkaefer
Copy link

kkaefer commented Dec 11, 2014

@mourner there's no difference in the on-wire represenation between sint32 and sint64. The only difference is for implementations, e.g. Google's Protobuf implementation provides a int64 when decoding (and requires one when encoding). For JavaScript implementations, there should be no difference since all numbers are stored as doubles anyway (or as int32 if they're small integers, depending on the JS implementation).

@mourner
Copy link
Member Author

mourner commented Dec 11, 2014

@kkaefer yeah, I originally meant int64 to int32, and there would be difference for negative numbers, but then learned about zig-zag encoding of sint. We can do either sint32 or sint64.

@mourner
Copy link
Member Author

mourner commented Dec 16, 2014

Closing in favor of #27

@mourner mourner closed this as completed Dec 16, 2014
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

3 participants