View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0023638 | Community | OCCT:Data Exchange | public | 2012-12-13 11:25 | 2024-12-12 13:24 |
Reporter | Assigned To | dpasukhi | |||
Priority | normal | Severity | minor | ||
Status | assigned | Resolution | reopened | ||
Product Version | 6.5.4 | ||||
Target Version | 7.8.0 | ||||
Summary | 0023638: Data Exchange - Reading IGES file produced invalid shape | ||||
Description | This problem is reported by ChengShengXiong (via Contact Form). The enclosed IGES file, when read in DRAW with default parameters, produces invalid shape. Setting read.surfacecurve.mode to -3 allows to lower tolerance, however do not remove invalidity. | ||||
Steps To Reproduce | pload MODELING DATAEXCHANGE # the following command is used in this case because it allows to lower the tolerance after import param read.surfacecurve.mode -3 igesread d:/cadbad.igs a * test bugs iges bug23638 | ||||
Tags | No tags attached. | ||||
Test case number | bugs iges bug23638 | ||||
2012-12-17 17:14 manager |
cadbad.zip (2,393,169 bytes) |
|
Branch CR23638 has been created by mkv. SHA-1: f939c7fa1ab7539835db389e2e7f5023e6a8a981 Detailed log of new commits: Author: mkv Date: Mon Jul 24 18:50:27 2017 +0300 Test for 0023638: Reading IGES file produced invalid shape |
|
Dear BugMaster, problem described in issue is reproduced on current state of OCCT. |
|
Branch CR23638 has been updated by apv. SHA-1: 0b31c7e9a1dfc0085b876c1d90dc593d685dd9db Detailed log of new commits: Author: apv Date: Fri Jul 28 10:19:54 2017 +0300 Test case adjustment |
|
Branch CR23638 has been updated forcibly by DenisOrlov. SHA-1: 2d6281a32e9d637d9b69bb786827e70808f60570 |
|
Branch CR23638 has been updated forcibly by DenisOrlov. SHA-1: 7d96f4e0d4070d12f8e52d0ccc14a46b52c61453 |
|
Branch CR23638 has been updated forcibly by DenisOrlov. SHA-1: d2f9004dea10d936abe3fffce0e929d1613e1921 |
|
Branch CR23638 has been updated forcibly by DenisOrlov. SHA-1: 537223925f9438f57e57a19bf6bbe14b6202f010 |
|
Branch CR23638 has been updated forcibly by DenisOrlov. SHA-1: 9600589d4816a63ac72c233d88f46c777565744e |
|
Branch CR23638 has been updated forcibly by DenisOrlov. SHA-1: a91875b65ee92ec5a57677899597de256702a29b |
|
Dear @DenisOrlov src/IGESData/IGESData_IGESModel.hxx Please remove Standard_EXPORT for inline methods. //! Returns ReShape fixed on previous step Standard_EXPORT Handle(ShapeBuild_ReShape) ReShape() { return myReShape; }; src/IGESToBRep/IGESToBRep_CurveAndSurface.cxx Do not comment useless code Handle(TCollection_HAsciiString) label = GetModel()->StringLabel(start); // Handle(TCollection_HAsciiString) aLabel = GetModel()->StringLabel(start); Moreover, I can't fine any normal changes on this file. It looks line a just no full code staly update. Please, if you want update code for file, that not impact on your task, create new task for it and make full style rework. src/ShapeAlgo/ShapeAlgo_ToolContainer.cxx Noy impacted changes src/ShapeAlgo/ShapeAlgo_ToolContainer.hxx Not impacted changes src/ShapeFix/ShapeFix_Shape.hxx Not impacted changes src/XSAlgo/XSAlgo_AlgoContainer.hxx Please use predefined definition. Not include for handle #include <ShapeBuild_ReShape.hxx> If you start to update style, do it fully. Spaces, names. But I want to see a new method, that use just one additional method. Not default and not nullptr. You can desigh methods to call one from one with just creating ReShape new object. Standard_EXPORT virtual TopoDS_Shape ProcessShape ( const TopoDS_Shape& shape, const Standard_Real Prec, const Standard_Real MaxTol, const Standard_CString rscfile, const Standard_CString seq, Handle(Standard_Transient)& info, const TopoDS_Shape& theShape, const Standard_Real thePrec, const Standard_Real theMaxTol, const Standard_CString thePrscfile, const Standard_CString thePseq, Handle(Standard_Transient)& theInfo, const Message_ProgressRange& theProgress = Message_ProgressRange(), const Standard_Boolean NonManifold = Standard_False) const; const Standard_Boolean theNonManifold = Standard_False, const Handle(ShapeBuild_ReShape)& theReShape = nullptr) const; src/XSAlgo/XSAlgo_AlgoContainer.cxx If you start to update one method, make it fully. |
|
Branch CR23638 has been updated forcibly by DenisOrlov. SHA-1: 44753060f8593659dbec814563ab7668ad57e1bb |
|
Dear ika, please review: OCCT: CR23638 PROD: NO Most of builds are ok, http://jenkins-test-10.nnov.opencascade.com/view/CR23638-master-DenisOrlov/view/PRODUCTS%20compile/ Tests relevant to the bug are passed. http://jenkins-test-10.nnov.opencascade.com/view/CR23638-master-DenisOrlov/view/COMPARE/ I Removed double healing of Iges group entities. |
|
Dear @DenisOrlov There a few remarks, please check src/IGESData/IGESData_IGESModel.hxx Not clear, what step is means? - + //! Returns ReShape fixed on previous step + Handle(ShapeBuild_ReShape) ReShape() { return myReShape; }; src/IGESToBRep/IGESToBRep_Actor.cxx Why you change code style? - shape = XSAlgo::AlgoContainer()->ProcessShape( shape, theeps, CAS.GetMaxTol(), - "read.iges.resource.name", - "read.iges.sequence", info, - aPS.Next()); + shape = XSAlgo::AlgoContainer()->ProcessShape( + shape, theeps, CAS.GetMaxTol(), + "read.iges.resource.name", "read.iges.sequence", + info, mymodel->ReShape(), aPS.Next()); src/IGESToBRep/IGESToBRep_CurveAndSurface.cxx I can't find any changes. Please revert or upgrade code style on each line that you update(not recommended). src/ShapeFix/ShapeFix_Shape.cxx Please put simple checking on the first position. - if(myMapFixingShape.Contains(aShapeNullLoc)) { - myShape.Location(L, Standard_False); + if( myMapFixingShape.Contains( aShapeNullLoc ) || isRecorded ) { + myShape.Location( L, Standard_False ); Please make const + Standard_Boolean isRecorded = Context()->IsNewShape(myShape); src/XSAlgo/XSAlgo_AlgoContainer.cxx You try to update code style. But not fully. Revert changes and apply only needed changes or make fully rework of tached methods. Why you split first parameter on the next line? + return ProcessShape( + theShape, thePrec, theMaxTol, thePrscfile, + thePseq, theInfo, aReShape, theProgress, theNonManifold); src/XSAlgo/XSAlgo_AlgoContainer.hxx Where is any description? + + Standard_EXPORT virtual TopoDS_Shape ProcessShape( + const TopoDS_Shape& theShape, const Standard_Real thePrec, const Standard_Real theMaxTol, + const Standard_CString thePrscfile, const Standard_CString thePseq, Handle(Standard_Transient)& theInfo, + const Handle(ShapeBuild_ReShape)& theReShape, + const Message_ProgressRange& theProgress = Message_ProgressRange(), + const Standard_Boolean theNonManifold = Standard_False) const; tests/bugs/iges/bug23638 Why you remove this? -param read.surfacecurve.mode -3 Why you chose 22487? TODO OCC11111, means that this bug should be fixed in the 11111 If it should be fixed put OCC00000 +puts "TODO OCC22487 ALL: Faulty shapes in variables faulty_1 to faulty_1" |
|
An addition remark, Please update summary of the commit description, you forgot "Data Exchange" 0023638: Data Exchange - Reading IGES file produced invalid shape |
|
Branch CR23638 has been updated forcibly by DenisOrlov. SHA-1: e81bb6021eb844e4c22106a95a385f4018ec1a21 |
|
param read.surfacecurve.mode -3 forces opencascade not to read 2d curves from a source file, but create it from the edges instead, most of time it works well, but in very specific cases, when a surface is created from a revolved curve and the edge is very close to the center of rotation, the resulting 2d curve is wrong. And that's the case of the current bug. So, the main recommendation here is not to set this parameter to -3. |
|
Dear @DenisOrlov All remarks for the src/XSAlgo/XSAlgo_AlgoContainer.cxx are not done. Please check. For all updated line you are an author. So you need to update names and code style. src/IGESData/IGESData_IGESModel.hxx There the general name is Gets. I think it will be better. And a desctiption are not clearly. And we need to add set, I think. For example, (it is just recommnedation) const Handle(ShapeBuild_ReShape)& ReShape() const { return } void SetReShape(const Handle(ShapeBuild_ReShape)& theReShape) { = } @@ -151,8 +151,8 @@ public: //! i.e. a string "Dnn" with nn = directory entry number (2*N-1) Standard_EXPORT Handle(TCollection_HAsciiString) StringLabel (const Handle(Standard_Transient)& ent) const Standard_OVERRIDE; - - + //! Returns ReShape fixed on previous step of healing + Handle(ShapeBuild_ReShape) ReShape() { return myReShape; }; |
|
Branch CR23638 has been updated forcibly by DenisOrlov. SHA-1: 5d0fbfcb7e32ee3ad817df4a01e168d2967aee53 |
|
Branch CR23638 has been updated forcibly by DenisOrlov. SHA-1: e885cebdbad70a6428424edac2eedd4706caa26b |
|
Dear @dorlov Please push the next commit in the new commit (do not make combining). Combine to one commit is necessary to integration. src/IGESData/IGESData_IGESModel.cxx ClearHeader should clear ReShape src/IGESData/IGESData_IGESModel.hxx //! Gets ReShape used to store a history of changes of the model's shapes const Handle(ShapeBuild_ReShape) ReShape() const { return myReShape; }; ->>>> Update comment and add & to return value and removing ; in the end //! Gets ReShape used to store a model's shapes changes const Handle(ShapeBuild_ReShape)& ReShape() const { return myReShape; } Please remove ; on the end //! Sets ReShape used to store a history of changes of the model's shapes void SetReShape(const Handle(ShapeBuild_ReShape)& theReShape) { myReShape = theReShape; }; src/IGESToBRep/IGESToBRep_Actor.cxx Please remove dummy spaces on the line end shape = XSAlgo::AlgoContainer()->ProcessShape(shape, theeps, CAS.GetMaxTol(), "read.iges.resource.name", "read.iges.sequence", info, mymodel->ReShape(), aPS.Next()); src/ShapeFix/ShapeFix_Shape.cxx Update code style, remove dummy space after ( and before ) if(isRecorded || myMapFixingShape.Contains(aShapeNullLoc)) { myShape.Location( L, Standard_False ); Please update name starting with aIsRecorded const Standard_Boolean isRecorded = Context()->IsNewShape(myShape); src/XSAlgo/XSAlgo_AlgoContainer.hxx I think we need to update styling of parameters for ProcessShape method. So, we need to separete each parameter on own line like this: https://dev.opencascade.org/doc/occt-7.6.0/overview/html/occt_contribution__coding_rules.html#autotoc_md359 It is just an example. After that we need to update documentation according this https://dev.opencascade.org/doc/occt-7.6.0/overview/html/occt_contribution__coding_rules.html#occt_coding_rules_4 In this case please describe parameter a little detailed. src/XSAlgo/XSAlgo_AlgoContainer.cxx Please split oneline statement to new line ( the best way to use {}) { if (theShape.IsNull()) return theShape; Handle(ShapeProcess_ShapeContext) aContext = Handle(ShapeProcess_ShapeContext)::DownCast(theInfo); Any variables within #ifdef directives are not updated #ifdef OCCT_DEBUG { static Standard_Integer time = 0; if (!time) std::cout << "Warning: XSAlgo_AlgoContainer::ProcessShape(): Sequence " << str.ToCString() << " is not defined in " << prscfile << " resource; do default processing" << std::endl; time++; } #endif Please remove dummy space after //purpose : (in your new method only) //======================================================================= //function : ProcessShape //purpose : //======================================================================= |
|
Branch CR23638 has been updated forcibly by DenisOrlov. SHA-1: a33f32db848e1bd06117fa218e78ea051bc8b348 |
|
Please create new commuit and push into the same branch If you think that all should be fine, then create combined commit and push into CR23638_1 src/ShapeFix/ShapeFix_Shape.cxx Please move { to the new line + if(aIsRecorded || myMapFixingShape.Contains(aShapeNullLoc)) { + myShape.Location(L, Standard_False); src/XSAlgo/XSAlgo_AlgoContainer.hxx It is not necessary to make the same column position for each argument, it is up to developer. As for me, i never do this, But it up to you + Standard_EXPORT virtual TopoDS_Shape ProcessShape(const TopoDS_Shape& theShape, + const Standard_Real thePrec, + const Standard_Real theMaxTol, + const Standard_CString thePrscfile, + const Standard_CString thePseq, + Handle(Standard_Transient)& theInfo, + const Handle(ShapeBuild_ReShape)& theReShape, + const Message_ProgressRange& theProgress = Message_ProgressRange(), + const Standard_Boolean theNonManifold = Standard_False) const; Please make the same doxygen update for the description old method ProcessShape + Standard_EXPORT virtual TopoDS_Shape ProcessShape (const TopoDS_Shape& theShape, + const Standard_Real thePrec, + const Standard_Real theMaxTol, + const Standard_CString thePrscfile, + const Standard_CString thePseq, + Handle(Standard_Transient)& theInfo, + const Message_ProgressRange& theProgress = Message_ProgressRange(), + const Standard_Boolean theNonManifold = Standard_False) const; |
|
Branch CR23638 has been updated by DenisOrlov. SHA-1: 1770122f5529c08cf9805d2b105631c1127a1157 Detailed log of new commits: Author: dorlov Date: Fri Mar 24 14:37:47 2023 +0000 0023638: Data Exchange - Reading IGES file produced invalid shape Applied minor changes in parameter's description and code formatting |
|
Branch CR23638_1 has been created by DenisOrlov. SHA-1: 51ef3120d6fbce5e858c421b631bdebd1faa3225 Detailed log of new commits: Author: dorlov Date: Fri Jan 13 13:25:17 2023 +0000 0023638: Data Exchange - Reading IGES file produced invalid shape Removed double healing of Iges group entities Added ShapeBuild_ReShape member to the IGESData_IGESModel class, shapes which are registered in ShapeBuild_ReShape class does not process to healing |
|
Dear dpasukhi, please review: OCCT: CR23638_1 PROD: NO Most of builds are ok, http://jenkins-test-10.nnov.opencascade.com/view/CR23638_1-master-DenisOrlov/view/OCCT%20compile/ Tests relevant to the bug are passed. http://jenkins-test-10.nnov.opencascade.com/view/CR23638_1-master-DenisOrlov/view/COMPARE/ I Removed double healing of Iges group entities. |
|
Dear bugmaster, Please integrate: OCCT: CR23638_1 PROD: NO All tests are OK, no regressions. |
|
Combination - OCCT branch : IR-2023-03-31 Products branch : master 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: Ubuntu2004-64: OCCT Total CPU difference: 18292.42000000035 / 17861.320000000374 [+2.41%] Products Total CPU difference: 12038.590000000031 / 11917.870000000039 [+1.01%] Windows-64-VC142: OCCT Total CPU difference: 20849.640625 / 20315.171875 [+2.63%] Products Total CPU difference: 14197.515625 / 13790.3125 [+2.95%] Image differences : No differences that require special attention Memory differences : No differences that require special attention |
|
Branch CR23638 has been deleted by mnt. SHA-1: 1770122f5529c08cf9805d2b105631c1127a1157 |
|
Branch CR23638_1 has been deleted by mnt. SHA-1: 51ef3120d6fbce5e858c421b631bdebd1faa3225 |
|
Reminder sent to: dkulikov Fix is not working. Must be reverted |
|
Revert PR: https://github.com/Open-Cascade-SAS/OCCT/pull/186 |
occt: master c479c4f6 2023-01-13 13:25:17 dorlov Committer: vglukhik Details Diff |
0023638: Data Exchange - Reading IGES file produced invalid shape Removed double healing of Iges group entities Added ShapeBuild_ReShape member to the IGESData_IGESModel class, shapes which are registered in ShapeBuild_ReShape class does not process to healing |
Affected Issues 0023638 |
|
mod - src/IGESData/IGESData_IGESModel.cxx | Diff File | ||
mod - src/IGESData/IGESData_IGESModel.hxx | Diff File | ||
mod - src/IGESToBRep/IGESToBRep_Actor.cxx | Diff File | ||
mod - src/ShapeFix/ShapeFix_Shape.cxx | Diff File | ||
mod - src/XSAlgo/XSAlgo_AlgoContainer.cxx | Diff File | ||
mod - src/XSAlgo/XSAlgo_AlgoContainer.hxx | Diff File | ||
mod - tests/bugs/iges/bug23638 | Diff File |
Date Modified | Username | Field | Change |
---|---|---|---|
2012-12-13 11:25 |
|
New Issue | |
2012-12-13 11:25 |
|
Assigned To | => gka |
2012-12-17 17:14 |
|
File Added: cadbad.zip | |
2017-07-24 18:50 | git | Note Added: 0068647 | |
2017-07-24 18:53 |
|
Test case number | => bugs iges bug23638 |
2017-07-27 18:15 |
|
Note Added: 0068812 | |
2017-07-28 10:20 | git | Note Added: 0068826 | |
2022-12-07 12:20 | ika | Assigned To | gka => DenisOrlov |
2023-01-13 16:25 | git | Note Added: 0112799 | |
2023-01-19 12:25 | git | Note Added: 0112882 | |
2023-01-19 13:20 | dpasukhi | Summary | Reading IGES file produced invalid shape => Data Exchange, Iges Import - Reading IGES file produced invalid shape |
2023-01-19 13:20 | dpasukhi | Summary | Data Exchange, Iges Import - Reading IGES file produced invalid shape => Data Exchange - Reading IGES file produced invalid shape |
2023-01-24 19:11 | ebelouso | Target Version | => 7.7.1 |
2023-01-25 12:57 | git | Note Added: 0112984 | |
2023-02-06 14:28 | git | Note Added: 0113083 | |
2023-02-10 19:29 | git | Note Added: 0113113 | |
2023-02-14 18:34 | git | Note Added: 0113138 | |
2023-02-14 18:41 | dpasukhi | Note Added: 0113139 | |
2023-02-16 16:22 | git | Note Added: 0113155 | |
2023-02-23 11:59 | DenisOrlov | Assigned To | DenisOrlov => ika |
2023-02-23 11:59 | DenisOrlov | Status | new => resolved |
2023-02-23 11:59 | DenisOrlov | Steps to Reproduce Updated | |
2023-02-23 11:59 | DenisOrlov | Note Added: 0113197 | |
2023-02-23 12:03 | dpasukhi | Assigned To | ika => dpasukhi |
2023-02-23 12:20 | dpasukhi | Note Added: 0113198 | |
2023-02-23 12:20 | dpasukhi | Assigned To | dpasukhi => DenisOrlov |
2023-02-23 12:20 | dpasukhi | Status | resolved => assigned |
2023-02-23 13:37 | dpasukhi | Note Added: 0113200 | |
2023-02-27 15:06 | git | Note Added: 0113206 | |
2023-02-27 15:24 | DenisOrlov | Assigned To | DenisOrlov => ika |
2023-02-27 15:24 | DenisOrlov | Status | assigned => resolved |
2023-02-27 15:24 | DenisOrlov | Note Added: 0113207 | |
2023-02-27 16:49 | dpasukhi | Note Added: 0113209 | |
2023-02-27 16:52 | dpasukhi | Note Edited: 0113209 | |
2023-02-27 16:53 | dpasukhi | Note Edited: 0113209 | |
2023-02-27 17:12 | git | Note Added: 0113210 | |
2023-02-27 17:46 | dpasukhi | Relationship added | related to 0033338 |
2023-02-27 19:10 | git | Note Added: 0113212 | |
2023-03-06 18:55 | ebelouso | Target Version | 7.7.1 => 7.8.0 |
2023-03-14 15:42 | ika | Assigned To | ika => dpasukhi |
2023-03-21 22:08 | dpasukhi | Assigned To | dpasukhi => DenisOrlov |
2023-03-21 22:08 | dpasukhi | Status | resolved => assigned |
2023-03-21 22:08 | dpasukhi | Note Added: 0113309 | |
2023-03-22 14:32 | git | Note Added: 0113312 | |
2023-03-22 14:34 | DenisOrlov | Assigned To | DenisOrlov => ika |
2023-03-22 14:34 | DenisOrlov | Status | assigned => resolved |
2023-03-22 14:34 | DenisOrlov | Assigned To | ika => dpasukhi |
2023-03-22 15:33 | dpasukhi | Assigned To | dpasukhi => DenisOrlov |
2023-03-22 15:33 | dpasukhi | Status | resolved => assigned |
2023-03-22 15:33 | dpasukhi | Note Added: 0113314 | |
2023-03-24 17:38 | git | Note Added: 0113324 | |
2023-03-24 17:42 | git | Note Added: 0113325 | |
2023-03-24 17:42 | DenisOrlov | Assigned To | DenisOrlov => ika |
2023-03-24 17:42 | DenisOrlov | Status | assigned => resolved |
2023-03-24 17:43 | DenisOrlov | Assigned To | ika => dpasukhi |
2023-03-27 11:50 | DenisOrlov | Note Added: 0113331 | |
2023-03-27 12:39 | dpasukhi | Assigned To | dpasukhi => bugmaster |
2023-03-27 12:39 | dpasukhi | Status | resolved => reviewed |
2023-03-27 12:39 | dpasukhi | Note Added: 0113333 | |
2023-04-02 15:53 | vglukhik | Note Added: 0113348 | |
2023-04-02 15:58 | vglukhik | Changeset attached | => occt master c479c4f6 |
2023-04-02 15:58 | vglukhik | Assigned To | bugmaster => vglukhik |
2023-04-02 15:58 | vglukhik | Status | reviewed => verified |
2023-04-02 15:58 | vglukhik | Resolution | open => fixed |
2023-04-12 20:16 | git | Note Added: 0113378 | |
2023-04-12 20:16 | git | Note Added: 0113379 | |
2024-12-11 21:07 | dpasukhi | Assigned To | vglukhik => dpasukhi |
2024-12-11 21:07 | dpasukhi | Status | verified => feedback |
2024-12-11 21:07 | dpasukhi | Resolution | fixed => reopened |
2024-12-11 21:09 | dpasukhi | Status | feedback => assigned |
2024-12-11 21:10 | dpasukhi | Note Added: 0116896 | |
2024-12-12 13:24 | dpasukhi | Note Added: 0116912 |