Archived issue #0027358

ShapeAnalysis_Curve::GetSamplePoints iteration logic isn't robust

CommunityOCCT:Shape Healingclosed24 public notes

Description

The current iterating loop is

Standard_Real step = ( last - first ) / (Standard_Real)( nbs - 1 );
Standard_Real par = first, stop = last - 0.5 * step;
for ( ; par < stop; par += step )
seq.Append(C.Value(par));
seq.Append(C.Value(last));

Such iterating isn't robust against some numerical errors appearing. Moreover for even smaller steps it can generate much more points, than nbs. Therefore it's proposed to use the following loop:

Standard_Real step = ( last - first ) / (Standard_Real)( nbs - 1 );
for (Standard_Integer i = 0; i < nbs; ++i)
seq.Append(curve->Value(first + step * i));
seq.Append(curve->Value(last));

Steps to reproduce

Not applicable

Public activity

24 archived notes

Participants are labeled by their role within this record.

01Commenter 22016-04-06 10:31

Branch [archived branch] has been created by Author.

[revision removed]

Detailed log of new commits:

Author: razmyslovich
Date: Wed Apr 6 09:30:58 2016 +0200

Make the iterating logic in ShapeAnalysis_Curve::GetSamplePoints more robust

02Commenter 22016-04-25 18:12

This modification is incorrect since the point for the last parameter added to the sequence twice.

03Commenter 22016-04-26 11:10

Branch [archived branch] has been updated by Author.

[revision removed]

Detailed log of new commits:

Author: razmyslovich
Date: Tue Apr 26 10:10:36 2016 +0200

0027358: Make the iterating logic in ShapeAnalysis_Curve::GetSamplePoints more robust - fix the points number

04Commenter 42016-04-26 11:43

Branch CR27358 is ready to be tested

05Commenter 22016-04-26 12:19

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

06Commenter 62016-04-26 13:34

Dear Commenter 1,
Branch CR27358 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
[revision removed]

There are following compilation errors:
Linux:
http://jenkins-test-07.nnov.opencascade.com:8080/view/CR27358-master/job/CR27358-master-OCCT-Debian70-64-opt-compile/1/parsed_console/
/dn54/builds/CR27358-master/Debian70-64-opt/OCCT/src/ShapeAnalysis/ShapeAnalysis_Curve.cxx:1035:60: error: 'nbs' was not declared in this scope

Windows:
http://jenkins-test-07.nnov.opencascade.com:8080/view/CR27358-master/job/CR27358-master-OCCT-Windows-64-VC10-opt-compile/1/parsed_console/
19>..\..\..\src\ShapeAnalysis\ShapeAnalysis_Curve.cxx(1035): error C2065: 'nbs' : undeclared identifier
19>..\..\..\src\ShapeAnalysis\ShapeAnalysis_Curve.cxx(1036): error C2065: 'nbs' : undeclared identifier

MacOS:
http://jenkins-test-07.nnov.opencascade.com:8080/view/CR27358-master/job/CR27358-master-OCCT-MacOS-opt-compile/1/parsed_console/
[ 33%] [user path removed]/builds/CR27358-master/MacOS-opt/OCCT/src/ShapeAnalysis/ShapeAnalysis_Curve.cxx:1035:60: error: use of undeclared identifier 'nbs'; did you mean 'nbp'?
  Standard_Real step = ( last - first ) / (Standard_Real)( nbs - 1 );
                                                           ^~~
                                                           nbp

[user path removed]/builds/CR27358-master/MacOS-opt/OCCT/src/ShapeAnalysis/ShapeAnalysis_Curve.cxx:1036:36: error: use of undeclared identifier 'nbs'; did you mean 'nbp'?
  for (Standard_Integer i = 0; i < nbs - 1; ++i)
                                   ^~~
                                   nbp

07Commenter 72016-04-26 13:34

Dear Author,
Branch [archived branch] has been rejected due to:
- compilation errors

08Commenter 22016-04-26 14:41

Branch [archived branch] has been updated by Author.

[revision removed]

Detailed log of new commits:

Author: razmyslovich
Date: Tue Apr 26 13:41:08 2016 +0200

    Merge branch 'CR27358' of git.dev.opencascade.org:occt into CR27358

    Conflicts:
        src/ShapeAnalysis/ShapeAnalysis_Curve.cxx

Author: razmyslovich
Date: Tue Apr 26 13:29:15 2016 +0200

    0027358: Make the iterating logic in ShapeAnalysis_Curve::GetSamplePoints more robust - clean build

Author: razmyslovich
Date: Tue Apr 26 13:13:13 2016 +0200

    0027358: Make the iterating logic in ShapeAnalysis_Curve::GetSamplePoints more robust - clean build

Author: razmyslovich
Date: Tue Apr 26 10:10:36 2016 +0200

    0027358: Make the iterating logic in ShapeAnalysis_Curve::GetSamplePoints more robust - fix the points number

Author: razmyslovich
Date: Wed Apr 6 09:30:58 2016 +0200

    Make the iterating logic in ShapeAnalysis_Curve::GetSamplePoints more robust

09Author2016-04-26 14:49

Sorry for compilation errors

10Commenter 102016-04-28 15:24

Branch CR27358 is ready to be tested

11Commenter 22016-04-29 13:45

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

12Commenter 22016-04-29 16:58

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

13Commenter 132016-05-04 12:53

Dear Commenter 1,
Branch CR27358 was rebased on current master of occt git-repository.
[revision removed]

14Commenter 142016-05-04 12:54

Dear Commenter 1,
Branch CR27358 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested on Release mode.
[revision removed]

There are following compilation errors:
Linux:
http://jenkins-test-07.nnov.opencascade.com:8080/view/CR27358-master/job/CR27358-master-OCCT-Debian70-64-opt-compile/1/parsed_console/
/dn54/builds/CR27358-master/Debian70-64-opt/OCCT/src/ShapeAnalysis/ShapeAnalysis_Curve.cxx:1035:60: error: 'nbs' was not declared in this scope

Windows:
http://jenkins-test-07.nnov.opencascade.com:8080/view/CR27358-master/job/CR27358-master-OCCT-Windows-64-VC10-opt-compile/1/parsed_console/
19>..\..\..\src\ShapeAnalysis\ShapeAnalysis_Curve.cxx(1035): error C2065: 'nbs' : undeclared identifier
19>..\..\..\src\ShapeAnalysis\ShapeAnalysis_Curve.cxx(1036): error C2065: 'nbs' : undeclared identifier

MacOS:
http://jenkins-test-07.nnov.opencascade.com:8080/view/CR27358-master/job/CR27358-master-OCCT-MacOS-opt-compile/1/parsed_console/
[user path removed]/builds/CR27358-master/MacOS-opt/OCCT/src/ShapeAnalysis/ShapeAnalysis_Curve.cxx:1035:60: error: use of undeclared identifier 'nbs'; did you mean 'nbp'?
  Standard_Real step = ( last - first ) / (Standard_Real)( nbs - 1 );
                                                           ^~~
                                                           nbp
[user path removed]/builds/CR27358-master/MacOS-opt/OCCT/src/ShapeAnalysis/ShapeAnalysis_Curve.cxx:1009:20: note: 'nbp' declared here
  Standard_Integer nbp =100*aK;
                   ^
[user path removed]/builds/CR27358-master/MacOS-opt/OCCT/src/ShapeAnalysis/ShapeAnalysis_Curve.cxx:1036:36: error: use of undeclared identifier 'nbs'; did you mean 'nbp'?
  for (Standard_Integer i = 0; i < nbs - 1; ++i)
                                   ^~~
                                   nbp

15Commenter 152016-05-04 12:58

Dear Author,
Branch [archived branch] has been rejected due to:
- compilation errors

Dear Author,
could you please use "Rebase..." function of TortoiseGit, do not use "Merge..." function.

16Author2016-05-11 15:29

Dear mkv,
sorry for this mess, I'm not using git that often...
Could you please still help me to commit my changes correctly?
My current local copy is switched to the branch. I did git->rebase, upstream: master. There were a couple of conflicts, but it worked fine, I pushed commit at the end and done. Now I would like to commit and push the correct file, but the change list is empty and commit is impossible. Did I something wrong? Should I just push now the changes.
Thanks in advance!
[signature removed]
Dima

17Commenter 22016-09-12 13:52

Branch [archived branch] has been created by Author.

[revision removed]

Detailed log of new commits:

Author: razmyslovich
Date: Mon Sep 12 12:52:36 2016 +0200

0027358: Make the iterating logic in ShapeAnalysis_Curve::GetSamplePoints more robust - rebased

Author: razmyslovich
Date: Mon Sep 12 11:17:48 2016 +0200

0027863: Fix Geom_BSplineSurface::SetVKnot function

18Author2016-09-12 13:54

Since my tries to make a rebase in the original branch were unsuccessful, I finally created a new branch using the current master state. Please, review the changes. Thank you!

19Commenter 22017-08-14 17:00

Branch [archived branch] has been created by Participant.

[revision removed]

Detailed log of new commits:

Author: razmyslovich
Date: Mon Sep 12 12:52:36 2016 +0200

    0027358: ShapeAnalysis_Curve::GetSamplePoints iteration logic isn't robust

    The iterating logic in ShapeAnalysis_Curve::GetSamplePoints() is made more robust: instead of iterative incrementing parameter by adding step, parameter at each point is calculated independently from index.
    This avoids possible accumulation of numeric errors, and ensures that generated points are equally spaced and their quantity is respected in all cases.

20Commenter 202017-08-15 10:47

The fix has been rebased on current master (branch CR27358_2), reviewed, and tested -- see job CR27358-master-abv. Please integrate

21Commenter 12017-08-15 12:10

Combination -
OCCT branch : [archived branch] : [revision removed]
Products branch : [archived branch]
was compiled on Linux, MacOS and Windows platforms and tested on optimize mode.

http://jenkins-test-10.nnov.opencascade.com/view/CR27358-master-abv/view/START%20BRANCH%20CERTIFICATION/

Number of compiler warnings:

OCCT :
Linux: 0 (0 on master)
Windows: 0 (0 on master)
MacOS : 0 (0 on master)

Products :
Linux: 5 (5 on master)
Windows: 0 (0 on master)
MacOS : 0 (0 on master)

Regressions/Differences/Improvements:
No regressions/differences

CPU differences:

Windows:
OCCT
Total CPU difference: 17276.112343598714 / 17295.75286949859 [-0.11%]
Producst
Total CPU difference: 7737.477998899967 / 7720.302288799964 [+0.22%]

Linux:
OCCT
Total CPU difference: 19493.150000000118 / 19456.560000000376 [+0.19%]
Products
Total CPU difference: 7727.790000000094 / 7725.120000000072 [+0.03%]

Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

22Commenter 22017-08-18 17:00

Branch [archived branch] has been deleted by Participant.

[revision removed]

23Commenter 22017-08-18 17:00

Branch [archived branch] has been deleted by Participant.

[revision removed]

24Commenter 22017-08-18 17:00

Branch [archived branch] has been deleted by Participant.

[revision removed]