Archived issue #0025656
Specification of semantic of Closed flag of an edge
Description
There 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.
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 reproduce
Not required
Public activity
26 archived notes
Participants are labeled by their role within this record.
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.
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.
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.
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.
Artem, please make necessary modifications.
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.
Branch [archived branch] has been created by Participant.
[revision removed]
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.
[revision removed]
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.
Dear Mikhail,
Please review branch CR25656.
Please review branch CR25656.
Branch [archived branch] has been created by Participant.
[revision removed]
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.
[revision removed]
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.
Dear Mikhail,
I've taken into account all your remarks. The updated branch is CR25656_1. Please, review.
I've taken into account all your remarks. The updated branch is CR25656_1. Please, review.
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);
--- 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);
Branch [archived branch] has been updated forcibly by Participant.
[revision removed]
[revision removed]
Dear Andrey, Mikhail,
I have changed the code according to your remarks. Please, review once again.
I have changed the code according to your remarks. Please, review once again.
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.
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.
Branch [archived branch] has been updated forcibly by Participant.
[revision removed]
[revision removed]
Dear Mikhail,
I have made changes according to your remarks. Please, review branch CR25656_1 once again.
I have made changes according to your remarks. Please, review branch CR25656_1 once again.
In line 327 of BRepSweep_NumLinearRegularSweep.cxx, the wire WireSeq.Value(ij) is used with uninitialized Closed flag.
Branch [archived branch] has been updated forcibly by Participant.
[revision removed]
[revision removed]
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.
There are still cases when Closed flag for wire and shell can be left unset.
Branch [archived branch] has been updated forcibly by Participant.
[revision removed]
[revision removed]
Dear, Mikhail,
Please, review current state of file BRepSweep_NumLinearRegularSweep.cxx on branch CR25656_1.
Please, review current state of file BRepSweep_NumLinearRegularSweep.cxx on branch CR25656_1.
OK.
Branch [archived branch] has been updated forcibly by Participant.
[revision removed]
[revision removed]
Branch CR25656_1 will be tested with CR20040_1.
Branch [archived branch] has been deleted by Participant.
[revision removed]
[revision removed]
Branch [archived branch] has been deleted by Participant.
[revision removed]
[revision removed]
Related records
- #0025345 · has duplicate · closedRevise and document usage of flag Closed in TopoDS_Shape
- #0025741 · related to · closedSpecification of semantic of Closed flag of a shape
- #0026027 · related to · closedVisualization, AIS_TexturedShape - back face culling option should not be overridden by texturing aspect