gumyr / build123d

A python CAD programming library
Apache License 2.0
432 stars 81 forks source link

offset a FilletPolyline results in 0 edges #292

Closed gumyr closed 1 year ago

gumyr commented 1 year ago
from build123d import *
from ocp_vscode import *

pts = [
    (11.421, 10.15),
    (11.421, 84.582),
    (19.213, 118.618),
    (17.543, 127.548),
    (-10.675, 127.548),
    (-10.675, 118.618),
    (-19.313, 118.618),
]
line = FilletPolyline(*pts, radius=3.177)
o_line = offset(line, amount=3.177)
print(o_line.edges())  # []
direct_o_line = line.wires()[0].offset_2d(3.177)
print(direct_o_line.edges())  # [...]
show([Vertex(*p, 0) for p in pts], line)

image

gumyr commented 1 year ago

This problem is specific to Algebra mode as:

with BuildPart() as g_bar:
    with BuildSketch() as bs:
        with BuildLine() as bl:
            FilletPolyline(*pts, radius=3.177)
            offset(amount=3.177)
        make_face()
    extrude(amount=6.35)

show(g_bar)

generates; image

bernhard-42 commented 1 year ago

The issue here is that the builder wraps the FilletPolyLine into a Curve. Hence,

# line = FilletPolyline(*pts, radius=3.177)
line = Curve(FilletPolyline(*pts, radius=3.177).wrapped)
o_line = offset(line, amount=3.177)
bs = make_face(o_line)
g_bar = extrude(bs, 6.35)

works as expected in algebra mode.

bernhard-42 commented 1 year ago

And Curve is a sub class of Compound which offset treats correctly. FilletPolyline is a Shape and Wire which offset ignores. Line 529++ of offset:

    for obj in object_list:
        if isinstance(obj, Compound):
            edges.extend(obj.get_type(Edge))
            faces.extend(obj.get_type(Face))
            solids.extend(obj.get_type(Solid))
        elif isinstance(obj, Solid):
            solids.append(obj)
        elif isinstance(obj, Face):
            faces.append(obj)
        elif isinstance(obj, Edge):
            edges.append(obj)

It is a not fully qualified condition (no else) which leads to the silent "death".

If I change offset line 529++ to

    for obj in object_list:
        if isinstance(obj, Compound):
            edges.extend(obj.get_type(Edge))
            faces.extend(obj.get_type(Face))
            solids.extend(obj.get_type(Solid))
        elif isinstance(obj, Solid):
            solids.append(obj)
        elif isinstance(obj, Face):
            faces.append(obj)
        elif isinstance(obj, Edge):
            edges.append(obj)
        elif isinstance(obj, Wire):
            edges.extend(obj.edges())
        else:
            raise TypeError(f"Unsupported type for {obj}")

the unwrapped algebra version works as the builder does.

jdegenstein commented 1 year ago

Should:

raise TypeError(f"Unsupported type for {obj}")

Instead be changed to something like this?

raise TypeError(f"Unsupported type {type(obj)} for {obj}")
bernhard-42 commented 1 year ago

yep, would be better. In any case, I think in cases like that we should ensure that an else branch is provided

gumyr commented 1 year ago

Fixed as described above with both Wire support and the else TypeError: image