CadQuery / cadquery

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

Feature discussion: construct workplanes on top of faces #713

Closed NothanUmber closed 3 years ago

NothanUmber commented 3 years ago

This is a follow up of https://github.com/CadQuery/cadquery/issues/709.

Here we had a cube which's six faces were selected, a circle constructed on each and those extruded. My expectation from a beginners perspective was that the resulting cylinders would be perpendicular to the individual faces. I learned that this is not the current behavior (the circles seem to be on planes parallel to the base workplane through some (?) point on the respective face - which creates overlaps). Is there already a convenient way to achieve the desired behavior? If not, would it perhaps make sense to introduce a new option for the inPlane parameter of workplanes? E.g. "parent"? This could align the workplane parallel to the respective parent plane instead of taking over the orientation of the base plane.

So the example from the issue above could look like that: result = (cq.Workplane("XY") .box(10.0,10.0,10.0,centered=(True,True,True)) .faces() .workplane("parent") .circle(1.0,forConstruction=False) .extrude(1.0,combine=True,clean=False)) show_object(result)

That would create a box with six cylinders coming out of the six faces perpendicularly - easy to understand and potentially helpful in many cases. The other option would be to change the default behavior. But not sure whether people rely on the current alignment behavior or whether most consider the behavior just as "undefined" for non-parallel selected faces? But if people rely on the current behavior then the propsal above would allow to keep the current default behavior. Would this make sense and be desirable?

fedorkotov commented 3 years ago

I agree that this feature would be very nice to have.

But there is a problem. How does one chose face workplane.

Probably a new api implementing this cool feature would have to take a callback that will determine workplace orientation.

(*) I know that calling normalAt() repeatedly is wasteful. This is just to get away with lambda instead of separate function in this examples

def main_body():
    return cq.Workplane("XY")\
    .box(10.0,10.0,10.0,centered=(True,True,True))\
    .union(
        cq.Workplane("XY")
        .move(15,0)
        .box(10.0,10.0,10.0,centered=(True,True,True)))    

result = \
    main_body()\
    .union(
        main_body()
        .faces()
        .each(
            lambda f: cq.Workplane(
                    cq.Plane(
                        f.Center(), 
                        f.normalAt().add(
                            cq.Vector(
                                f.normalAt().x +1,
                                f.normalAt().y +1, 
                                f.normalAt().z +1))\
                            .cross(f.normalAt()), 
                        f.normalAt()), origin=f.Center())
                .circle(1).extrude(1).vals()[0],
            useLocalCoordinates = True))

image

def main_body():
    return cq.Workplane("XY")\
    .box(10.0,10.0,10.0,centered=(True,True,True))\
    .union(
        cq.Workplane("XY")
        .move(15,0)
        .box(10.0,10.0,10.0,centered=(True,True,True)))    

result = \
    main_body()\
    .union(
        main_body()
        .faces()
        .each(
            lambda f: cq.Workplane(
                    cq.Plane(
                        f.Center(), 
                        f.normalAt().add(
                            cq.Vector(
                                f.normalAt().x +1,
                                f.normalAt().y +1, 
                                f.normalAt().z +1))\
                            .cross(f.normalAt()), 
                        f.normalAt()), origin=f.Center())
                .rect(3,3).extrude(1).vals()[0], # only this line is different
            useLocalCoordinates = True))

image

NothanUmber commented 3 years ago

Thanks @fedorkotov for the example how to do this with each and lambda functions! Having the opportunity to override things sounds good (and is in some cases certainly necessary). Doesn't each face have a local coordinate system? So, couldn't the x-axis of the current selected face be used? This would also conveniently be orthogonal to the face's normal. Center of face for workplane center sounds plausible as default.

adam-urbanczyk commented 3 years ago

Faces don't have an LCS - current workplane has. -1 on this feature request. It'll be difficult to implement/maintain and would change current semantics.

If you want to achieve this kind of things use each, eachPoint, cutEach. In the future it might be possible to use cq.Sketch for this kind of operations too.

Related issue: #661

adam-urbanczyk commented 3 years ago

@jmwright @marcus7070 provisionally adding won't do

NothanUmber commented 3 years ago

Ok, understood. Imho it would just help if Cadquery would act a little more - as expected. At least from a beginner's perspective I am often confronted with the situation that I don't really grasp the internal logic why something behaves the way it does - e.g. when placing workplanes on surfaces. Placement often appears almost arbitrary to me and I have to use relative center commands to move the new scetch from where it is placed to where I want to have it. And turn it and align it as I want it. Predicting what a call does without looking at the output can often be quite hard because of the way internal state is transformed in sometimes non obvious ways.

I understand that you don't want to change things without a very good reason - makes totally sense. But in some ways at least from my current (still limited) perspective I am not sure whether additions (or even breaking changes) regarding plausible defaults and conventions can really be avoided when the goal is to make Cadquery intuitive to use and most importantly behaving predictively (without having to look into the implementation to understand what is going on or to fall back to trial and error).

I'll continue the road for some more time (as in some cases I think it's just me, not the tool), but I think it would help a lot to give Cadquery e.g. to some of your friends to get first-hand feedback of the first contact experience from people who optimally have some domain or even CAD experience but aren't Cadquery devs themselves.

fedorkotov commented 3 years ago

I have an idea of a face coordinate system selection method.

Face coordinate system selection algorithm

As I suggested above origin is at face center, Z is face normal at origin and x axis can be chosen as flows

  1. Chose a set V of at least 3 unit vectors v_i with additional unique weights w_i. These vectors do not have to be linearly independent but their span (linear hull) should be all 3D vector space for this method to work on arbitrary faces. In some cases this requirement can be relaxed.
  2. Select v of V with minimum projection on Z (face normal). If several vectors of V have the same projection of Z, select one of them with minimum weight
  3. Select X axis as normalize(v - (v dot Z)*Z) (subtract from v it's projection on Z and normalize result

In other words X axis is selected as close as possible to one of selected set of vectors. I have implemented this algorithm in attached module apply_to_all.py.

typical usage is something like

result = apply_to_each_face(
        main_body.faces("filter here"),
        XAxisClosestTo(VECTORS_LIST),
        lambda wp, face:  wp.draw_single_volume())

This algorithm is not universal but IMHO it makes using each much easier in a lot of cases. It is usable as a module separate from cadquery but it would look nicer as a Workplane method I think.

result = 
        main_body.faces("filter here")\
        .apply_to_each_face(
            XAxisClosestTo(VECTORS_LIST),
            lambda wp, face:  wp.draw_single_volume())

Examples

All examples below assume these imports

from apply_to_all import \
    apply_to_each_face, \
    XAxisClosestTo, \
    WORLD_AXIS_UNIT_VECTORS_XYZ, \
    WORLD_AXIS_UNIT_VECTORS_XZY, \
    WORLD_AXIS_UNIT_VECTORS_YXZ, \
    WORLD_AXIS_UNIT_VECTORS_YZX, \
    WORLD_AXIS_UNIT_VECTORS_ZXY, \
    WORLD_AXIS_UNIT_VECTORS_ZYX

Example 1

def main_body():
    return cq.Workplane("XY")\
    .box(10.0,10.0,10.0,centered=(True,True,True))\
    .union(
        cq.Workplane("XY")
        .move(15,0)
        .box(10.0,10.0,10.0,centered=(True,True,True))) 

result =\
    main_body().union(
    apply_to_each_face(
        main_body().faces(),
        XAxisClosestTo(WORLD_AXIS_UNIT_VECTORS_XYZ),
        lambda wp, face:\
            wp.rect(2,4).extrude(1)))

image

Example 2

def main_body():
    return cq.Workplane("XY")\
    .polygon(8, 10.0)\
    .extrude(5)

result =\
    main_body().union(
    apply_to_each_face(
        main_body().faces("#Z"),
        XAxisClosestTo(WORLD_AXIS_UNIT_VECTORS_ZXY),
        lambda wp, face:\
            wp.rect(1,2).extrude(1)))

image

Example 3

def main_body():
    return cq.Workplane("XY")\
    .polygon(6, 10.0)\
    .extrude(3, taper=45)

result =\
    main_body().faces("<Z").shell(-0.5).cut(
    apply_to_each_face(
        main_body().faces("not <Z"),
        XAxisClosestTo(WORLD_AXIS_UNIT_VECTORS_ZXY),
        lambda wp, face:\
            wp.add(face)\
              .wires()\
              .toPending()\
              .offset2D(-0.8)\
              .extrude(-2)))

image

apply_to_all.py.txt

NothanUmber commented 3 years ago

Looks really promising! Thanks particularly for adding the examples 2 and 3 where faces aren't parallel to XY, XZ or YZ anymore and somewhere in the middle of the body. This happened a lot to me in my first "bigger" project - and these are the cases where it was more/very difficult to select and place things. E.g. when faces get created as the result of boolean intersections of non-cubic bodies the spatial placement of that face and particularly the orientation and positioning of sketches placed on such a face can be not so easy to describe formally, particularly if all these bodies are also parametric. It's easy to say what is wanted but it can be hard to formulate (with my current knowledge level) in CadQuery.

marcus7070 commented 3 years ago

CadQuery is structured in layers, from the low-level to high-level:

  1. OCP/OCCT
  2. occ_impl
  3. fluent api

Each layer gets more restrictive with what can be modelled, but much more simple to use. The concept of a 2D workplane is not required to produce a model, it's just a simplification that makes CQ (and other CAD software) easier to use. I would say that having multiple workplanes active at once is too complicated, and even more complicated than just using the occ_impl layer.

marcus7070 commented 3 years ago

Also, here is the current method of doing @fedorkotov's example 2.

def box(face):
    origin = face.Center()
    normal = face.normalAt()
    x_dir = cq.Vector(0, 0, 1).cross(normal)
    plane = cq.Plane(origin, x_dir, normal)
    loc = cq.Location(plane)
    box = cq.Solid.makeBox(2, 1, 1, pnt=cq.Vector(-1, -0.5, 0)).located(loc)
    return box

result = (
    cq.Workplane()
    .polygon(8, 10)
    .extrude(5)
)
result = result.union(
    result
    .faces("#Z")
    .each(box)
)

I'm sure there is a better way to create that Location rather than using a Plane intermediate, and I'm thinking Workplane.each could use a combine option which would avoid that final union step.

marcus7070 commented 3 years ago

Ok, understood. Imho it would just help if Cadquery would act a little more - as expected. At least from a beginner's perspective I am often confronted with the situation that I don't really grasp the internal logic why something behaves the way it does

I would really encourage you to read the source. I always have a second window open with the source code showing when I write CQ models.

The code for Workplane.workplane is a bit messy though, sorry. We have a few improvements planned for that method, I hope to refactor and tidy the code at the same time, so users can more easily understand how that method works.

fedorkotov commented 3 years ago

@marcus7070

I would say that having multiple workplanes active at once is too complicated [..]

But it's the whole point of using each. If you call each on faces presumably you want to position something relative to each face. Otherwise you would just create it in some other way. So typical use case of each includes two sub-tasks

  1. Choosing coordinate frame for each face
  2. Actually modelling in that frame

Now when using each I have to do both of these things manually. But why do I have to? Why do I have to think of normals, vector products and projections when all I want is to place a thingamajig in the center of each face? Cadquery could have automated coordinate system selection for me in a lot of cases. That is why I still think that something like my apply_to_each_face could be a useful addition to cadquery

By the way I added one more X axis selection algorithm XAxisInPlane. X axis is selected in one of user-specified planes. Now apply_to_each_face can do this example which it previously could not.

from apply_to_all import \
    apply_to_each_face, \
    XAxisInPlane, \
    WORLD_XY_NORMAL, \
    WORLD_ZX_NORMAL, \
    WORLD_YZ_NORMAL

def main_body():
    return cq.Workplane("XY")\
    .polygon(6, 10.0)\
    .extrude(3, taper=45)    

result =\
    main_body().union(
    apply_to_each_face(
        main_body().faces("#Z"),
        XAxisInPlane([
            WORLD_XY_NORMAL, 
            WORLD_YZ_NORMAL]),
        lambda wp, face:\
            wp.rect(2,1).extrude(2))) 

image

apply_to_all.py.txt

Jojain commented 3 years ago

Putting the implementation/maintenance of this apart I also think this is pretty useful. If it doesn't find it's place in cadquery directly I think this could be a nice plugin to have.

As @fedorkotov said dealing with normal cross product and stuff makes it annoying and may also just be too difficult for the user that is not too familiar with these concepts.

fedorkotov commented 3 years ago

Can't refrain of sharing one more cool-looking thingy

def main_body():
    return cq.Workplane("XY")\
    .rect(10.0, 10.0)\
    .extrude(10)    

result =\
    main_body().union(
    apply_to_each_face(
        main_body().faces(),
        XAxisClosestTo(WORLD_AXIS_UNIT_VECTORS_ZXY),
        lambda wp, face:\
            wp.add(face)\
              .wires()\
              .toPending()\
              .twistExtrude(10, 45)))\
    .edges()\
    .fillet(3)

image

NothanUmber commented 3 years ago

Also, here is the current method of doing @fedorkotov's [example 2] [...] x_dir = cq.Vector(0, 0, 1).cross(normal)

Thank you for this example! Does this assume that the normal is already orthogonal to the absolute Z-axis? (edit: apparently the vector can have arbitrary length - so no, but that way x would always be parallel to the global XY plane)

E.g. Blender allows to switch to a local coordinate system for each element (e.g. plane). It is often very easy to formulate transformations or locations in the local coordinate system but can be hard in the global system as one has to essentially apply all transformations that were applied to the plane also to the target vectors. (That was at least my workaround when selecting faces and putting workplanes on top just didn't work the way I expected - always constructing small parts from the XY workplane and then transforming stuff myself and just using an assembly to put things together. Which works but is a lot more difficult (and longer) than if I could put a workplane in a predictable way on an already existing face).

Edit: I think the way how Blender retrieves the local coordinate system is to have the local coordinate system as an empty joined to each element. And this empty is transformed together with the element. So if I rotate the element I implicitly also rotate the local coordinate system. That way they can do this even without having a full construction history of things. For new faces that were created e.g. from boolean operations I think they use normal vector, a vector parallel to the first edge and the cross product of those (but there I am not sure). Additionally it is possible to explicitly transform the local coordinate system relative to the global one, so if the default ends up with something undesired it can manually be changed - which is of course easier in a system without history that just stores the current state.

Edit2: My assumption that x is just parallel to the first edge in Blender in "normal coordinates" is apparently wrong (they have global, local, normal, gimbal, view and custom coordinate systems): image

marcus7070 commented 3 years ago

I don't have enough time to comment on this properly, but just wanted to let you know you raise some good points @fedorkotov and I'll get back to this soon.

Does this assume that the normal is already orthogonal to the absolute Z-axis?

Not orthogonal, just that the normal is not in line with the Z-axis. Although it's also pretty easy to check if the normal is in line with cq.Vector(0, 0, 1) and then use (0, 1, 0) in the cross product instead.

adam-urbanczyk commented 3 years ago

Alright @fedorkotov @NothanUmber let's keep some things organized:

  1. Workplane (cq.Workplane) is a fundamental concept of CQ. It will not change in the foreseeable future and multiple wokrplanes at once won't be supported.
  2. Construction of new workplanes based on selection (cq.Workplane.wokrplane) can be improved, extended. AFAIR there are some issues on this topic (#464, #357)
  3. each and co have no relation to single or multiple workplanes support. Again, there is some space to improve/extend those methods.

@NothanUmber I don't know Blender, but likely it uses very different semantics than CQ - It does not help to refer to LCS from Blender because this concept does not exist in CQ. Side-note: In general for complex shapes it not very obvious how the LCS should look (center, corner, which normal for curved faces...)

@fedorkotov great that you want to contribute the code to plugins!

NothanUmber commented 3 years ago

Sure, not trying to convert CQ to Blender, it's good that both exist and it is good to have maintainers with a clear vision of the usage philosophy for each application! Just thought it might help regarding the question fedorkotov raised - how to determine the x axis for a workplane that is aligned with a face - which from my understanding isn't too far off from a normal based "local" coordinate system. But on second look his proposal might not be too different from what they do anyways.

fedorkotov commented 3 years ago

My plugin that provides functionality requested by @NothanUmber is merged. I think this can be closed now.

jmwright commented 3 years ago

Here's a direct link to the plugin for reference.

https://github.com/CadQuery/cadquery-plugins/tree/main/plugins/apply_to_each_face

NothanUmber commented 3 years ago

Thanks @fedorkotov, this is really useful, indeed!

Have experimented a little with the extension. Perhaps some "convenience" defaults might make things even easier to use? E.g. when having symmetric constructs on the faces, people might not care about the x axis (thus a default might make sense). And in most cases one probably doesn't need the face in the lambda. But having all these options (including providing your own selector function) is of course good if needed! Edit: Perhaps the union could also be put into applyToEachFace? Would there be a disadvantage?

from typing import Callable, Union

import cadquery as cq
import apply_to_each_face as af

def applyToEachFace2(
    wp: cq.Workplane,
    f_draw: Union[Callable[[cq.Workplane], cq.Workplane], Callable[[cq.Workplane, cq.Face], cq.Workplane]],
    f_workplane_selector: Union[Callable[[cq.Face], cq.Workplane], str] = None,
    union=True
) -> cq.Workplane:
    if f_workplane_selector is None:
        _f_workplane_selector = af.XAxisClosestTo(
            af.WORLD_AXIS_UNIT_VECTORS_XYZ)
    elif isinstance(f_workplane_selector, str):
        if f_workplane_selector is "XYZ":
            _f_workplane_selector = af.XAxisClosestTo(
                af.WORLD_AXIS_UNIT_VECTORS_XYZ)
        elif f_workplane_selector is "XZY":
            _f_workplane_selector = af.XAxisClosestTo(
                af.WORLD_AXIS_UNIT_VECTORS_XZY)
        elif f_workplane_selector is "YXZ":
            _f_workplane_selector = af.XAxisClosestTo(
                af.WORLD_AXIS_UNIT_VECTORS_YXZ)
        elif f_workplane_selector is "YZX":
            _f_workplane_selector = af.XAxisClosestTo(
                af.WORLD_AXIS_UNIT_VECTORS_YZX)
        elif f_workplane_selector is "ZXY":
            _f_workplane_selector = af.XAxisClosestTo(
                af.WORLD_AXIS_UNIT_VECTORS_ZXY)
        elif f_workplane_selector is "ZYX":
            _f_workplane_selector = af.XAxisClosestTo(
                af.WORLD_AXIS_UNIT_VECTORS_ZYX)

        elif f_workplane_selector is "XY_ZX_YZ":
            _f_workplane_selector = af.XAxisInPlane(
                af.WORLD_AXIS_PLANES_XY_ZX_YZ)
        elif f_workplane_selector is "XY_YZ_ZX":
            _f_workplane_selector = af.XAxisInPlane(
                af.WORLD_AXIS_PLANES_XY_YZ_ZX)
        elif f_workplane_selector is "YZ_XY_ZX":
            _f_workplane_selector = af.XAxisInPlane(
                af.WORLD_AXIS_PLANES_YZ_XY_ZX)
        elif f_workplane_selector is "YZ_ZX_XY":
            _f_workplane_selector = af.XAxisInPlane(
                af.WORLD_AXIS_PLANES_YZ_ZX_XY)
        elif f_workplane_selector is "ZX_XY_YZ":
            _f_workplane_selector = af.XAxisInPlane(
                af.WORLD_AXIS_PLANES_ZX_XY_YZ)
        elif f_workplane_selector is "ZX_YZ_XY":
            _f_workplane_selector = af.XAxisInPlane(
                af.WORLD_AXIS_PLANES_ZX_YZ_XY)
        else:
            raise Exception(f"unknown X alignment {f_workplane_selector}")

    else:
        _f_workplane_selector = f_workplane_selector
    if f_draw.__code__.co_argcount == 1:
        _f_draw = lambda wp, _: f_draw(wp)
    else:
        _f_draw = f_draw

    if union:
        return wp.union(
            wp.applyToEachFace(
                _f_workplane_selector,
                _f_draw),
            wp)
    else:
        return wp.applyToEachFace(
                _f_workplane_selector,
                _f_draw)

cq.Workplane.applyToEachFace2 = applyToEachFace2

def main_body():
    return cq.Workplane("XY")\
    .box(3, 3, 3)

def substructure(workplane):
    return workplane.rect(2,1).extrude(1)

obj = main_body().faces().applyToEachFace2(substructure, "XYZ")
NothanUmber commented 3 years ago

@marcus7070: Think I found one major cause for my initial confusion and why the center of the workplane on a face appeared almost random to me: The default centerOption is ProjectedOrigin, I expected the workplane to be centered on the face though and corrected this with manual relative center-calls, shaking my head (instead of looking into the implementation...). In the example below it is still relatively easy to comprehend how the center point was calculated, but in general ProjectedOrigin, being quite dependent on the internal state of the workplane can be challenging to predict (particularly if one isn't expecting the projection). Using CenterOfMass as default might have been more predictable - but I understand that this cannot be changed now anymore as it would break existing code.

There are a number of places in the docu that might be confusing though (the statements there aren't really wrong because in these examples the ProjectedOrigin is coincidentally equivalent to the CenterOfMass but at least I drew the wrong conclusions from these): https://cadquery.readthedocs.io/en/latest/quickstart.html?highlight=center%20of%20mass#more-holes https://cadquery.readthedocs.io/en/latest/quickstart.html?highlight=workplane%20default%20mass#add-the-holes etc. Might perhaps be better to explicitly set centerOption to "CenterOfMass" in these tutorials?

from typing import Callable, Union

import cadquery as cq

class MyWorkplane(cq.Workplane):
    def substructure(workplane):
        return workplane.workplane().rect(2,1).extrude(1)
    def substructure2(workplane):
        return workplane.workplane(centerOption="CenterOfMass").rect(2,1).extrude(1)

obj = MyWorkplane("XY")\
    .box(3, 3, 3)\
    .faces(">Z").substructure()\
    .faces(">X").substructure()

obj2 = MyWorkplane("XY")\
    .center(0, 5)\
    .box(3, 3, 3)\
    .faces(">Z").substructure2()\
    .faces(">X").substructure2()

grafik

adam-urbanczyk commented 3 years ago

ProjectedOrigin is indeed the new default and won't be changed again. BTW: this issue is not the right place for this discussion.

NothanUmber commented 3 years ago

Ah, ok, it was changed. Can you please point me to the discussion about this topic so I can understand the rationale?

adam-urbanczyk commented 3 years ago

Rationale: for asymmetric faces centerOfMass is unpredictable/useless.

NothanUmber commented 3 years ago

Ok, so perhaps I just haven't come across a case where ProjectedOrigin would have been what I was looking for. But for me I know now what to do. Adapting the docu might be helpful though.

NothanUmber commented 3 years ago

@adam-urbanczyk sorry if my assessment comes across as harsh. Intuition can be very subjective and depend a lot on background and use-case. What makes sense as a default for me might not make sense for others and vice versa. Perhaps it would be best not to make assumptions about defaults in cases where there is no obviously "right" answer and try to be as explicit as possible/necessary? Will at least for me mention the centerOption explicitly in any case, even if it should one day be ProjectedOrigin.

Think I ran into a similar trap with the applyToEachFace2 proposal above, making an arbitrary assumption about a good default x-axis direction, which might at least as often be wrong as right. So probably at least in this case it might be better to not provide a default at all (like in @fedorkotov 's original implementation) and perhaps just make expressing the desired direction explicitly as succinct as possible (e.g. with the "XYZ" etc. abbreviation strings for the 12 most used variants).