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

Implement importBrep and vtkPolyData export #735

Merged
merged 59 commits into from
Jun 22, 2021
Merged

Implement importBrep and vtkPolyData export #735

merged 59 commits into from
Jun 22, 2021

Conversation

adam-urbanczyk
Copy link
Member

@adam-urbanczyk adam-urbanczyk commented Apr 20, 2021

Related to #650 and #281

  • Write to brep
  • Stream support
  • toVtkPolyData
  • repr_javascript
  • disable keyboard interaction in notebooks
  • vtk from assy
  • cq_directive based on vtk.js
  • tests

Highlights

Rendering in jupyter notebooks:
image

@adam-urbanczyk adam-urbanczyk marked this pull request as draft April 20, 2021 06:59
@codecov
Copy link

codecov bot commented Apr 20, 2021

Codecov Report

Merging #735 (06b683a) into master (0f249df) will increase coverage by 1.17%.
The diff coverage is 98.23%.

Impacted file tree graph

@@            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     
Impacted Files Coverage Δ
cadquery/occ_impl/shapes.py 93.76% <91.89%> (+1.38%) ⬆️
cadquery/occ_impl/assembly.py 96.03% <97.67%> (+0.79%) ⬆️
cadquery/assembly.py 92.72% <100.00%> (+1.02%) ⬆️
cadquery/cq.py 92.30% <100.00%> (+0.98%) ⬆️
cadquery/cqgi.py 81.55% <100.00%> (+0.15%) ⬆️
cadquery/occ_impl/exporters/__init__.py 93.33% <100.00%> (+0.26%) ⬆️
cadquery/occ_impl/exporters/assembly.py 100.00% <100.00%> (ø)
cadquery/occ_impl/exporters/vtk.py 100.00% <100.00%> (ø)
cadquery/occ_impl/geom.py 88.67% <100.00%> (+0.17%) ⬆️
cadquery/occ_impl/jupyter_tools.py 100.00% <100.00%> (ø)
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0f249df...06b683a. Read the comment docs.

@bernhard-42
Copy link
Contributor

I know it's early times for this implementation, but here are some comments on the parameters and defaults of toVtkPolyData:

Possible changes for the defaults:

  • angularTolerance
    Opencascade recommends for the angular deflection a degree angle of 12-20°, which is 0.21-0.35 rad ("Angular deflection is relatively simple and allows using a default value (12-20 degrees)"). Since going down to 0.1 (i.e. 5.7°) comes with a significant time penalty (see below) and people might just use the default, one could consider to set the default to 0.2 or even 0.3

  • tolerance
    From my understanding, the shape mesher determines the linear deflection according to the shape size and uses the tolerance arg to refine it (see examples below). Since it is not that obvious for users how to select it (and the timings are not so critical) maybe setting a default like 0.001 would be helpful?

Possible additional parameters:

  • triangulate: bool, default=False
    if True, apply vtkTriangleFilter on the poly data (for us pythreejs users :-) )

  • isolines: int, default=0
    Would it be an idea to expose one parameter isolines for u an v so that a user could select wether to get Iso lines (isolines>0) or not (isolines=0)?

  • parallel: bool, default=False
    For larger objects having a possibility to leverage multicore CPUs would be nice.

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

image

A small example:

image

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

@adam-urbanczyk
Copy link
Member Author

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.

@adam-urbanczyk adam-urbanczyk marked this pull request as ready for review June 14, 2021 16:04
@adam-urbanczyk
Copy link
Member Author

There are still some js glitches (I'll ask on the vtk.js forum), but I think that you can start reviewing.

@marcus7070
Copy link
Member

@adam-urbanczyk, sorry to annoy you with unsupported installation questions, but when I build this through Nix I get a single failed test for:

> =================================== FAILURES ===================================
> ______________________ TestCadQuery.testBrepImportExport _______________________
> [gw6] linux -- Python 3.8.9 /nix/store/q6gfck5czr67090pwm53xrdyhpg6bx67-python3-3.8.9/bin/python3.8
> self = <tests.test_cadquery.TestCadQuery testMethod=testBrepImportExport>
>     def testBrepImportExport(self):
>         # import/export to file
>         s = Workplane().box(1, 1, 1).val()
>         s.exportBrep("test.brep")
>         si = Shape.importBrep("test.brep")
>         self.assertTrue(si.isValid())
>         self.assertAlmostEqual(si.Volume(), 1)
>         # import/export to BytesIO
>         from io import BytesIO
>         bio = BytesIO()
> >       s.exportBrep(bio)
> tests/test_cadquery.py:4561:
> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
> self = <cadquery.occ_impl.shapes.Solid object at 0x7fff8e2b7fa0>
> f = <_io.BytesIO object at 0x7fff8e5b7c20>
>     def exportBrep(self, f: Union[str, BytesIO]) -> bool:
>         """
>         Export this shape to a BREP file
>         """
> >       rv = BRepTools.Write_s(self.wrapped, f)
> E       TypeError: Write_s(): incompatible function arguments. The following argument types are supported:
> E           1. (Sh: OCP.TopoDS.TopoDS_Shape, File: str, theProgress: OCP.Message.Message_ProgressRange = <OCP.Message.Message_ProgressRange object at 0x7fffe19a5bf0>) -> bool
> E       
> E       Invoked with: <OCP.TopoDS.TopoDS_Solid object at 0x7fff8e2d5c70>, <_io.BytesIO object at 0x7fff8e5b7c20>
> cadquery/occ_impl/shapes.py:468: TypeError

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.

Copy link
Member

@jmwright jmwright left a 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!

cadquery/cq.py Show resolved Hide resolved
cadquery/occ_impl/assembly.py Show resolved Hide resolved
cadquery/occ_impl/exporters/__init__.py Show resolved Hide resolved
cadquery/occ_impl/exporters/assembly.py Show resolved Hide resolved
cadquery/occ_impl/shapes.py Show resolved Hide resolved
@marcus7070
Copy link
Member

This is not completely relevant to this PR, but:

when I build this through Nix I get a single failed test for:

> =================================== FAILURES ===================================
> ______________________ TestCadQuery.testBrepImportExport _______________________

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 OCP.BRepTools.BRepTools.Write_s(... iostream ...) it had a different name to what was in symbols_mangled_linux.dat and was being dropped. I corrected my pywrap includes and I now have the correct overloaded methods for BRepTools.Write_s.

Copy link
Member

@marcus7070 marcus7070 left a 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!

cadquery/occ_impl/shapes.py Outdated Show resolved Hide resolved
cadquery/occ_impl/exporters/assembly.py Outdated Show resolved Hide resolved
cadquery/occ_impl/exporters/vtk.py Show resolved Hide resolved
cadquery/occ_impl/assembly.py Show resolved Hide resolved
cadquery/occ_impl/shapes.py Show resolved Hide resolved
doc/assy.rst Outdated Show resolved Hide resolved
@adam-urbanczyk
Copy link
Member Author

Alright, thanks for all the comments. I propose to wait 24h with this and then squash/merge.

@marcus7070
Copy link
Member

No last second changes @adam-urbanczyk? I'll squash and merge in a bit then.

@marcus7070 marcus7070 merged commit e00ac83 into master Jun 22, 2021
@marcus7070
Copy link
Member

That was a huge effort, thank you very much @adam-urbanczyk!

@Jojain
Copy link
Contributor

Jojain commented Jun 23, 2021

I had not time to ask before merging but :
What this PR is for and what does it allow to do ?
To my knowledge toVTKPolyData allows to convert a cadquery model in data readable by VTK ? So this allows to render cq models in software based on VTK like Paraview ? Or am I getting it all wrong ?

Thank you in advance.

@adam-urbanczyk
Copy link
Member Author

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.

@adam-urbanczyk
Copy link
Member Author

Also (de)serialization of cq.Shape objects to BytesIO - should be handy for caching

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.

5 participants