gumyr / build123d

A python CAD programming library
Apache License 2.0
575 stars 94 forks source link

Loft produces unexpected faces of GeomType.BSPLINE or filter_by(Plane) should work with GeomType.BSPLINE shapes #756

Closed dalibor-frivaldsky closed 3 weeks ago

dalibor-frivaldsky commented 3 weeks ago

The following code creates the shape in the picture below:

with BuildPart() as mount:
    with BuildSketch():
        Rectangle((1+16+4)*mm, 20*mm, align=(Align.MIN, Align.CENTER))
    with BuildSketch(Pos(1*mm, 0, 4*mm)):
        Rectangle(16*mm, 20*mm, align=(Align.MIN, Align.CENTER))
    loft()
Screenshot 2024-11-04 at 14 49 17

I would like to filter out the trapezoidal faces with mount.faces().filter_by(Plane.XZ), however, this returns an empty list.

I have narrowed down the issue to the implemenation of the Shape.filter_by method, specifically the plane_parallel_predicate. The shape being filtered needs to be both an instance of Face and of a GeomType.PLANE. The loft creates the trapezoidal faces as GeomType.BSPLINE, which fails the second part of the check and results in the empty list after applying the filter.

I don't know whether the issue is with the loft creating the faces as b-splines (which maybe seems unnecessary?) or whether the filter_by function should be able to work with (plannar?) b-spline faces. What do you think?

gumyr commented 3 weeks ago

Unfortunately, the OCCT CAD kernel returns these BSPLINE faces where a PLANAR face would be more appropriate. The check for planar needs to be change from just looking at the geom_type property to something like this:

from build123d import *
from OCP.BRep import BRep_Tool
from OCP.GeomLib import GeomLib_IsPlanarSurface
from ocp_vscode import show_all

def is_face_planar(face: Face) -> bool:
    surface = BRep_Tool.Surface_s(face.wrapped)
    is_planar = GeomLib_IsPlanarSurface(surface, 1e-6)
    return is_planar.IsPlanar()

with BuildPart() as mount:
    with BuildSketch():
        Rectangle((1 + 16 + 4) * MM, 20 * MM, align=(Align.MIN, Align.CENTER))
    with BuildSketch(Pos(1 * MM, 0, 4 * MM)):
        Rectangle(16 * MM, 20 * MM, align=(Align.MIN, Align.CENTER))
    loft()

for f in mount.faces():
    print(f"{f.geom_type} is planar: {is_face_planar(f)}")
GeomType.PLANE is planar: True
GeomType.BSPLINE is planar: True
GeomType.PLANE is planar: True
GeomType.BSPLINE is planar: True
GeomType.PLANE is planar: True
GeomType.PLANE is planar: True

I'm thinking a new property should be added to the Face class, is_planar : bool that does what is shown here.

dalibor-frivaldsky commented 3 weeks ago

I assume with the is_planar property in place, the code in plane_parallel_predicate:

if isinstance(shape, Face) and shape.geom_type == GeomType.PLANE:

would change to:

if isinstance(shape, Face) and shape.is_planar:

That looks good to me.

gumyr commented 3 weeks ago

Now:

non_trap = mount.faces().filter_by(Plane.XZ, reverse=True)

results in: image

dalibor-frivaldsky commented 3 weeks ago

I have found one more place where the behavior of loft() causes issues:

with BuildPart() as mount:
    with BuildSketch():
        Rectangle((1+16+4)*mm, 20*mm, align=(Align.MIN, Align.CENTER))
    with BuildSketch(Pos(1*mm, 0, 4*mm)):
        Rectangle(16*mm, 20*mm, align=(Align.MIN, Align.CENTER))
    loft()

    with BuildSketch(mount.faces()[1]):
        Rectangle(4*mm, 4*mm)
    extrude(amount=4*mm)

This fails with "ValueError: Planes can only be created from planar faces" on BuildSketch(mount.faces()[1]) when creating a Plane object from the provided trapezoid face, which is of GeomType.BSPLINE. The place to fix this seems to be here

geometry.py

elif arg_face:
    # Determine if face is planar
    surface = BRep_Tool.Surface_s(arg_face.wrapped)
    if not isinstance(surface, Geom_Plane):
        raise ValueError("Planes can only be created from planar faces")

I can give this a try and fix it. I've already created a dev environment for build123d in the meantime and implemented partial fix for the original issue.

In general, the underlying loft() kernel implementation is behaving fairly unexpectedly in multiple ways. As another example, creating a Plane from the top and bottom faces (parallel with Plane.XY) sets their x_dir vector to a seemengly arbitrary value (shown below). But I don't think build123d is the place to try to fix this.

with BuildPart() as mount:
    with BuildSketch():
        Rectangle((1+16+4)*mm, 20*mm, align=(Align.MIN, Align.CENTER))
    with BuildSketch(Pos(1*mm, 0, 4*mm)):
        Rectangle(16*mm, 20*mm, align=(Align.MIN, Align.CENTER))
    loft()

with BuildSketch(mount.faces() | Plane.XY):
        Rectangle(4*mm, 4*mm)
    extrude(amount=4*mm)
image