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

fix zig-zag decoding #51

Merged
merged 1 commit into from Jul 28, 2022

Conversation

blaugold
Copy link
Contributor

To make bitwise operations platform independent the
docs
recommend coercing to signed 32 bit integers.

Fixes #7

To make bitwise operations platform independent the
[docs](https://dart.dev/guides/language/numbers#what-should-you-do)
recommend coercing to signed 32 bit integers.

Fixes greensopinion#7
@@ -24,7 +24,7 @@ _Command _decodeCommand(int command) {
int _decodeCommandLength(int command) => command >> 3;

@pragma('vm:prefer-inline')
int _decodeZigZag(int value) => (value >> 1) ^ -(value & 1);
int _decodeZigZag(int value) => ((value >> 1) ^ -(value & 1)).toSigned(32);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, this looks fantastic.

The vector tile spec 2.1 indicates:

A geometry is encoded as a sequence of 32 bit unsigned integers

Any concern with using signed 32-bit integers? Wondering if it might affect the results.

Also curious as to why toSigned(32) is applied after the bitwise operations, rather than before. Should we be doing something like this instead?

int _decodeZigZag(int value) {
  final value32 = value.toSigned(32)
  return (value32 >> 1) ^ -(value32 & 1);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The decoded parameter values fit into signed 32-bit integers:

Parameter values greater than pow(2,31) - 1 or less than -1 * (pow(2,31) - 1) are not supported.

The results of bitwise operations are different between native and web because of the different underlying representations. I'm not sure how this works exactly, but for 32-bit chunks we can get consistent results everywhere by interpreting the results of bit-wise operations as signed 32-bit numbers.

For bit manipulation, consider explicitly operating on 32-bit chunks, which are consistent on all platforms. To force a signed interpretation of a 32-bit chunk, use int.toSigned(32).

@greensopinion
Copy link
Owner

Screenshot with this and #52 in a web browser 🎉

Screen Shot 2022-07-27 at 6 15 48 PM

Still appears to work fine with this and [vector_map_tiles](https://github.com/greensopinion/flutter-vector-map-tiles) (mac application tested)

Screen Shot 2022-07-27 at 6 18 36 PM

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.

Rendering on web looks broken
2 participants