View Issue Details

IDProjectCategoryView StatusLast Update
0031361Open CASCADEOCCT:Modeling Algorithmspublic2023-03-19 20:09
Reporterakaftasev Assigned Tomgerus 
PrioritynormalSeverityminor 
Status closedResolutionfixed 
PlatformWindowsOSVC++ 2013 
Product Version7.4.0 
Target Version7.7.0Fixed in Version7.6.3 
Summary0031361: Modeling algorithms, GeomFill_Pipe - an exception arises when building tube
DescriptionWhen try to build tube on two circles in on surface and bezier curve, an exception arise
Steps To Reproduce
pload MODELING
circle c1 0 0 0 10
circle c2 0 100 0 10
beziercurve curve 4 0 0 0 100 0 0 100 100 0 0 100 0
tuyau t curve c1 c2
TagsNo tags attached.
Test case numberbugs/modalg_7/bug31361

Activities

msv

2020-09-15 12:34

developer   ~0094821

An exception was caught 0000023C32AB1DA0 : Standard_NumericError: FLT INVALID OPERATION

The parameters of construction are incorrect, because circles lie in the same plane in that the path is located. So, we cannot expect any good result of such operation. However, raising such exception should not be done. The best behavior is to report an error status, or at least to throw such meaningful program exception.

git

2022-02-25 10:43

administrator   ~0107061

Branch CR31361 has been created by mgerus.

SHA-1: d857ebd309cba983f0e69ee0205cf0b171e86225


Detailed log of new commits:

Author: mgerus
Date: Fri Feb 25 10:39:29 2022 +0300

    0031361: Modeling algorithms - An exception arise when building tube
    
    Add status flag to GeomFill_Pipe

git

2022-02-25 12:48

administrator   ~0107063

Branch CR31361 has been updated by mgerus.

SHA-1: 50722a8072691193e06a8958a8b871503432fafc


Detailed log of new commits:

Author: mgerus
Date: Fri Feb 25 12:48:20 2022 +0300

    0031361: Modeling algorithms - An exception arise when building tube
    
    Delete isDone from GeomFill_Pipe, changed to myStatus
    Remove non-ASCI symbols from comments

git

2022-03-01 10:39

administrator   ~0107115

Branch CR31361 has been updated forcibly by mgerus.

SHA-1: e8ca2d7ec76f6964f7f3d6a8cdc1302ff2e30688

git

2022-03-14 16:37

administrator   ~0107281

Branch CR31361 has been updated by mgerus.

SHA-1: 25e47d6c905f82cef700b0169082e0b7ede49138


Detailed log of new commits:

Author: mgerus
Date: Mon Mar 14 16:37:19 2022 +0300

    fix after tests

git

2022-03-15 09:34

administrator   ~0107288

Branch CR31361 has been updated by mgerus.

SHA-1: b0e722d0fa64913c164434805e73eb4d87e30602


Detailed log of new commits:

Author: mgerus
Date: Tue Mar 15 09:34:36 2022 +0300

    fix after tests v2

git

2022-03-16 13:27

administrator   ~0107303

Branch CR31361 has been updated forcibly by mgerus.

SHA-1: 52e7ecf2b0633deeeaa6fb730684377be6343a2d

git

2022-03-17 10:14

administrator   ~0107316

Branch CR31361 has been updated forcibly by mgerus.

SHA-1: 443634fa2317a33eceb0d7107781feecaa663190

mgerus

2022-03-17 14:18

developer   ~0107326

Please review the branch CR31361.
http://jenkins-test-occt/view/CR31361-master-MGERUS/view/COMPARE/

Branches for Integration:
OCCT - CR31361
Products - Not

azv

2022-03-21 14:03

administrator   ~0107375

Last edited: 2022-03-21 14:04

Dear Maxim,

Please, consider the following remarks:

src/GeomFill/GeomFill_CorrectedFrenet.cxx

1. What do we check within this statement?
518:    if (Tangent.Crossed(prevTangent).SquareMagnitude() < Precision::SquareConfusion() &&
519:      (!(abs(Tangent.Dot(prevTangent)) - 1 < Precision::Confusion()) || Tangent.Dot(prevTangent) + 1 < Precision::Confusion()))

Looks like we are verifying that Tangent and prevTangent vectors are collinear and their dot-product should be greater than 1. I do not understand such strong restriction, don't we required the collinearity only?

2. Suppose, Standard_False should be returned here.
520:    {
521:      return 0;
522:    }



src/GeomFill/GeomFill_Pipe.cxx

3. Get rid of spaces in line 754.

4. I propose to update the status here instead of defining additional flag myIsPerform.
 
782:    else
783:    {
784:      myIsPerform = Standard_False;
785:    }


General remark

5. Please, also provide a test case proving the problem is fixed.

git

2022-03-24 10:59

administrator   ~0107411

Branch CR31361 has been updated by mgerus.

SHA-1: e21e98e8fac7b10bd1de4e5b5ad3149abe34ad37


Detailed log of new commits:

Author: mgerus
Date: Thu Mar 24 10:59:48 2022 +0300

    0031361: Modeling algorithms - An exception arise when building tube
    
    Add fixes after code review

git

2022-03-28 08:32

administrator   ~0107441

Branch CR31361 has been updated forcibly by mgerus.

SHA-1: e99ade9bd15c32766eaa14534c91c8365729c232

kgv

2022-03-28 14:11

developer   ~0107446

   Pipe.Perform(1.e-4, Standard_False,  Cont);
   if (!Pipe.IsDone()) {
     di << "GeomFill_Pipe cannot make a surface\n";
-    return 1;
+    return 0;
   }

Messages related to algorithm failures should be started with "Error: " prefix.

git

2022-03-28 15:03

administrator   ~0107447

Branch CR31361 has been updated by mgerus.

SHA-1: 39b2e96ed47bb10066642d6653cf96ca257e3e54


Detailed log of new commits:

Author: mgerus
Date: Mon Mar 28 15:03:26 2022 +0300

    # Add fixes after code review v2

azv

2022-03-28 23:26

administrator   ~0107454

GeomFill_Pipe.cxx

I suppose, status GeomFill_ImpossibleContact could be kept, so the following line is not necessary.
842:    myStatus = GeomFill_PipeNotOk;


Maxim, I have reviewed all commits in your branch, so could you rebase it to the current master and squash into a single commit?

git

2022-03-29 09:19

administrator   ~0107457

Branch CR31361 has been updated forcibly by mgerus.

SHA-1: 510083c77ddba617c56e060cd04cc3da9d638144

git

2022-03-31 10:16

administrator   ~0107502

Branch CR31361 has been updated forcibly by mgerus.

SHA-1: 57193d45f93869b531aa184739d6e6e6e5dc1766

kgv

2022-03-31 12:24

developer   ~0107508

@mgerus

+    if (myLoc->SetCurve(myAdpPath))
+    {
+      TColGeom_SequenceOfCurve SeqC;

Please use early return:
> if (!myLoc->SetCurve(myAdpPath))
> {
> myStatus = GeomFill_ImpossibleContact;
> return;
> }
> ...

+ inline GeomFill_PipeError GeomFill_Pipe::GetStatus() const
+{
+  return myStatus;

Please move to .hxx header.

+tuyau t curve c1 c2
\ No newline at end of file

Please add newline at end of files.

   //! Returns whether approximation was done.
     Standard_Boolean IsDone() const;
+  //! Return status of execution
+    GeomFill_PipeError GetStatus() const;

Please put an empty line between methods.

   Standard_Boolean myPolynomial;
-
-
+  //! Execution status
+  GeomFill_PipeError myStatus;

Consider moving the field in front of Boolean flags for sorting fields by their size if this wouldn't break logical grouping.

+      // sequence des sections
...
+      // verification de l'orientation
...
+      // creation de la NSections

Consider fixing trivially-translatable French comments why you here.

- void GeomFill_ConstantBiNormal::SetCurve(const Handle(Adaptor3d_Curve)& C) 
+ Standard_Boolean GeomFill_ConstantBiNormal::SetCurve(const Handle(Adaptor3d_Curve)& C) 
 {
   GeomFill_TrihedronLaw::SetCurve(C);
     if (! C.IsNull()) { 
     frenet->SetCurve(C);
   }
+    return Standard_True;
 }

Please fix indentation.

@@ -40,7 +40,7 @@ class GeomFill_LocationLaw : public Standard_Transient
 public:  

-  Standard_EXPORT virtual void SetCurve (const Handle(Adaptor3d_Curve)& C) = 0;
+  Standard_EXPORT virtual Standard_Boolean SetCurve (const Handle(Adaptor3d_Curve)& C) = 0;

Please add description to the method, including newly added return value.

git

2022-03-31 15:04

administrator   ~0107514

Branch CR31361 has been updated by mgerus.

SHA-1: f6b3263582db2eb3341bc56fee56e542acea63a4


Detailed log of new commits:

Author: mgerus
Date: Thu Mar 31 15:04:31 2022 +0300

    Add fixes after review

git

2022-04-01 08:21

administrator   ~0107526

Branch CR31361 has been updated by mgerus.

SHA-1: adf5a4a59a42e2881c3bcf051e70a12e18daed07


Detailed log of new commits:

Author: mgerus
Date: Fri Apr 1 08:21:13 2022 +0300

    add fix for linux

kgv

2022-04-01 09:10

developer   ~0107531

@mgerus
 //==================================================================
 //Function: SetCurve
-//Purpose :
+//Purpose : return Standard_True
 //==================================================================

Documentation should be put into .hxx class header to appear in reference manual generated by Doxygen.
The legacy "Purpose:" topping could be left empty.

git

2022-04-01 09:32

administrator   ~0107532

Branch CR31361 has been updated by mgerus.

SHA-1: d7f21363507d63aecf388f9e58d21c1c1b337f5e


Detailed log of new commits:

Author: mgerus
Date: Fri Apr 1 09:32:50 2022 +0300

    add fix after review v2

kgv

2022-04-01 10:37

developer   ~0107536

-  
+  //! @return Standard_True in case if execution end correctly, owerwise return Standard_False
   Standard_EXPORT virtual Standard_Boolean SetCurve (const Handle(Adaptor3d_Curve)& C) = 0;

It is desired to also describe the purpose of this method and it's arguments, not just a return value.
`owerwise` - misprint.

git

2022-04-04 10:00

administrator   ~0107579

Branch CR31361 has been updated forcibly by mgerus.

SHA-1: 59596c99b7c34953865ad76a2596bd013669e8c3

kgv

2022-04-04 11:05

developer   ~0107582

Last edited: 2022-04-04 11:05

-//Purpose :
+//Purpose : .

Unrelated and incorrect change.

azv

2022-04-04 11:56

administrator   ~0107585

src/GeomFill/GeomFill_ConstantBiNormal.cxx:69-80
//==================================================================
//Function : SetCurve
//Purpose : 
//==================================================================
 Standard_Boolean GeomFill_ConstantBiNormal::SetCurve(const Handle(Adaptor3d_Curve)& C) 
{
  GeomFill_TrihedronLaw::SetCurve(C);
  if (! C.IsNull()) { 
    frenet->SetCurve(C);
  }
  return Standard_True;
}


No need to add pre-function comment. There is lack such comments in this file.

Return result of nested SetCurve operation instead of Standard_True.

src/GeomFill/GeomFill_ConstantBiNormal.hxx:48
  Standard_EXPORT virtual Standard_Boolean SetCurve (const Handle(Adaptor3d_Curve)& C) Standard_OVERRIDE;


Please, add the description similar to other places.

src/GeomFill/GeomFill_CorrectedFrenet.cxx:340 Fix indentation.

src/GeomFill/GeomFill_CorrectedFrenet.hxx:54 Please, add new line to separate comment from the previous method.

src/GeomFill/GeomFill_CurveAndTrihedron.hxx:56
  //! @return Standard_True

This function does not return true always. Please, update the return value similar to GeomFill_CorrectedFrenet::SetCurve.

src/GeomFill/GeomFill_DiscreteTrihedron.cxx:144
  return Standard_True;

Shouldn't we return here the result of GeomFill_TrihedronLaw::SetCurve(C) from line 122? Or should we return isSngl value instead?

src/GeomFill/GeomFill_LocationDraft.cxx:115. I suppose, the retult of myLaw->SetCurve(C) should be returned here.

src/GeomFill/GeomFill_LocationLaw.hxx:42. Add a description here.

src/GeomFill/GeomFill_Pipe.hxx:279
  // TODO constructors

Odd line. To be removed.

src/GeomFill/GeomFill_Pipe.lxx:74. No new line at the end of the file.

git

2022-04-04 14:24

administrator   ~0107591

Branch CR31361 has been updated by mgerus.

SHA-1: 9169b879141563d20ebfd75dbd20cbd1b1d1534b


Detailed log of new commits:

Author: mgerus
Date: Mon Apr 4 14:23:57 2022 +0300

    fixes after review

git

2022-04-05 08:09

administrator   ~0107616

Branch CR31361 has been updated by mgerus.

SHA-1: ce8e216c8d32e00492ef27ae0095292179e44de9


Detailed log of new commits:

Author: mgerus
Date: Tue Apr 5 08:09:02 2022 +0300

    fixes after review: remove tabs

git

2022-04-05 09:12

administrator   ~0107618

Branch CR31361 has been updated forcibly by azv.

SHA-1: ecc80341394c017f8990f239a0aa1af420e1c182

azv

2022-04-05 11:37

administrator   ~0107634

Branches to be integrated:
OCCT: CR31361
Products: NOT

smoskvin

2022-04-09 09:49

administrator   ~0107785

Combination -
OCCT branch : IR-2022-04-08
master SHA - 7021de2fe7a69d4c788ccf43b8b096dbcc8597c8
49e51745631c52b6c452c65adae4d6dfa21a1b1e
Products branch : IR-2022-04-08 SHA - e16d959d441765c483049307ba7293173532103a
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: 18452.87000000039 / 18490.94000000032 [-0.21%]
Products
Total CPU difference: 11761.310000000152 / 11719.920000000124 [+0.35%]
Windows-64-VC14:
OCCT
Total CPU difference: 20539.828125 / 20552.71875 [-0.06%]
Products
Total CPU difference: 13248.21875 / 13242.609375 [+0.04%]


Image differences :
No differences that require special attention

Memory differences :
No differences that require special attention

git

2022-04-10 10:49

administrator   ~0107813

Branch CR31361 has been deleted by mnt.

SHA-1: ecc80341394c017f8990f239a0aa1af420e1c182

kgv

2022-07-20 15:02

developer   ~0109896

@azv,

This patch breaks public API and ABI, I suggest withdrawing this patch from 7.6.3 branch.

Related Changesets

occt: master b2ec2f5d

2022-04-04 10:00:24

mgerus


Committer: smoskvin Details Diff
0031361: Modeling algorithms - An exception arise when building tube

* Add status flag to GeomFill_Pipe
* Add myIsPerform flag to GeomFill_Pipe
* Add checking vectors in GeomFill_CorrectedFrenet
* Add boolean return to SetCurve virtual methods
* Add test case
Affected Issues
0031361
mod - src/GeometryTest/GeometryTest_SurfaceCommands.cxx Diff File
mod - src/GeomFill/GeomFill_ConstantBiNormal.cxx Diff File
mod - src/GeomFill/GeomFill_ConstantBiNormal.hxx Diff File
mod - src/GeomFill/GeomFill_CorrectedFrenet.cxx Diff File
mod - src/GeomFill/GeomFill_CorrectedFrenet.hxx Diff File
mod - src/GeomFill/GeomFill_CurveAndTrihedron.cxx Diff File
mod - src/GeomFill/GeomFill_CurveAndTrihedron.hxx Diff File
mod - src/GeomFill/GeomFill_DiscreteTrihedron.cxx Diff File
mod - src/GeomFill/GeomFill_DiscreteTrihedron.hxx Diff File
mod - src/GeomFill/GeomFill_Frenet.cxx Diff File
mod - src/GeomFill/GeomFill_Frenet.hxx Diff File
mod - src/GeomFill/GeomFill_GuideTrihedronAC.cxx Diff File
mod - src/GeomFill/GeomFill_GuideTrihedronAC.hxx Diff File
mod - src/GeomFill/GeomFill_GuideTrihedronPlan.cxx Diff File
mod - src/GeomFill/GeomFill_GuideTrihedronPlan.hxx Diff File
mod - src/GeomFill/GeomFill_LocationDraft.cxx Diff File
mod - src/GeomFill/GeomFill_LocationDraft.hxx Diff File
mod - src/GeomFill/GeomFill_LocationGuide.cxx Diff File
mod - src/GeomFill/GeomFill_LocationGuide.hxx Diff File
mod - src/GeomFill/GeomFill_LocationLaw.hxx Diff File
mod - src/GeomFill/GeomFill_Pipe.cxx Diff File
mod - src/GeomFill/GeomFill_Pipe.hxx Diff File
mod - src/GeomFill/GeomFill_Pipe.lxx Diff File
mod - src/GeomFill/GeomFill_TrihedronLaw.cxx Diff File
mod - src/GeomFill/GeomFill_TrihedronLaw.hxx Diff File
add - tests/bugs/modalg_7/bug31361 Diff File

Issue History

Date Modified Username Field Change
2020-02-07 13:31 akaftasev New Issue
2020-02-07 13:31 akaftasev Assigned To => msv
2020-09-15 12:34 msv Note Added: 0094821
2020-09-15 12:34 msv Target Version 7.5.0 => 7.6.0
2021-08-29 18:53 msv Target Version 7.6.0 => 7.7.0
2022-02-24 14:18 azv Assigned To msv => mgerus
2022-02-24 14:18 azv Status new => assigned
2022-02-25 10:43 git Note Added: 0107061
2022-02-25 12:48 git Note Added: 0107063
2022-03-01 10:39 git Note Added: 0107115
2022-03-14 16:37 git Note Added: 0107281
2022-03-15 09:34 git Note Added: 0107288
2022-03-16 13:27 git Note Added: 0107303
2022-03-17 10:14 git Note Added: 0107316
2022-03-17 14:18 mgerus Assigned To mgerus => azv
2022-03-17 14:18 mgerus Status assigned => resolved
2022-03-17 14:18 mgerus Note Added: 0107326
2022-03-21 14:03 azv Assigned To azv => mgerus
2022-03-21 14:03 azv Status resolved => assigned
2022-03-21 14:03 azv Note Added: 0107375
2022-03-21 14:04 azv Note Edited: 0107375
2022-03-24 10:59 git Note Added: 0107411
2022-03-24 11:13 kgv Summary Modeling algorithms - An exception arise when building tube => Modeling algorithms, GeomFill_Pipe - an exception arises when building tube
2022-03-24 11:13 kgv Steps to Reproduce Updated
2022-03-28 08:33 git Note Added: 0107441
2022-03-28 08:33 mgerus Assigned To mgerus => azv
2022-03-28 08:33 mgerus Status assigned => resolved
2022-03-28 14:11 kgv Note Added: 0107446
2022-03-28 15:03 git Note Added: 0107447
2022-03-28 23:26 azv Assigned To azv => mgerus
2022-03-28 23:26 azv Status resolved => assigned
2022-03-28 23:26 azv Note Added: 0107454
2022-03-29 09:19 git Note Added: 0107457
2022-03-31 08:43 mgerus Assigned To mgerus => azv
2022-03-31 08:43 mgerus Status assigned => resolved
2022-03-31 10:16 git Note Added: 0107502
2022-03-31 12:24 kgv Note Added: 0107508
2022-03-31 12:47 azv Assigned To azv => mgerus
2022-03-31 12:47 azv Status resolved => assigned
2022-03-31 15:04 git Note Added: 0107514
2022-04-01 08:21 git Note Added: 0107526
2022-04-01 09:10 kgv Note Added: 0107531
2022-04-01 09:32 git Note Added: 0107532
2022-04-01 10:37 kgv Note Added: 0107536
2022-04-04 10:00 git Note Added: 0107579
2022-04-04 10:01 mgerus Assigned To mgerus => azv
2022-04-04 10:02 mgerus Status assigned => resolved
2022-04-04 11:05 kgv Note Added: 0107582
2022-04-04 11:05 kgv Note Edited: 0107582
2022-04-04 11:56 azv Assigned To azv => mgerus
2022-04-04 11:56 azv Status resolved => assigned
2022-04-04 11:56 azv Note Added: 0107585
2022-04-04 14:24 git Note Added: 0107591
2022-04-04 14:24 mgerus Assigned To mgerus => azv
2022-04-05 08:09 git Note Added: 0107616
2022-04-05 09:12 git Note Added: 0107618
2022-04-05 11:36 azv Status assigned => resolved
2022-04-05 11:37 azv Assigned To azv => bugmaster
2022-04-05 11:37 azv Status resolved => reviewed
2022-04-05 11:37 azv Note Added: 0107634
2022-04-09 09:49 smoskvin Status reviewed => tested
2022-04-09 09:49 smoskvin Note Added: 0107785
2022-04-09 09:52 smoskvin Test case number => bugs/modalg_7/bug31361
2022-04-10 10:42 smoskvin Changeset attached => occt master b2ec2f5d
2022-04-10 10:42 mgerus Assigned To bugmaster => mgerus
2022-04-10 10:42 mgerus Status tested => verified
2022-04-10 10:42 mgerus Resolution open => fixed
2022-04-10 10:49 git Note Added: 0107813
2022-06-19 15:04 azv Target Version 7.7.0 => 7.6.3
2022-07-20 15:02 kgv Note Added: 0109896
2022-07-20 15:02 kgv Target Version 7.6.3 => 7.7.0
2023-03-19 20:09 vglukhik Status verified => closed
2023-03-19 20:09 vglukhik Fixed in Version => 7.6.3