View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0031047 | Community | OCCT:Modeling Algorithms | public | 2019-10-08 17:39 | 2021-07-10 12:40 |
| Reporter | BenjaminBihler | Assigned To | bugmaster | ||
| Priority | normal | Severity | minor | ||
| Status | closed | Resolution | fixed | ||
| Platform | g++ 9.2.0 | OS | Windows | ||
| Product Version | 7.4.0 | ||||
| Target Version | 7.6.0 | Fixed in Version | 7.6.0 | ||
| Summary | 0031047: Modeling Algorithms - BRepExtrema_DistShapeShape gives wrong result | ||||
| Description | The distance of a vertex to a face is computed. It should be zero, but it is computed to be 47.1989. I use mingw-w64 with g++ 9.2.0, but this actually shouldn't make a difference. Please see the code in "Steps To Reproduce". | ||||
| Steps To Reproduce | Test case bugs modalg_7 bug31047 | ||||
| Tags | No tags attached. | ||||
| Test case number | bugs/modalg_6/bug31047 | ||||
|
|
Face.brep (118,509 bytes) |
|
|
A slight translation of the test point makes the distance measurement work. At least I can use this here to detect whether my measurement is wrong in a special test case. See this code for the disturbed (and therefore correct) calculation: #include <TopoDS_Face.hxx> #include <BRepExtrema_DistShapeShape.hxx> #include <TopoDS.hxx> #include <Precision.hxx> #include <TopExp_Explorer.hxx> #include <BRepTools.hxx> #include <BRep_Builder.hxx> #include <BRepBuilderAPI_MakeVertex.hxx> #include <string> #include <vector> #include <iostream> int main(int, char*[]) { BRep_Builder builder; TopoDS_Shape face; BRepTools::Read(face, "Face.brep", builder); gp_Pnt testPoint1(-79.14602309143586467143904883414507, -282.66130124109884036442963406443596, 0.00000000000000000000000000000000); BRepExtrema_DistShapeShape distanceComputer1( BRepBuilderAPI_MakeVertex(testPoint1), face); if (distanceComputer1.IsDone()) { std::cout << "Calculated Distance = " << distanceComputer1.Value() << std::endl; if (distanceComputer1.Value() > Precision::Confusion()) { std::cout << "This is wrong! The distance should be (close to) zero." << std::endl; } } else { std::cout << "Calculation failed." << std::endl; } std::cout << std::endl << "Translating test point..." << std::endl; gp_Pnt testPoint2 = testPoint1.Translated(gp_Vec(0.0, 0.1, 0.0)); BRepExtrema_DistShapeShape distanceComputer2( BRepBuilderAPI_MakeVertex(testPoint2), face); if (distanceComputer2.IsDone()) { std::cout << "Calculated Distance = " << distanceComputer2.Value() << std::endl; if (distanceComputer2.Value() > Precision::Confusion()) { std::cout << "This is wrong! The distance should be (close to) zero." << std::endl; } } else { std::cout << "Calculation failed." << std::endl; } return 0; } |
|
|
I have updated steps to reproduce with a simple draw script. It gives the error in the current OCCT version. |
|
|
The cause of the bug is incorrect work of BRepClass_FaceClassifier. See script 2 in steps to reproduce. The possible fix is to use BRepTopAdaptor_FClass2d for classification. |
|
|
我在求两个曲面的距离时遇到了相似的问题。 两个曲面均是周期曲面且相交，第一个曲面在周期方向曲率存在较大的波动，第二个曲面为回转面。 调用过程：BrepExtrema_DistShapeShape->BrepExtrema_DistSS->BrepExt_ExtFF->Extrema_GenExtSS, 最后一个步骤中，均匀取了20*20个离散点并求最近点对，进而确定Extrema_FunDistSS, math_BFGS的初始值，进行求解。根据该步骤所求的结果，计算对应曲面上的法向量，发现两个法向量夹角为180.005°。 我猜测是这里出了问题。 版本：7.3.0 |
|
|
Dear Mechanocider, Very few people who uses this bugtracker can read Chinese. Please use English for your posts. 亲爱的机械编码器， 使用此bugtracker的人很少能读中文。 请使用英文撰写您的帖子。 |
|
|
Dear msv, are BRepClass_FaceClassifier and BRepTopAdaptor_FClass2d serving the same purpose? Is BRepTopAdaptor_FClass2d more correct/stable? This post https://www.opencascade.com/content/classify-point-given-face seems to indicate that it is faster. But I have found no more information on that. Thank you, Benjamin |
|
|
BRepTopAdaptor_FClass2d tries to use discrete representation of the boundary to solve the problem using just intersections of linear segments. In case of point located too close to bounds it calls geometric classifier BRepClass_FaceClassifier. |
|
|
Dear msv, I need to calculate distance between two surfaces, one is VPeriodic surface and another is surface of revolution. The two surfaces are clearly intersecting while BRepExtrema_DistShapeShape give the result 0.088, it's wrong. My code like this: BRepExtrema_DistShapeShape ext; ext.SetFlag(Extrema_ExtFlat_MIN); ext.LoadS1(face1); ext.LoadS2(face2); ext.Perform(); if (ext.IsDone()) { double dist = ext.Value(); } I debuged sources and got the following simplified call stack: BRepExtrema_DistShapeShape::Perform DistanceMapMap(myMapF1, myMapF2, myBF1, myBF2) BRepExtrema_DistSS BRepExt_ExtFF Extrema_ExtSS Extrema_GenExtSS Then I got two closest points(UV(1),UV(2),UV(3),UV(4)) from Extrema_GenExtSS::Perform(). I calculated normal directions from the two specified points on corresponding surface, and the angle between the two direction is 180.005°。丷丷 And I think 20*20 sampling points for start point is not enough about this situation. Test version:7.3.0 Sorry I cannot give you a test case. sorry for my bad posts. Thank you so much! |
|
|
Dear Mechanicoder, now your post is much clearer :). Thank you. However, it is better to work with each bug in a separate ticket. The current one is about distance between vertex and face. Your case is distance between two faces. So, I kindly ask you to create a new bug with shapes to reproduce the problem. Without your particular shapes we hardly can reproduce it, and so cannot fix. |
|
|
Branch CR31047 has been created by andrey.bulychev.ext_170479. SHA-1: 9c0eb1cf5264cd754c21b9e68c7908d228fe3006 Detailed log of new commits: Author: abulyche Date: Mon Apr 12 13:13:14 2021 +0300 0031047: Modeling Algorithms - BRepExtrema_DistShapeShape gives wrong result Added map of edges to BrepClass_Edge Added searching of vertices with high tolerance to BrepClass_Intersector.cxx |
|
|
Branch CR31047 has been updated forcibly by andrey.bulychev.ext_170479. SHA-1: 0663d7a83f43d2b78db58bad85a9768e908f16a0 |
|
|
Branch CR31047 has been updated forcibly by andrey.bulychev.ext_170479. SHA-1: ccc1e1bb78af4509d38b0e6e33277ac027773d18 |
|
|
Branch CR31047 has been updated forcibly by andrey.bulychev.ext_170479. SHA-1: 418eb40ed5a534226fd5cdfc81daf5c2592962aa |
|
|
Branch CR31047 has been updated forcibly by andrey.bulychev.ext_170479. SHA-1: a59a277961e71a7e384a79e856413b30f7b98128 |
|
|
Branch CR31047 has been updated forcibly by andrey.bulychev.ext_170479. SHA-1: 39332472e3be1ccf6fd7a2831bf6b87146ccd3e5 |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: ca62d8d8aaa9fd7f2380ecd9b7f7811b1eca8d24 |
|
|
Debugging |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: cfcef65dc59525d84951a67bd177d571973e43c4 |
|
|
Analysis |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: d9ada77bb18d653fe205c73a548ec064d8258fc1 |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: 760039ff32f47246466be22db58cd4132bfb71b2 |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: 7f22347e6a5303613090f96c473c90ae668349de |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: b6580bed6a2b45099e53804c6ce4d4089fe3e810 |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: b85854b94a8e504222561093ad6a92bb18226f0e |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: d459f6b29361464a4e4cefa607a643b1335c7496 |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: a633dee751c6b0c20893148c7ea69874686810d8 |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: 4ecdcc993b2d283dc219a9c491328fca328b7909 |
|
2021-07-05 01:01 developer |
bug31047.brep (118,509 bytes) |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: 218263031ee47f843bf3088b7384a92d3985c91c |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: aa3f2d0b6240ca2d8fb7a9d75311609090e9dd0d |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: 299a438be53a32471d9861ca8fef7ac4009c135f |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: cab4e366bd5d82d31e92c1646fe8052cc86ba9f2 |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: 07fdbd65a3f5c608d509e09b5687db7978473b7a |
|
|
Test results CR31047-master-abulyche |
|
|
+const TopoDS_Edge& GetNextEdge() const; Method naming is inconsistent to other properties in class (redundant "Get"). Please also put description to new (and old, if not a big deal) methods.
+inline const TopoDS_Edge& BRepClass_Edge::GetNextEdge() const
+{
+ return myNextEdge;
Please avoid putting inline methods into .lxx file (obsolete style) - trivial/short methods could be inlined in place of method declaration. +//function : GetNextEdge +//purpose : . Please avoid copying trailing spaces while copying old code.
+void BRepClass_Edge::SetNextEdge(const TopTools_IndexedDataMapOfShapeListOfShape& theMapVE)
+{
+ if (!theMapVE.IsEmpty())
+ {
+ if (!myEdge.IsNull())
+ {
...
+Geom2dInt_GInter& CheckSkip(Geom2dInt_GInter& theInter,
+{
+ if (!theE.Edge().IsNull() && !theE.Face().IsNull())
+ {
+ Standard_Boolean anIsLSkip = Standard_False;
+ TopoDS_Vertex aVl; // the last vertex of current edge
+
+ Handle(Geom2d_Curve) aSkipC2D;
+
+ aVl = TopExp::LastVertex(theE.Edge(), Standard_True);
+ if (!aVl.IsNull())
+ {
It is preferred putting return/break statements instead of nesting if statements https://dev.opencascade.org/doc/overview/html/occt_contribution__coding_rules.html#autotoc_md372 + const TopTools_ListOfShape anEdgesL = theMapVE.FindFromKey(aVl); ... + TopoDS_Shape aTmpE = anIt.Value(); It is desired avoiding redundant copies and use references when possible. --- a/src/BRepClass/BRepClass_Edge.hxx +++ b/src/BRepClass/BRepClass_Edge.hxx ... +#include <TopExp.hxx> Header includes used only within .cxx file should be put into .cxx file.
BRepClass_Intersector::BRepClass_Intersector()
{
+ myMaxTolerance = 0.1;
Initialization list is a preferred syntax for class fields initialization. +const Standard_Real& BRepClass_Intersector::GetMaxTolerance() const Primitive types (double, int, float) are normally returned by copy rather than by reference. + const Standard_Real& GetMaxTolerance() const; It is also preferred inlining trivial methods into .hxx. Moreover, it is necessary adding Standard_EXPORT to public/protected methods defined in .cxx or this API will cause linker errors (Standard_EXPORT is not necessary for inlined methods). + for (i = 1; i <= aNbPnts; ++i) Please reduce iterator declaration scope to for(;;) loop, when it is not used outside if the loop. + aMinDist = sqrt(aMinDist); Sqrt(). |
|
|
Geom2dInt_GInter& CheckSkip(Geom2dInt_GInter& theInter, .... theInter = Geom2dInt_GInter(theCGA, theDL, theCur, aDE, Precision::PConfusion(), Precision::PIntersection()); } } } } } } } } } } return theInter; method returns theInter twice: as method return and as parameter theInter change. |
|
|
Please, confider and correct code according to remarks |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: 9fa994439440b0102e50614f21e07e0092fbf6fe |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: e802d1118b6002d95ea442e185a4c1f1132c2854 |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: 7ebdcb8aefa0d63dc6f36b9d0dd2656c4ac7c158 |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: b7411e0fa2fec2cb710e04f2ea0c61b79836b83a |
|
|
Branch CR31047 is ready for review |
|
|
Branch seems to be valid |
|
|
+ TopoDS_Vertex aVl = TopExp::LastVertex(myEdge, Standard_True); + if (aVl.IsNull() || aVl.IsSame(TopExp::FirstVertex(myEdge, Standard_True))) Please use TopExp::Vertices() to query both vertices at once.
+ if (theMapVE.FindFromKey(aVl).Extent() == 2)
+ {
+ for (TopTools_ListIteratorOfListOfShape anIt(theMapVE.FindFromKey(aVl)); anIt.More(); anIt.Next())
It is counter-efficient duplicating the same map lookup. NCollection_IndexedDataMap::Seek() is preferred in this case. + Standard_Real aMinDist = RealLast(), aDist; ... + aDist = anExtPC2d.SquareDistance(i); Please declare variable locally.
+ if (aMinInd) {
Please avoid comparing non-Boolean types with 0 like that - it would be more clear "if (aMinInd != 0)".
+ {
+ return myMaxTolerance;
+ }
Broken indentation. |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: b1ca152129bd4a3336ad84cf399c2531201f6015 |
|
|
Branch CR31047 has been updated forcibly by abulychev-ext. SHA-1: 79826265e820adb9fb2a2f65229883ea90e853ec |
|
|
Branch CR31047 is ready for review Test results http://jenkins-test-occt/view/CR31047-master-abulyche/view/COMPARE/ |
|
|
+ if ((*aListE).Extent() == 2) Unusual syntax, aListE->Extent() is more common. |
|
|
Combination - OCCT branch : IR-2021-07-09 master SHA - cb7f92396f80270e13206be742038ca0ce04571a a87b7ddc8cb44606b91e3f37113847c3f5f50fdc Products branch : IR-2021-07-09 SHA - a0de44eb21bec792a53984d50cb770f6679dbbd8 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: Debian80-64: OCCT Total CPU difference: 17537.450000000343 / 17428.46000000034 [+0.63%] Products Total CPU difference: 11490.74000000011 / 11521.620000000124 [-0.27%] Windows-64-VC14: OCCT Total CPU difference: 19309.859375 / 19266.640625 [+0.22%] Products Total CPU difference: 12831.40625 / 12854.9375 [-0.18%] Image differences : No differences that require special attention Memory differences : No differences that require special attention |
|
|
Branch CR31047 has been deleted by mnt. SHA-1: 79826265e820adb9fb2a2f65229883ea90e853ec |
|
occt: master cb7f9239 2021-04-12 10:13:14 Committer: bugmaster Details Diff |
0031047: Modeling Algorithms - BRepExtrema_DistShapeShape gives wrong result Added map of edges to BrepClass_Edge Added searching of vertices with high tolerance to BrepClass_Intersector.cxx Added check for hitting a vertex with high tolerance Added the creation of a segment for the correct work of the classifier |
Affected Issues 0031047 |
|
| mod - src/BRepClass/BRepClass_Edge.cxx | Diff File | ||
| mod - src/BRepClass/BRepClass_Edge.hxx | Diff File | ||
| mod - src/BRepClass/BRepClass_FaceExplorer.cxx | Diff File | ||
| mod - src/BRepClass/BRepClass_FaceExplorer.hxx | Diff File | ||
| mod - src/BRepClass/BRepClass_Intersector.cxx | Diff File | ||
| mod - src/BRepClass/BRepClass_Intersector.hxx | Diff File | ||
| add - tests/bugs/modalg_6/bug31047 | Diff File | ||
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2019-10-08 17:39 | BenjaminBihler | New Issue | |
| 2019-10-08 17:39 | BenjaminBihler | Assigned To | => msv |
| 2019-10-08 17:39 | BenjaminBihler | File Added: Face.brep | |
| 2019-10-10 08:28 | BenjaminBihler | Note Added: 0087997 | |
| 2019-10-14 16:06 |
|
Note Added: 0088107 | |
| 2019-10-14 16:06 |
|
Steps to Reproduce Updated | |
| 2019-10-14 16:28 |
|
Note Added: 0088108 | |
| 2019-10-14 16:28 |
|
Steps to Reproduce Updated | |
| 2019-10-14 16:29 |
|
Assigned To | msv => ifv |
| 2019-10-14 16:29 |
|
Status | new => assigned |
| 2019-10-15 09:33 | Mechanicoder | Note Added: 0088128 | |
| 2019-10-15 09:47 |
|
Note Added: 0088129 | |
| 2019-10-15 09:49 |
|
Note Edited: 0088129 | |
| 2019-10-15 09:51 | BenjaminBihler | Note Added: 0088130 | |
| 2019-10-15 10:01 |
|
Note Added: 0088131 | |
| 2019-10-17 14:44 | Mechanicoder | Note Added: 0088255 | |
| 2019-10-17 14:52 |
|
Note Added: 0088258 | |
| 2019-12-17 13:48 | kgv | Summary | BRepExtrema_DistShapeShape gives wrong result => Modeling Algorithms - BRepExtrema_DistShapeShape gives wrong result |
| 2020-09-15 16:27 |
|
Target Version | 7.5.0 => 7.6.0 |
| 2020-09-17 09:55 |
|
Assigned To | ifv => akulagin |
| 2021-03-04 16:59 |
|
Assigned To | akulagin => abulyche |
| 2021-04-12 13:23 | git | Note Added: 0100240 | |
| 2021-04-15 13:38 | git | Note Added: 0100314 | |
| 2021-04-16 08:19 | git | Note Added: 0100327 | |
| 2021-04-19 18:09 | git | Note Added: 0100461 | |
| 2021-04-19 18:51 | git | Note Added: 0100462 | |
| 2021-05-04 14:33 | git | Note Added: 0100760 | |
| 2021-05-11 12:48 | bugmaster | Assigned To | abulyche => abulychev-ext |
| 2021-05-12 12:18 | git | Note Added: 0100912 | |
| 2021-05-25 12:30 |
|
Note Added: 0101353 | |
| 2021-05-25 17:22 |
|
Note Edited: 0101353 | |
| 2021-05-27 11:42 | git | Note Added: 0101420 | |
| 2021-05-28 13:07 |
|
Note Added: 0101444 | |
| 2021-05-31 19:11 | git | Note Added: 0101526 | |
| 2021-06-02 17:27 | git | Note Added: 0101553 | |
| 2021-06-20 13:08 | git | Note Added: 0101944 | |
| 2021-06-24 13:51 | git | Note Added: 0102027 | |
| 2021-06-24 17:24 | git | Note Added: 0102034 | |
| 2021-06-24 23:08 | git | Note Added: 0102043 | |
| 2021-07-01 16:25 | git | Note Added: 0102212 | |
| 2021-07-01 18:50 | git | Note Added: 0102216 | |
| 2021-07-05 01:01 |
|
File Added: bug31047.brep | |
| 2021-07-05 01:03 | git | Note Added: 0102270 | |
| 2021-07-05 01:15 | git | Note Added: 0102271 | |
| 2021-07-05 21:23 | git | Note Added: 0102289 | |
| 2021-07-06 11:19 | git | Note Added: 0102297 | |
| 2021-07-06 11:23 | git | Note Added: 0102298 | |
| 2021-07-06 13:32 |
|
Note Added: 0102302 | |
| 2021-07-06 13:32 |
|
Assigned To | abulychev-ext => ifv |
| 2021-07-06 13:32 |
|
Status | assigned => resolved |
| 2021-07-06 13:32 |
|
Steps to Reproduce Updated | |
| 2021-07-06 14:18 | kgv | Note Added: 0102303 | |
| 2021-07-06 14:19 | kgv | Note Edited: 0102303 | |
| 2021-07-06 14:49 |
|
Note Added: 0102305 | |
| 2021-07-06 14:52 |
|
Note Added: 0102306 | |
| 2021-07-06 14:52 |
|
Assigned To | ifv => abulychev-ext |
| 2021-07-06 14:52 |
|
Status | resolved => assigned |
| 2021-07-07 16:36 | git | Note Added: 0102334 | |
| 2021-07-07 20:57 | git | Note Added: 0102348 | |
| 2021-07-08 11:20 | git | Note Added: 0102360 | |
| 2021-07-08 11:29 | git | Note Added: 0102361 | |
| 2021-07-08 14:01 |
|
Note Added: 0102371 | |
| 2021-07-08 14:01 |
|
Assigned To | abulychev-ext => ifv |
| 2021-07-08 14:01 |
|
Status | assigned => resolved |
| 2021-07-08 14:11 |
|
Note Added: 0102374 | |
| 2021-07-08 14:11 |
|
Assigned To | ifv => bugmaster |
| 2021-07-08 14:11 |
|
Status | resolved => reviewed |
| 2021-07-08 14:18 | kgv | Note Added: 0102375 | |
| 2021-07-08 14:19 | kgv | Note Edited: 0102375 | |
| 2021-07-08 14:21 |
|
Assigned To | bugmaster => abulychev-ext |
| 2021-07-08 14:21 |
|
Status | reviewed => assigned |
| 2021-07-08 16:22 | git | Note Added: 0102377 | |
| 2021-07-08 17:38 | git | Note Added: 0102381 | |
| 2021-07-09 11:22 |
|
Note Added: 0102391 | |
| 2021-07-09 11:22 |
|
Assigned To | abulychev-ext => kgv |
| 2021-07-09 11:22 |
|
Status | assigned => resolved |
| 2021-07-09 11:43 | kgv | Note Added: 0102392 | |
| 2021-07-09 11:43 | kgv | Assigned To | kgv => bugmaster |
| 2021-07-09 11:43 | kgv | Status | resolved => reviewed |
| 2021-07-10 12:23 | bugmaster | Note Added: 0102434 | |
| 2021-07-10 12:23 | bugmaster | Status | reviewed => tested |
| 2021-07-10 12:28 | bugmaster | Test case number | => bugs/modalg_6/bug31047 |
| 2021-07-10 12:30 | bugmaster | Changeset attached | => occt master cb7f9239 |
| 2021-07-10 12:30 | bugmaster | Status | tested => verified |
| 2021-07-10 12:30 | bugmaster | Resolution | open => fixed |
| 2021-07-10 12:40 | git | Note Added: 0102437 |
