CadQuery / cadquery

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

`Edge.startPoint` does not respect OCCT orientation #831

Open marcus7070 opened 3 years ago

marcus7070 commented 3 years ago

I think I should be able to reverse an edge with e.wrapped.Reversed().

>>> e = cq.Edge.makeLine(cq.Vector(0, 0, 0), cq.Vector(0, 0, 1))
>>> e_rev = cq.Edge(e.wrapped.Reversed())

But the start and end points are not what you would expect:

>>> e.startPoint() == e_rev.endPoint()
False
>>> e.startPoint() == e_rev.startPoint()
True

I think (but I'm not confident) when we grab the geom adaptor here: https://github.com/CadQuery/cadquery/blob/ab876acc84a7df273bb368549656912a980feae3/cadquery/occ_impl/shapes.py#L1225-L1237 we need to check for self.wrapped.Orientation() and invert the parameter if necessary. Probably also need to do similar in a few other places that use the geom adaptor.

But this feels like a common task in OCCT, there might be a more direct OCCT function we could use?

jdthorpe commented 3 years ago

II was trying to iterate over a wire and noticed that the edges in a wire are not oriented in the way I would expect from the description in the Repairing tools for wires page on the OCCT site:

edges in the wire go in a sequential order (the end of a preceding edge is the start of a following one)

I was trying to iterate over the edges in a wire hoping to get them tip-to-tail, but they were not oriented that way.

Here's a concrete example:

# generate a random shape and get a wire from a face
wire = (cq.Workplane("XY")
    .tag("origin")
    .rect(5, 1)
    .workplaneFromTagged("origin")
    .move(3,0.75001)
    .circle(2)
    .extrude(1)
    .faces(">Z").wires().item(0))

# grab the edges from the wire
edges= wire.edges().vals()

for i in range(len(edges)):
     # print the difference between the previous end point and the current starting point
     print(edges[i].startPoint() - edges[i-1].endPoint())

which for me yields:

Vector: (0.0, 0.0, 0.0)
Vector: (0.0, 0.0, 0.0)
Vector: (-0.42307073025186237, 0.9999999999999711, 0.0)
Vector: (-0.4230707302518628, 0.9999999999999716, 0.0)

This feels related to the OP, and if it is not, is there a canonical way to iterate over the edges in wire in the aforementioned orientation?

adam-urbanczyk commented 3 years ago

It sounds like it is related. One of your edges has a REVERSED orientation.

marcus7070 commented 3 years ago

Also pretty close to what I was trying to do when I noticed this.

adam-urbanczyk commented 3 years ago

This would be the relevant method: https://dev.opencascade.org/doc/refman/html/class_shape_analysis___edge.html#a1760cfca538e691b2abb02716435f801

adam-urbanczyk commented 3 years ago

Or even better: https://dev.opencascade.org/doc/refman/html/class_shape_analysis.html#af7f28a20b25686dea3ecf852fc481d0f

marcus7070 commented 3 years ago

Thanks @adam-urbanczyk, they do look like the right methods for start and end points. Any advice on how to handle Edge.locationAt?

>>> e.locationAt(0.2).toTuple()
((0.0, 0.0, 0.2), (0.0, -0.0, 0.0))
>>> e_rev.locationAt(0.2).toTuple()
((0.0, 0.0, 0.2), (0.0, -0.0, 0.0))

I think a lot more of those Mixin1D methods are not going to handle reverse orientation edges as well.

adam-urbanczyk commented 3 years ago

I'd say: either state in the docs that it operates in the geometry domain (and thus ignores the orientation specified on the topo level) or add a check on the orientation and reverse the parameter when needed.