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 big-endian binary stl reading #26

Merged
merged 4 commits into from Apr 12, 2018

Conversation

Projects
None yet
3 participants
@hyperair
Contributor

hyperair commented Oct 1, 2017

This PR fixes a deserialization issue (all floats end up wrong) that occurs when reading binary STLs on a big-endian machine, e.g. powerpc.

Fix reading of binary STLs in BE architectures
Also use uint32_t instead of int for header_num_facets as per the binary STL
spec.
@hroncok

This comment has been minimized.

Member

hroncok commented Oct 1, 2017

Thanks for you contribution. However, this doesn't compile on Mac, and it produces some errors with -Werror. Would you be able to fix those problems?

@hyperair

This comment has been minimized.

Contributor

hyperair commented Oct 1, 2017

@hroncok Could you fetch again? I think I've already fixed them.

@hroncok

This comment has been minimized.

Member

hroncok commented Oct 1, 2017

I'm not fetching, I'm watching the Travis CI integration here.

@hyperair

This comment has been minimized.

Contributor

hyperair commented Oct 1, 2017

Oh oops, looks like a different error from the one I fixed. I'll have a look, thanks.

hyperair added some commits Oct 1, 2017

Change comment style of portable_endian.h to /* */
Fixes this warning:
src/portable_endian.h:1:1: error: C++ style comments are not allowed in ISO C90 [-Werror]
@hyperair

This comment has been minimized.

Contributor

hyperair commented Oct 1, 2017

@hroncok It's green now.

@hroncok

This comment has been minimized.

Member

hroncok commented Oct 1, 2017

Great job! Do you need this to be included in the 0.98.x series, or merging to master and released "when ready" is fine?

@hroncok hroncok self-requested a review Oct 1, 2017

@hyperair

This comment has been minimized.

Contributor

hyperair commented Oct 1, 2017

hyperair added a commit to hyperair/Slic3r that referenced this pull request Oct 1, 2017

Partially revert the disabling of admesh on big-endian architectures
Partially revert 2085a48. This is in
preparation for applying the upstream fix from admesh at
admesh/admesh#26

hyperair added a commit to hyperair/Slic3r that referenced this pull request Oct 1, 2017

hyperair added a commit to hyperair/Slic3r that referenced this pull request Oct 1, 2017

lordofhyphens added a commit to slic3r/Slic3r that referenced this pull request Oct 1, 2017

@evansque

This comment has been minimized.

evansque commented Apr 12, 2018

Nice going @hyperair
@hroncok will this be merged anytime soon?

@hroncok

This comment has been minimized.

Member

hroncok commented Apr 12, 2018

This has slipped from my radar. I'll merge today.

@hroncok hroncok merged commit 64e86ea into admesh:master Apr 12, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hroncok

This comment has been minimized.

Member

hroncok commented Apr 12, 2018

Let's keep this in master for a while before it's merged to 0.98.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment