From 3840bd1154e5c889ba64afb73f4b216dbd05bc00 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Fri, 21 Sep 2018 10:22:24 +0200 Subject: [PATCH 1/5] Limit tolerance on wire self-intersection to edge segment length --- src/ifcgeom/IfcGeomFunctions.cpp | 244 +++++++++++++++---------------- 1 file changed, 122 insertions(+), 122 deletions(-) diff --git a/src/ifcgeom/IfcGeomFunctions.cpp b/src/ifcgeom/IfcGeomFunctions.cpp index 02a145dd3c..1fd1b78d1a 100644 --- a/src/ifcgeom/IfcGeomFunctions.cpp +++ b/src/ifcgeom/IfcGeomFunctions.cpp @@ -156,6 +156,126 @@ #endif #endif +namespace { + TopTools_ListOfShape copy_operand(const TopTools_ListOfShape& l) { +#if OCC_VERSION_HEX < 0x70000 + TopTools_ListOfShape r; + TopTools_ListIteratorOfListOfShape it(l); + for (; it.More(); it.Next()) { + r.Append(BRepBuilderAPI_Copy(it.Value())); + } + return r; +#else + // On OCCT 7.0 and higher BRepAlgoAPI_BuilderAlgo::SetNonDestructive(true) is + // called. Not entirely sure on the behaviour before 7.0, so overcautiously + // create copies. + return l; +#endif + } + + TopoDS_Shape copy_operand(const TopoDS_Shape& s) { +#if OCC_VERSION_HEX < 0x70000 + return BRepBuilderAPI_Copy(s); +#else + return s; +#endif + } + + double min_edge_length(const TopoDS_Shape& a) { + double min_edge_len = std::numeric_limits::infinity(); + TopExp_Explorer exp(a, TopAbs_EDGE); + for (; exp.More(); exp.Next()) { + GProp_GProps prop; + BRepGProp::LinearProperties(exp.Current(), prop); + double l = prop.Mass(); + if (l < min_edge_len) { + min_edge_len = l; + } + } + return min_edge_len; + } + + double min_vertex_edge_distance(const TopoDS_Shape& a, double t) { + TopExp_Explorer exp(a, TopAbs_VERTEX); + + double M = std::numeric_limits::infinity(); + + for (; exp.More(); exp.Next()) { + if (exp.Current().Orientation() != TopAbs_FORWARD) { + continue; + } + + const TopoDS_Vertex& v = TopoDS::Vertex(exp.Current()); + gp_Pnt p = BRep_Tool::Pnt(v); + + TopExp_Explorer exp2(a, TopAbs_EDGE); + for (; exp2.More(); exp2.Next()) { + const TopoDS_Edge& e = TopoDS::Edge(exp2.Current()); + TopoDS_Vertex v1, v2; + TopExp::Vertices(e, v1, v2); + + if (v.IsSame(v1) || v.IsSame(v2)) { + continue; + } + + BRepAdaptor_Curve crv(e); + Extrema_ExtPC ext(p, crv); + if (!ext.IsDone()) { + continue; + } + + for (int i = 1; i <= ext.NbExt(); ++i) { + const double m = sqrt(ext.SquareDistance(i)); + if (m < M && m > t) { + M = m; + } + } + } + } + + return M; + } + + bool is_manifold(const TopoDS_Shape& a) { + TopTools_IndexedDataMapOfShapeListOfShape map; + TopExp::MapShapesAndAncestors(a, TopAbs_EDGE, TopAbs_FACE, map); + + for (int i = 1; i <= map.Extent(); ++i) { + if (map.FindFromIndex(i).Extent() != 2) { + return false; + } + } + + return true; + } + + bool is_manifold(const TopTools_ListOfShape& l) { + TopTools_ListOfShape r; + TopTools_ListIteratorOfListOfShape it(l); + for (; it.More(); it.Next()) { + if (!is_manifold(it.Value())) { + return false; + } + } + return true; + } + + void bounding_box_overlap(double p, const TopoDS_Shape& a, const TopTools_ListOfShape& b, TopTools_ListOfShape& c) { + Bnd_Box A; + BRepBndLib::Add(a, A); + + TopTools_ListIteratorOfListOfShape it(b); + for (; it.More(); it.Next()) { + Bnd_Box B; + BRepBndLib::Add(it.Value(), B); + + if (A.Distance(B) < p) { + c.Append(it.Value()); + } + } + } +} + bool IfcGeom::Kernel::create_solid_from_compound(const TopoDS_Shape& compound, TopoDS_Shape& shape) { TopTools_ListOfShape face_list; TopExp_Explorer exp(compound, TopAbs_FACE); @@ -2937,7 +3057,7 @@ bool IfcGeom::Kernel::wire_intersections(const TopoDS_Wire& wire, TopTools_ListO // TopoDS_Face face = BRepBuilderAPI_MakeFace(wire, true).Face(); // ShapeAnalysis_Wire saw(wd, face, getValue(GV_PRECISION)); - const double eps = getValue(GV_PRECISION) * 10.; + const double eps = (std::min)(min_edge_length(wire) / 2., getValue(GV_PRECISION) * 10.); for (int i = 2; i < n; ++i) { @@ -2974,6 +3094,7 @@ bool IfcGeom::Kernel::wire_intersections(const TopoDS_Wire& wire, TopTools_ListO BRep_Tool::Curve(wd->Edge(j + 1), u21, u22) ); + // @todo: extend this to work in case of multiple extrema and curved segments. if ((unbounded_intersects = (ecc.NbExtrema() == 1 && ecc.Distance(1) < eps))) { ecc.Parameters(1, U1, U2); } @@ -3257,127 +3378,6 @@ bool IfcGeom::Kernel::boolean_operation(const TopoDS_Shape& a, const TopoDS_Shap return succesful; } #else - -namespace { - TopTools_ListOfShape copy_operand(const TopTools_ListOfShape& l) { -#if OCC_VERSION_HEX < 0x70000 - TopTools_ListOfShape r; - TopTools_ListIteratorOfListOfShape it(l); - for (; it.More(); it.Next()) { - r.Append(BRepBuilderAPI_Copy(it.Value())); - } - return r; -#else - // On OCCT 7.0 and higher BRepAlgoAPI_BuilderAlgo::SetNonDestructive(true) is - // called. Not entirely sure on the behaviour before 7.0, so overcautiously - // create copies. - return l; -#endif - } - - TopoDS_Shape copy_operand(const TopoDS_Shape& s) { -#if OCC_VERSION_HEX < 0x70000 - return BRepBuilderAPI_Copy(s); -#else - return s; -#endif - } - - double min_edge_length(const TopoDS_Shape& a) { - double min_edge_len = std::numeric_limits::infinity(); - TopExp_Explorer exp(a, TopAbs_EDGE); - for (; exp.More(); exp.Next()) { - GProp_GProps prop; - BRepGProp::LinearProperties(exp.Current(), prop); - double l = prop.Mass(); - if (l < min_edge_len) { - min_edge_len = l; - } - } - return min_edge_len; - } - - double min_vertex_edge_distance(const TopoDS_Shape& a, double t) { - TopExp_Explorer exp(a, TopAbs_VERTEX); - - double M = std::numeric_limits::infinity(); - - for (; exp.More(); exp.Next()) { - if (exp.Current().Orientation() != TopAbs_FORWARD) { - continue; - } - - const TopoDS_Vertex& v = TopoDS::Vertex(exp.Current()); - gp_Pnt p = BRep_Tool::Pnt(v); - - TopExp_Explorer exp2(a, TopAbs_EDGE); - for (; exp2.More(); exp2.Next()) { - const TopoDS_Edge& e = TopoDS::Edge(exp2.Current()); - TopoDS_Vertex v1, v2; - TopExp::Vertices(e, v1, v2); - - if (v.IsSame(v1) || v.IsSame(v2)) { - continue; - } - - BRepAdaptor_Curve crv(e); - Extrema_ExtPC ext(p, crv); - if (!ext.IsDone()) { - continue; - } - - for (int i = 1; i <= ext.NbExt(); ++i) { - const double m = sqrt(ext.SquareDistance(i)); - if (m < M && m > t) { - M = m; - } - } - } - } - - return M; - } - - bool is_manifold(const TopoDS_Shape& a) { - TopTools_IndexedDataMapOfShapeListOfShape map; - TopExp::MapShapesAndAncestors(a, TopAbs_EDGE, TopAbs_FACE, map); - - for (int i = 1; i <= map.Extent(); ++i) { - if (map.FindFromIndex(i).Extent() != 2) { - return false; - } - } - - return true; - } - - bool is_manifold(const TopTools_ListOfShape& l) { - TopTools_ListOfShape r; - TopTools_ListIteratorOfListOfShape it(l); - for (; it.More(); it.Next()) { - if (!is_manifold(it.Value())) { - return false; - } - } - return true; - } - - void bounding_box_overlap(double p, const TopoDS_Shape& a, const TopTools_ListOfShape& b, TopTools_ListOfShape& c) { - Bnd_Box A; - BRepBndLib::Add(a, A); - - TopTools_ListIteratorOfListOfShape it(b); - for (; it.More(); it.Next()) { - Bnd_Box B; - BRepBndLib::Add(it.Value(), B); - - if (A.Distance(B) < p) { - c.Append(it.Value()); - } - } - } -} - bool IfcGeom::Kernel::boolean_operation(const TopoDS_Shape& a, const TopTools_ListOfShape& b_, BOPAlgo_Operation op, TopoDS_Shape& result, double fuzziness) { bool success = false; BRepAlgoAPI_BooleanOperation* builder; From bf4b5928dd066be4ecda370340e6372c26aa7030 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Fri, 21 Sep 2018 10:22:37 +0200 Subject: [PATCH 2/5] Fail on empty face early --- src/ifcgeom/IfcGeomFaces.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/ifcgeom/IfcGeomFaces.cpp b/src/ifcgeom/IfcGeomFaces.cpp index d0bf5c8175..d2cc535f2d 100644 --- a/src/ifcgeom/IfcGeomFaces.cpp +++ b/src/ifcgeom/IfcGeomFaces.cpp @@ -106,6 +106,11 @@ bool IfcGeom::Kernel::convert(const IfcSchema::IfcFace* l, TopoDS_Shape& face) { IfcSchema::IfcFaceBound::list::ptr bounds = l->Bounds(); + // Fail on this early as it can cause issues later on + if (bounds->size() == 0) { + return false; + } + Handle(Geom_Surface) face_surface; const bool is_face_surface = l->is(IfcSchema::Type::IfcFaceSurface); From 7205f0165eeff6186a0c4dfc6ab57f2d418bb68a Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Fri, 21 Sep 2018 11:21:51 +0200 Subject: [PATCH 3/5] Work around private copy constructor on older versions of occt --- src/ifcgeom/IfcGeomFunctions.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/ifcgeom/IfcGeomFunctions.cpp b/src/ifcgeom/IfcGeomFunctions.cpp index 1fd1b78d1a..8c8060eda4 100644 --- a/src/ifcgeom/IfcGeomFunctions.cpp +++ b/src/ifcgeom/IfcGeomFunctions.cpp @@ -157,7 +157,7 @@ #endif namespace { - TopTools_ListOfShape copy_operand(const TopTools_ListOfShape& l) { + void copy_operand(const TopTools_ListOfShape& l, TopTools_ListOfShape& r) { #if OCC_VERSION_HEX < 0x70000 TopTools_ListOfShape r; TopTools_ListIteratorOfListOfShape it(l); @@ -169,7 +169,7 @@ namespace { // On OCCT 7.0 and higher BRepAlgoAPI_BuilderAlgo::SetNonDestructive(true) is // called. Not entirely sure on the behaviour before 7.0, so overcautiously // create copies. - return l; + r.Assign(l); #endif } @@ -3381,7 +3381,7 @@ bool IfcGeom::Kernel::boolean_operation(const TopoDS_Shape& a, const TopoDS_Shap bool IfcGeom::Kernel::boolean_operation(const TopoDS_Shape& a, const TopTools_ListOfShape& b_, BOPAlgo_Operation op, TopoDS_Shape& result, double fuzziness) { bool success = false; BRepAlgoAPI_BooleanOperation* builder; - TopTools_ListOfShape b; + TopTools_ListOfShape B, b; if (op == BOPAlgo_CUT) { builder = new BRepAlgoAPI_Cut(); bounding_box_overlap(getValue(GV_PRECISION), a, b_, b); @@ -3420,7 +3420,8 @@ bool IfcGeom::Kernel::boolean_operation(const TopoDS_Shape& a, const TopTools_Li #endif builder->SetFuzzyValue(fuzz); builder->SetArguments(s1s); - builder->SetTools(copy_operand(b)); + copy_operand(b, B); + builder->SetTools(B); builder->Build(); if (builder->IsDone()) { TopoDS_Shape r = *builder; From 297812d4e2f249987a53cb68b43a45a0cdd3734e Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Fri, 21 Sep 2018 11:43:06 +0200 Subject: [PATCH 4/5] Better handling of consecutive circular arc segments --- src/ifcgeom/IfcGeomWires.cpp | 70 +++++++++++++++++++++++++++++------- 1 file changed, 58 insertions(+), 12 deletions(-) diff --git a/src/ifcgeom/IfcGeomWires.cpp b/src/ifcgeom/IfcGeomWires.cpp index 704e8abbb7..bc7294a15c 100644 --- a/src/ifcgeom/IfcGeomWires.cpp +++ b/src/ifcgeom/IfcGeomWires.cpp @@ -43,6 +43,8 @@ #include #include +#include + #include #include #include @@ -109,14 +111,57 @@ namespace { // Returns new wire with the edge replaced by a linear edge with the vertex v moved to p TopoDS_Wire adjust(const TopoDS_Wire& w, const TopoDS_Vertex& v, const gp_Pnt& p) { - BRep_Builder b; - TopoDS_Vertex v2; - b.MakeVertex(v2, p, BRep_Tool::Tolerance(v)); + TopTools_IndexedDataMapOfShapeListOfShape map; + TopExp::MapShapesAndAncestors(w, TopAbs_VERTEX, TopAbs_EDGE, map); - ShapeBuild_ReShape reshape; - reshape.Replace(v.Oriented(TopAbs_FORWARD), v2); + bool all_linear = true, single_circle = false, first = true; + TopTools_ListOfShape edges; + if (map.FindFromKey(v, edges)) { + TopTools_ListIteratorOfListOfShape it(edges); + for (; it.More(); it.Next()) { + const TopoDS_Edge& e = TopoDS::Edge(it.Value()); + double _, __; + Handle(Geom_Curve) crv = BRep_Tool::Curve(e, _, __); + const bool is_line = crv->DynamicType() == STANDARD_TYPE(Geom_Line); + const bool is_circle = crv->DynamicType() == STANDARD_TYPE(Geom_Circle); + all_linear = all_linear && all_linear; + single_circle = first && is_circle; + } + } - return TopoDS::Wire(reshape.Apply(w)); + if (all_linear) { + BRep_Builder b; + TopoDS_Vertex v2; + b.MakeVertex(v2, p, BRep_Tool::Tolerance(v)); + + ShapeBuild_ReShape reshape; + reshape.Replace(v.Oriented(TopAbs_FORWARD), v2); + + return TopoDS::Wire(reshape.Apply(w)); + } else if (single_circle) { + TopoDS_Vertex v1, v2; + TopExp::Vertices(w, v1, v2); + + gp_Pnt p1, p2, p3; + p1 = v.IsEqual(v1) ? p : BRep_Tool::Pnt(v1); + p3 = v.IsEqual(v2) ? p : BRep_Tool::Pnt(v2); + + double a, b; + Handle(Geom_Curve) crv = BRep_Tool::Curve(TopoDS::Edge(edges.First()), a, b); + crv->D0((a + b) / 2., p2); + + GC_MakeCircle mc(p1, p2, p3); + if (!mc.IsDone()) { + throw IfcGeom::geometry_exception("Failed to adjust circle"); + } + + TopoDS_Edge edge = BRepBuilderAPI_MakeEdge(mc.Value(), p1, p3).Edge(); + BRepBuilderAPI_MakeWire builder; + builder.Add(edge); + return builder.Wire(); + } else { + throw IfcGeom::geometry_exception("Unexpected wire to adjust"); + } } // A wrapper around BRepBuilderAPI_MakeWire that makes sure segments are connected either by moving end points or by adding intermediate segments @@ -193,17 +238,20 @@ namespace { const bool is_line1 = c1->DynamicType() == STANDARD_TYPE(Geom_Line); const bool is_line2 = c2->DynamicType() == STANDARD_TYPE(Geom_Line); - // Adjust the segment that is linear - if (is_line1) { + const bool is_circle1 = c1->DynamicType() == STANDARD_TYPE(Geom_Circle); + const bool is_circle2 = c2->DynamicType() == STANDARD_TYPE(Geom_Circle); + + // Preferably adjust the segment that is linear + if (is_line1 || (is_circle1 && !is_line2)) { mw_.Add(adjust(w1, w12, p2)); Logger::Message(Logger::LOG_ERROR, "Adjusted edge end-point with distance " + boost::lexical_cast(dist) + " on:", inst_->entity); - } else if (is_line2 && !last) { + } else if ((is_line2 || is_circle2) && !last) { mw_.Add(w1); override_next_ = true; next_override_ = p1; Logger::Message(Logger::LOG_ERROR, "Adjusted edge end-point with distance " + boost::lexical_cast(dist) + " on:", inst_->entity); } else { - // If both aren't linear an edge is added + // In all other cases an edge is added mw_.Add(w1); mw_.Add(BRepBuilderAPI_MakeEdge(p1, p2)); Logger::Message(Logger::LOG_ERROR, "Added additional segment to close gap with length " + boost::lexical_cast(dist) + " to:", inst_->entity); @@ -721,8 +769,6 @@ bool IfcGeom::Kernel::convert(const IfcSchema::IfcSubedge* l, TopoDS_Wire& resul #ifdef USE_IFC4 -#include - bool IfcGeom::Kernel::convert(const IfcSchema::IfcIndexedPolyCurve* l, TopoDS_Wire& result) { IfcSchema::IfcCartesianPointList* point_list = l->Points(); From 6b8681b74a06766283929f7bc70c888a69da9ede Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Fri, 21 Sep 2018 11:45:36 +0200 Subject: [PATCH 5/5] That was bad --- src/ifcgeom/IfcGeomFunctions.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/ifcgeom/IfcGeomFunctions.cpp b/src/ifcgeom/IfcGeomFunctions.cpp index 8c8060eda4..155136a744 100644 --- a/src/ifcgeom/IfcGeomFunctions.cpp +++ b/src/ifcgeom/IfcGeomFunctions.cpp @@ -159,12 +159,10 @@ namespace { void copy_operand(const TopTools_ListOfShape& l, TopTools_ListOfShape& r) { #if OCC_VERSION_HEX < 0x70000 - TopTools_ListOfShape r; TopTools_ListIteratorOfListOfShape it(l); for (; it.More(); it.Next()) { r.Append(BRepBuilderAPI_Copy(it.Value())); } - return r; #else // On OCCT 7.0 and higher BRepAlgoAPI_BuilderAlgo::SetNonDestructive(true) is // called. Not entirely sure on the behaviour before 7.0, so overcautiously