CadQuery / cadquery

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

Make offset2D act on objects instead of pending wires #1563

Closed blastrock closed 2 months ago

blastrock commented 2 months ago

Hi,

This PR is a bugfix, but I'm not sure this is the desired behavior, or even if this is a bug in the first place.

Here's a script to describe my issue:

    cq.Workplane("XY")
    .rect(5, 5)
    # .offset2D(1)
    .workplane(offset=5)
    .rect(3, 5)
    .workplane(offset=5)
    .rect(10, 20)
    # .offset2D(1)
    .consolidateWires()
    # .loft()

This object contains 3 rects on top of each other. Now if I uncomment the offset2D calls, I expect them to act on the single rect just before each call, so that the bottom and top rects get rounded, and the middle one is left untouched.

What actually happens is that offset2D calls _consolidateWires and acts on all pending wires of all the workplanes on this stack, so the second offset2D will add an offset to all 3 rects (the first one will be offset twice).

This patch allows offset2D to not act on all pending wires but only on current objects. This gives me the desired result, the middle rect is not offset and has sharp edges:

image image


There are a few questions I have about this:

Note that tests are broken, I am waiting for more input about whether this is the right thing to do before trying to fix them.

dov commented 2 months ago

This can actually be solved by my PR #1574 which allows adding wires directly to the WorkPlane. With this accepted, you could then create a closed wire for each of your three rectangles, and then offset2D (or use Wire.fillet() with #1573) only the first and last rectangle.

Actually even without #1574 being accepted you can easily get this functionality by monkey patching:

def addWire(self, wire):
    """Add a wire at the current point"""
    return self.eachpoint(lambda v : wire.moved(v))

cq.Workplane.addWire = addWire

I also see a problem with your PR only acting on the the last wire, since you might work on several disjoint objects, created by disjoint wires, and you would probably like to run offset2D() on each of them.

adam-urbanczyk commented 2 months ago

Please use cq.Sketch for this kind of use cases: https://cadquery.readthedocs.io/en/latest/sketch.html#workplane-integration

blastrock commented 2 months ago

Thanks for the answers! I struggled, but I managed to work it out with the sketch API!

In case someone wants to do the same: you need to do a .wires().offset(x).clean() to get the right sketch, and then you need to place each sketch with placeSketch as shown it the last example of this page