DiscussionsIssue archiveOCCT:Modeling Algorithms

Archived issue #0032557

Modeling Data - Use of BRepBuilderAPI_NurbsConvert create 2d p-curves with errors

CommunityOCCT:Modeling Algorithmsclosed34 public notes

Search issues

Description

With the given STEP file `abc_0_31-mod.step` given in the attached zip file, the code `debug_issue.cpp` (also in the zip file) shows that the conversion to NURBS using `BRepBuilderAPI_NurbsConvert` create an incorrect wire, with 2d p-curve accumulating so much imprecision that the end points of the loop of p-curve in the wire no longer coincide.

Given the data set, the incriminated wire the one of the third face. I will attach screenshots...

The attached .cpp file loops on the edges of the wire, and displays the maximal distance between two endpoints (of p-curves) that should coincide. That process is run before and after the conversion to NURBS.

Steps to reproduce

Test case
bugs/moddata_3/bug32557

Public activity

34 archived notes

Participants are labeled by their role within this record.

01Author
I have upload a general view of the STEP face that has the incorrect wire (after the conversion to NURBS).

I have also uploaded a drawing of the wire, in the 2d parametric space, before and after the conversion to NURBS, as well as a zoom on the problematic zone.
02Author
This bug occurs with OCCT version 7.5.0 (as shipped by Linux Fedora 34), and with the master branch of OCCT (commit dated Sun Jun 27 20:13:49 2021 +0300).

The bug was not present in OCCT version 7.4.0. That version 7.4.0 was ship in Linux Fedora 32, and I recompiled it locally to check again, just before the submission of this issue.
03Author
Hi, It seems this buggy behaviour has been introduced in OCCT 7.5.0 by the commit

> [revision removed]
> Author: ifv <[email removed]>
> Date: Wed Dec 18 13:53:11 2019 +0300
>
> 0031242: Scaling with different coefficients along axes produces invalid shape
>
> GeomConvert_1.cxx:
> Creation periodic BSpline surfaces from trimmed periodic surface if trim is boundaries of periodic domain is allowed
>
> BRepTools_NurbsConvertModification.cxx:
> Checking domain of 2dCurves if surfaces are periodic is added
>
> Test case tests/bugs/mesh/bug30008_2 is modified according to current behavior
>
> Test case tests/bugs/modalg_7/bug31242 is added

It seems to be a regression.

I can for now go back to OCCT 7.4.0, as a workaround.

What are your intention for that bug? Will your revert [revision removed] (0031242) in future OpenCASCADE releases?
04Author
Here is a Github link to the culprit commit: https://github.com/Open-Cascade-SAS/OCCT/commit/[revision removed].
05Commenter 1
Commenter 2, thanks for preliminary analysis, the bug #0031242 has been linked to this one (it is inaccessible to public, but there is nothing interesting in description).
> Hi, It seems this buggy behaviour has been introduced in OCCT 7.5.0 by the commit
06Author
Hi @kvg, that is interesting to now that [revision removed] was actually a fix for an existing bug. It means you cannot easily revert the commit, because the bug #0031242 (whatever it is) would reappear.
07Commenter 7
Dear Igor, could you see if this regression can be fixed by the next release?
08Commenter 8
Debugging
09Commenter 9
This problem can be fixed by next release
10Commenter 3
Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: ifv
Date: Fri Sep 24 16:24:34 2021 +0300

    0032557: Modeling Data - Use of BRepBuilderAPI_NurbsConvert create 2d p-curves with errors
    
    BRepTools/BRepTools_NurbsConvertModification.cxx -
     Checking domain of 2dCurves if surfaces are periodic is improved
    
    tests/bugs/moddata_3/bug32557 - test case added
11Commenter 3
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
12Author
I confirm branch CR32557 fixes the bug in my setup. Thanks for the fix.
13Commenter 13
Branch 32557 is ready for review

Branches for integration
OCCT - CR32557
Products - not
14Commenter 14
Igor, could you test if it fixes also the linked bug #0032454?
15Author
Hi,

Even if branch `CR32557` fixes most of my test cases, I found one for which there is still a distance between 2d curves once converted in NURBS. See the attached file `Attachment 12 (ZIP)`, with the data set `abc_0_522.step` from the public ABC repository. This time, the file debug_issue.cpp uses `ShapeAnalysis_WireOrder` to find the maximal distance between edges of the wire, because the wire is not correctly ordered in the file.

Commenter 4 It looks like the same bug, right?
16Commenter 16
Combination -
OCCT branch : [archived branch]
master SHA - [revision removed]
[revision removed]
Products branch : [archived branch] SHA - [revision removed]
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
Total CPU difference: 17464.590000000462 / 17476.150000000354 [-0.07%]
Products
Total CPU difference: 11405.930000000111 / 11390.180000000097 [+0.14%]
Windows-64-VC14:
OCCT
Total CPU difference: 19332.171875 / 19335.34375 [-0.02%]
Products
Total CPU difference: 12772.859375 / 12782.828125 [-0.08%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
17Commenter 3
Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: ifv
Date: Mon Oct 4 14:48:03 2021 +0300

    CheckAndSegment added
18Commenter 3
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
19Commenter 19
Issue is reopen because of new problem provided by @Irineau, see note #104424
20Author
The branch CR32557_1 does fix the issue with the file abc_0_522.step,
but there are other files from the ABC repository that got worst with
the new commits from CR32557_1, compared to CR32557.

I have attached two new files:

  Attachment 3 (STEP)
  Attachment 7 (STEP)

With those two files, and a lot of other from the ABC repository, now I have an exception raised:

  terminate called after throwing an instance of 'Standard_ConstructionError'

Here is the relevant backtrace:

#5 0x00007ffff61a75a9 in __cxa_throw () from /lib64/libstdc++.so.6
#6 0x00007fff402977f5 in CheckSurfaceData (SPoles=..., SUKnots=..., SVKnots=..., SUMults=..., SVMults=..., UDegree=2,
    VDegree=1, UPeriodic=<optimized out>, VPeriodic=<optimized out>)
    at OCCT/src/Geom/Geom_BSplineSurface.cxx:85
#0000007 0x00007fff40297d8f in Geom_BSplineSurface::Geom_BSplineSurface (this=0x6e5c680, this@entry=0x1, Poles=..., Weights=...,
    UKnots=..., VKnots=..., UMults=..., VMults=..., UDegree=<optimized out>, VDegree=<optimized out>,
    UPeriodic=<optimized out>, VPeriodic=<optimized out>)
    at OCCT/src/Geom/Geom_BSplineSurface.cxx:259
#0000008 0x00007fff402970bf in Geom_BSplineSurface::Copy (this=<optimized out>)
    at OCCT/src/Geom/Geom_BSplineSurface.cxx:140
#0000009 0x00007fff402c1b0b in GeomAdaptor::MakeSurface (HS=..., theTrimFlag=false)
    at OCCT/src/GeomAdaptor/GeomAdaptor.cxx:134
#10 0x00007fff40474356 in computePeriodicity (theSurf=..., theUPeriod=@0x7fffffffb878: 0, theVPeriod=@0x7fffffffb870: 0)
    at OCCT/src/ProjLib/ProjLib_ComputeApproxOnPolarSurface.cxx:119
#0000011 0x00007fff4046f1e5 in ProjLib_ComputeApproxOnPolarSurface::BuildInitialCurve2d (this=this@entry=0x7fffffffc730, Curve=...,
    Surf=...) at OCCT/src/ProjLib/ProjLib_ComputeApproxOnPolarSurface.cxx:932
#12 0x00007fff4046e202 in ProjLib_ComputeApproxOnPolarSurface::Perform (this=this@entry=0x7fffffffc730, InitialCurve2d=...,
    Curve=..., S=...) at OCCT/src/ProjLib/ProjLib_ComputeApproxOnPolarSurface.cxx:794
#0000013 0x00007fff4046d8db in ProjLib_ComputeApproxOnPolarSurface::ProjLib_ComputeApproxOnPolarSurface (this=0x7fffffffc730,
    theInitialCurve2d=..., theCurve=..., theSurface=..., theTolerance3D=<optimized out>)
    at OCCT/src/ProjLib/ProjLib_ComputeApproxOnPolarSurface.cxx:517
#0000014 0x00007fff408c8a03 in BRepTools_NurbsConvertModification::NewCurve2d (this=0x6d86a60, E=..., F=..., newE=..., newF=...,
    Curve2d=..., Tol=@0x7fffffffc940: 9.[revision removed]-08)
    at OCCT/src/BRepTools/BRepTools_NurbsConvertModification.cxx:488
#0000015 0x00007fff408c4188 in BRepTools_Modifier::Rebuild (this=<optimized out>, this@entry=0x7fffffffd310, S=..., M=...,
    theNewGeom=<optimized out>, theProgress=...)
    at OCCT/src/BRepTools/BRepTools_Modifier.cxx:410
#0000016 0x00007fff408c3df2 in BRepTools_Modifier::Rebuild (this=<optimized out>, this@entry=0x7fffffffd310, S=..., M=...,
    theNewGeom=<optimized out>, theProgress=...)
    at OCCT/src/BRepTools/BRepTools_Modifier.cxx:363
#0000017 0x00007fff408c3df2 in BRepTools_Modifier::Rebuild (this=<optimized out>, this@entry=0x7fffffffd310, S=..., M=...,
    theNewGeom=<optimized out>, theProgress=...)
    at OCCT/src/BRepTools/BRepTools_Modifier.cxx:363
#0000018 0x00007fff408c29a4 in BRepTools_Modifier::Perform (this=0x7fffffffd310, M=..., theProgress=...)
    at OCCT/src/BRepTools/BRepTools_Modifier.cxx:147
#0000019 0x00007fff41040243 in BRepBuilderAPI_ModifyShape::DoModif (this=this@entry=0x7fffffffd2c0)
    at OCCT/src/BRepBuilderAPI/BRepBuilderAPI_ModifyShape.cxx:77
#0000020 0x00007fff410403ad in BRepBuilderAPI_ModifyShape::DoModif (this=0x7fffffffd2c0, S=..., M=...)
    at OCCT/src/BRepBuilderAPI/BRepBuilderAPI_ModifyShape.cxx:126
#0000021 0x00007fff41040701 in BRepBuilderAPI_NurbsConvert::Perform (this=this@entry=0x7fffffffd2c0, S=...)
    at OCCT/src/BRepBuilderAPI/BRepBuilderAPI_NurbsConvert.cxx:63
#0000022 0x00007fff41040682 in BRepBuilderAPI_NurbsConvert::BRepBuilderAPI_NurbsConvert (this=0x7fffffffd2c0, S=...,
    Copy=<optimized out>) at OCCT/src/BRepBuilderAPI/BRepBuilderAPI_NurbsConvert.cxx:50
21Commenter 3
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
22Commenter 22
Dear @Irineau, could you please test new variant of branch CR32557_1 on your data repository?
23Author
Hi Commenter 4, I have added a few test cases that fail similarly, on the last [revision removed]:

Attachment 5 (STEP)
Attachment 6 (STEP)
Attachment 1 (STEP)
Attachment 2 (STEP)
Attachment 4 (STEP)
24Author
Actually, the call stack is not always the same, and the exception thrown is also different:

           name status time error
Attachment 6 (STEP) FAILED 3.05515 terminate called after throwing an instance of 'Standard_ConstructionError'
 Attachment 1 (STEP) FAILED 3.04846 terminate called after throwing an instance of 'Standard_ConstructionError'
 Attachment 2 (STEP) FAILED 3.00421 terminate called after throwing an instance of 'Standard_ConstructionError'
Attachment 5 (STEP) FAILED 3.03978 terminate called after throwing an instance of 'Standard_RangeError'
 Attachment 4 (STEP) FAILED 3.02421 terminate called after throwing an instance of 'Standard_RangeError'

but that seems to be always somewhere in a call to `BRepBuilderAPI_NurbsConvert::Perform`.
25Commenter 3
Branch [archived branch] has been deleted by Participant.

[revision removed]
26Commenter 3
Branch [archived branch] has been created by Participant.

[revision removed]


Detailed log of new commits:

Author: ifv
Date: Mon Oct 4 14:48:03 2021 +0300

    0032557: Modeling Data - Use of BRepBuilderAPI_NurbsConvert create 2d p-curves with gaps
    
    GeomConvert/GeomConvert_1.cxx - method CheckAndSegment is added to adjust surface periodic boundaries
    bug32557 - test is updated by new cases
27Commenter 27
Branch CR32557_1 is ready for review

Branches for integration:
OCCT - CR32557_1
Products - not

Patch seems to fix issue #0032454
Test results:

http://jenkins-test-occt.nnov.opencascade.com/view/CR32557_1-master-ifv/view/COMPARE/
28Commenter 28
What is about the difference in images?
IMAGE bugs modalg_7 bug31242: bug31242.png differs
29Commenter 29
In my opinion, reason of image differences for bugs modalg_7 bug31242 is any problem in isoline builder. Really result of CR32557_1 is better than result of master, for example tolerances are:
CR32557_1
## Tolerances on the shape result (nbFaces:1 nbEdges:4 nbVtx:4)

    Face : Min 1.00000e-07 Max 1.00000e-07

    Edge : Min 3.22903e-07 Max 8.73714e-02

    Vertex : Min 5.00000e-06 Max 8.73714e-02

master
## Tolerances on the shape result (nbFaces:1 nbEdges:4 nbVtx:4)

    Face : Min 1.00000e-07 Max 1.00000e-07

    Edge : Min 4.48635e-06 Max 2.12449e-01

    Vertex : Min 9.94636e-02 Max 2.12449e-01

Surfaces are the same, pcurves are differ a bit
30Commenter 30
Branches for integration:
OCCT - CR32557_1
Products - not
31Commenter 31
Debugging
32Commenter 32
Combination -
OCCT branch : [archived branch]
master SHA - [revision removed]
[revision removed]
Products branch : [archived branch] SHA - [revision removed]
was compiled on Linux, MacOS and Windows platforms and tested in optimize mode.

Number of compiler warnings:
No new/fixed warnings

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:
Debian80-64:
OCCT
Total CPU difference: 17728.310000000427 / 17750.15000000046 [-0.12%]
Products
Total CPU difference: 11578.090000000113 / 11624.980000000118 [-0.40%]
Windows-64-VC14:
OCCT
Total CPU difference: 19746.84375 / 19683.796875 [+0.32%]
Products
Total CPU difference: 12740.0 / 10994.3125 [+15.88%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
33Commenter 3
Branch [archived branch] has been deleted by Participant.

[revision removed]
34Commenter 3
Branch [archived branch] has been deleted by Participant.

[revision removed]

Related records