-
Notifications
You must be signed in to change notification settings - Fork 82
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
Update registry.jl #388
Update registry.jl #388
Conversation
Change to stl binary/ascii type detection. Removed check on value of final attr bits, which caused a failure to parse for me, incorrectly treating it as an ascii type stl. If the eof is in the correct place, this value does not matter.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #388 +/- ##
==========================================
- Coverage 91.69% 91.66% -0.03%
==========================================
Files 10 10
Lines 710 708 -2
==========================================
- Hits 651 649 -2
Misses 59 59 ☔ View full report in Codecov by Sentry. |
Could you add a small test file? |
Yeah sure. Do you mean:
|
Add a test file here: Lines 356 to 366 in d30fbd7
|
…ctual attribute data. Should match as a binary STL, rather than an ASCII STL.
I've included the file as a test case. Please let me know if you would like any other work, or if you can support merging. I guess eventually supporting STLs with attribute data would be nice, but for this specific problem (a binary stl, with nonzero 'attribute byte count' but actually containing no attribute byte data), things work correctly rather than being misconstrued as an ASCII stl. |
Just bumping this again @SimonDanisch @sjkelly It works, the test is in, the change non-breaking. If there is anything I can do to help get this merged let me know. |
Change to stl binary/ascii type detection. Removed check on value of final attr bits, which caused a failure to parse for me, incorrectly treating it as an ascii type stl. If the eof is in the correct place, this value does not matter.
@SimonDanisch Maybe as a MeshIO contributor you can give this a quick look? Its a tiny change!