gumyr/build123d

Add Solid and 3D Compound bug

Opened this issue · 6 comments

The following:

p = Sphere(1) + Compound(Sphere(1))

gives this contradictory error:

ValueError: Only shapes with the same dimension can be added

This is a contrived example, however, such a scenario can often arise when using Algebra mode.

A Compound has no dimension as it can contain 1, 2 or 3 dimensional objects. To "fix" this the Compound wrapper would have to be stripped off effectively reversing what the user did. Do you have a more realistic example?

A Compound is a generic container so its dimension is equal to the dimension of its contained shapes.

Compound<3>() + Part() = Compound<3>()
Compound<2>() + Sketch() = Compound<2>()
Compound<?>() + Sketch() = Compound<2>()
Compound<3>() + Sketch() = Error
Compound<?>() + Compound<?>() = Compound<?>()

An example without calling Compound:

p = Sphere(1) + Sphere(1).fuse(Sphere(1))

Everything here is obviously 3D, yet i get ValueError: Only shapes with the same dimension can be added

A Compound is a generic container so its dimension is equal to the dimension of its contained shapes.

Not always, Compounds are more generic:

c = Compound(
    [
        Circle(1),
        Line((0, 0, 0), (0, 0, 5)).edge(),
        Pos(0, 0, 6) * Cone(1, 0, 2),
    ]
)

is a valid Compound. In ocp_vscode I have to treat them differently, i.e. unroll these "mixed" Compounds.

To achieve what you want, currently you have to wrap the fuse result into a Part:

a = Sphere(1) + Part(Box(3, 0.2, 0.2).fuse(Box(0.2, 0.2, 3)))

So issue to me is less that Part + Compound should work (it can't for mixed Compounds).

I am not sure fuse should automatically convert to a Part (it is on the direct API level), but maybe a best practice is to:
Either not mixin direct API calls and use proper algebra operators, i.e. Sphere(1) + Box(3, 0.2, 0.2) + Box(0.2, 0.2, 3), or to wrap the result into a Part if using direct API is necessary.

How would I write a dimension agnostic operation then? For example, the builtin mirror function only returns the mirrored object but I want a version that returns the symmetric object:

def mirror_keep(obj: Union[Edge, Wire, Face, Compound, Curve, Sketch, Part]):
    return Compound(children=[obj, mirror(obj)])

This does not work due to the above issue. I could instead do

return obj + mirror(obj)

but that is an unneeded and much more expensive operation (in some cases this difference can be very significant)

You can do:

  • Part([b, mirror(b)])
  • Sketch([s, mirror(s)])
  • Curve([w, mirror(w)])
  • ...
    with b = Pos(0,1,0) * Box(1,1,1) and s = b.shell() and w = b.wire()

Not sure you need a function or an assembly. Compound (and Part, Sketch and Curve are Compounds) should do what you want.

If you want a function, check the type of the input object and depending on the type use the above

The dimension check is only done with the + operator not the fuse method so if you'd like to avoid the check just use the fuse method directly - assuming the specific situation is going to result in something that makes sense.