Open NothanUmber opened 3 years ago
Here the same with higher quality parameters (it seems to be an interative approximation approach):
new_surface_thickness_0 = cq.Workplane().interpPlate(surf_edges=edges_with_face_constraints, thickness=0.0, combine=False, clean=False, degree=3, nbPtsOnCur=20, nbIter=4, anisotropy=False, tol2d=0.00001, tol3d=0.0001, tolAng=0.01, tolCurv=0.1, maxDeg=4, maxSegments=300 )
This looks great to me. Making that surface with current CQ would be impossible (I think?).
I'm interested to hear @adam-urbanczyk opinion though.
I'm definitely +1 for extending makeNSidedSurface
. I'll try to understand why G2
is ignored.
Face.makeNSidedSurface is based on the OCCT class BRepOffsetAPI_MakeFilling. This class supports an interesting capability that is currently not exposed via makeNSidedSurface (and it's interpPlate wrappers in Solid and Workplane): It allows to define adjacent faces as continuity constraints, so the newly created surface is tangential towards these faces.
Think it can be useful for scenarios where smooth surface transitions are desired.
Have played around with this and have a first proposal checked in here: https://github.com/NothanUmber/cadquery Think it makes sense to discuss this first before opening a pull request.
There are some usage examples at the bottom of this file: https://github.com/NothanUmber/cadquery/blob/master/examples/Ex101_InterpPlate.py
This proposal assumes that instead of just providing a list of boundary edges for the new surface, one can optionally provide a list of tuples of boundary edges and adjacent faces, the new surface should be tangential towards. The advantage of this interface design approach would be that one wouldn't have to provide a face constraint for all edges - e.g. if the surface shouldn't be tangential towards all adjacent faces or if some edge isn't even adjacent to any face. Currently not providing face constraints for all edges works unreliably though. In some cases the underlaying BRepOffsetAPI_MakeFilling will throw an error if only some edges have continuity face constraints and some don't. And in some cases it works. So the current interface proposal would put both the responsibility and the possibilities in the hands of the user - partial constraints (thus mixing tuples of edges and faces with just edges) are possible but may fail. Another approach might be to e.g. raise an exception if edge-face tuples and sole edges are mixed, so either all edges have to be constrained or none. (Less possibilities for errors, but semi-constrained cases that would actually work would also be ruled out that way). Or even a separate tangentialFaces parameter that has to contain one face for each edge in edges? (Would also only work for the fully constrained case)
There are still a number of open points:
I am currently still struggling with getting interpPlate to work with thickness<>0. The underlaying BRepOffset_MakeOffset.MakeOffsetShape() call doesn't seem to terminate (I cancelled it after 30 minutes). See last example in Ex101_InterpPlate.py. If we don't get this to run then perhaps it might be an option to expose the continuity capabilities for makeNSidedSurface but not for Solid./Workplan.einterpPlane? (makeNSidedSurface is executing the same code as before when no face-continuity constraints are provided, so it is backwards compatible)
for some reason I haven't understood yet, the last face constraint has to be added with option isBound=False, otherwise MakeFilling.Build will fail with the famous "BRep_API: command not done." Even when setting isBound to False the last constraint IS considered and the edge also part of the surface boundary. One could again expose this to the user (e.g. via an optional third entry in the tuple, so it would be (Edge, Face, boolean) tuples. But for the moment I am handling that internally because it's hard to rationalize why one should do it this way. (At least I don't understand it yet).
Currently only G1 continuity seems to be supported. Thus I currently hardcoded that. If more continuity options should get supported by OCCT, adding a new parameter to expose this should be easy
There is an additional Add-builder method in BRepOffsetAPI_MakeFilling that allows to provide face constraints that are not associated to edges - so OCCT would have to figure out by itself which edges match. Sounds good in theory, didn't get this to work in practice though. Face constraints added that way just got ignored in my experiements. If that would work then one could add face constraints independent of edges (e.g. in a separate faceConstraints parameter) instead of these edge-face tuples.
A little unrelated: The current Solid.interpPlate implementation expects a Workplane with wires on the stack as surf_edges parameter (instead of a "list of ordered or unordered CadQuery wires" as mentioned in the parameter description). If Solid.interpPlate is considered part of the direct API which better shouldn't depend on the fluent API (to prevent circular depenencies and to keep layers separated) it might perhaps be an opportunity to deprecate the usage of Workplanes here - or to deprecate the existing interpPlate method and provide a new one with a new interface? Or one could factor out the functionality from Solid.interpPlate to Workplane.interpPlate (where usage of Workplanes as parameters might be more suitable)? Currently I haven't done any of these changes, would probably better to separate this out.
Here a minimal useage example from the file above: o = 20 # outer side length i = 5 # inner side length h = 20 # height
surface_1 = cq.Face.makeFromWires(cq.Wire.makePolygon([cq.Vector(-i,-i,h), cq.Vector(-i,i,h), cq.Vector(-o,i,0), cq.Vector(-o,-i,0)])) surface_2 = cq.Face.makeFromWires(cq.Wire.makePolygon([cq.Vector(-5, -5, h), cq.Vector(i,-i,h), cq.Vector(i,-o,0),cq.Vector(-i,-o,0)])) surface_3 = cq.Face.makeFromWires(cq.Wire.makePolygon([cq.Vector(5, -5, h), cq.Vector(i,i,h), cq.Vector(o,i,0),cq.Vector(o,-i,0)])) surface_4 = cq.Face.makeFromWires(cq.Wire.makePolygon([cq.Vector(5, 5, h), cq.Vector(-i,i,h), cq.Vector(-i,o,0),cq.Vector(i,o,0)]))
connection_edge_surface_1 = cq.StringSyntaxSelector(">Z").filter(surface_1.Edges())[0] connection_edge_surface_2 = cq.StringSyntaxSelector(">Z").filter(surface_2.Edges())[0] connection_edge_surface_3 = cq.StringSyntaxSelector(">Z").filter(surface_3.Edges())[0] connection_edge_surface_4 = cq.StringSyntaxSelector(">Z").filter(surface_4.Edges())[0]
edges_with_face_constraints = [ (connection_edge_surface_1, surface_1), (connection_edge_surface_2, surface_2), (connection_edge_surface_3, surface_3), (connection_edge_surface_4, surface_4) ] new_surface_nsided = cq.Face.makeNSidedSurface( edges=edges_with_face_constraints, points = [], maxSegments=120)
obj = cq.Workplane().add(surface_1).add(surface_2).add(surface_3).add(surface_4).add(new_surface_nsided)
Which would look like this: (The yellow face is the newly constructed one, the white faces were provided as tangential face constraints)
What do you think?