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

Add protobuf support to @xviz/io #500

Merged
merged 2 commits into from
Jul 25, 2019
Merged

Add protobuf support to @xviz/io #500

merged 2 commits into from
Jul 25, 2019

Conversation

twojtasz
Copy link
Contributor

  • Initial support for a file with a magic header PBE1 followed by our protobuf envelope and other types.
  • Enumerations are not handled most efficiently so we are converting to a Javascript Object at a performance penalty in this change. Will address in a follow-up
  • Primitive color is normalized to an array of bytes prior to encoding in Protobuf. This creates some roundtrip testing issues. The plan is to change color in a follow-up to bytes.
  • We have a custom protobuf.js module that can decode arrays to TypedArrays, which will be addressed in a follow-up

@twojtasz twojtasz requested a review from jlisee July 17, 2019 23:57
Copy link
Contributor

@jlisee jlisee left a comment

Choose a reason for hiding this comment

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

See comments on #499 since I think the same code is in both PRs (probably meant to stack these)

@coveralls
Copy link

Coverage Status

Coverage increased (+1.9%) to 63.356% when pulling a1c1232 on tpw/add-protobuf-support into 0d1acce on master.

@twojtasz twojtasz merged commit 575b053 into master Jul 25, 2019
@twojtasz twojtasz deleted the tpw/add-protobuf-support branch September 5, 2019 01:02
alexhaislip pushed a commit to Smart-Ag/xviz that referenced this pull request Aug 2, 2021
- Support protobuf with @xviz/io reader, writer, provider implementations
- Format includes `PBE1` header followed by protobuf messages
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.

3 participants