Anonymous | Login 2020-10-01 07:00 MSK Project: All Projects Community Open CASCADE
 My View | View Issues | Change Log | Roadmap

View Issue Details  Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0026489Open CASCADE[OCCT] OCCT:Modeling Algorithmspublic2015-07-28 12:102016-04-20 15:48
Reporterpkv
Assigned Toski
PrioritynormalSeverityminor
StatusclosedResolutionfixed
PlatformOSOS Version
Product Version[OCCT] 7.0.0
Target Version[OCCT] 7.0.0Fixed in Version[OCCT] 7.0.0
Summary0026489: The class ShapeUpgrade_UnifySameDomain provides the results that are wrong or difficult to explain.
DescriptionThe class ShapeUpgrade_UnifySameDomain provides the result that is wrong or at least difficult to explain.

There are two kinds of the problem with the algorithm above.

I . The result obtained by the algorithm itself is wrong
II. The result obtained by the method:
TopoDS_Shape ShapeUpgrade_UnifySameDomain::Generated (const TopoDS_Shape& aShape) const
is wrong.

Steps To Reproduce1. Preface
The command zubuild is to obtain the result of the algorithm itself
Syntax:
zubuild r s
s - input shape
r - resulting shape

The command zugener is to obtain the result of the method Generated(...) of the algorithm
Syntax:
zugener x s
s - input shape
x - shape returned by the method Generated(s)

The commands zubuild, zugener are in attached file a.cxx

2. Cases
#--------------------------- Case 1
restore r003 b1
explode b1 w
zubuild r b1_1
nbshapes r
#
# The result of the command nbshapes:
Number of shapes in r
VERTEX : 8
EDGE : 8
WIRE : 1
FACE : 0
...
# Problem: As it seems the result r should contain 4 vertices and 4 edges.

#--------------------------- Case 2
restore r002 b1
zubuild r b1
nbshapes r
#
# The result of the command nbshapes:
Number of shapes in r
VERTEX : 8
EDGE : 8
WIRE : 1
FACE : 1
...
# Problem: As it seems the result r should contain 4 vertices and 4 edges.

#--------------------------- Case 3
restore r002 b1
zubuild r b1
explode b1 f
#
zugener x b1_1
# The result of zugener is x (x=r)

zugener x b1_2
# null shape

zugener x b1_3
# null shape

* Problem: As it seems the result of commads
zugener x b1_2
zugener x b1_3
should be r too.
TagsNo tags attached.
Test case numberbugs heal bug26489_1, bug26489_2, bug26489_3, bug26489_4, bug26489_5, bug26489_6
Attached Files r002 (2,471 bytes) 2015-07-28 12:12
r003 (2,065 bytes) 2015-07-28 12:12
a.cxx (1,797 bytes) 2015-07-28 12:13
p01usd.brep (1,058 bytes) 2015-07-29 13:01
3cir.brep (830 bytes) 2015-08-06 14:51

Relationships
 child of 0025462 verified bugmaster BRepAlgoAPI_Fuse not work correctly for a planar face

 Notes git (administrator) 2015-07-29 12:51 Branch CR26489 has been created by isn. SHA-1: cd0cf5b5c3913c4ced158afe7ec002db82b9c6b7 Detailed log of new commits: Author: isn Date: Tue Jul 28 22:01:20 2015 +0300     0026489: The class ShapeUpgrade_UnifySameDomain provides the results that are wrong or difficult to explain     --- Try to merge all possible sub-sequences in the sequence of edges isn (developer) 2015-07-29 13:03 edited on: 2015-07-29 13:03 additional test file have been added (p01usd.brep) ----- restore p01usd.brep poly mkplane ff poly unifysamedom res ff ----- isn (developer) 2015-07-29 13:16 please check branch CR26489. It should give more accurate result with input faces (test case#2) isn (developer) 2015-07-29 14:49 branch have been updated msv (developer) 2015-07-29 16:36 Remarks: 810: NCollection_Sequence subSeq = SubsSeqsEdges(i); use const& to avoid copying. 610: SubsSeqsEdges.ChangeFirst().Append(SubsSeqsEdges.ChangeLast()); It seems this statement inserts edges in not correct order. May be Prepend should be used? In MergeEdges, in branch with circles (line 688) there is no treatment similar to lines branch. Returning false is not allowed anymore. 637: continue; This is dangerous when LastClosed is true (infinite loop). 749: if (j == aChain.Length() - 1 && IsClosed && !LastClosed) The last check !LastClosed is useless, as this line is unreachable with LastClosed==true. 794: UnionEdges = aChain; It is better to avoid copying using Clear and Append methods. git (administrator) 2015-07-30 16:25 Branch CR26489_1 has been created by isn. SHA-1: e29fbb2790cd4f53bb3783104bbbeb37bd338b42 Detailed log of new commits: Author: isn Date: Thu Jul 30 15:46:18 2015 +0300     0026489: The class ShapeUpgrade_UnifySameDomain provides the results that are wrong or difficult to explain.     another version msv (developer) 2015-07-30 18:01 1421: TopoDS_Edge E; This line is not needed any more. Somewhere you changed TopTools_SequenceOfShape to NCollection_Sequence. It is unjustified, especially taking into account that you everywhere still use cast operation TopoDS::Edge() when take an item from the sequence. I propose to use the same class TopTools_SequenceOfShape everywhere. 871: NCollection_Sequence> SubsSeqsEdges; 872: NCollection_Sequence UnionEdges; 873: NCollection_Sequence anUnionStats; All these sequences are expected to be synchronized. It is better to define a struct containing an input sequence of edges and a merged edge. The boolean flag is not needed, as IsNull() property of shape can be used for that. Then define a sequence of such structs. It will simplify logic and reduce both memory usage and computation time. The method MergeEdges does not use the argument aFace. So, I propose to remove it in all functions that constitute this chain of calls. 808: if (BRep_Tool::Degenerated(anEdge)) 809: return Standard_False; I think we should omit this cycle. Degenerated edges must not impede merging other edges. They must be left alone. Therefore I propose to remember their vertex in a separate map in order to not merge edges through this vertex, and remove degenerated edge from the sequence at all. 832: if(aChain.Length() Precision::Confusion()) Misprint: the same ade1 is used twice. 722: if( t1 == GeomAbs_Circle && t2 == GeomAbs_Line || t2 == GeomAbs_Circle && t1 == GeomAbs_Line) Why do you check only lines and circles? What about other types, like ellipse, parabola, hyperbola? It is better to compare types directly (type1 != type2), excluding bspline and bezier. Place checking for compatibility of types before checking for derivatives equality, because it is more quick. 536: if(c3d1.IsNull()) break; 544: if(c3d2.IsNull()) break; In this case both IsUnionOfLinesPossible and IsUnionOfCirclesPossible can be left true, and it will be a mistake. I would return false here. 589: E.Reverse(); 643: E.Reverse(); Why do you reverse it? 605: if (!MC.IsDone() || MC.Value().IsNull()) { In this block probably it is needed to copy orientation from the edge of which the curve C1 was taken. 596: Handle(Geom_Circle) C1 = Handle(Geom_Circle)::DownCast(c3d1); c3d1 is not always belong to the first edge in sequence. It is better to take this curve again. msv (developer) 2015-07-31 12:32 Repeated remarks: 1422: TopoDS_Edge E; This line is not needed any more. 695: line is too long. Try to limit the lines to 100 characters width. 818: if (BRep_Tool::Degenerated(anEdge)) Here it is needed to remove the edge from the SeqEdges, otherwise the next cycle can fail. 847: if(aChain.Length()