gumyr / build123d

A python CAD programming library
Apache License 2.0
382 stars 72 forks source link

Add Solid and 3D Compound bug #625

Open MatthiasJ1 opened 1 month ago

MatthiasJ1 commented 1 month ago

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.

gumyr commented 1 month ago

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?

MatthiasJ1 commented 1 month ago

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

bernhard-42 commented 1 month ago

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.

MatthiasJ1 commented 1 month ago

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)

bernhard-42 commented 1 month ago

You can do:

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

gumyr commented 1 month ago

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.