gumyr / build123d

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

Calling method in `add` behaves differently from adding a variable #351

Closed qwelyt closed 10 months ago

qwelyt commented 10 months ago

MWE

from ocp_vscode import show, show_object, reset_show, set_port, set_defaults, get_defaults, Camera
set_port(3939)

import build123d as bd

w = 19.05
l = 19.05
cut = 14.07

def repeater(n,p=None):
    t = bd.Rectangle(w,l)-bd.Rectangle(cut,cut) if p is None else p
    skt = [loc*t for loc in bd.GridLocations(0,l,1,n)]
    return skt

def caller_1(n):
    with bd.BuildSketch() as skt:
        bd.add(repeater(n))
    return skt.sketch

def caller_2(n):
    ts = repeater(n)
    with bd.BuildSketch() as skt:
        bd.add(ts)
    return skt.sketch

repeaters = [(bd.Sketch()+repeater(i+1)).move(bd.Location((i*(w+2)*-1,l*3,0))) for i in range(5)]
caller1s = [caller_1(i+1).move(bd.Location((i*(w+2),0,0))) for i in range(5)]
caller2s = [caller_2(i+1).move(bd.Location((i*(w+2),l*6,0))) for i in range(5)]
show(
    repeaters,
    caller1s,
    caller2s,
    reset_camera=Camera.KEEP
)

Results: 20231022_213807

Top right is caller2s.
Middle left is repeaters.
Bottom right is caller1s.

I would expect all these pyramids to look the same. But for some reason calling the repeaters-function inside add bleeds a rectangle, somehow. Yet, by just putting it in a variable first and then putting that variable in add the problem is gone.

@jdegenstein also found that this variant behaves like it should

def caller(n):
    asdf = bd.Sketch() + repeater(n)
    return asdf

so it seems to be something odd related to the builders add.

Ruudjhuu commented 10 months ago

This is a result of an active BuildSketch() context.

If the function bd.Rectangle() is executed inside a BuildSketch() context, the rectangle is automatically added due to the default mode=Mode.ADD.

So your code path is: caller_1 -> BuildSketch.__enter__ -> repeater -> Rectangle(mode=Mode.ADD) - Rectangle(mode=Mode.ADD) -> add(mode=Mode.ADD) -> Buildsketch.__exit__

If you use mode=Mode.PRIVATE in bd.Rectangle() you'll probably have the behavior you expected.

qwelyt commented 10 months ago

Indeed. Doing

t = bd.Rectangle(w,l,mode=bd.Mode.PRIVATE)-bd.Rectangle(cut,cut, mode=bd.Mode.PRIVATE) if p is None else p

removes the strange squares.

So there is a work-around. But should you need it?

Ruudjhuu commented 10 months ago

Well, I think this behaviour is as intended. You seem to mix the 2 APIs provided by build123d. This is possible with 1 exception: using algebra API inside builder context which you do. Repeater is written with the algebra API, and caller1 is written with the builder API. See Docs

gumyr commented 10 months ago

As described above, mixing the Build and Algebra APIs as done here is not supported. Here is an implementation of caller with the Builder API:

def caller_3(n, p=None):
    with bd.BuildSketch() as skt:
        with bd.GridLocations(0, l, 1, n):
            if p is None:
                bd.Rectangle(w, l)
                bd.Rectangle(cut, cut, mode=bd.Mode.SUBTRACT)
            else:
                p
    return skt.sketch

with creates the desired objects. image Even detecting this situation is extremely difficult to do - this won't be fixed.

qwelyt commented 10 months ago

I'm not so sure that this is only related to mixing algebra and builder.

from ocp_vscode import show, show_object, reset_show, set_port, set_defaults, get_defaults, Camera
set_port(3939)

import build123d as bd

w = 19.05
l = 19.05
cut = 14.07

def repeater(n,p=None):
    with bd.BuildSketch() as skt:
        with bd.GridLocations(w,l,1,n):
            if p is None:
                bd.Rectangle(w,l)
                bd.Rectangle(cut,cut, mode=bd.Mode.SUBTRACT)
            else:
                bd.add(p)
    return skt

def caller1(n):
    with bd.BuildSketch() as skt:
        with bd.Locations((0,0),(w+2,0), ((w+2)*2,0)):
            bd.add(repeater(n=n,p=bd.Rectangle(w,l)))
    return skt.sketch

def caller2(n):
    with bd.BuildSketch() as skt:
        with bd.Locations((0,0),(w+2,0), ((w+2)*2,0)):
            bd.add(repeater(n=n))
    return skt.sketch

show(
    caller1(3),
    caller2(3).move(bd.Location((0,l*4,0))),
    reset_camera=Camera.KEEP
)

20231027_231405 The top one uses the default shape, caller2. The bottom one specifies p with something.

Why are results so different?

qwelyt commented 10 months ago

Or maybe it is. But it becomes somewhat convoluted to do things like this.

def caller1(n):
    with bd.BuildSketch() as t:
        bd.Rectangle(w,l)
    with bd.BuildSketch() as skt:
        with bd.Locations((0,0),(w+2,0), ((w+2)*2,0)):
            bd.add(repeater(n=n,p=t.sketch))
    return skt.sketch

Doing that instead of just adding the rectangle (or Sketch()+Rectangle(w,l)) works. So yes, perhaps the modes do mess this up. But I would like for this to be way more clear when you are trying to debug things like this. Printing the objects returns the exact same thing.

Sketch at 0x7f40ada08510, label(), #children(0)
Sketch at 0x7f5445b47e10, label(), #children(0)

Guess which sketch is witch?
If you don't do t.sketch and just send in t you ofc get the context

<build123d.build_sketch.BuildSketch object at 0x7f3082d11f10>

Since the origin of the sketch seem to matter, can this be included in the print?
Or perhaps throw an error as it is not supported. That might be better.