Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in Shape.split() #698

Closed
makerC-LV opened this issue Sep 13, 2024 · 1 comment
Closed

Bug in Shape.split() #698

makerC-LV opened this issue Sep 13, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@makerC-LV
Copy link

makerC-LV commented Sep 13, 2024

I build an object and split it, and that works. However if I fillet some of the edges before splitting it, there's an error. Here's a minimal example:

import build123d as b3d
from build123d import Align, Axis, Box, Compound, Keep, Location, Plane, Polygon, extrude, faces, fillet, offset, split
from ocp_vscode import show

a = 100
b = 200
c = 50
h = 10
d = 2
o = 25

def example():
    t = Polygon([(0, 0, 0), (a, 0, 0), (a,
                b, 0), (0, c, 0)], align=Align.MIN)
    obj = extrude(t, h)
    top_edges = obj.edges().group_by(Axis.Z)[-1]
    # if the line below is commented out, the example runs, if not, throws error
    # obj = fillet(top_edges, d)

    right = split(obj, bisect_by=Plane.YZ.offset(o), keep=Keep.TOP)
    left = split(obj, bisect_by=Plane.YZ.offset(o), keep=Keep.BOTTOM)
    return [left, right]


show(example())

Observation by Bernhard on Discord:
I think this is a bug in Shape.split: BRepAlgoAPI_Splitter returns a list of one Compound instead of two Solids. If I change

        if keep == Keep.BOTH:
            result = Compound(downcast(splitter.Shape()))
        else:
            parts = list(Compound(downcast(splitter.Shape())))
            tops = []

to

        parts = list(Compound(downcast(splitter.Shape())))
        if len(parts) == 1 and isinstance(parts[0], Compound):
            parts = list(parts[0])

        if keep == Keep.BOTH:
            result = Compound(parts)
        else:
            tops = []

then it works.

@gumyr gumyr added the bug Something isn't working label Oct 4, 2024
@gumyr gumyr added this to the Not Gating Release 1.0.0 milestone Oct 4, 2024
@gumyr
Copy link
Owner

gumyr commented Oct 4, 2024

Thanks for pointing this out. The problem with Compounds of Compounds has happened before so I've created an unwrap method for Compounds that remove these redundant layers. With this change the fix looks like:

        result = Compound(downcast(splitter.Shape())).unwrap(fully=False)
        if keep != Keep.BOTH:
            tops = []
            bottoms = []
            for part in result:
                ...

Compounds are full iterables now with a __Len__ method that can help when working with them.

@gumyr gumyr closed this as completed Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants