[Problem] Regression on toNurbs() #9760
Comments
luzpaz
added
3rd party: OCC
Regression
|
@bgbsww suspected occt 7.7 bug, would be nice to know if it works on 7.8 |
|
Unfortunately, I get the exact no attribute error above. @adrianinsaval But that also fails with the Unknown exception shown on the forum. So no 7.8 fix. |
|
Which I think means it is unlikely to go away in the future. The old version used to return legit edges; the new one seems to return empty ones, leading to the exception - there is no curve defined.
Hmm. Stepping into CorrectVertexTol, it does nothing in a failing test case. More digging. |
Lots of debugging historyI wrote a test that triggers the failure in 7.7.x. Then I determined that the test only fails when the UI is up. Freecad -c -t runs just fine. That seems like a big clue.Run without the UI, we try toNurbs on: and everything passes. With the UI up, the shape has more to it: So what's up with the Polygon3D with deflection, I wonder. Good: Bad: #0 BRepTools_NurbsConvertModification::NewPolygon(TopoDS_Edge const&, opencascade::handle<Poly_Polygon3D>&) (this=0x5555580ea8f0, theEdge=..., thePoly=...) And the actual error is thrown in BRepAdaptor_Curve::Initialize after a failed call to BRep_Tool::CurveOnSurface |
Lots of debugging historyIn TestPartApp.py: ``` def testIssue9760(self): doc = self.Doc doc.addObject('Sketcher::SketchObject', 'Sketch') doc.Sketch.Placement = App.Placement(App.Vector(0.000000, 0.000000, 0.000000), App.Rotation(0.000000, 0.000000, 0.000000, 1.000000)) doc.Sketch.MapMode = "Deactivated" ActiveSketch = doc.getObject('Sketch') p1 = App.Vector(0.367431,3.345679,0) p2 = App.Vector(5.658839,8.910781,0) p3 = App.Vector(11.223940,3.966051,0) z = App.Vector(0,0,1) ActiveSketch.addGeometry(Part.Circle(p1,z,10),True) ActiveSketch.addConstraint(Sketcher.Constraint('Weight',0,1.000000)) ActiveSketch.addGeometry(Part.Circle(p2,z,10),True) ActiveSketch.addConstraint(Sketcher.Constraint('Equal',0,1)) ActiveSketch.addGeometry(Part.Circle(p3,z,10),True) ActiveSketch.addConstraint(Sketcher.Constraint('Equal',0,2)) ActiveSketch.addGeometry(Part.BSplineCurve([p1,p2,p3],None,None,False,2,None,False),False) # App.Vector(0.367431,3.34568),App.Vector(5.65884,8.91078),App.Vector(11.2239,3.96605)],None,None,False,2,None,False),False) conList = [] conList.append(Sketcher.Constraint('InternalAlignment:Sketcher::BSplineControlPoint',0,3,3,0)) conList.append(Sketcher.Constraint('InternalAlignment:Sketcher::BSplineControlPoint',1,3,3,1)) conList.append(Sketcher.Constraint('InternalAlignment:Sketcher::BSplineControlPoint',2,3,3,2)) ActiveSketch.addConstraint(conList) del conList ActiveSketch.exposeInternalGeometry(3) |
Lots of debugging historyOkay, the Polygon3d is normal, shows up in a 7.6.3 build and works fine.The differences I can determine between the two shape objects are: Bad: Good: I wasn't planning on becoming the tolerance guy, but that's a pretty glaring value. Digging... Using fixTolerance on the nurbs shape corrects the tolerance but doesn't create the missing curve. |
|
Ok! The key difference is that in 7.7.0 the method If that override is replaced by |
|
What did that take 10 minutes? Here's the bug and OCCT needs to fix it: Should be ( note the else and return when a curve create fails ): @chennes did I read somewhere that you're set up as an OpenCascade contributor? Or is there someone else that can present this to them? |
|
I have signed their CLA, but won't be able to submit this patch until next week - I am away from the computer that I use for OCCT work. Can anyone write a DRAW script demonstrating the error? |
Lots of debugging historyI think that ``` bsplinecurve bc 2 2 0 3 1 3 0 3 0 1 5 9 0 1 12 4 0 1 mkedge e1 bc nurbsconvert nb e1 dump nb lprops nb ``` is a start, but of course it doesn't fail :( More digging to figure out how to show the curve of the edge inside nb. Explode will only provide the vertexes. |
Lots of debugging historyComparing what should be about the same OCCT shape between FreeCAD and DRAW, I think I see why this differs when you run FreeCAD headless that causes it to work. Using the C++ DumpJson calls:Nurbs fail in FreeCAD with display: toNurbs succeed in Draw: The second "CurveRepresentation" of {"className": "BRep_Polygon3D" is what triggers the problem, and I don't think this can be replicated in Draw :( I don't know where it comes from and if it can be avoided in FreeCAD |
Lots of debugging history``` >>> p1 = App.Vector(0, 3, 0) >>> p2 = App.Vector(5, 9, 0) >>> p3 = App.Vector(12, 4, 0) >>> bs = Part.BSplineCurve([p1,p2,p3],None,None,False,2,None,False) >>> e1 = Part.Edge(bs) >>> n2 = e1.toNurbs() >>> n2.Edges[0].Curve >>> s1 = Part.Shape(e1) >>> o1 = Part.show(s1,"testSpline") >>> n3 = o1.Shape.toNurbs() >>> n3.Edges[0].Curve Traceback (most recent call last): File "", line 1, in Base.FreeCADError: Unknown exception while reading attribute 'Curve' of object 'TopoShape' >>> e1.toNurbs().Edges[0].Curve Traceback (most recent call last): File "", line 1, in Base.FreeCADError: Unknown exception while reading attribute 'Curve' of object 'TopoShape' ``` The modifications to the edge object happen when we build a mesh to be able to display it here: https://github.com/FreeCAD/FreeCAD/blob/3ba0c3d7958dad891e125e065a41d95faac47c6e/src/Mod/Part/Gui/ViewProviderExt.cpp#L958C1-L958C1We should be able to simulate that in Draw... Which does result in the second curve representation with Polygon3D, but still no crash. On to the subtleties. |
|
Invalid shapes created by NURBS conversion in 7.7.0 and after. The added implementation of BRepTools_NurbsConvertModification::NewPolygon appears to miss the case where degenerated edges exist and returns the wrong flag value. DRAWEXE test script: DRAWEXE test runs: Patch: @chennes When you get to it, here's the complete package for OCCT. Please let me know if you need anything else. For the peanut gallery, yes, this bug is fixed with that patch in place. |
|
I hate Mantis 😁 -- https://tracker.dev.opencascade.org/view.php?id=33576 |
|
Is the patch submitted somewhere else? |
|
I have not yet submitted the patch as a PR, I have switched computers since the last time I did that, I have to get my keys set up. I was hoping the OCCT folks would render an opinion first, since it really looks like they made that change on purpose. |
|
I agree the method was added on purpose, and it solves a problem. I think it's an implementation oversight that they missed this one case. It doesn't make sense to avoid doing the processing because you couldn't make the curve, and then claim you did something. That path wouldn't show up in the problem they were fixing. Method needs to stay, results need to be correct. We'll see what they say. |
|
It was really the comment "// skip processing degenerated edges" that gave me pause. To me that sounds like they know they are just deciding not to do anything with them, for whatever reason. When I am back home I'll submit the PR anyway, in part just to see what their CI does with it. |
|
@chennes
Did you get a response from OCCT? |
|
I added the patch provided to the 7.8.1 package in conda-forge. I'll will start working on moving the weekly builds to that in a week or so |
Is there an existing issue for this?
Version
0.21 (Development)
Full version info
Subproject(s) affected?
None
Problem description
If you select an edge and send it to the Python console then
the created edge has no underlying Curve. This breaks JoinCurves in the Curves WB and likely other things. It may be associated with OCC 7.7.x. See forum https://forum.freecad.org/viewtopic.php?p=687077#p687077
Anything else?
No response
Code of Conduct
The text was updated successfully, but these errors were encountered: