Unifying make-faces-from-wires routines

Hi!
In my efforts to improve revolve, it was decided it’s a good idea to unify some of the code for making faces with holes from wires (from sketches, and not).

There are 5 routines I’m aware of:

  • in PartDesign
  • in Part Extrude
  • in Part Make Face From Sketch
  • in Part Revolve
  • the new one I made for Part Revolve

I’m trying to digest them. So far, I’ve digested PartDesign one, and here’s the outline of what it does:

PartDesign FaceMaker

  1. sort wires by diagonal lengths of their bounding boxes. This ensures that outer wires come before inner (hole) wires in the list.
  2. split wires into groups, each group corresponding to a single face. This is done by adding wires one by one to the groups, by testing if the first vertex of the wire being added is inside a temporary face made of outer wire of the group.
  3. make the faces (there is a check that the orientation of the inner wire is correct, by making a temporary face and comparing normal)

Capability. Makes set of faces with holes.
Requirements/Limitations:

  • faces must be on one plane (but it’s not required to know the plane beforehand).
  • if wires are intersecting, the result may be horribly broken (not just a compound of faces that intersect - some faces may have badly formed holes)
  • takes list of wires as input, so pre-processing/wrapping is required for compound traversal and looped edge support.

FaceMaker I made for revolve:

  1. traverse compounds. find all looped wires and edges and make faces out of them. Bypass faces and shells, fail on anything else.

Capabilities:

  • arbitrary set of planes
  • for intersecting wires, intersecting faces are made (not terribly invalid)
  • dumb trivially simple stupid code
    Limitations.
  • no support for holes

model refine has some similar functionality also.

https://github.com/blobfish/FreeCAD_sf_master/blob/master/src/Mod/Part/App/modelRefine.cpp#L294

The more I think of it, the more it becomes clear there is no facemaker code to rule them all.
So I think of making a bunch of facemaker classes (derived from OCC BRepBuilderAPI_MakeShape), integrate them into typesystem, and offer general facemaker method in toposhape that just takes class name as an argument.
Is it worth going this far?

I have had this feeling also.

To add another story to this. Screwmaker fails on OCCT 6.9 and 7.0. It fails in a routine that make faces with a hole in it. What worked before, results now in a outer shell with a body in it after the extrusion.
I did not dig much into it, but this is what I do remember.

Ulrich