MantisBT - Community
View Issue Details
0031402Community[OCCT] OCCT:Modeling Datapublic2020-03-04 10:502020-12-05 13:01
martinsiggel 
bugmaster 
normalmajor 
verifiedfixed 
[OCCT] 7.4.0 
[OCCT] 7.6.0* 
bugs/moddata_3/bug31402_1,bug31402_2
0031402: Modeling Data - Geom_BSplineSurface::Segment() produces wrong result
Using Geom_BSplineSurface::Segment on the attached surface, with the parameters
surface->Segment(0., 1., 0.49999999999999988898, 0.66666666666666662966);

does not trim the surface correctly. surface->Bounds reports the following bounds after trimming:

(0.0, 1.0, 0.3333333333333333, 0.6666666666666666)

Expected bounds would be:
(0.0, 1.0, 0.5, 0.66666667)

I suppose a tolerance issue in the knot insertion algorithm.
BRep_Builder b;
TopoDS_Shape s;
BRepTools::Read(s, "surface.brep", b);

auto surface = Handle(Geom_BSplineSurface)::DownCast(BRep_Tool::Surface(TopoDS::Face(s)));

surface->Segment(0., 1., 0.49999999999999988898, 0.66666666666666662966);

double u1, u2, v1, v2;
surface->Bounds(u1, u2, v1, v2);
No tags attached.
? surface.brep (4,352) 2020-03-04 10:50
https://tracker.dev.opencascade.org/
Issue History
2020-03-04 10:50martinsiggelNew Issue
2020-03-04 10:50martinsiggelAssigned To => msv
2020-03-04 10:50martinsiggelFile Added: surface.brep
2020-03-04 11:38martinsiggelNote Added: 0090808
2020-09-15 12:27msvProduct Version7.5.0 => 7.4.0
2020-09-15 12:27msvTarget Version7.5.0 => 7.6.0*
2020-11-02 11:30szyAssigned Tomsv => aavtamon
2020-11-02 11:30szyStatusnew => assigned
2020-11-12 11:04gitNote Added: 0096686
2020-11-12 16:52gitNote Added: 0096723
2020-11-12 17:22kgvSummaryGeom_BSplineSurface::Segment produces wrong result => Modeling Data - Geom_BSplineSurface::Segment() produces wrong result
2020-11-12 17:23kgvDescription Updatedbug_revision_view_page.php?rev_id=23995#r23995
2020-11-12 17:23kgvSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=23997#r23997
2020-11-12 22:24gitNote Added: 0096732
2020-11-13 12:35gitNote Added: 0096743
2020-11-13 12:42gitNote Added: 0096744
2020-11-13 14:48aavtamonNote Added: 0096746
2020-11-13 14:48aavtamonAssigned Toaavtamon => msv
2020-11-13 14:48aavtamonStatusassigned => resolved
2020-11-13 18:11msvNote Added: 0096753
2020-11-13 18:14msvNote Added: 0096754
2020-11-13 18:16msvNote Added: 0096755
2020-11-13 18:16msvAssigned Tomsv => ifv
2020-11-14 00:01aavtamonNote Added: 0096772
2020-11-16 13:05ifvNote Added: 0096791
2020-11-16 13:05ifvAssigned Toifv => aavtamon
2020-11-16 13:05ifvStatusresolved => assigned
2020-11-16 13:21ifvRelationship addedrelated to 0030645
2020-11-27 15:15gitNote Added: 0097110
2020-12-02 10:33gitNote Added: 0097231
2020-12-02 14:25gitNote Added: 0097236
2020-12-02 14:28aavtamonNote Added: 0097237
2020-12-02 14:28aavtamonAssigned Toaavtamon => ifv
2020-12-02 14:28aavtamonStatusassigned => resolved
2020-12-03 11:32ifvNote Added: 0097262
2020-12-03 11:32ifvAssigned Toifv => aavtamon
2020-12-03 11:32ifvStatusresolved => assigned
2020-12-03 11:34ifvNote Edited: 0097262bug_revision_view_page.php?bugnote_id=97262#r24128
2020-12-03 14:25gitNote Added: 0097271
2020-12-03 14:28aavtamonNote Added: 0097272
2020-12-03 14:28aavtamonAssigned Toaavtamon => ifv
2020-12-03 14:28aavtamonStatusassigned => resolved
2020-12-03 15:49ifvNote Added: 0097274
2020-12-03 15:49ifvAssigned Toifv => aavtamon
2020-12-03 15:49ifvStatusresolved => assigned
2020-12-03 15:58ifvNote Edited: 0097274bug_revision_view_page.php?bugnote_id=97274#r24130
2020-12-03 21:33gitNote Added: 0097286
2020-12-04 10:28aavtamonNote Added: 0097300
2020-12-04 10:28aavtamonAssigned Toaavtamon => ifv
2020-12-04 10:28aavtamonStatusassigned => resolved
2020-12-04 10:34ifvNote Added: 0097301
2020-12-04 10:34ifvAssigned Toifv => bugmaster
2020-12-04 10:34ifvStatusresolved => reviewed
2020-12-05 11:57bugmasterNote Added: 0097350
2020-12-05 11:57bugmasterStatusreviewed => tested
2020-12-05 12:11bugmasterTest case number => bugs/moddata_3/bug31402_1,bug31402_2
2020-12-05 12:15bugmasterChangeset attached => occt master 1e08a76f
2020-12-05 12:15bugmasterStatustested => verified
2020-12-05 12:15bugmasterResolutionopen => fixed
2020-12-05 13:01gitNote Added: 0097356
2020-12-05 13:01gitNote Added: 0097367
2020-12-05 13:01gitNote Added: 0097369

Notes
(0090808)
martinsiggel   
2020-03-04 11:38   
I tried to track the error down:
The v-knot insertion of 0.499999999... is almost at v=0.5, which is a knot of the surface. As this is within the knot insertion tolerance, the a knew knot is not inserted but the multiplicity of 0.5 is increased. The knot vector's size does not change. Fine so far.

The problem seems to lie in BSplCLib::LocateParameter. Here, the value of v is just outside the tolerance to the next knot (difference to the next knot is 1.1e-16, tolerance (eps) is 5.6e-17).

Conclusion: The tolerances of knot insertion and parameter location do not match and must be adapted.
(0096686)
git   
2020-11-12 11:04   
Branch CR31402 has been created by aavtamon.

SHA-1: 7af830d5780e75790b08c4d572dba51fb210130e


Detailed log of new commits:

Author: aavtamon
Date: Thu Nov 12 09:38:50 2020 +0300

    0031402: Modeling Data - Geom_BSplineSurface::Segment produces wrong result
    
    - Added test case bug31402 in tests/bugs/moddata_3;
    - Added test data file bug31402.brep in data/occ;
    - Added new test command OCC31402 in QABugs_20.cxx;
    - Added tolerance as a default parameter in one of BSplCLib::LocateParameter declarations;
    - Changed calls of LocateParameter method in Geom_BSplineSurface::segment.
(0096723)
git   
2020-11-12 16:52   
Branch CR31402 has been updated forcibly by aavtamon.

SHA-1: 962605742c961f79d4f260597de293228db633c5
(0096732)
git   
2020-11-12 22:24   
Branch CR31402 has been updated forcibly by aavtamon.

SHA-1: 40ac7d258eda547429be6574767f79ea7dd7dd55
(0096743)
git   
2020-11-13 12:35   
Branch CR31402 has been updated forcibly by aavtamon.

SHA-1: 2d45160898b8a015f1e4dd9cf9c98f448c0cd28a
(0096744)
git   
2020-11-13 12:42   
Branch CR31402 has been updated forcibly by aavtamon.

SHA-1: 0b204a460ad1344783bedb6fe91f198a8fc85780
(0096746)
aavtamon   
2020-11-13 14:48   
Dear Mikhail,
could you please check it?
http://jenkins-test-12.nnov.opencascade.com/view/CR31402-master-aavtamon/view/ALL/ [^]
(0096753)
msv   
2020-11-13 18:11   
checkshape s
is extra command, as we don't need to check the shape in this test.

Do not put the test brep file in the source repository.
(0096754)
msv   
2020-11-13 18:14   
In the commit message, it is better not to list the files and methods that you have changed. It is rather more interesting to know the logical changes.
(0096755)
msv   
2020-11-13 18:16   
Dear Igor,
Could you review the logical part? May be it is needed also to see if the tolerance is to be added in other LocateParameter methods?
Or, may be consider another idea of fix?
(0096772)
aavtamon   
2020-11-14 00:01   
In this case method "Segment" changes the surface belonging to the shape, so it is worth to check the shape after reproduce steps implementation. The following lines show results of checkshape call:
 
checkshape s
 **** probleme de SameParameter au point :
         0.272727272727273 1.38777878078145e-016 1
   Erreur detectee :1.35236664162281 Tolerance :1e-007
 **** probleme de SameParameter au point :
         0.272727272727273 1.38777878078145e-016 1
   Erreur detectee :1.35236664162281 Tolerance :1e-007
On Shape faulty_1 :
BRepCheck_UnorientableShape
Shape faulty_2 on shape faulty_1 :
BRepCheck_InvalidCurveOnClosedSurface
BRepCheck_InvalidSameParameterFlag

Faulty shapes in variables faulty_1 to faulty_2
(0096791)
ifv   
2020-11-16 13:05   
In my opinion, using tolerance in LocateParameter(...) is not good idea.
Algorithm of this method must locate parameter with maximal possible precision and correspond with allowed distance between knots, see method CheckSurfaceData(...):
.....
  for (i = SUKnots.Lower(); i < SUKnots.Upper(); i++) {
    if (SUKnots(i+1) - SUKnots(i) <= Epsilon(Abs(SUKnots(i)))) {
      throw Standard_ConstructionError("Geom_BSplineSurface: UKnots interval
                                       values too close");
    }
  }
....
This method is rather popular and often used in Spline calculations, inappropriate value of tolerance can cause wrong definition of knot interval.
I think it would better to handle tolerance (EpsU, EpsV) at level of method Segment() or InsertU(V)Knots(...).

Some remarks:
If we change something in Geom_BSplineSurface, it is necessary to do the same for Geom_BSplineCurve and Geom2d_BSplineCurve, otherwise Segment(...) for surface will be work correctly, but for isoline of this surface it will work wrongly.

It is not necessary to make special QAcommand, there is Draw command "segsur", which can be used in tests:
Draw[45]> help segsur
segsur : segsur name Ufirst Ulast Vfirst Vlast [Utol [Vtol]]
(0097110)
git   
2020-11-27 15:15   
Branch CR31402_1 has been created by aavtamon.

SHA-1: 4dd6ba131f5d2d8121180b6717344b397d854ea0


Detailed log of new commits:

Author: aavtamon
Date: Fri Nov 27 15:08:33 2020 +0300

    0031402: Modeling Data - Geom_BSplineSurface::Segment() produces wrong result
    
    In Geom_BSplineSurface, Geom_BSplineCurve and Geom2d_BSplineCurve changed Eps calculation from Max to Min in method Segment().
(0097231)
git   
2020-12-02 10:33   
Branch CR31402_2 has been created by aavtamon.

SHA-1: 5ac765e98da1f4232e0dffa303a3d43a54f3e9a1


Detailed log of new commits:

Author: aavtamon
Date: Wed Dec 2 10:18:15 2020 +0300

    0031402: Modeling Data - Geom_BSplineSurface::Segment() produces wrong result
    
    In the method Segment() index1 needs to be checked as well as index2 in Geom_BSplineSurface and Geom2d_BSplineCurve
    (Geom_BSplineCurve already has this check).
    New test case bug31402_1 has been added to check surface.
(0097236)
git   
2020-12-02 14:25   
Branch CR31402_2 has been updated forcibly by aavtamon.

SHA-1: 3a26a7c7ee62a0131bc12eb73e0a6107851df69b
(0097237)
aavtamon   
2020-12-02 14:28   
Could you please check an another solution?
Test results are here:
http://jenkins-test-12.nnov.opencascade.com/view/CR31402_2-master-aavtamon/view/COMPARE/ [^]
(0097262)
ifv   
2020-12-03 11:32   
(edited on: 2020-12-03 11:34)
1. At beginning of Geom2d_BSplineCurve::Segment(...) (lines 671-683) there is piece of code:
  U1=aU1;
  U2=aU2;
  n1=knots->Lower();
  n2=knots->Upper();
  for (i=n1; i<=n2; ++i) {
    U=knots->Value(i);
    if (Abs(U-aU1)<=Eps) {
      U1=U;
    }
    else if (Abs(U-aU2)<=Eps) {
      U2=U;
    }
  }

which seems to be unnecessary after your modification.

In other side, checking for index2 in Geom_BSplineCurve::Segment(...) contains additional condition to prevent index1 == index2, see line 568:
  if ( Abs(knots->Value(index2+1)-U) <= Eps || index2 == index1)
    index2++;

Such condition should be added for Geom2d_BSplineCurve and Geom_BSplineSurface

(0097271)
git   
2020-12-03 14:25   
Branch CR31402_2 has been updated by aavtamon.

SHA-1: 8303ebb0f15a80c4813aa3f4605df0175d1f0869


Detailed log of new commits:

Author: aavtamon
Date: Thu Dec 3 14:18:23 2020 +0300

    The unnecessary code block in Geom2d_BSplineCurve has been deleted,
    and checking index2 block has beed extended.

(0097272)
aavtamon   
2020-12-03 14:28   
Thank you for your remarks, I've applied them.
And do I need to start another full test session to check new changes?
(0097274)
ifv   
2020-12-03 15:49   
(edited on: 2020-12-03 15:58)
Of course, you should retest branch after modification of code, except "cosmetic" ones or comments.

Now U1 and U2 are useless, it is better to replace them by aU1 and aU2 in code.

(0097286)
git   
2020-12-03 21:33   
Branch CR31402_2 has been updated forcibly by aavtamon.

SHA-1: 5f8c684ae90ab261594e46fb388829a05cc7919a
(0097300)
aavtamon   
2020-12-04 10:28   
The test results tend to be the same:
http://jenkins-test-12.nnov.opencascade.com/view/CR31402_2-master-aavtamon/view/COMPARE/ [^]
(0097301)
ifv   
2020-12-04 10:34   
Seems to be valid

Branches for integration:
OCCT - CR31402_2
Products - not
(0097350)
bugmaster   
2020-12-05 11:57   
Combination -
OCCT branch : IR-2020-12-04
master SHA - 1e08a76f1e872a1f38931a6c3e8cf71396fc1209
a206de37fbfa0bf71bd534ae47192bbec23b8522
Products branch : IR-2020-12-04 SHA - cb56638fb6b31df41b28133f69a1dc202dcb56aa
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: 18090.67999999998 / 18052.040000000085 [+0.21%]
Products
Total CPU difference: 12342.950000000128 / 12326.850000000131 [+0.13%]
Windows-64-VC14:
OCCT
Total CPU difference: 19751.5 / 19705.828125 [+0.23%]
Products
Total CPU difference: 13772.515625 / 13793.796875 [-0.15%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention
(0097356)
git   
2020-12-05 13:01   
Branch CR31402_2 has been deleted by inv.

SHA-1: 5f8c684ae90ab261594e46fb388829a05cc7919a
(0097367)
git   
2020-12-05 13:01   
Branch CR31402_1 has been deleted by inv.

SHA-1: 4dd6ba131f5d2d8121180b6717344b397d854ea0
(0097369)
git   
2020-12-05 13:01   
Branch CR31402 has been deleted by inv.

SHA-1: 0b204a460ad1344783bedb6fe91f198a8fc85780