gumyr/build123d

Add `BRepFill_Filling` based constrained surface creation

Opened this issue · 9 comments

Currently the surface construction methods in build123d do not provide great control over adjacent face continuity requirements. I propose to add BRepFill_Filling as a build123d direct API Face method.

Further reading: https://neweopencascade.wordpress.com/2019/03/15/build-constrained-surfaces-with-brepoffsetapi_makefilling-part-i/

I was able to create a rough proof of concept, and it does function as expected:

image

Working prototype:

from typing import Iterable, Union, Tuple
from enum import Enum, auto
from OCP.BRepFill import BRepFill_Filling
from OCP.GeomAbs import *  # import specific

# put in build_enums.py
class Continuity(Enum):
    """geometric continuity"""

    C0 = auto()
    G1 = auto()
    G2 = auto()

    def __repr__(self):
        return f"<{self.__class__.__name__}.{self.name}>"

# put in topology.py ?
_continuity_dict = {
    Continuity.C0: GeomAbs_C0,
    Continuity.G1: GeomAbs_G1,
    Continuity.G2: GeomAbs_G2,
}

# put in topology.py as a classmethod of Face
@classmethod
def make_surface_patch(
    cls,
    edge_face_constraint: Iterable[
        Union[
            Tuple[Edge, Face, Continuity],
            Tuple[Edge, Continuity],
        ]
    ],
) -> Face:
    # input validation
    for efc in edge_face_constraint:
        if not isinstance(efc[0], Edge):
            raise ValueError(f"Provided {type(efc[0])} is not an Edge or Wire")

    patch = BRepFill_Filling()
    for efc in edge_face_constraint:
        if isinstance(efc[1], Continuity) and len(efc) == 2:
            patch.Add(efc[0].wrapped, _continuity_dict[efc[1]])
        else:
            patch.Add(efc[0].wrapped, efc[1].wrapped, _continuity_dict[efc[2]])
    patch.Build()
    result = patch.Face()

    return cls(result)


Face.make_surface_patch = make_surface_patch

This looks good to me. At some point we're going to need some good surface modelling documentation with all of these different methods to choose from.

@gumyr a few more question for you -- do you think the "UI" makes sense? The tuples supply the e.g. edge/face/continuity pairing. Is this consistent with other parts of build123d? I wanted it to be very explicit which parts "go together". I also decided against any kind of topology exploration (to automatically pair edges/faces) to avoid complexity and not to mention that it might not even be possible.

There are a few other things I have noticed:

  1. According to the OCCT docs it is possible to add a punctual constraint via a gp_Pnt. I currently think it is probably worth adding support for this. This does not require a constraint.
  2. As far as I can tell (and I could be very wrong) I do not see the point for the user to be able to supply the continuity for unsupported edges, they seem to only use C0 continuity.

Would you mind sharing the code you used to create the heart value example? It would be easier to discuss the API by looking at a real example.

One thing that might help is to rename the Continuity Enum to terms more commonly used but this is easier said than done.

IMO the ability to specify continuity is very useful.

@gumyr Here is a version of the example that you referenced (combine with the earlier prototype code):

r = 30
angle = 40
offs = 100

plnA = Plane.XY
with BuildSketch() as s:
    Circle(r)
    split(bisect_by=Plane.XZ)

plnB = Plane.XY.rotated((0, -angle, 0)).offset(offs + 20)
with BuildSketch(plnB) as s2:
    Circle(r)
    split(bisect_by=Plane.XZ)

plnC = Plane.XY.rotated((0, angle + 20, 0)).offset(offs - 10)
with BuildSketch(plnC) as s3:
    Circle(r)
    split(bisect_by=Plane.XZ)

selA = s.sketch.edges().sort_by(Axis.Y)[0]
selB = s2.sketch.edges().sort_by(Axis.Y)[0]
selC = s3.sketch.edges().sort_by(Axis.Y)[0]

with BuildLine(Plane.XZ) as l:
    m1 = Spline([selA @ 0, selB @ 0], tangents=[plnA.z_dir, plnB.z_dir])
    m2 = Spline([selA @ 1, selC @ 1], tangents=[plnA.z_dir, plnC.z_dir])
    m3 = Spline([selB @ 1, selC @ 0], tangents=[-plnB.z_dir, plnC.z_dir])

# create construction geometry (support faces)
ex1 = Edge.extrude(m1.edge(), (0, 10, 0))
ex2 = Edge.extrude(m2.edge(), (0, 10, 0))
ex3 = Edge.extrude(m3.edge(), (0, 10, 0))

exA = Edge.extrude(selA, -10 * plnA.z_dir)
exB = Edge.extrude(selB, 10 * plnB.z_dir)
exC = Edge.extrude(selC, 10 * plnC.z_dir)

# position constraints on surface:
n1 = Spline([selA @ 0.5, selB @ 0.5], tangents=[(0, 0, 1), plnB.z_dir])
n2 = Spline([selA @ 0.5, selC @ 0.5], tangents=[(0, 0, 1), plnC.z_dir])
n3 = Spline(
    [selB @ 0.5, selC @ 0.5],
    tangents=[-plnB.z_dir, plnC.z_dir],
    tangent_scalars=[1.1, 1.1],
)

top_face = Face.make_surface_patch(
    edge_face_constraint=[
        (m1.edge(), ex1, Continuity.G1),
        (m2.edge(), ex2, Continuity.G1),
        (m3.edge(), ex3, Continuity.G1),
        (selA.edge(), exA, Continuity.G1),
        (selB.edge(), exB, Continuity.G1),
        (selC.edge(), exC, Continuity.G1),
        (n1.edge(), Continuity.C0),
        (n2.edge(), Continuity.C0),
        (n3.edge(), Continuity.C0),
    ]
)

image

@MatthiasJ1 as you can see I AM specifying continuity above, the question is if "unsupported edges" need a continuity mode input because they only seem to work with C0 continuity.

What about adding a couple methods to make the interface a little more user friendly?

top_face = Face.make_surface_patch(
    edge_face_constraint=[
        m1.tangent_continuous(ex1),
        m2.curature_continuous(ex2),
        ...
        n1,
        n2,
        n3,
    ]
)

The m1.tangent_continuous(ex1) method could return a tuple of (m1.edge(), ex1, Continuity.G1) so the make_surface_patch API wouldn't actually change. The C0 constraint doesn't actually provide any more information so I don't think it's required - am I missing something here?

@gumyr I agree that the C0 constraint is not providing anything, so that can be default behavior without the user needing to specify. Regarding the helper functions, I am not sure they are adding enough value to be worth it -- so I prefer to not add them for now.

Here is the new proposed method signature:

# put in topology.py as a classmethod of Face
@classmethod
def make_surface_patch(
    cls,
    edge_face_constraint: Iterable[
        Union[
            Tuple[Edge, Face, Continuity],
            Edge,
            VectorLike,
        ]
    ],
) -> Face:

Does VectorLike make sense in this application (will be converted to gp_Pnt)

gumyr commented

Looks fine to me. This is an advanced feature so I expect users will have more trouble with defining their surface than figuring out the syntax. VectorLike is more consistent than Vector across all the build123d APIs. Tuple isn't really required, tuple is the recommendation IIRC.