View Issue Details

Related ChangesetsJump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
0025656Open CASCADEOCCT:Modeling Datapublic2014-12-24 14:342015-05-14 15:31
ReportermsvAssigned Tobugmaster  
PrioritynormalSeverityminor 
Status closedResolutionfixed 
Product Version6.8.0 
Target Version6.9.0Fixed in Version6.9.0 
Summary0025656: Specification of semantic of Closed flag of an edge
DescriptionThere is no clear understanding of what Closed flag of an edge means.

1. Does it mean "the geometric representation of this edge is a closed curve"?
2. Does it mean "the geometric representation of this edge has its end points near to each other in some tolerance"?
3. Does it mean "the topological boundaries of this edge are presented by the same vertex"?

Looking at the methods of the class BRep_Builder, I encountered inconsistency in this regard.

The method UpdateEdge(edge, curve3d) sets the flag Closed according to the flag IsClosed of the curve. It is the closeness of the 1st kind.

The method UpdateEdge(edge, pcurve, surface) does not take care of the flag Closed at all.

The method UpdateEdge(edge, pcurve1, pcurve2, surface) sets the flag Closed when both pcurves are closed. This will correspond to the case of closed seam edge, e.g. on a torus. It is the closeness also of the 1st kind.

The method Range(edge, first, last) sets the flag Closed if there is a 3D curve for which first and last points are distanced not more than on the edge tolerance. It is the closeness of the 2nd kind.

The method Range(edge, surface, first, last) sets the flag Closed if there is a 2D curve on the given surface for which first and last points in 3D space are distanced not more than on the edge tolerance. It is the closeness of the 2nd kind. Note that this method provides Closed flag for the cases for which the method UpdateEdge(edge, pcurve, surface) does not.

These inconsistencies lead to various indefinite behaviors of algorithms that deal with such edges. For example, there is a bug #20040, in which thrusections algorithm suffers due to absence of Closed flag on the edge, which is constructed on a closed 2D curve on surface.

The goal of this investigation is to analyze all places in OCCT when the flag Closed of an edge is used, classify them in order to elaborate common consistent approach, and update the source code of OCCT according to this approach.
Steps To ReproduceNot required
TagsNo tags attached.
Test case numberNot needed

Relationships

has duplicate 0025345 closedabv Open CASCADE Revise and document usage of flag Closed in TopoDS_Shape 
related to 0025741 closedbugmaster Open CASCADE Specification of semantic of Closed flag of a shape 
related to 0026027 closedbugmaster Open CASCADE Visualization, AIS_TexturedShape - back face culling option should not be overridden by texturing aspect 

Activities

msv

2014-12-24 14:44

developer   ~0035678

Note that depending on the result of analysis, the setting of the flag Closed by the methods of BRep_Builder UpdateEdge and Range may be deprecated.

msv

2014-12-24 14:46

developer   ~0035679

In the last assumption will be the case, this flag must be set by procedure that sets the same vertex as boundaries of the edge.

azv

2014-12-26 17:22

administrator   ~0035773

The result of investigation is following:

The Closed flag of TopoDS_Shape should be used to identify that a shape bounds a volume or an area. According this concept, this flag should affect on shells and wires only, because a closed shell determines a volume in 3D space, and a closed wire determines an area on a face. As far as, edges are used as a part of wire, no need to worry about Closed flag on edges.

As a result, the closedness of an edge should be determined by coincidence of its vertices. It should not use any geometric representations. The verification of the closedness will be moved to BRep_Tool::IsClosed(const TopoDS_Shape&) method.

msv

2014-12-26 17:27

developer   ~0035776

Artem, please make necessary modifications.

abv

2014-12-26 17:50

manager   ~0035778

I suggest we agree that flag Closed should not be used for faces, edges, solids, and compounds. This will allow to exclude processing of these types of shapes in BRep_Tool::IsClosed(), thus saving some processing time.

git

2014-12-30 08:06

administrator   ~0035856

Branch CR25656 has been created by azv.

SHA-1: 747b32a7998052531e1c7a8923452adf59c5357a


Detailed log of new commits:

Author: azv
Date: Tue Dec 30 07:37:51 2014 +0300

    0025656: Specification of semantic of Closed flag of an edge
    
    1. Using of the "Closed" flag was unified:
      a) this flag is applicable for TopoDS_Wire and TopoDS_Shell only, because these entities may hedge an area in 2D space or a volume in 3D space correspondingly;
      b) other types of TopoDS shapes are passing over this flag;
      c) changing of this flag should be controlled by high-level algorithms (not BRep_Builder).
    2. Implemented verification of the closedness of edges. An edge is closed if and only if its first and last vertices are the same.
    3. Test cases were changed according to new behavior.

azv

2014-12-30 08:08

administrator   ~0035857

Dear Mikhail,

Please review branch CR25656.

git

2014-12-30 13:45

administrator   ~0035883

Branch CR25656_1 has been created by azv.

SHA-1: f65b5512b7013bda6975246bfa4e6869d8c61833


Detailed log of new commits:

Author: azv
Date: Tue Dec 30 07:37:51 2014 +0300

    0025656: Specification of semantic of Closed flag of an edge
    
    1. Using of the "Closed" flag was unified:
      a) this flag is applicable for TopoDS_Wire and TopoDS_Shell only, because these entities may hedge an area in 2D space or a volume in 3D space correspondingly;
      b) other types of TopoDS shapes are passing over this flag;
      c) changing of this flag should be controlled by high-level algorithms (not BRep_Builder).
    2. Implemented verification of the closedness of edges. An edge is closed if and only if its first and last vertices are the same.
    3. Test cases were changed according to new behavior.

azv

2014-12-30 13:47

administrator   ~0035884

Dear Mikhail,

I've taken into account all your remarks. The updated branch is CR25656_1. Please, review.

abv

2014-12-30 14:33

manager   ~0035889

Please avoid replacing simple checks by complex ones even if they are equivalent, like here:

--- a/src/BRepBuilderAPI/BRepBuilderAPI_Sewing.cxx
+++ b/src/BRepBuilderAPI/BRepBuilderAPI_Sewing.cxx
@@ -582,8 +582,8 @@ TopoDS_Edge BRepBuilderAPI_Sewing::SameParameterEdge(const TopoDS_Edge& edgeFirs
     //check that edges merged valid way (for edges having length less than specified
     //tolerance
     // Check if edges are closed
- Standard_Boolean isClosed1 = V11.IsSame(V12);
- Standard_Boolean isClosed2 = V21.IsSame(V22);
+ Standard_Boolean isClosed1 = BRep_Tool::IsClosed(edge1);
+ Standard_Boolean isClosed2 = BRep_Tool::IsClosed(edge2);

git

2014-12-30 14:51

administrator   ~0035892

Branch CR25656_1 has been updated forcibly by azv.

SHA-1: cf384692d3fd5d2a0f4cccf87e466c756341280b

azv

2014-12-30 14:54

administrator   ~0035893

Dear Andrey, Mikhail,

I have changed the code according to your remarks. Please, review once again.

msv

2014-12-30 17:43

developer   ~0035908

Remarks.

1) BRepSweep_NumLinearRegularSweep.cxx::Shape(const TopoDS_Shape& aGenS,
                      const Sweep_NumShape& aDirS)
check the flag sepwires, if it is true than newWire is not used, WireSeq is used instead.

2) IGESToBRep_BRepEntity::TransferEdge
The condition dist1f + dist2l + Precision::Confusion() <= dist1l + dist2f will lead to that we will reverse the edge even in equal case, but it is not what we want for this case.
Instead, dist1f + dist2l - dist1l - dist2f < Precision::Confusion() must be used.

git

2015-01-12 08:51

administrator   ~0035942

Branch CR25656_1 has been updated forcibly by azv.

SHA-1: 5ea829200db6c064ea8dcf08a0ee979ddb0de9cc

azv

2015-01-12 08:53

administrator   ~0035943

Dear Mikhail,

I have made changes according to your remarks. Please, review branch CR25656_1 once again.

msv

2015-01-12 12:35

developer   ~0035950

In line 327 of BRepSweep_NumLinearRegularSweep.cxx, the wire WireSeq.Value(ij) is used with uninitialized Closed flag.

git

2015-01-12 13:44

administrator   ~0035953

Branch CR25656_1 has been updated forcibly by azv.

SHA-1: a98ca96cbc582cd02a9ff331b56358e374ef35b6

azv

2015-01-12 13:50

administrator   ~0035954

I've checked the code of BRepSweep_NumLinearRegularSweep::Shape(const TopoDS_Shape&, const Sweep_NumShape&) once again, and found that it is not necessary to assign the Closed flag for newWire and newShell, because these shapes are used for construction of myShapes(iGenS, iDirS), which Closed flag is assigned in the line 398.

msv

2015-01-12 16:10

developer   ~0035961

There are still cases when Closed flag for wire and shell can be left unset.

git

2015-01-12 16:25

administrator   ~0035966

Branch CR25656_1 has been updated forcibly by azv.

SHA-1: 9d580cf717dd1e04909f5c895c17e45e827b8041

azv

2015-01-12 16:26

administrator   ~0035967

Dear, Mikhail,

Please, review current state of file BRepSweep_NumLinearRegularSweep.cxx on branch CR25656_1.

msv

2015-01-12 16:45

developer   ~0035969

OK.

git

2015-01-16 15:33

administrator   ~0036148

Branch CR25656_1 has been updated forcibly by mkv.

SHA-1: 6b18aebe71763e962989c84561ad952b2ce91496

apn

2015-01-21 12:00

administrator   ~0036303

Branch CR25656_1 will be tested with CR20040_1.

git

2015-01-26 12:31

administrator   ~0036517

Branch CR25656_1 has been deleted by inv.

SHA-1: 6b18aebe71763e962989c84561ad952b2ce91496

git

2015-01-26 12:32

administrator   ~0036533

Branch CR25656 has been deleted by inv.

SHA-1: 747b32a7998052531e1c7a8923452adf59c5357a

Related Changesets

occt: master da72a17c

2014-12-30 04:37:51

azv


Committer: bugmaster Details Diff		 0025656: Specification of semantic of Closed flag of an edge

1. Using of the "Closed" flag was unified:
a) this flag is applicable for TopoDS_Wire and TopoDS_Shell only, because these entities may hedge an area in 2D space or a volume in 3D space correspondingly;
b) other types of TopoDS shapes are passing over this flag;
c) changing of this flag should be controlled by high-level algorithms (not BRep_Builder).
2. Implemented verification of the closedness of edges. An edge is closed if and only if its first and last vertices are the same.
3. Test cases were changed according to new behavior.		 Affected Issues
0025656
mod - src/BRep/BRep_Builder.cxx Diff File
mod - src/BRep/BRep_Tool.cdl Diff File
mod - src/BRep/BRep_Tool.cxx Diff File
mod - src/BRepAlgo/BRepAlgo_Loop.cxx Diff File
mod - src/BRepFeat/BRepFeat_MakeCylindricalHole.cxx Diff File
mod - src/BRepFill/BRepFill.cxx Diff File
mod - src/BRepFill/BRepFill_Evolved.cxx Diff File
mod - src/BRepFill/BRepFill_Filling.cxx Diff File
mod - src/BRepFill/BRepFill_Generator.cxx Diff File
mod - src/BRepFill/BRepFill_NSections.cxx Diff File
mod - src/BRepFill/BRepFill_OffsetWire.cxx Diff File
mod - src/BRepFill/BRepFill_Pipe.cxx Diff File
mod - src/BRepFill/BRepFill_Section.cxx Diff File
mod - src/BRepFill/BRepFill_ShapeLaw.cxx Diff File
mod - src/BRepLib/BRepLib.cxx Diff File
mod - src/BRepLib/BRepLib_MakeWire.cxx Diff File
mod - src/BRepLib/BRepLib_MakeWire_1.cxx Diff File
mod - src/BRepOffset/BRepOffset_Offset.cxx Diff File
mod - src/BRepOffsetAPI/BRepOffsetAPI_ThruSections.cxx Diff File
mod - src/BRepPrim/BRepPrim_Builder.cxx Diff File
mod - src/BRepPrimAPI/BRepPrimAPI_MakeOneAxis.cxx Diff File
mod - src/BRepPrimAPI/BRepPrimAPI_MakeWedge.cxx Diff File
mod - src/BRepProj/BRepProj_Projection.cxx Diff File
mod - src/BRepSweep/BRepSweep_NumLinearRegularSweep.cxx Diff File
mod - src/BRepSweep/BRepSweep_Rotation.cxx Diff File
mod - src/BRepTools/BRepTools_ReShape.cxx Diff File
mod - src/ChFi3d/ChFi3d_Builder_1.cxx Diff File
mod - src/IGESToBRep/IGESToBRep_BRepEntity.cxx Diff File
mod - src/LocOpe/LocOpe_SplitShape.cxx Diff File
mod - src/ShapeBuild/ShapeBuild_ReShape.cxx Diff File
mod - src/ShapeFix/ShapeFix_Edge.cxx Diff File
mod - src/ShapeFix/ShapeFix_Solid.cxx Diff File
mod - src/ShapeProcess/ShapeProcess_ShapeContext.cxx Diff File
mod - src/ShapeUpgrade/ShapeUpgrade_FaceDivideArea.cxx Diff File
mod - src/TopoDS/TopoDS_TShape.cdl Diff File
mod - src/TopOpeBRepBuild/TopOpeBRepBuild_Builder1.cxx Diff File
mod - src/TopOpeBRepBuild/TopOpeBRepBuild_Builder1_1.cxx Diff File
mod - src/TopOpeBRepBuild/TopOpeBRepBuild_Builder1_2.cxx Diff File
mod - src/TopOpeBRepBuild/TopOpeBRepBuild_FaceBuilder.cxx Diff File
mod - src/TopOpeBRepBuild/TopOpeBRepBuild_makeedges.cxx Diff File
mod - tests/bugs/moddata_3/bug25202_1 Diff File
mod - tests/bugs/moddata_3/bug25202_3 Diff File
mod - tests/bugs/moddata_3/bug25202_4 Diff File

Issue History

Date Modified Username Field Change
2014-12-24 14:34 msv New Issue
2014-12-24 14:34 msv Assigned To => msv
2014-12-24 14:37 abv Summary Investigation of semantic of Closed flag of an edge => Specification of semantic of Closed flag of an edge
2014-12-24 14:40 msv Summary Specification of semantic of Closed flag of an edge => Investigation of semantic of Closed flag of an edge
2014-12-24 14:40 msv Description Updated
2014-12-24 14:40 msv Assigned To msv => azv
2014-12-24 14:40 msv Status new => assigned
2014-12-24 14:41 msv Summary Investigation of semantic of Closed flag of an edge => Specification of semantic of Closed flag of an edge
2014-12-24 14:44 msv Note Added: 0035678
2014-12-24 14:46 msv Note Added: 0035679
2014-12-26 17:22 azv Note Added: 0035773
2014-12-26 17:27 msv Note Added: 0035776
2014-12-26 17:50 abv Note Added: 0035778
2014-12-30 08:06 git Note Added: 0035856
2014-12-30 08:08 azv Note Added: 0035857
2014-12-30 08:08 azv Assigned To azv => msv
2014-12-30 08:08 azv Status assigned => resolved
2014-12-30 08:08 azv Steps to Reproduce Updated
2014-12-30 09:33 azv Relationship added has duplicate 0025345
2014-12-30 13:25 msv Assigned To msv => azv
2014-12-30 13:25 msv Status resolved => assigned
2014-12-30 13:45 git Note Added: 0035883
2014-12-30 13:47 azv Note Added: 0035884
2014-12-30 13:47 azv Assigned To azv => msv
2014-12-30 13:47 azv Status assigned => resolved
2014-12-30 14:33 abv Note Added: 0035889
2014-12-30 14:33 abv Assigned To msv => azv
2014-12-30 14:33 abv Status resolved => assigned
2014-12-30 14:51 git Note Added: 0035892
2014-12-30 14:54 azv Note Added: 0035893
2014-12-30 14:54 azv Assigned To azv => msv
2014-12-30 14:54 azv Status assigned => resolved
2014-12-30 17:43 msv Note Added: 0035908
2014-12-30 17:43 msv Assigned To msv => azv
2014-12-30 17:43 msv Status resolved => assigned
2015-01-12 08:51 git Note Added: 0035942
2015-01-12 08:53 azv Note Added: 0035943
2015-01-12 08:53 azv Assigned To azv => msv
2015-01-12 08:53 azv Status assigned => resolved
2015-01-12 12:35 msv Note Added: 0035950
2015-01-12 12:35 msv Assigned To msv => azv
2015-01-12 12:35 msv Status resolved => assigned
2015-01-12 13:44 git Note Added: 0035953
2015-01-12 13:50 azv Note Added: 0035954
2015-01-12 13:50 azv Assigned To azv => msv
2015-01-12 13:50 azv Status assigned => resolved
2015-01-12 16:10 msv Note Added: 0035961
2015-01-12 16:10 msv Assigned To msv => azv
2015-01-12 16:10 msv Status resolved => assigned
2015-01-12 16:25 git Note Added: 0035966
2015-01-12 16:26 azv Note Added: 0035967
2015-01-12 16:26 azv Assigned To azv => msv
2015-01-12 16:26 azv Status assigned => resolved
2015-01-12 16:45 msv Note Added: 0035969
2015-01-12 16:45 msv Assigned To msv => bugmaster
2015-01-12 16:45 msv Status resolved => reviewed
2015-01-16 15:33 git Note Added: 0036148
2015-01-16 15:48 mkv Assigned To bugmaster => mkv
2015-01-21 12:00 apn Note Added: 0036303
2015-01-21 20:20 apn Test case number => Not needed
2015-01-21 20:20 apn Assigned To mkv => apn
2015-01-22 16:34 apn Assigned To apn => bugmaster
2015-01-22 16:34 apn Status reviewed => tested
2015-01-22 17:03 msv Relationship added related to 0025741
2015-01-23 16:19 bugmaster Changeset attached => occt master da72a17c
2015-01-23 16:19 bugmaster Status tested => verified
2015-01-23 16:19 bugmaster Resolution open => fixed
2015-01-26 12:31 git Note Added: 0036517
2015-01-26 12:32 git Note Added: 0036533
2015-04-07 12:31 kgv Relationship added related to 0026027
2015-05-14 15:28 aiv Status verified => closed
2015-05-14 15:31 aiv Fixed in Version => 6.9.0