View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0023638CommunityOCCT:Data Exchangepublic2012-12-13 11:252023-04-12 20:16
ReporterszvAssigned Tovglukhik  
PrioritynormalSeverityminor 
Status verifiedResolutionfixed 
Product Version6.5.4 
Target Version7.8.0 
Summary0023638: Data Exchange - Reading IGES file produced invalid shape
DescriptionThis 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 Reproducepload 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
TagsNo tags attached.
Test case numberbugs iges bug23638

Attached Files

  • cadbad.zip (2,393,169 bytes)

Relationships

related to 0033338 assigneddpasukhi Open CASCADE Coding - IGESToBRep_CurveAndSurface's method has unreachable code 

Activities

szv

2012-12-17 17:14

manager  

cadbad.zip (2,393,169 bytes)

git

2017-07-24 18:50

administrator   ~0068647

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

mkv

2017-07-27 18:15

tester   ~0068812

Dear BugMaster,
problem described in issue is reproduced on current state of OCCT.

git

2017-07-28 10:20

administrator   ~0068826

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

git

2023-01-13 16:25

administrator   ~0112799

Branch CR23638 has been updated forcibly by DenisOrlov.

SHA-1: 2d6281a32e9d637d9b69bb786827e70808f60570

git

2023-01-19 12:25

administrator   ~0112882

Branch CR23638 has been updated forcibly by DenisOrlov.

SHA-1: 7d96f4e0d4070d12f8e52d0ccc14a46b52c61453

git

2023-01-25 12:57

administrator   ~0112984

Branch CR23638 has been updated forcibly by DenisOrlov.

SHA-1: d2f9004dea10d936abe3fffce0e929d1613e1921

git

2023-02-06 14:28

administrator   ~0113083

Branch CR23638 has been updated forcibly by DenisOrlov.

SHA-1: 537223925f9438f57e57a19bf6bbe14b6202f010

git

2023-02-10 19:29

administrator   ~0113113

Branch CR23638 has been updated forcibly by DenisOrlov.

SHA-1: 9600589d4816a63ac72c233d88f46c777565744e

git

2023-02-14 18:34

administrator   ~0113138

Branch CR23638 has been updated forcibly by DenisOrlov.

SHA-1: a91875b65ee92ec5a57677899597de256702a29b

dpasukhi

2023-02-14 18:41

administrator   ~0113139

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.

git

2023-02-16 16:22

administrator   ~0113155

Branch CR23638 has been updated forcibly by DenisOrlov.

SHA-1: 44753060f8593659dbec814563ab7668ad57e1bb

DenisOrlov

2023-02-23 11:59

developer   ~0113197

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.

dpasukhi

2023-02-23 12:20

administrator   ~0113198

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"

dpasukhi

2023-02-23 13:37

administrator   ~0113200

An addition remark,
Please update summary of the commit description, you forgot "Data Exchange"
0023638: Data Exchange - Reading IGES file produced invalid shape

git

2023-02-27 15:06

administrator   ~0113206

Branch CR23638 has been updated forcibly by DenisOrlov.

SHA-1: e81bb6021eb844e4c22106a95a385f4018ec1a21

DenisOrlov

2023-02-27 15:24

developer   ~0113207

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.

dpasukhi

2023-02-27 16:49

administrator   ~0113209

Last edited: 2023-02-27 16:53

 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; };

git

2023-02-27 17:12

administrator   ~0113210

Branch CR23638 has been updated forcibly by DenisOrlov.

SHA-1: 5d0fbfcb7e32ee3ad817df4a01e168d2967aee53

git

2023-02-27 19:10

administrator   ~0113212

Branch CR23638 has been updated forcibly by DenisOrlov.

SHA-1: e885cebdbad70a6428424edac2eedd4706caa26b

dpasukhi

2023-03-21 22:08

administrator   ~0113309

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  : 
//=======================================================================

git

2023-03-22 14:32

administrator   ~0113312

Branch CR23638 has been updated forcibly by DenisOrlov.

SHA-1: a33f32db848e1bd06117fa218e78ea051bc8b348

dpasukhi

2023-03-22 15:33

administrator   ~0113314

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;

git

2023-03-24 17:38

administrator   ~0113324

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

git

2023-03-24 17:42

administrator   ~0113325

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

DenisOrlov

2023-03-27 11:50

developer   ~0113331

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.

dpasukhi

2023-03-27 12:39

administrator   ~0113333

Dear bugmaster,
Please integrate:
OCCT: CR23638_1
PROD: NO

All tests are OK, no regressions.

vglukhik

2023-04-02 15:53

administrator   ~0113348

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

git

2023-04-12 20:16

administrator   ~0113378

Branch CR23638 has been deleted by mnt.

SHA-1: 1770122f5529c08cf9805d2b105631c1127a1157

git

2023-04-12 20:16

administrator   ~0113379

Branch CR23638_1 has been deleted by mnt.

SHA-1: 51ef3120d6fbce5e858c421b631bdebd1faa3225

Related Changesets

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

Issue History

Date Modified Username Field Change
2012-12-13 11:25 szv New Issue
2012-12-13 11:25 szv Assigned To => gka
2012-12-17 17:14 szv File Added: cadbad.zip
2017-07-24 18:50 git Note Added: 0068647
2017-07-24 18:53 mkv Test case number => bugs iges bug23638
2017-07-27 18:15 mkv 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