Localization of make_face changed

[jdegenstein]

from build123d import *

with BuildSketch(Plane.YZ) as s:
    with BuildLine(Plane.YZ) as l:
        n2 = JernArc((0,46),(1,0),40,-90)
        n3 = Line(n2@1,n2@0)
    make_face()

show_object(l.line)
show_object(s.sketch)

running the above code on build123d commit 943 results in the following screenshot in my static build of CQ-editor

Running the same code on my static build of CQ-editor with build123d commit 1018 results in the following screenshot:

It appears that something has changed, which results in unexpected behavior (the prior behavior made more sense to me)

[jdegenstein]

#OLD BEHAVIOR (commit 943)
s.sketch.bounding_box().center()
Out[5]: Vector: (0.0, 20.000000000000007, 26.000000000000004)

#NEW BEHAVIOR (commit 1018)
s.sketch.bounding_box().center()
Out[2]: Vector: (0.0, -26.000000000000007, -20.000000000000004)

Should be useful as a test to prevent this regression again in the future

[gumyr]

We need to discuss this (refer to sketching on other planes.

Given this code:

with BuildSketch(Plane.YZ) as s:
    # <working on a local Plane.XY>
    with BuildLine(Plane.YZ) as l:
        n2 = JernArc((0, 46), (1, 0), 40, -90)
        n3 = Line(n2 @ 1, n2 @ 0)
    make_face()
    # </working on a local Plane.XY>

the creation of lines on Plane.YZ (which is perpendicular to Plane.XY) means that the only way to create content that is applicable to the Sketch is to use the Z value of the Curve objects that can create 3D lines. This is almost certainly not what users expect to happen. What's generated now is the result of coordinate system mapping and isn't necessarily what should happen. If BuildLine is created on Plane.XY the following is generated which is what I believe to be correct:

So... I'm proposing that when BuildLine is called with a BuildSketch parent, if the given Plane isn't Plane.XY a ValueError is generated explaining that the user needs to work on the local Plane.XY. BuildLine used with a parent of BuildPart wouldn't have this check as it's likely to be creating a 3D path.

Is there another point of view on this issue?

[jdegenstein]

Full disclosure, I am coming at this from a usability perspective and therefore somewhat neglecting practical considerations. In my "perfect world" I could construct a BuildLine context anywhere (e.g. on the face of a 3D object) and then convert that into a face and extrude it (without translation or rotation). For some reason unknown to me something has changed. I used to be able to do this (as of build123d commit 943 LINK to static CQ-editor build ) which was a huge benefit of build123d over the much more restrictive way in which CQ deals with "2D". Being able to "draw locally" is a key feature of many CAD packages, so having to draw only on Plane.XY (or later relocate back to Plane.XY) seems like a significant limitation to me.

Consider the following improved example:

from build123d import *

with BuildPart() as p:
    Box(70,70,70)
    pln = Plane(p.part.faces().sort_by(Axis.Y)[-1])
    with BuildSketch(pln) as s:
        with BuildLine(pln) as l:
            m1 = CenterArc((0,0),30,0,300)
            m2 = SagittaArc((0,0),m1@1,-5)
            m3 = Line(m1@0,(0,0))
        make_face()
    extrude(amount=30)

show_object(l.line)
show_object(s.sketch)
show_object(p.part,options=rand_color(alpha=.5))

As you said, the current workaround is to put the nested BuildLine instance context back on Plane.XY, which results in correct sketch positioning but loses the convenience of having the visual preview of where the BuildLine will go:

with BuildPart() as p:
    Box(70,70,70)
    pln = Plane(p.part.faces().sort_by(Axis.Y)[-1])
    with BuildSketch(pln) as s:
        with BuildLine() as l: # <-- NOW ON Plane.XY
            m1 = CenterArc((0,0),30,0,300)
            m2 = SagittaArc((0,0),m1@1,-5)
            m3 = Line(m1@0,(0,0))
        make_face()
    extrude(amount=30)

The modified code results in the same behavior on commit 943 and commit 1033

Is it possible to restore the old behavior? That is what I think makes the most sense personally.

n.b. for testing convenience --> link to static build of CQ-editor with commit 1033

[gumyr]

Let's change the example a little to illustrate the problem:

from build123d import *
from ocp_vscode import *

with BuildPart() as p:
    with BuildSketch(Plane.XY.offset(-35)):
        Rectangle(70, 70)
    extrude(amount=70, taper=5)
    # Box(70, 70, 70)

    pln = Plane(p.part.faces().sort_by(Axis.Y)[-1])
    with BuildSketch(pln) as s:
        with BuildLine(pln) as l:
            # with BuildLine() as l:
            m1 = CenterArc((0, 0), 30, 0, 300)
            m2 = SagittaArc((0, 0), m1 @ 1, -5)
            m3 = Line(m1 @ 0, (0, 0))
        make_face()
    extrude(amount=30)

show(l, s, p)

With a face that isn't aligned to a major plane the math gets crazy and the bump on the side is no longer centered on the face. However with:

from build123d import *
from ocp_vscode import *

with BuildPart() as p:
    with BuildSketch(Plane.XY.offset(-35)):
        Rectangle(70, 70)
    extrude(amount=70, taper=5)
    # Box(70, 70, 70)

    pln = Plane(p.part.faces().sort_by(Axis.Y)[-1])
    with BuildSketch(pln) as s:
        # with BuildLine(pln) as l:
        with BuildLine() as l:
            m1 = CenterArc((0, 0), 30, 0, 300)
            m2 = SagittaArc((0, 0), m1 @ 1, -5)
            m3 = Line(m1 @ 0, (0, 0))
        make_face()
    extrude(amount=30)

show(l, s, p)

the bump is correctly positioned.

I agree that this can be confusing and it's desirable to draw the line "directly on the object"; however, I don't see how I can generalize this to work in all cases. ocp_vscode viewer does show both the local and global version of the sketch so one can see where things will go.

There needs to be a local sketch plane - this could be done by introducing a new local Plane and a whole new set of selectors that work only on this plane or leave things as they are and educate users that sketches are created on Plane.XY.

[Jojain]

I'm in favor of forcing the BuildLine to be in XY (i.e cannot change the plane) if its used in a sketch context. Since BuildSketch(Plane.YZ) will first draw on XY then relocate on YZ and you cannot actually REALLY draw on some other plane than XY. I think a nested BuildLine should also be forced to be drawn on XY otherwise it doesnt make any sense and the transforms gets all over the place.

The problem I have with the draw on XY then relocate is that it feels very weird when you start to try to project things to the plane.

Consider the following workflow :

• I create a box
• I want to create a sketch on a plane ZY
• I want to project the topleft corner and the top right corner of that box
• I want to draw a line that goes from these two projection.

With BuildLine nested and forced to be on XY the project method will project against plane XY while I actually wanted to project against ZY. Ofc I can provide the plane ZY to project but then I would need to transform back the ZY points onto XY to have them where I want on XY.

When you try to think about what is going on here its SUPER complicated and SUPER unintuitive. Drawing on XY has definitly benefits but also big drawbacks. This + the fact that it displays things on XY and not where the user expects it are raising difficulty for people in my opinion.

[gumyr]

Let's try that out:

from build123d import *
from ocp_vscode import *

with BuildPart() as bp:
    b = Box(10, 10, 10)
    with BuildSketch(Plane.ZY) as bs:
        pnts = project(
            b.vertices().group_by(Axis.X)[0].group_by(Axis.Z)[-1], mode=Mode.PRIVATE
        )
        with BuildLine() as bl:
            Polyline(*pnts, (0, 0), close=True)
        make_face()
    extrude(amount=5, mode=Mode.SUBTRACT)
show(bp, bs, bl)

This is what you intended to happen isn't it?

I really wish there was a simpler, more direct way to get around the sketching on Plane.XY - I personally think this is the weakest part of build123d.

[Jojain]

If you put the project in the BuildLine context here is what happens :

from build123d import *
from ocp_vscode import *

with BuildPart() as bp:
    b = Box(10, 10, 10)
    with BuildSketch(Plane.ZY) as bs:
        with BuildLine() as bl:
            pnts = project(
                b.vertices().group_by(Axis.X)[0].group_by(Axis.Z)[-1], mode=Mode.PRIVATE
            )
            Polyline(*pnts, (0, 0), close=True)
        make_face()
    extrude(amount=5, mode=Mode.SUBTRACT)
show(bp, bs, bl, *pnts)

I understand why, but this will lead you to have to repeat yourself hundred of times when people ask why it does this. It is working AS IT SHOULD. However it is not working AS USER EXPECT. I also agree that the sketching part is the only thing counter intuitive in B3D and if we could have a better solution it would be much better.

Once you are done with all the other features you want for 1.0.0 I would advice you to take a step back and try to consider solving this issue. With enough brain power we might find a solution that works better than what is currently written.