CadQuery / cadquery

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

[Bug] Incorrect transform chaining in nested assemblies #1628

Closed lenianiva closed 3 weeks ago

lenianiva commented 1 month ago

Consider this example (the make_arrow and mark_plane functions are used for visualization only)

import cadquery as Cq

def make_arrow(size: float = 0.1) -> Cq.Workplane:
    """
    Makes an arrow object pointing +Z
    """
    cone = Cq.Solid.makeCone(
        radius1 = size,
        radius2 = 0,
        height=size)
    result = (
        Cq.Workplane("XY")
        .cylinder(radius=size / 2, height=size, centered=(True, True, False))
        .union(cone.located(Cq.Location((0, 0, size))))
    )
    result.faces("<Z").tag("dir_rev")
    return result
def mark_plane(self: Cq.Assembly,
               tag: str,
               size: float = 0.1,
               color: Cq.Color = Cq.Color(0, 0, 0, 1)) -> Cq.Assembly:
    """
    Adds a marker to make a plane constraint visible
    """
    name = tag.replace("?", "__T").replace("/", "__Z") + "_marker"
    print("Marker name:", name)
    return (
        self
        .add(make_arrow(size), name=name, color=color)
        .constrain(tag, f"{name}?dir_rev", "Plane", param=180)
    )

Cq.Assembly.markPlane = mark_plane

### Bug ###

def makeCube() -> Cq.Workplane:
    result = (
        Cq.Workplane()
        .box(1.5, 1, 1)
    )
    v = Cq.Vector(0, 0, -1)
    (
        result.faces(">X").workplane().moveTo(.5, .5)
        .eachpoint(Cq.Edge.makeLine(v * (-1), v).moved, useLocalCoordinates=True)
        .tag("special")
    )
    #result.faces(">X").tag("special")
    return result
def two_cubes():

    return (
        Cq.Assembly()
        .add(makeCube(), name="c1", color=Cq.Color(0, 0.7, 0.7, .5))
        .add(makeCube(), name="c2", color=Cq.Color(0, 0.7, 0.3, .5),
             loc=Cq.Location((0, 1, 0), (0, 0, 1), 90))
        #.constrain("c1@faces@>Y", "c2@faces@>Y", "Plane")
        #.constrain("c1@faces@>X", "c2@faces@>X", "Axis", param=90)
        #.solve()
    )
def three_cubes():
    return (
        Cq.Assembly()
        .add(makeCube(), name="c", color = Cq.Color(0.8, 0, 0.8, .5))
        .add(two_cubes(), name="a")
        .constrain("c", "Fixed")
        .constrain("a/c1@faces@<Y", "c@faces@>Y", "Plane")
        .constrain("a/c1@faces@>X", "c@faces@>X", "Axis", param=45)
    )

inner = (
    three_cubes()
    .markPlane("a/c2?special", color=Cq.Color(1, 1, 1, 1))
    .solve()
)
assembly = (
    Cq.Assembly()
    .add(inner, name="inner")
    .markPlane("inner/a/c2?special")
    .solve()
)
show_object(assembly)

There is a sequence of nested assemblies in this structure

inner
|- c
|- a
   |- c1
   |- c2

I added two arrows, a white one, which is solved at the a level, and a black one, solved at the outermost level. These two arrows are constrained to the same object, inner/a/c2?special, so their locations and orientations should be identical, but what I got is this:

Screenshot 2024-07-13 at 14 39 54

The white arrow, which is solved along with the inner assembly, stays in place, but the black arrow is off.

I discovered this behaviour when I made an assembly with mounting holes. The holes are nested in several layers of subassemblies and the position of the holes are divergent from the Edge objects which mark the centre of these holes.

Is this expected behaviour? How can I prevent the divergence of tag locations in subassemblies?

lenianiva commented 1 month ago

I think I found the error. The transform chaining order in _subloc is wrong. This fixes the bug:

def _subloc(self, name: str) -> Tuple[Cq.Location, str]:
    """
       Calculate relative location of an object in a subassembly.

       Returns the relative positions as well as the name of the top assembly.
       """

    rv = Cq.Location()
    obj = self.objects[name]
    name_out = name

    if obj not in self.children and obj is not self:
        locs = []
        while not obj.parent is self:
            locs.append(obj.loc)
            obj = cast(Cq.Assembly, obj.parent)
            name_out = obj.name

        rv = reduce(lambda l1, l2: l1 * l2, reversed(locs)) # <- fix

    return (rv, name_out)
Cq.Assembly._subloc = _subloc

Note the fix is to reverse the locs array. image

This makes sense since the transform is in the order (grand_child, child, this, parent, ...), and location transform should be multiplied in the order parent_t * this_t * child_t * ...

If the maintainers are fine with this fix I'll open a PR.

lorenzncode commented 1 month ago

Thank you for the example code. It is not exactly a minimal reproducible example and can be simplified - not always easy! In the future, I'd also suggest to follow the import convention used in the docs import cadquery as cq.

Here is an attempt at a MRE.

import cadquery as cq
from cadquery.occ_impl.shapes import *

b1 = box(1, 1, 1)
b2 = cq.Workplane().add(b1)
b2.faces(">Z").vertices("<XY").tag("vtag")

cone1 = cq.Workplane().add(cone(0.2, 2))
cone1.vertices(">Z").tag("conetip")
cone1.faces("<Z").tag("conebase")

sub1 = (
    cq.Assembly()
    .add(b1, name="partI", color=cq.Color("green"))
    .add(
        b2,
        name="partJ",
        color=cq.Color("blue"),
        loc=cq.Location((0, 0, 1), (0, 0, 1), 45),
    )
)

assy1 = (
    cq.Assembly().add(b1, name="partX", color=cq.Color("red")).add(sub1, name="sub1")
)
assy1 = assy1.add(cone1, name="marker1", color=cq.Color("green"))
assy1.constrain("partX", "Fixed")

# green box stacked on its side
assy1.constrain("sub1/partI@faces@>Z", "FixedAxis", (0, 1, 0))
assy1.constrain("partX@faces@>Z", "sub1/partI@faces@<Y", "Plane")

assy1.constrain("marker1?conetip", "sub1/partJ?vtag", "Point")
assy1.constrain("marker1?conebase", "FixedAxis", (0, 0, 1))
assy1 = assy1.solve()

assy2 = (
    cq.Assembly()
    .add(assy1, name="assy1")
    .add(cone1, name="marker2", color=cq.Color("red"))
)
assy2.constrain("assy1/partX", "Fixed")
assy2.constrain("marker2?conetip", "assy1/sub1/partJ?vtag", "Point")
assy2.constrain("marker2?conebase", "FixedAxis", (0, 0.2, 1))
assy2.solve()

show_object(assy1)
show_object(assy2)

The green cone is used as the "arrow" marker. The tip of the green cone points to a vertex on the blue box (OK). The tip of the red cone is expected to also point to the vertex on the blue box (it does not - the result is not expected).

image

lenianiva commented 1 month ago

Thank you for the example code. It is not exactly a minimal reproducible example and can be simplified - not always easy! In the future, I'd also suggest to follow the import convention used in the docs import cadquery as cq.

Here is an attempt at a MRE.

import cadquery as cq
from cadquery.occ_impl.shapes import *

b1 = box(1, 1, 1)
b2 = cq.Workplane().add(b1)
b2.faces(">Z").vertices("<XY").tag("vtag")

cone1 = cq.Workplane().add(cone(0.2, 2))
cone1.vertices(">Z").tag("conetip")
cone1.faces("<Z").tag("conebase")

sub1 = (
    cq.Assembly()
    .add(b1, name="partI", color=cq.Color("green"))
    .add(
        b2,
        name="partJ",
        color=cq.Color("blue"),
        loc=cq.Location((0, 0, 1), (0, 0, 1), 45),
    )
)

assy1 = (
    cq.Assembly().add(b1, name="partX", color=cq.Color("red")).add(sub1, name="sub1")
)
assy1 = assy1.add(cone1, name="marker1", color=cq.Color("green"))
assy1.constrain("partX", "Fixed")

# green box stacked on its side
assy1.constrain("sub1/partI@faces@>Z", "FixedAxis", (0, 1, 0))
assy1.constrain("partX@faces@>Z", "sub1/partI@faces@<Y", "Plane")

assy1.constrain("marker1?conetip", "sub1/partJ?vtag", "Point")
assy1.constrain("marker1?conebase", "FixedAxis", (0, 0, 1))
assy1 = assy1.solve()

assy2 = (
    cq.Assembly()
    .add(assy1, name="assy1")
    .add(cone1, name="marker2", color=cq.Color("red"))
)
assy2.constrain("assy1/partX", "Fixed")
assy2.constrain("marker2?conetip", "assy1/sub1/partJ?vtag", "Point")
assy2.constrain("marker2?conebase", "FixedAxis", (0, 0.2, 1))
assy2.solve()

show_object(assy1)
show_object(assy2)

The green cone is used as the "arrow" marker. The tip of the green cone points to a vertex on the blue box (OK). The tip of the red cone is expected to also point to the vertex on the blue box (it does not - the result is not expected).

image

I don't think this is any more simple than my example. The simplest you can do is to check the Assembly._subloc of a/c2?special and inner/a/c2?special and notice that they are unequal.