CadQuery / cadquery

A python parametric CAD scripting framework based on OCCT
https://cadquery.readthedocs.io
Other
3.12k stars 288 forks source link

Syntactic sugar for boolean operations #568

Closed RubenRubens closed 3 years ago

RubenRubens commented 3 years ago

Short and sweet.

import cadquery as cq

Box = cq.Workplane('XY').box(1, 1, 1, centered=(False, False, False))
Sphere = cq.Workplane('XY').sphere(1)

# Union
op1 = Box + Sphere

# Difference
op2 = Box - Sphere

# Intersection
op3 = Box * Sphere

Here's the fork.

michaelgale commented 3 years ago

Oh wow! Why haven't we thought of that before! I like it. I would propose adding support for the "|" (union) and the "&" (intersection) operators so that it is consistent with python's use of these operators for the same operations of the set builtin type.

michaelgale commented 3 years ago

Question: do expressions such as the following also work:

op1 = Box.rotate((0,0,0), (0,0,1), 45).translate((-1, 0, 0)) + Sphere.translate((0, 0, 2))
adam-urbanczyk commented 3 years ago

They have to -> rotate and translate return a Workplane.

+1 on supporting & and |. While we are at it xor (^) could be also considered.

michaelgale commented 3 years ago

@adam-urbanczyk Of course--I realized just after posting--but I left it as a "dumb" question to remind myself to think before posting! :)

michaelgale commented 3 years ago

This is likely a bit of rabbit hole, but could we also consider extending this syntactic sugar to support operations such as:

# translation
op1 = Box + (0, 0, 2)
op1 = (Box - (0, 0, 2)) + (Box + (0, 0, 2))

# scaling
op1 = 3 * Box
# op1 is a box scaled 3x larger in all axes
op1 = (1, 1, 3) * Box
# op1 is box scaled 3x taller in Z (X,Y remain unscaled)

This would require a bit more parsing of argument types to determine if the "+/-" operators apply a CSG operation or simply a translation. However, it does leverage the flexibility of python's untyped capabilities. Conceptually, an expression such as r = box1 + box2 is no different than r = box + (0, 0, 1)--its to python's credit that implementing these expressions is a possibility in the first place.

Lastly, would we also consider adding support for "+=" and "-=" too? e.g.

Box += Sphere
Box -= Box.translate((0, 0, -3))
RubenRubens commented 3 years ago

Short and sweet is the priority for me. I strongly disagree with making operations overcomplicated. From my point of view we have two options:

(A) Support +, - and * symbols

They are supported in other projects such as SolidPython (a python transpiler for OpenSCAD) and SolidMonty (a failed project I did myself some time ago). I know that in maths sum and union are not the same operation but... who cares? This way in CQ, + would mean union in solid1 + solid2 and sum in number1 + number2. It's just fine.

(B) Support Python's set syntax

Symbol Operation
\| union
& intersection
- difference
^ symmetric difference

This way we can add * as a scaling operator (fantastic idea!). I believe using + as a translation operations is a terrible idea. Ugly and overcomplicated.

Furthermore, this is not about supporting as many symbols as possible. It's about supporting the most useful syntax most of the time.

RubenRubens commented 3 years ago

Box += Sphere just works. Python comes with batteries included.

adam-urbanczyk commented 3 years ago

I'm -1 on supporting scaling and translations.

For me a and b are both fine, actually both could be implemented.

RubenRubens commented 3 years ago

OK. If we are not supporting scaling then I prefer option A.

I'm afraid The Zen of Python does not apply in this case:

There should be one—and preferably only one—obvious way to do it.
Although that way may not be obvious at first unless you're Dutch.
michaelgale commented 3 years ago

If we are going to support these operator based CSG operations, then I think we should aim for a complete implementation. Therefore, can we consider this:

Symbol dunder method Operation Example
+ __add__ union r = a + b
\| __or__ union r = a \| b
+= __iadd__ in place union r += a
\|= __ior__ in place union r \|= a
- __sub__ cut r = a - b
-= __isub__ in place cut r -= a
& __and__ intersection r = a & b
&= __iand__ in place intersection r &= a
^ __xor__ symmetric difference r = a ^ b
^= __ixor__ in place symmetric difference r ^= a
* __rmul__ scale (all axis with a scalar) r = 2.5 * a
* __mul__ scale (all axis with a scalar) r = a * 2.5
*= __imul__ in place scale r *= 2.5
@ __rmatmul__ scale (per axis with tuple, list, Vector) r = (1, 1, 2) @ a
@= __imatmul__ in place scale (multi axis) r @= (1, 1, 2)
  1. I'm not so sure that the ^ (symmetric difference) is really required. It's not a common CSG operation and I can't think of a use case where it would be useful (however, I'm sure some do exist).
  2. Note that scaling is proposed to use either the scalar or matrix multiply operators separately. This implementation is more in line with its corresponding mathematical operation and enforces the order of matrix operation for the multi-axis scaling.
  3. I'm disappointed that the translation sugar was shot down--oh well.
  4. Obviously the in place methods are redundant in most cases but are shown for completeness.
michaelgale commented 3 years ago

Ok, forgive me--I can't let go of the translations! If there are two operations I would gladly become a type-2 diabetic with syntactic sugar its replacing translate and especially rotate. I use these two operations heavily. The rotate method I find burdensome with its 3 arguments (2x of which are tuples with the first almost always (0,0,0)).

Therefore can I risk provoking the community by proposing the following:

Sym dunder method Operation Example Equivalent CQ
>> __rrshift__ translation r = a >> (0, 0, 2) r = a.translate((0, 0, 2))
>>= __irshift__ i.p. translation r >>= (0, 0, 2) r = r.translate((0, 0, 2))
** __rpow__ rotation with Euler angles r = a ** (0, 0, 45) r = a.rotate((0, 0, 0), (0, 0, 1), 45)
**= __ipow__ i.p. rotation with Euler angles r **= (0, 0, 45) r = r.rotate((0, 0, 0), (0, 0, 1), 45)

As an example,

r = a.translate((0, 0, 5)).union(b.rotate((0, 0, 0), (0, 0, 1), 90).translate((3, 0, 0)))

becomes

r = (a >> (0, 0, 5)) + ((b ** (0, 0, 90)) >> (3, 0, 0))
marcus7070 commented 3 years ago

The link between ** and a rotation is a bit of a stretch there @michaelgale. But there is a method I found called infix operators that might let you do something neater: http://tomerfiliba.com/blog/Infix-Operators/ (eg. defining a |rot| pseudo-operator).

Added benefit here is that these would require no modifications to CQ's Workplane class, so you could implement them outside of CQ.

michaelgale commented 3 years ago

@marcus7070 Oh wow, thanks for the infix operator link! That is a very tempting way of compacting some of these verbose operations. While I like the functionality it brings, it feels like a non-pythonic workaround and would make the code look somewhat strange and unconventional. Whereas overloading operators which roughly correspond to the semantics of the solid operation look and feel nicer.

michaelgale commented 3 years ago

@marcus7070 Yes, the ** is a bit of stretch. But, perhaps we can think of it like rotation in the complex plane. That is, you multiply a vector by e**(i theta) (The rotation angle is in exponent). I know it's a stretch, but it is based on a thin analogy to something! :)

adam-urbanczyk commented 3 years ago

I don't have bandwidth ATM to discuss in detail but:

@michaelgale

(1) do you really need anisotropic scaling? It changes the underlying geometry type, which might have unexpected results. Exposing this op to the users like that will increase the support effort for CQ and I think that there are better ways to build models. (1a) do you really need isotropic scaling? (2) @ fits better for rotation,translation,... - note that general transform is a matrix. It could also make sense to overload it for the Matrix and Location class. (3) >> looks really confusing to me in this context.

@RubenRubens let's not be too religious on zens of Python. Duplicating + and & won't hurt anyone and will be more forgiving for the users.

All in all operators are nice, but let's not be too trigger happy with them.

michaelgale commented 3 years ago

@adam-urbanczyk Thank you for your feedback.

  1. No. Honestly, scaling is not really a priority and I can't think of a time where I've used it meaningfully. Its suggestion was merely for the sake of completeness rather than a burning requirement.
  2. Yes, overloading the @ would make more sense in the geom classes since all linear transformations eventually reduce to a matrix operation. No sense conflating these operations within the higher level CQ workplane stack context.
  3. Now that I reflect, my desire for syntactic sugar for translate and rotate where stimulated by my over-excitement at the sugar for boolean CSG! Therefore, I will withdraw my suggestion for sugary linear transforms!
adam-urbanczyk commented 3 years ago

Closed by #569