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

OFF import: Handle binary format #4994

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mmuman
Copy link
Contributor

@mmuman mmuman commented Feb 17, 2024

Finally found a way to generate binary OFF files:
https://github.com/pmp-library/pmp-library

However it seems it saves them as little-endian, while the specs says it should be big-endian. So we just try and support both, and do best-effort at autodetecting it.

Validated with all the previous samples.

@kintel
Copy link
Member

kintel commented Feb 17, 2024

Could you add a binary file as a test?

@mmuman
Copy link
Contributor Author

mmuman commented Feb 17, 2024

If you want to try, you need pmp-library/pmp-library#197 then:

mkdir build
cd build
cmake ..
make -j
./mconvert -b -i foo.off -o foo.b.off

@mmuman
Copy link
Contributor Author

mmuman commented Feb 17, 2024

Could you add a binary file as a test?

Sure. Not sure where to add that though, are they auto-generated at build time?

@mmuman
Copy link
Contributor Author

mmuman commented Feb 17, 2024

Seems macOS doesn't like the IEEE-754 test… I wanted to make sure we'd get compatible floats when reading…

@kintel
Copy link
Member

kintel commented Feb 17, 2024

For tests, read https://github.com/openscad/openscad/blob/master/doc/testing.txt
..and look at e.g. the OBJ file tests as an example:
https://github.com/openscad/openscad/blob/master/tests/data/obj/cube.obj
https://github.com/openscad/openscad/blob/master/tests/data/obj/dodecahedron.obj

list(APPEND EXPORT_OBJ_TEST_FILES ${TEST_SCAD_DIR}/obj/obj-import-export_cube.scad)

@kintel
Copy link
Member

kintel commented Feb 17, 2024

Looks like you're just missing parentheses for the macOS issue

@mmuman
Copy link
Contributor Author

mmuman commented Feb 17, 2024

Looks like you're just missing parentheses for the macOS issue

No it also gives the error

Finally found a way to generate binary OFF files:
https://github.com/pmp-library/pmp-library

However it seems it saves them as little-endian, while the specs says it
should be big-endian. So we just try and support both, and do
best-effort at autodetecting it.
vertcube.off from https://people.sc.fsu.edu/~jburkardt/data/off/off.html

This file has colors, comments, and some whitespace

vertcube.le.b.off is the binary converted from it with mconvert from
PMP-library, as little-endian.
Seems the STL import just assumes everyone has 32bit standard floats.
#error Byte order undefined or unknown. Currently only BOOST_ENDIAN_BIG_BYTE and BOOST_ENDIAN_LITTLE_BYTE are supported.
#endif

// Specs says data is big-endian, however PMP-library used little-endian
Copy link
Member

Choose a reason for hiding this comment

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

I would say this is a PMP bug. Should we really follow that example and support software outputting buggy files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It depends if it's been used widely or not…

@mmuman
Copy link
Contributor Author

mmuman commented Feb 17, 2024

Let's see how they handle pmp-library/pmp-library#198

@kintel
Copy link
Member

kintel commented Mar 6, 2024

Looks like the PMP issue was resolved

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

Successfully merging this pull request may close these issues.

None yet

2 participants