-
Notifications
You must be signed in to change notification settings - Fork 291
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
Implement importBrep and vtkPolyData export #735
Conversation
Codecov Report
@@ Coverage Diff @@
## master #735 +/- ##
==========================================
+ Coverage 94.64% 95.82% +1.17%
==========================================
Files 32 32
Lines 7249 8804 +1555
Branches 789 998 +209
==========================================
+ Hits 6861 8436 +1575
+ Misses 255 227 -28
- Partials 133 141 +8
Continue to review full report at Codecov.
|
I know it's early times for this implementation, but here are some comments on the parameters and defaults of Possible changes for the defaults:
Possible additional parameters:
Personally I found the tolerance=0.001 and angularTolerance of 0.3 was a great compromise of visual quality and tessellation duration. Some timings: The large RC example A small example: with def compute(shape, deviation=0.0001, angular_deviation=0.3, iso_lines=0, info=False):
vs = OCP.IVtkOCC.IVtkOCC_Shape(shape)
sd = OCP.IVtkVTK.IVtkVTK_ShapeData()
sm = OCP.IVtkOCC.IVtkOCC_ShapeMesher(
theDevCoeff=deviation,
theDevAngle=angular_deviation,
theNbUIsos=iso_lines,
theNbVIsos=iso_lines)
sm.Build(vs,sd)
return pv.PolyData(sd.getVtkPolyData()), sm |
Will look into this, but the defaults are based on current defaults for other ops. I don't want to change them just because. I also propose to skip the isolines completely. |
There are still some js glitches (I'll ask on the vtk.js forum), but I think that you can start reviewing. |
@adam-urbanczyk, sorry to annoy you with unsupported installation questions, but when I build this through Nix I get a single failed test for:
I think you've solved the same thing in CadQuery/OCP#47; did you make some extra changes outside of the OCP repo that I've missed? I'm using the latest CadQuery/OCP@b058865, CadQuery/pywrap@f8869e5 and pybind11 v2.6.1. My versions for clang, gcc, jinja2, pyparsing and everything else I can think of all match the logs from the latest OCP CI build. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adam-urbanczyk Thanks for all the work you've put into this!
This is not completely relevant to this PR, but:
In my OCP build process I was accidentally giving pywrap the includes from libc++ instead of libstdc++, so when pywrap generated the mangled name for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would like to have the default camera position as iso rather than top-down, but that can be another PR.
I think we can remove sphinxcadquery from environment.yml? There are also two more lines for sphinxcadquery in conf.py that could also be removed.
The build_docs.sh in the root dir is incorrect now, docs need to be built from the doc dir so the relative link to vslot_2020_1.dxf works.
Thanks so much for all this work @adam-urbanczyk!
Alright, thanks for all the comments. I propose to wait 24h with this and then squash/merge. |
No last second changes @adam-urbanczyk? I'll squash and merge in a bit then. |
That was a huge effort, thank you very much @adam-urbanczyk! |
I had not time to ask before merging but : Thank you in advance. |
Yes, it allows convert to vtk data structures and save as vtp or vtkjs. Additionally this PR implemented rendering in jupyter and our docs based on vtk.js. |
Also (de)serialization of cq.Shape objects to BytesIO - should be handy for caching |
Related to #650 and #281
Highlights
Rendering in jupyter notebooks: