View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0031402CommunityOCCT:Modeling Datapublic2020-03-04 10:502020-12-05 13:01
Reportermartinsiggel Assigned Tobugmaster  
PrioritynormalSeveritymajor 
Status closedResolutionfixed 
Product Version7.4.0 
Target Version7.6.0Fixed in Version7.6.0 
Summary0031402: Modeling Data - Geom_BSplineSurface::Segment() produces wrong result
DescriptionUsing 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.
Steps To Reproduce
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);
TagsNo tags attached.
Test case numberbugs/moddata_3/bug31402_1,bug31402_2

Attached Files

  • surface.brep (4,352 bytes)

Activities

martinsiggel

2020-03-04 10:50

developer  

surface.brep (4,352 bytes)

martinsiggel

2020-03-04 11:38

developer   ~0090808

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.

git

2020-11-12 11:04

administrator   ~0096686

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.

git

2020-11-12 16:52

administrator   ~0096723

Branch CR31402 has been updated forcibly by aavtamon.

SHA-1: 962605742c961f79d4f260597de293228db633c5

git

2020-11-12 22:24

administrator   ~0096732

Branch CR31402 has been updated forcibly by aavtamon.

SHA-1: 40ac7d258eda547429be6574767f79ea7dd7dd55

git

2020-11-13 12:35

administrator   ~0096743

Branch CR31402 has been updated forcibly by aavtamon.

SHA-1: 2d45160898b8a015f1e4dd9cf9c98f448c0cd28a

git

2020-11-13 12:42

administrator   ~0096744

Branch CR31402 has been updated forcibly by aavtamon.

SHA-1: 0b204a460ad1344783bedb6fe91f198a8fc85780

aavtamon

2020-11-13 14:48

developer   ~0096746

Dear Mikhail,
could you please check it?
http://jenkins-test-12.nnov.opencascade.com/view/CR31402-master-aavtamon/view/ALL/

msv

2020-11-13 18:11

developer   ~0096753

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.

msv

2020-11-13 18:14

developer   ~0096754

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.

msv

2020-11-13 18:16

developer   ~0096755

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?

aavtamon

2020-11-14 00:01

developer   ~0096772

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

ifv

2020-11-16 13:05

developer   ~0096791

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]]

git

2020-11-27 15:15

administrator   ~0097110

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().

git

2020-12-02 10:33

administrator   ~0097231

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.

git

2020-12-02 14:25

administrator   ~0097236

Branch CR31402_2 has been updated forcibly by aavtamon.

SHA-1: 3a26a7c7ee62a0131bc12eb73e0a6107851df69b

aavtamon

2020-12-02 14:28

developer   ~0097237

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/

ifv

2020-12-03 11:32

developer   ~0097262

Last edited: 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

git

2020-12-03 14:25

administrator   ~0097271

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.

aavtamon

2020-12-03 14:28

developer   ~0097272

Thank you for your remarks, I've applied them.
And do I need to start another full test session to check new changes?

ifv

2020-12-03 15:49

developer   ~0097274

Last edited: 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.

git

2020-12-03 21:33

administrator   ~0097286

Branch CR31402_2 has been updated forcibly by aavtamon.

SHA-1: 5f8c684ae90ab261594e46fb388829a05cc7919a

aavtamon

2020-12-04 10:28

developer   ~0097300

The test results tend to be the same:
http://jenkins-test-12.nnov.opencascade.com/view/CR31402_2-master-aavtamon/view/COMPARE/

ifv

2020-12-04 10:34

developer   ~0097301

Seems to be valid

Branches for integration:
OCCT - CR31402_2
Products - not

bugmaster

2020-12-05 11:57

administrator   ~0097350

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

git

2020-12-05 13:01

administrator   ~0097356

Branch CR31402_2 has been deleted by inv.

SHA-1: 5f8c684ae90ab261594e46fb388829a05cc7919a

git

2020-12-05 13:01

administrator   ~0097367

Branch CR31402_1 has been deleted by inv.

SHA-1: 4dd6ba131f5d2d8121180b6717344b397d854ea0

git

2020-12-05 13:01

administrator   ~0097369

Branch CR31402 has been deleted by inv.

SHA-1: 0b204a460ad1344783bedb6fe91f198a8fc85780

Related Changesets

occt: master 1e08a76f

2020-12-02 07:18:15

aavtamon


Committer: bugmaster Details Diff		 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 cases bug31402_1, bug31402_2 has been added.

The unnecessary code block in Geom2d_BSplineCurve has been deleted,
and checking index2 block has beed extended.		 Affected Issues
0031402
mod - src/Geom/Geom_BSplineSurface.cxx Diff File
mod - src/Geom2d/Geom2d_BSplineCurve.cxx Diff File
add - tests/bugs/moddata_3/bug31402_1 Diff File
add - tests/bugs/moddata_3/bug31402_2 Diff File

Issue History

Date Modified Username Field Change
2020-03-04 10:50 martinsiggel New Issue
2020-03-04 10:50 martinsiggel Assigned To => msv
2020-03-04 10:50 martinsiggel File Added: surface.brep
2020-03-04 11:38 martinsiggel Note Added: 0090808
2020-09-15 12:27 msv Product Version 7.5.0 => 7.4.0
2020-09-15 12:27 msv Target Version 7.5.0 => 7.6.0
2020-11-02 11:30 szy Assigned To msv => aavtamon
2020-11-02 11:30 szy Status new => assigned
2020-11-12 11:04 git Note Added: 0096686
2020-11-12 16:52 git Note Added: 0096723
2020-11-12 17:22 kgv Summary Geom_BSplineSurface::Segment produces wrong result => Modeling Data - Geom_BSplineSurface::Segment() produces wrong result
2020-11-12 17:23 kgv Description Updated
2020-11-12 17:23 kgv Steps to Reproduce Updated
2020-11-12 22:24 git Note Added: 0096732
2020-11-13 12:35 git Note Added: 0096743
2020-11-13 12:42 git Note Added: 0096744
2020-11-13 14:48 aavtamon Note Added: 0096746
2020-11-13 14:48 aavtamon Assigned To aavtamon => msv
2020-11-13 14:48 aavtamon Status assigned => resolved
2020-11-13 18:11 msv Note Added: 0096753
2020-11-13 18:14 msv Note Added: 0096754
2020-11-13 18:16 msv Note Added: 0096755
2020-11-13 18:16 msv Assigned To msv => ifv
2020-11-14 00:01 aavtamon Note Added: 0096772
2020-11-16 13:05 ifv Note Added: 0096791
2020-11-16 13:05 ifv Assigned To ifv => aavtamon
2020-11-16 13:05 ifv Status resolved => assigned
2020-11-27 15:15 git Note Added: 0097110
2020-12-02 10:33 git Note Added: 0097231
2020-12-02 14:25 git Note Added: 0097236
2020-12-02 14:28 aavtamon Note Added: 0097237
2020-12-02 14:28 aavtamon Assigned To aavtamon => ifv
2020-12-02 14:28 aavtamon Status assigned => resolved
2020-12-03 11:32 ifv Note Added: 0097262
2020-12-03 11:32 ifv Assigned To ifv => aavtamon
2020-12-03 11:32 ifv Status resolved => assigned
2020-12-03 11:34 ifv Note Edited: 0097262
2020-12-03 14:25 git Note Added: 0097271
2020-12-03 14:28 aavtamon Note Added: 0097272
2020-12-03 14:28 aavtamon Assigned To aavtamon => ifv
2020-12-03 14:28 aavtamon Status assigned => resolved
2020-12-03 15:49 ifv Note Added: 0097274
2020-12-03 15:49 ifv Assigned To ifv => aavtamon
2020-12-03 15:49 ifv Status resolved => assigned
2020-12-03 15:58 ifv Note Edited: 0097274
2020-12-03 21:33 git Note Added: 0097286
2020-12-04 10:28 aavtamon Note Added: 0097300
2020-12-04 10:28 aavtamon Assigned To aavtamon => ifv
2020-12-04 10:28 aavtamon Status assigned => resolved
2020-12-04 10:34 ifv Note Added: 0097301
2020-12-04 10:34 ifv Assigned To ifv => bugmaster
2020-12-04 10:34 ifv Status resolved => reviewed
2020-12-05 11:57 bugmaster Note Added: 0097350
2020-12-05 11:57 bugmaster Status reviewed => tested
2020-12-05 12:11 bugmaster Test case number => bugs/moddata_3/bug31402_1,bug31402_2
2020-12-05 12:15 bugmaster Changeset attached => occt master 1e08a76f
2020-12-05 12:15 bugmaster Status tested => verified
2020-12-05 12:15 bugmaster Resolution open => fixed
2020-12-05 13:01 git Note Added: 0097356
2020-12-05 13:01 git Note Added: 0097367
2020-12-05 13:01 git Note Added: 0097369