gumyr / build123d

A python CAD programming library
Apache License 2.0
521 stars 89 forks source link

Sketch: Faces with normals in opposite directions aren't combined into a single Face #267

Closed gumyr closed 1 year ago

gumyr commented 1 year ago

Consider the following example:

from build123d import *
from ocp_vscode import *

f1 = Face.make_from_wires(Wire.make_polygon([(1, 0), (2, 0), (2, 1), (1, 1), (1, 0)]))
f2 = Face.make_from_wires(Wire.make_polygon([(0, 0), (0, 1), (1, 1), (1, 0), (0, 0)]))
f3 = f1.fuse(f2)
print(f3.show_topology("Face"))
show_all()

image

Compound at 0x7fccaa533bf0, Center(1.0, 0.5, 0.0)
├── Face at 0x7fccaf5b4070, Center(1.5, 0.5, 0.0)
└── Face at 0x7fccaa533bb0, Center(0.5, 0.5, 0.0)

When using add to add new faces to a sketch, the normals must all face in the positive Z direction.

gumyr commented 1 year ago

More data:

with BuildSketch() as t1:
    Rectangle(1, 1, align=Align.MIN)
    with Locations((1, 0)):
        Rectangle(1, 1, align=Align.MIN)

f1 = Face.make_from_wires(Wire.make_polygon([(1, 0), (2, 0), (2, 1), (1, 1), (1, 0)]))
f2 = Face.make_from_wires(Wire.make_polygon([(0, 0), (1, 0), (1, 1), (0, 1), (0, 0)]))
f3 = f1.fuse(f2)

t1 is a single Face but f3 is a Compound of two faces. In both cases the normals are in the +ve Z direction.

bernhard-42 commented 1 year ago

I looked into it and I think one needs to reverse the wires of the face in order to flip it completely:

Turns out that it seems sufficient to flip the LINE edges:

def reverse_wire(wire):
    wire_builder = BRepBuilderAPI_MakeWire()
    combined_edges = TopTools_ListOfShape()
    for edge in reversed(wire.edges()):
        v = edge.vertices()
        t = edge.geom_type()
        if t == "LINE":
            rev_edge = BRepBuilderAPI_MakeEdge(v[1].wrapped, v[0].wrapped).Edge()
        else:
            rev_edge = edge.wrapped
        combined_edges.Append(rev_edge)
    wire_builder.Add(combined_edges)
    wire_builder.Build()
    return Wire(wire_builder.Wire())

It also seems sufficient to reverse the outer_wire:

def flip_face(face):
    rev_outer_wire = []
    inner_wires = []
    for wire in face.wires():
        if wire == face.outer_wire():
            rev_outer_wire = reverse_wire(wire)
        else:
            inner_wires.append(wire)

    return Face.make_from_wires(rev_outer_wire, inner_wires)

Example:

f1 = Face.make_from_wires(
    Wire.make_polygon([(1, 0), (1.5, 0.5), (1, 2), (3, 1), (2, 0), (1, 0)])
)
f1 = (
    fillet(f1.vertices()[2:4], 0.2)
    - Pos(1.8, 1.2, 0) * Rectangle(0.2, 0.4)
    - Pos(2, 0.5, 0) * Circle(0.2)
).faces()[0]

f2 = Face.make_from_wires(
    Wire.make_polygon([(1, 0), (1.5, -1), (2, -1), (2, 0), (1, 0)])
)
show(f1, f2)
image
show(flip_face(f1), f2)
image

Now let's fuse it:

f3 = flip_face(f1) + f2

show(f3)
print(f3.show_topology("Face"))

# +Compound at 0x169e67370, Center(1.931176925864295, 0.40255330702035463, 0.0)
# └── Face at 0x157f784b0, Center(1.931176925864295, 0.40255330702035463, 0.0)
image

Not sure it would work for all wires with all line types, but thought to share

bernhard-42 commented 1 year ago

More data:

with BuildSketch() as t1:
    Rectangle(1, 1, align=Align.MIN)
    with Locations((1, 0)):
        Rectangle(1, 1, align=Align.MIN)

f1 = Face.make_from_wires(Wire.make_polygon([(1, 0), (2, 0), (2, 1), (1, 1), (1, 0)]))
f2 = Face.make_from_wires(Wire.make_polygon([(0, 0), (1, 0), (1, 1), (0, 1), (0, 0)]))
f3 = f1.fuse(f2)

t1 is a single Face but f3 is a Compound of two faces. In both cases the normals are in the +ve Z direction.

Try f1.fuse(f2).clean() or f1 + f2 This leads to the expected result

and both the builder and the algebra mode apply clean.after fuse. Hence the difference as far as I can see

bernhard-42 commented 1 year ago

Assume the above flip_face would work in general, then the part missing in fuse would be to check whether all to_fuse faces are PLANE and then flip_face(face) of all face with face.normal_at() == -self.normal_at().

bernhard-42 commented 1 year ago

In any case, this is just a sketch of a solution for this problem. But it does look promising to me

gumyr commented 1 year ago

Thanks for looking into this. There is an OCP method Complemented that does the flip of a face - whith is what is used in the Face.__neg__ operation. Here are some examples:

from build123d import *
from ocp_vscode import *

test = Face.make_from_wires(Wire.make_rect(1, 1), [Wire.make_circle(0.2)])
print(test.wrapped.Orientation())
print(test.normal_at())
test_reversed = Face(test.wrapped.Complemented().Moved(Location((1.5, 0, 0)).wrapped))
print(test_reversed.wrapped.Orientation())
print(test_reversed.normal_at())
test_neg = -test.moved(Location((0, 1.5, 0)))
print(test_neg.wrapped.Orientation())
print(test_neg.normal_at())

show_all()
TopAbs_Orientation.TopAbs_FORWARD
Vector: (-0.0, 0.0, -1.0)
TopAbs_Orientation.TopAbs_REVERSED
Vector: (0.0, -0.0, 1.0)
TopAbs_Orientation.TopAbs_REVERSED
Vector: (0.0, -0.0, 1.0)

image

The fix should be just finding the flipped faces and using - to flip them back before adding them to the sketch.

gumyr commented 1 year ago

Fixed with commit 6aa434042b019dc2bb2d0daa6a0ee58d4008f5a7 The change in the build_common.py was quite small:

-                        reoriented_face = plane.to_local_coords(face)
-                        aligned.append(
-                            reoriented_face.moved(
-                                Location((0, 0, -reoriented_face.center().Z))
-                            )
-                        )
-                    else:
+                        face: Face = plane.to_local_coords(face)
+                        face.move(Location((0, 0, -face.center().Z)))
+                    if face.normal_at().Z > 0:  # Flip the face if up-side-down
                         aligned.append(face)
+                    else:
+                        aligned.append(-face)

Now:

f1 = Face.make_from_wires(
    Wire.make_polygon([(1, 0), (1.5, 0.5), (1, 2), (3, 1), (2, 0), (1, 0)])
)
f1 = (
    fillet(f1.vertices()[2:4], 0.2)
    - Pos(1.8, 1.2, 0) * Rectangle(0.2, 0.4)
    - Pos(2, 0.5, 0) * Circle(0.2)
).faces()[0]
print(f"{f1.normal_at()=}")

f2 = Face.make_from_wires(
    Wire.make_polygon([(1, 0), (1.5, -1), (2, -1), (2, 0), (1, 0)])
)
print(f"{f2.normal_at()=}")
with BuildSketch() as flip_test:
    add(f1)
    add(f2)

show(f1, f2, flip_test.sketch.moved(Location((-2, 0, 0))))

results in: image