Archived issue #0029745

Modeling Data - GeomAdaptor_Surface::VIntervals fails on periodic surfaces

CommunityOCCT:Modeling Dataclosed143 public notes

Description

Suppose:
_ we have a V periodic NURBS Surface;
_ we have a GeomAdaptor_Surface with V limits outside of the period;
_ the surface has discontinuities;
_ we call GeomAdaptor_Surface::VIntervals to get a list of continuous subintervals.

That method will build an U iso-curve and get such intervals on that; at the end, however, that intervals are bounded with:
  T(T.Lower()) = myVFirst;
  T(T.Lower() + myNbVIntervals) = myVLast;

The problem arises since the value returned by the curve Intervals(...) method will be within the period, so those values will be invalid.

Example:
_ surface with periodic V knot vector ranging [0 ; 4];
_ surface adaptor with limits [5 ; 6].
_ Intervals(...) on the ISO curve returns [1,2,3];
_ the statements reported above will transform the result in [5,2,6].

This will lead to a crash in subsequent processing.

A solution along the line of the following might work:
  if (mySurface->IsVPeriodic())
    for (Standard_Integer i=T.Lower()+1;i<T.Lower()+myNbVIntervals;++i)
      T(i)+=myVFirst-T(T.Lower());
  T(T.Lower()) = myVFirst;
  T(T.Lower() + myNbVIntervals) = myVLast;

Notice the problem should also occur in U direction.

Steps to reproduce

QAcommand:
OCC29745 bc1 1 0 12

Public activity

143 archived notes

Participants are labeled by their role within this record.

01Commenter 22020-11-06 11:58

Dear Author, can you attach more detailed steps to reproduce current bug, because according to this description it is hard to reproduce it

02Author2020-12-18 18:35

TL;DR
Changes made in 7.4.0 prevent this bug from happening again.
However, I'm not sure the new result is correct either.

The problem arose while importint a STeP model and I'm having an hard time reproducing it programmatically (even with 7.3.0).

In any case, please take a look at the following code:
  TColgp_Array2OfPnt Poles(1,2,1,3);
  Poles(1,1)=gp_Pnt(0,0,0); Poles(2,1)=gp_Pnt(0,0,1);
  Poles(1,2)=gp_Pnt(1,0,0); Poles(2,2)=gp_Pnt(1,0,1);
  Poles(1,3)=gp_Pnt(1,1,0); Poles(2,3)=gp_Pnt(1,1,1);
  //Poles(1,4)=gp_Pnt(0,0,0); Poles(2,4)=gp_Pnt(0,0,1);
  TColStd_Array1OfReal UKnots(1,2),VKnots(1,4),ResultsA(1,4),ResultsB(1,4);
  UKnots(1)=0; UKnots(2)=1;
  VKnots(1)=0; VKnots(2)=1; VKnots(3)=2; VKnots(4)=3;
  TColStd_Array1OfInteger UMult(1,2),VMult(1,4);
  UMult(1)=2; UMult(2)=2;
  VMult(1)=1; VMult(2)=1; VMult(3)=1; VMult(4)=1;
  Handle(Geom_BSplineSurface) S=new Geom_BSplineSurface(Poles,UKnots,VKnots,UMult,VMult,1,1,false,true);
  GeomAdaptor_Surface A(S,0,1,0,3);
  A.VIntervals(ResultsA,GeomAbs_C1);
  for (TColStd_Array1OfReal::const_iterator i(ResultsA.begin());i!=ResultsA.end();++i) std::cout<<*i<<" ";
  std::cout<<std::endl;
  GeomAdaptor_Surface B(S,0,1,3,6);
  B.VIntervals(ResultsB,GeomAbs_C1);
  for (TColStd_Array1OfReal::const_iterator i(ResultsB.begin());i!=ResultsB.end();++i) std::cout<<*i<<" ";
  std::cout<<std::endl;

Output I'd expect (but possibly I'm wrong):
0 1 2 3
3 4 5 6

Output I get (with OC 7.3.0):
0 1 2 3
3 6 0 0

Notice here two values are missing.
This is not the original problem I reported and which I'm unable to reproduce now: in that case what I was getting in the second line was the equivalent of 3 1 2 6.

7.4.0 changed this behaviour: at line 441 of GeomAdaptor_Surface.cxx, "break" was replaces with a "return".
So the code at the end of the function, that changes only the first and last value, is not executed, and what I originally reported cannot happen anymore.

Still I'm wondering whether "3 6" is to be expected or if we just exchanged a bug for another.

03Commenter 22021-01-26 11:37

Fixed GeomAdaptor_Curve::LocalContinuity() for periodic surface

04Commenter 32021-01-28 14:12

Branch [archived branch] has been updated by Commenter 2.

[revision removed]

Detailed log of new commits:

Author: Commenter 2
Date: Thu Jan 28 14:12:53 2021 +0300

Fixed GeomAdaptor_Curve::NbIntervals() method to periodic curves

05Commenter 22021-01-28 14:13

For previous commit

06Commenter 22021-02-01 15:27

Debbuging

07Commenter 32021-02-03 18:09

Branch [archived branch] has been updated forcibly by Commenter 2.

[revision removed]

08Commenter 22021-02-03 18:26

Fixed GeomAdaptor_Curve::Intervals() method to periodic curves

09Commenter 32021-02-03 21:46

Branch [archived branch] has been updated forcibly by Commenter 2.

[revision removed]

10Commenter 32021-02-05 11:43

Branch [archived branch] has been updated by Commenter 2.

[revision removed]

Detailed log of new commits:

Author: Commenter 2
Date: Fri Feb 5 11:44:33 2021 +0300

some fixes for negative intervals

Author: Commenter 2
Date: Thu Feb 4 15:17:42 2021 +0300

delete bad definition

11Commenter 32021-02-05 16:53

Branch [archived branch] has been updated by Commenter 2.

[revision removed]

Detailed log of new commits:

Author: Commenter 2
Date: Fri Feb 5 16:54:21 2021 +0300

Change definition of local continuity only within one period

12Commenter 22021-02-05 16:59

Analyzing results and fixing regressions

13Commenter 22021-02-08 18:10

Debuging

14Commenter 32021-02-08 18:28

Branch [archived branch] has been created by Commenter 2.

[revision removed]

Detailed log of new commits:

Author: Commenter 2
Date: Fri Jan 15 10:50:18 2021 +0300

    0029745: Modeling Data - GeomAdaptor_Surface::VIntervals fails on periodic surfaces

    Fixed GeomAdaptor_Curve::LocalContinuity() for periodic surface within one period
    Fixed GeomAdaptor_Curve::NbIntervals() method to periodic curves
    Fixed GeomAdaptor_Curve::Intervals() method to periodic curves

15Commenter 32021-02-18 11:33

Branch [archived branch] has been updated forcibly by Commenter 2.

[revision removed]

16Commenter 162021-03-04 17:39

Dear Andrey, I think that Geom2dAdaptor_Curve should be modified too in order to have the same behaviour for 2d and 3d curves. In general, codes for 2d and 3d cases must be practically the same, because works only with 1d parameters of splines, so think about using any template approach.

17Commenter 172021-03-05 09:11

Review

18Commenter 32021-03-05 14:26

Branch [archived branch] has been created by Commenter 2.

[revision removed]

Detailed log of new commits:

Author: Commenter 2
Date: Fri Jan 15 10:50:18 2021 +0300

    update tests

    change test

    0029745: Modeling Data - GeomAdaptor_Surface::VIntervals fails on periodic surfaces

    Fixed GeomAdaptor_Curve::LocalContinuity() for periodic surface within one period.
    Fixed GeomAdaptor_Curve::NbIntervals() method to periodic curves.
    Fixed GeomAdaptor_Curve::Intervals() method to periodic curves.
    Deleted definition of length from tests.
    Update Geom2dAdaptor_Curve to the same behavior.

19Commenter 32021-03-05 14:27

Branch [archived branch] has been updated forcibly by Commenter 2.

[revision removed]

20Commenter 32021-03-05 18:44

Branch [archived branch] has been updated forcibly by Commenter 2.

[revision removed]

21Commenter 22021-03-05 20:55

Branch for review:
OCCT - CR29745_2

22Commenter 22021-03-05 20:55

Results of testing:
http://jenkins-test-12.nnov.opencascade.com/view/CR29745-master-Commenter 2/view/COMPARE/

23Commenter 232021-03-15 08:23

Fix seems to be wrong because it does not provide correct results if trim interval crosses boundary between sequential periodic intervals.

24Commenter 32021-11-11 05:56

Branch [archived branch] has been created by Participant.

[revision removed]

Detailed log of new commits:

Author: Commenter 1
Date: Thu Nov 11 05:55:32 2021 +0300

    0029745: Modeling Algorithms - GeomAdaptor_Surface::VIntervals fails on periodic surfaces

    Fixed GeomAdaptor_Curve::LocalContinuity() for periodic surface;
    Fixed GeomAdaptor_Curve::NbIntervals() method for periodic curves

25Commenter 32021-11-16 15:50