[BUG] Pocket clipper does not generate correct paths for circles

[giannissc]

[voneiden]

Possible to provide the code for a MRE? And slightly elaborate the issue? Is it the excessive retracts?

[giannissc]

From what I understood this is an issue with pyclipper. It seem to be working by discretizing paths into points and then offsetting those points. Hence if the precision of the discetization is too low, arcs and splines are a very low resolution approximation of the real thing. If you increase the precision then pyclipper generates hundreds of tiny line segment and a result hundreds of G1 commands taking tiny steps.

[voneiden]

Correct, clipper works only with linear segments. See #11 for notes on exploring using OCCT for offsetting. Unfortunately it's a very buggy road. Short summary out of my memory:

Clipper pros

• Fast
• Stable

Clipper cons

• Linear segments only
• Tiny scaling errors

OCCT pros

• Can retain pure geometry.. in theory (arcs, circles, b-splines)

OCCT cons

• Offsetting can create invalid geometry
• Offsetting can create inverse geometry when scaling down
• Offsetting can create malformed geometry (b-splines)
• Circle offset bug (btw, is your workaround for this different from the one in cq-cam?)
• Slower

Currently the clipper pocketing is the default one because it's far more reliable, but I'm certainly open for attempting to put more effort on the OCCT pocketing. Profiling still uses OCCT offsetting (generally it's a bit harder to hit these edge cases in the offsets used for profiling)

[voneiden]

Precision issues in arc/circle interpolation can also be related to my recent changes in #44

Clippers ArcTolerance was a bit tricky to tie into precision and I may have made poor choices there.

[giannissc]

From my experiments clipper is much slower (if you increase the precision to get smooth paths)

There is a library in rust called kurbo that recently implemented robust offset curves. The library itself contains all the primitives you can find in OCCT but the offset logic was for splines. I will ask around in their forum to check if it can be generalized and if it is I can start creating bindings for python

[giannissc]

Just generated my first gcode using cq-cam!! I didn't manage to test it today but I will do so first thing tomorrow!

[giannissc]

btw, is your workaround for this different from the one in cq-cam

My problem with the offsetting provided by cadquery and subsequently OCCT is that it is not translation and rotation invariant. So if you were to translate or rotate a design before using the offset it would appear in the wrong place. My solution is a bit convoluted and I am not sure that it is the most efficient way to do thing but it has been robust so far (apart from when working with cq_cam for some reason; then I have to reverse to the old offset2D code).
Long story short: (1) I calculate the center and normal for the original wire, (2) run the offset2D code, (3) compare the original center with the offset center and the angle between the original normal and the offset normal, (4) if there was no change I do nothing and if there was I translate and rotate to the correct location.

Here is my code:

def vectorAngle(self:Vector, normal:Vector):
    dot_product = np.dot(self.toTuple(), normal.toTuple())
    return np.arccos(dot_product)*180/math.pi

def triangleFromPoints(point1:Vector, point2: Vector, point3:Vector):
    line1 = Edge.makeLine(point1, point2)
    line2 = Edge.makeLine(point1, point3)
    return Wire.assembleEdges([line1,line2]).close()

def remap(self: Wire, center_from: Vector, normal_from: Vector):
    center_to = self.CenterOfBoundBox()
    normal_to = Face.makeFromWires(self,[]).normalAt()

    # Translation
    self = self.translate(center_from-center_to)
    
    # Angle Calculation
    angle = normal_from.angle(normal_to)
    
    # Rotation Axis
    face_wire = Wire.makeTriangle(center_from, center_from+normal_from, center_from+normal_to)
    face_normal = Face.makeFromWires(face_wire,[]).normalAt()
    
    axis_start = center_from
    axis_end = center_from + face_normal
    
    # Rotation
    return self.rotate(axis_start,axis_end, -angle)

def offset2DfixWire(self: Wire, d: float, kind: Literal["arc", "intersection", "tangent"] = "arc") -> List["Wire"]:
    wire = self
    center_from = self.CenterOfBoundBox()
    normal_from = Face.makeFromWires(wire,[]).normalAt()
    
    kind_dict = {
        "arc": GeomAbs_JoinType.GeomAbs_Arc,
        "intersection": GeomAbs_JoinType.GeomAbs_Intersection,
        "tangent": GeomAbs_JoinType.GeomAbs_Tangent,
    }

    offset = BRepOffsetAPI_MakeOffset()
    offset.Init(kind_dict[kind])
    offset.AddWire(self.wrapped)
    offset.Perform(d)

    obj = downcast(offset.Shape())

    if isinstance(obj, TopoDS_Compound):
        ws = [self.__class__(el.wrapped) for el in Compound(obj)]
    else:
        ws = [self.__class__(obj)]

    rv = list(chain(w.remap(center_from, normal_from) for w in ws))
    
    return rv

def offset2DfixWorkplane(self: Workplane, d: float, kind: Literal["arc", "intersection", "tangent"] = "arc", forConstruction: bool = False) -> Workplane:
    ws = self._consolidateWires()
    rv = list(chain.from_iterable(w.offset2D(d, kind) for w in ws))

    self.ctx.pendingEdges = []
    
    if forConstruction:
        for wire in rv:
            wire.forConstruction = True
        self.ctx.pendingWires = []
    else:
        self.ctx.pendingWires = rv

    return self.newObject(rv)

def toBox(self:BoundBox):
    x = self.xlen
    y = self.ylen
    z = self.zlen
    
    point = self.center
    point.x = point.x-x/2
    point.y = point.y-y/2
    point.z = point.z-z/2
    
    return Solid.makeBox(x,y,z, pnt=point)

if (__name__ == 'cq_additions'):
    Vector.angle = vectorAngle
    Wire.makeTriangle = triangleFromPoints
    Wire.remap = remap
    Wire.offset2D = offset2DfixWire
    Workplane.offset2D = offset2DfixWorkplane
    BoundBox.toBox = toBox

[giannissc]

I scanned through OCCT bugs and roadmap and it seem like they are going to address some offsetting bugs for OCCT 7.8!

[voneiden]

That's interesting, I need to check that particular bug out. I'm also delighted to see that you've already taken a deep dive into this topic on the OCCT side.

Should we try implementing a centralized robust_offset method that tries to handle OCCT offsetting while also trying to work around all the possible known issues?

[giannissc]

I just spent the entire morning investigating why my offset bug fix does not work on cq_cam and I think I have an explanation. It might require me to dig in deeper. So the bug is a bit more comlex than I originally thought so I will be posting a more detailed discussion later on with code samples.

[giannissc]

Is this the circle offset bug btw? Cause I think it is still happening..!

This bug is so weird cause I have the exact same design in two other files and both of them render correctly..!

[giannissc]

I will also need to open separate issues for the offsetting bug to keep the discussion relevant. On another note, I have talked to the kurbo guys and they are keen on supporting our usecase. Their code does indeed work with beziers for which they provide curve fitting algorithms to convert arbitrary curves to beziers. The you can execute the offsetting code and you get beziers (quadratic) as an output which you can then flatten and generate G0/G1 commands. I also asked if there is a way to convert bezier back into arcs to support G2/G3 commands and while they don't support this usecase just yet they are happy to add it. I think I will investigate this further locally and then try to integrate it into cq_cam

[giannissc]

Should we try implementing a centralized robust_offset method that tries to handle OCCT offsetting while also trying to work around all the possible known issues?

That's not that bad of an idea!

[voneiden]

kurbo is definitely pretty interesting, very interested to see how that will evolve. How did you find out about it?

Is this the circle offset bug btw?

Uh, impossible to say just from the picture what's going on there, but that sure does look pretty amusing. The circle offset bug in OCCT that I've made the workaround against should result in an offset that is on a different plane in the direction of the face normal, so most likely this is something completely else. Is it clipper or OCCT offset? In any case that feels/looks like a bug in our code rather.

[voneiden]

Ref to the separate issue you created on offsetting: #61