View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0031361 | Open CASCADE | OCCT:Modeling Algorithms | public | 2020-02-07 13:31 | 2023-03-19 20:09 |
Reporter | akaftasev | Assigned To | |||
Priority | normal | Severity | minor | ||
Status | closed | Resolution | fixed | ||
Platform | Windows | OS | VC++ 2013 | ||
Product Version | 7.4.0 | ||||
Target Version | 7.7.0 | Fixed in Version | 7.6.3 | ||
Summary | 0031361: Modeling algorithms, GeomFill_Pipe - an exception arises when building tube | ||||
Description | When 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 | ||||
Tags | No tags attached. | ||||
Test case number | bugs/modalg_7/bug31361 | ||||
|
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. |
|
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 |
|
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 |
|
Branch CR31361 has been updated forcibly by mgerus. SHA-1: e8ca2d7ec76f6964f7f3d6a8cdc1302ff2e30688 |
|
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 |
|
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 |
|
Branch CR31361 has been updated forcibly by mgerus. SHA-1: 52e7ecf2b0633deeeaa6fb730684377be6343a2d |
|
Branch CR31361 has been updated forcibly by mgerus. SHA-1: 443634fa2317a33eceb0d7107781feecaa663190 |
|
Please review the branch CR31361. http://jenkins-test-occt/view/CR31361-master-MGERUS/view/COMPARE/ Branches for Integration: OCCT - CR31361 Products - Not |
|
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. |
|
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 |
|
Branch CR31361 has been updated forcibly by mgerus. SHA-1: e99ade9bd15c32766eaa14534c91c8365729c232 |
|
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. |
|
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 |
|
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? |
|
Branch CR31361 has been updated forcibly by mgerus. SHA-1: 510083c77ddba617c56e060cd04cc3da9d638144 |
|
Branch CR31361 has been updated forcibly by mgerus. SHA-1: 57193d45f93869b531aa184739d6e6e6e5dc1766 |
|
+ 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. |
|
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 |
|
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 |
|
//================================================================== //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. |
|
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 |
|
- + //! @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. |
|
Branch CR31361 has been updated forcibly by mgerus. SHA-1: 59596c99b7c34953865ad76a2596bd013669e8c3 |
|
-//Purpose : +//Purpose : . Unrelated and incorrect change. |
|
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. |
|
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 |
|
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 |
|
Branch CR31361 has been updated forcibly by azv. SHA-1: ecc80341394c017f8990f239a0aa1af420e1c182 |
|
Branches to be integrated: OCCT: CR31361 Products: NOT |
|
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 |
|
Branch CR31361 has been deleted by mnt. SHA-1: ecc80341394c017f8990f239a0aa1af420e1c182 |
|
This patch breaks public API and ABI, I suggest withdrawing this patch from 7.6.3 branch. |
occt: master b2ec2f5d 2022-04-04 10:00:24
Committer: |
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 |
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 |
|
Note Added: 0094821 | |
2020-09-15 12:34 |
|
Target Version | 7.5.0 => 7.6.0 |
2021-08-29 18:53 |
|
Target Version | 7.6.0 => 7.7.0 |
2022-02-24 14:18 |
|
Assigned To | msv => mgerus |
2022-02-24 14:18 |
|
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 |
|
Assigned To | mgerus => azv |
2022-03-17 14:18 |
|
Status | assigned => resolved |
2022-03-17 14:18 |
|
Note Added: 0107326 | |
2022-03-21 14:03 |
|
Assigned To | azv => mgerus |
2022-03-21 14:03 |
|
Status | resolved => assigned |
2022-03-21 14:03 |
|
Note Added: 0107375 | |
2022-03-21 14:04 |
|
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 |
|
Assigned To | mgerus => azv |
2022-03-28 08:33 |
|
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 |
|
Assigned To | azv => mgerus |
2022-03-28 23:26 |
|
Status | resolved => assigned |
2022-03-28 23:26 |
|
Note Added: 0107454 | |
2022-03-29 09:19 | git | Note Added: 0107457 | |
2022-03-31 08:43 |
|
Assigned To | mgerus => azv |
2022-03-31 08:43 |
|
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 |
|
Assigned To | azv => mgerus |
2022-03-31 12:47 |
|
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 |
|
Assigned To | mgerus => azv |
2022-04-04 10:02 |
|
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 |
|
Assigned To | azv => mgerus |
2022-04-04 11:56 |
|
Status | resolved => assigned |
2022-04-04 11:56 |
|
Note Added: 0107585 | |
2022-04-04 14:24 | git | Note Added: 0107591 | |
2022-04-04 14:24 |
|
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 |
|
Status | assigned => resolved |
2022-04-05 11:37 |
|
Assigned To | azv => bugmaster |
2022-04-05 11:37 |
|
Status | resolved => reviewed |
2022-04-05 11:37 |
|
Note Added: 0107634 | |
2022-04-09 09:49 |
|
Status | reviewed => tested |
2022-04-09 09:49 |
|
Note Added: 0107785 | |
2022-04-09 09:52 |
|
Test case number | => bugs/modalg_7/bug31361 |
2022-04-10 10:42 |
|
Changeset attached | => occt master b2ec2f5d |
2022-04-10 10:42 |
|
Assigned To | bugmaster => mgerus |
2022-04-10 10:42 |
|
Status | tested => verified |
2022-04-10 10:42 |
|
Resolution | open => fixed |
2022-04-10 10:49 | git | Note Added: 0107813 | |
2022-06-19 15:04 |
|
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 |