cadquery icon indicating copy to clipboard operation
cadquery copied to clipboard

Sketch close returns start point of for construction edge when closing non construction edge

Open lorenzncode opened this issue 1 year ago • 1 comments

Sketch close() closes an edge that is not for construction with an edge created with forConstruction=True.

I attempted to close the triangle with itself, however, it was closed with the starting point of the for construction arc.

import cadquery as cq

s0 = (
    cq.Sketch()
    .arc((0, 0), 30, 0, 120, tag="e0", forConstruction=True)
    .edges(tag="e0")
    .distribute(1, rotate=False)
    .segment((0, 0), (10, 2))
    .segment((10, -2))
    .close()
    # .assemble()
)

show_object(s0, name="s0")

issue1

Uncommenting assemble():

issue1_assemble

A solution is to omit close by explicitly specifying the edge end/start point:

s0 = (
    cq.Sketch()
    .arc((0, 0), 30, 0, 120, tag="e0", forConstruction=True)
    .edges(tag="e0")
    .distribute(1, rotate=False)
    .segment((0, 0), (10, 2))
    .segment((10, -2))
    .segment((0, 0))
    .assemble()
)

without_close

Solution using callback:

def triangle(loc):
    s = cq.Sketch().segment((0, 0), (10, 2)).segment((10, -2)).close().assemble()
    return s.moved(loc)


s0 = (
    cq.Sketch()
    .arc((0, 0), 30, 0, 120, tag="e0", forConstruction=True)
    .edges(tag="e0")
    .distribute(1, rotate=False)
    .each(triangle)
)

Is there a scenario where it is desired to close an edge in non construction mode with an edge in construction mode? If not, perhaps Sketch _startPoint can be changed to something like this:

    def _startPoint(self) -> Vector:

        if not self._edges:
            raise ValueError("No free edges available")

        # find the first edge matching current edge mode
        mode = self._edges[-1].forConstruction 
        for edge in reversed(self._edges):
            if edge.forConstruction != mode:
                break
            prevedge = edge

        e = prevedge

        # current implementation selects first edge:
        # e = self._edges[0]

        return e.startPoint()

I didn't find any failing tests with this change.

lorenzncode avatar Nov 18 '23 20:11 lorenzncode

Thanks @lorenzncode , I think that the proposal makes sense.

adam-urbanczyk avatar Jan 04 '24 06:01 adam-urbanczyk