CadQuery / cadquery

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

BRepBuilderAPI_DisconnectedWire when using Wire.assembleEdges() on Wire.Edges() #1130

Closed voneiden closed 2 years ago

voneiden commented 2 years ago

I've discovered an edge (attached as brep) that returns ever so slightly inconsistent values for various curve positions. It appears to wobble around the actual curve. The error is not huge - seems to be about 5e-7. But it's enough to break WireBuilder (DisconnectedWire).

miss

The edge geomType is OFFSET. The underlying geom type is BSPLINE. Wild guess: it's an OCCT bug related to offset bsplines..

import cadquery as cq

edge = cq.Edge.importBrep('edge.brep')

interpolations = []
for i in range(200):
    i = i/10000.0
    p = edge.positionAt(i).toTuple()
    interpolations.append(cq.Vertex.makeVertex(*p))
show_object(edge, 'edge')
show_object(cq.Vertex.makeVertex(*edge.startPoint().toTuple()), 'start')
show_object(cq.Vertex.makeVertex(*edge.endPoint().toTuple()), 'end')

show_object(interpolations, 'interpolations')

edge.zip

voneiden commented 2 years ago

Another angle, looking "along" the edge with high zoom, shows how the renderer interpolates the edge vs what positionAt actually returns.

i2

My current feeling is that the bspline offset curve is not exact, but OCCT has some kind of workaround that allows the edge to be part of a valid wire regardless.

Edit: Just to highlight how it looks like in the context of the wire, pictured is the joint of two edges.

i5

lorenzncode commented 2 years ago

The wobble appears to be a render tolerance issue. I noticed the wobble is gone after changing the cq-editor 3D Viewer options, e.g.: Deviation 1e-7 Angular deviation 1e-6

This is the start of the edge, with paramAt values: 0.0 0.0001 0.0002 0.0003 0.0004

The vertices appear to lie on the curve, although the first does not coincide with the start of the displayed edge.

image

adam-urbanczyk commented 2 years ago

@voneiden is this resolved? You were using rendered edge/wire as your reference. That has it's own precision as @lorenzncode pointed out.

voneiden commented 2 years ago

@adam-urbanczyk No, the problem has been narrowed down (and the title needs updating) but in principle it is not resolved. I'm not sure how to explain this with words though, please see the attached picture:

image

The edge starts at "???" but this point is not accessible via edge.startPoint() or edge.positionAt(0).

It's unexpected behaviour so I think it's good to have it documented at least here. Anyway, I have a gut feeling that this is an OCCT bug/feature and beyond CQ to do anything about.

But feel free to close the issue. I've managed to work around this so the only pain remaining is not understanding the root cause.

adam-urbanczyk commented 2 years ago

Why do you assume that the red line is the reference (i.e. why is the discretization for rendering better)? If it is not, I don't think that such small deviations are relevant for visualization TBH.

voneiden commented 2 years ago

The visualization is OK, it's the underlying geometry that is broken or at least very unusual.

Consider for example the following simplified use case: I'd like to convert each (non-linear) edge in a wire into a simple linear edge from start to end.

edges = wire.Edges()
new_edges = [cq.Edge.makeLine(edge.startPoint(), edge.endPoint()) for edge in edges]
new_wire = cq.Wire.assembleEdges(new_edges)    

If the wire contains an edge like described in this issue, the operation will fail with BRepBuilderAPI_DisconnectedWire because the startPoint of the edge does not actually lie on the wire even though the edge is part of the wire.

I can post a brep of such a wire if you're interested to take a look. The wire was a result of offset2D operation.

adam-urbanczyk commented 2 years ago

Yes, please post. I'm still interested why you assume that the red wire is correct.

voneiden commented 2 years ago

Why do you assume that the red line is the reference (i.e. why is the discretization for rendering better)?

I'm still interested why you assume that the red wire is correct.

I'm not following you on the intent of these questions. I don't think were quite on the same page. Anyway, if we focus on just the geometry for a moment and ignore the visualizations:

My hypothesis: Wires must consist of one or more continuous segments of edges

If the hypothesis is true, then the snippet below must never fail.

import cadquery as cq

wire = cq.Wire.importBrep('wire.brep')
assert wire.IsClosed()
edges = wire.Edges()
new_edges = [cq.Edge.makeLine(edge.startPoint(), edge.endPoint()) for edge in edges]
new_wire = cq.Wire.assembleEdges(new_edges)

raises

OCP.StdFail.StdFail_NotDone: BRep_API: command not done (BRepBuilderAPI_WireError.BRepBuilderAPI_DisconnectedWire)

with this wire

wire.zip

So my hypothesis is wrong. Is my hypothesis based on a false premise or does OCCT break the rules?

lorenzncode commented 2 years ago

I'm not an OCCT expert, however, it might be that you are trying to reassemble with higher tolerance than the underlying brep data.

The brep file shows some shapes with tolerance 1e-7 and others with lower tolerance e.g. 0.000176148 (line 1809).

BRepTools.Dump_s more human readable output format.:

TShape # 12 : VERTEX    01011010 0x5633a2ef0cb0

    Tolerance : 0.000176148
    - Point 3D : 12.5151, 1.44298, 0

Try combine which is successful in this case with disconnected edges.

import cadquery as cq

points = [
    [(0, 0, 0), (1.0000000, 0, 0)],
    [(1.0000003, 0, 0), (5, 5, 0)],
]

edges = []

for p in points:
    e = cq.Edge.makeLine(p[0], p[1])
    edges.append(e)

#wire = cq.Wire.assembleEdges(edges)  # fails
wire = cq.Wire.combine(edges)         # success
adam-urbanczyk commented 2 years ago

Why do you assume that the red line is the reference (i.e. why is the discretization for rendering better)?

I'm still interested why you assume that the red wire is correct.

I'm not following you on the intent of these questions. I don't think were quite on the same page. Anyway, if we focus on just the geometry for a moment and ignore the visualizations:

I'm trying to understand the issue you are reporting.

My hypothesis: Wires must consist of one or more continuous segments of edges

Kind of, they are continuous up to a certain tolerance (see https://dev.opencascade.org/doc/refman/html/class_b_rep___tool.html#a6fa2106e68bd301fd841abfa62202d02)

If the hypothesis is true, then the snippet below must never fail.

If the tolerance of individual edges is set correctly (see https://github.com/Open-Cascade-SAS/OCCT/blob/5614b1369a232f43e59cb9be4f5f8efd2075608d/src/BRepLib/BRepLib_MakeWire.cxx#L226)

import cadquery as cq

wire = cq.Wire.importBrep('wire.brep')
assert wire.IsClosed()
edges = wire.Edges()
new_edges = [cq.Edge.makeLine(edge.startPoint(), edge.endPoint()) for edge in edges]
new_wire = cq.Wire.assembleEdges(new_edges)

raises

OCP.StdFail.StdFail_NotDone: BRep_API: command not done (BRepBuilderAPI_WireError.BRepBuilderAPI_DisconnectedWire)

with this wire

wire.zip

So my hypothesis is wrong. Is my hypothesis based on a false premise or does OCCT break the rules?

As @lorenzncode noticed, there seems to be something weird with the tolerance of the wire/edges. Naive check of plotting the individual edges yields this

afbeelding

so some of the edges of your wire seem to have bigger gaps. It seems that combine (i.e. ShapeAnalysis_FreeBounds.ConnectEdgesToWires_s) works even for tight tolerances, which I find confusing. Maybe it implements the check in a different way than BrepLib_MakeWire. If you are interested, you might want to compare the implementations in OCCT.

voneiden commented 2 years ago

Thank you both, good clarifications. The pieces have fallen together.

So when I do cq.Edge.makeLine I lose the original tolerance of the edge vertices and that's the reason I can't rebuild the wire. By copying the tolerance using BRep_Builder.UpdateVertex and BRep_Tool.Tolerance_s I can get the above working.

Apparently the offset algorithm does increase tolerances in certain cases.

import cadquery as cq
from OCP.BRep import BRep_Tool, BRep_Builder

wire = cq.Wire.importBrep('wire.brep')
assert wire.IsClosed()
edges = wire.Edges()
new_edges = []
builder = BRep_Builder()
for edge in edges:
    new_edge = cq.Edge.makeLine(edge.startPoint(), edge.endPoint())
    vtx = edge.Vertices()
    new_vtx = new_edge.Vertices()
    builder.UpdateVertex(new_vtx[0].wrapped, BRep_Tool.Tolerance_s(vtx[0].wrapped))
    builder.UpdateVertex(new_vtx[1].wrapped, BRep_Tool.Tolerance_s(vtx[1].wrapped))
    new_edges.append(new_edge)
new_wire = cq.Wire.assembleEdges(new_edges)

Thanks again. I think we can close this? It's a feature, not a bug. :smiling_face_with_tear: