From 2eaae2bda89071750f481d673c1ce689f5b73287 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Wed, 12 Sep 2018 13:26:11 +0200 Subject: [PATCH 01/11] Use BVH tree for wire intersections --- src/ifcgeom/IfcGeomFunctions.cpp | 50 ++++++++++++++++++++++++++------ src/ifcparse/IfcLogger.cpp | 6 +++- 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/ifcgeom/IfcGeomFunctions.cpp b/src/ifcgeom/IfcGeomFunctions.cpp index feb851b803..f23397c923 100644 --- a/src/ifcgeom/IfcGeomFunctions.cpp +++ b/src/ifcgeom/IfcGeomFunctions.cpp @@ -44,6 +44,9 @@ #include #include +#include +#include + #include #include #include @@ -143,6 +146,7 @@ #include "../ifcparse/IfcSIPrefix.h" #include "../ifcparse/IfcFile.h" #include "../ifcgeom/IfcGeom.h" +#include "../ifcgeom/IfcGeomTree.h" #if OCC_VERSION_HEX < 0x60900 #ifdef _MSC_VER @@ -2876,10 +2880,7 @@ bool IfcGeom::Kernel::wire_intersections(const TopoDS_Wire& wire, TopTools_ListO } int n = count(wire, TopAbs_EDGE); - if (n < 3 || n > 128) { - if (n > 128) { - Logger::Notice("Too many segments for detection of self-intersections"); - } + if (n < 3) { wires.Append(wire); return false; } @@ -2889,8 +2890,15 @@ bool IfcGeom::Kernel::wire_intersections(const TopoDS_Wire& wire, TopTools_ListO // ... to be sure to get consecutive edges BRepTools_WireExplorer exp(wire); + IfcGeom::impl::tree tree; + + int edge_idx = 0; for (; exp.More(); exp.Next()) { wd->Add(exp.Current()); + if (n > 64) { + // tfk: indices in tree are 0-based vd 1-based in wiredata + tree.add(edge_idx++, exp.Current()); + } } bool intersected = false; @@ -2898,13 +2906,37 @@ bool IfcGeom::Kernel::wire_intersections(const TopoDS_Wire& wire, TopTools_ListO // tfk: Extrema on infinite curves proved to be more robust. // TopoDS_Face face = BRepBuilderAPI_MakeFace(wire, true).Face(); // ShapeAnalysis_Wire saw(wd, face, getValue(GV_PRECISION)); - - for (int i = 2; i < n; ++i) { - for (int j = 0; j < i - 1; ++j) { - if (i == n - 1 && j == 0) continue; + + const double eps = getValue(GV_PRECISION) * 10.; + for (int i = 2; i < n; ++i) { + + std::vector js; + if (n > 64) { + Bnd_Box b; + BRepBndLib::Add(wd->Edge(i + 1), b); + b.Enlarge(eps); + js = tree.select_box(b, false); + } else { + boost::push_back(js, boost::irange(0, i - 1)); + } + + for(std::vector::const_iterator it = js.begin(); it != js.end(); ++it) { + int j = *it; + + if (n > 64) { + if (j > i) { + continue; + } + if ((std::max)(i, j) - (std::min)(i, j) <= 1) { + continue; + } + } + + // Only check non-consecutive edges + if (i == n - 1 && j == 0) continue; + bool unbounded_intersects; - const double eps = getValue(GV_PRECISION) * 10.; double u11, u12, u21, u22, U1, U2; GeomAPI_ExtremaCurveCurve ecc( diff --git a/src/ifcparse/IfcLogger.cpp b/src/ifcparse/IfcLogger.cpp index 2fcc9d3c2f..a9b43907dd 100644 --- a/src/ifcparse/IfcLogger.cpp +++ b/src/ifcparse/IfcLogger.cpp @@ -42,7 +42,11 @@ namespace { } os << message << std::endl; if (entity) { - os << entity->toString() << std::endl; + std::string instance_string = entity->toString(); + if (instance_string.size() > 259) { + instance_string = instance_string.substr(0, 256) + "..."; + } + os << instance_string << std::endl; } } From 12623dd1e5b1d829ef9b6a1911663d15ca6526a7 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Wed, 12 Sep 2018 13:57:41 +0200 Subject: [PATCH 02/11] Only install/strip on Release build --- nix/build-all.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nix/build-all.py b/nix/build-all.py index 98de3f4322..2279801d02 100644 --- a/nix/build-all.py +++ b/nix/build-all.py @@ -556,7 +556,7 @@ run_cmake("", cmake_args=[ logger.info("\rBuilding executables... ") __check_call__([make, "-j%s" % (IFCOS_NUM_BUILD_PROCS,)], cwd=executables_dir) -__check_call__([make, "install/strip"], cwd=executables_dir) +__check_call__([make, "install/strip" if BUILD_CFG == "Release" else "install"], cwd=executables_dir) # On OSX the actual Python library is not linked against. ADDITIONAL_ARGS="" From 0df2bfadef0f415e9451d92807e0fc4e5ada3f8c Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Wed, 12 Sep 2018 16:44:30 +0200 Subject: [PATCH 03/11] Don't sew overly complex geometries even as part of boolean operations #455 --- src/ifcgeom/IfcGeom.h | 17 +++++++++++++++++ src/ifcgeom/IfcGeomFunctions.cpp | 15 ++++++++++++++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/ifcgeom/IfcGeom.h b/src/ifcgeom/IfcGeom.h index d6472287cc..16832cce65 100644 --- a/src/ifcgeom/IfcGeom.h +++ b/src/ifcgeom/IfcGeom.h @@ -73,6 +73,23 @@ if ( it != cache.T.end() ) { e = it->second; return true; } #endif namespace IfcGeom { + class IFC_PARSE_API geometry_exception : public std::exception { + protected: + std::string message; + public: + geometry_exception(const std::string& m) + : message(m) {} + virtual ~geometry_exception() throw () {} + virtual const char* what() const throw() { + return message.c_str(); + } + }; + + class IFC_PARSE_API too_many_faces_exception : public geometry_exception { + public: + too_many_faces_exception() + : geometry_exception("Too many faces for operation") {} + }; class IFC_GEOM_API Cache { public: diff --git a/src/ifcgeom/IfcGeomFunctions.cpp b/src/ifcgeom/IfcGeomFunctions.cpp index f23397c923..98cf09568e 100644 --- a/src/ifcgeom/IfcGeomFunctions.cpp +++ b/src/ifcgeom/IfcGeomFunctions.cpp @@ -174,12 +174,23 @@ bool IfcGeom::Kernel::create_solid_from_compound(const TopoDS_Shape& compound, T bool IfcGeom::Kernel::create_solid_from_faces(const TopTools_ListOfShape& face_list, TopoDS_Shape& shape) { bool valid_shell = false; + + int max_faces = getValue(GV_MAX_FACES_TO_SEW); + if (max_faces == -1) { + max_faces = 1000; + } + + if (face_list.Extent() > max_faces) { + throw too_many_faces_exception(); + } + TopTools_ListIteratorOfListOfShape face_iterator; BRepOffsetAPI_Sewing builder; builder.SetTolerance(getValue(GV_PRECISION)); builder.SetMaxTolerance(getValue(GV_PRECISION)); builder.SetMinTolerance(getValue(GV_PRECISION)); + for (face_iterator.Initialize(face_list); face_iterator.More(); face_iterator.Next()) { builder.Add(face_iterator.Value()); } @@ -1324,7 +1335,9 @@ IfcGeom::BRepElement

* IfcGeom::Kernel::create_brep_for_representation_and_pro } else { convert_openings(product,openings,shapes,trsf,opened_shapes); } - } catch(...) { + } catch (const std::exception& e) { + Logger::Message(Logger::LOG_ERROR, std::string("Error processing openings for: ") + e.what() + ":", product->entity); + } catch(...) { Logger::Message(Logger::LOG_ERROR,"Error processing openings for:",product->entity); } if (settings.get(IteratorSettings::USE_WORLD_COORDS)) { From 452ab6e5bc185948f0debb796d254556cf957dd5 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Mon, 17 Sep 2018 11:33:40 +0200 Subject: [PATCH 04/11] Fix flaw in vertex-edge distance used to limit tolerance in boolean ops --- src/ifcgeom/IfcGeomFunctions.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ifcgeom/IfcGeomFunctions.cpp b/src/ifcgeom/IfcGeomFunctions.cpp index 98cf09568e..eb0f8c9c4f 100644 --- a/src/ifcgeom/IfcGeomFunctions.cpp +++ b/src/ifcgeom/IfcGeomFunctions.cpp @@ -3295,7 +3295,7 @@ namespace { TopoDS_Vertex v1, v2; TopExp::Vertices(e, v1, v2); - if (v.IsEqual(v1) || v.IsEqual(v2)) { + if (v.IsSame(v1) || v.IsSame(v2)) { continue; } From 51a9bb0ac13d9810d20e06cf51b91c082c7eb1ab Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Mon, 17 Sep 2018 12:00:49 +0200 Subject: [PATCH 05/11] Retry boolean operations if output is non-manifold on manifold input --- src/ifcgeom/IfcGeomFunctions.cpp | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/ifcgeom/IfcGeomFunctions.cpp b/src/ifcgeom/IfcGeomFunctions.cpp index eb0f8c9c4f..9f3a06a46a 100644 --- a/src/ifcgeom/IfcGeomFunctions.cpp +++ b/src/ifcgeom/IfcGeomFunctions.cpp @@ -3317,6 +3317,30 @@ namespace { 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; + } + } bool IfcGeom::Kernel::boolean_operation(const TopoDS_Shape& a, const TopTools_ListOfShape& b, BOPAlgo_Operation op, TopoDS_Shape& result, double fuzziness) { @@ -3376,7 +3400,12 @@ bool IfcGeom::Kernel::boolean_operation(const TopoDS_Shape& a, const TopTools_Li success = BRepCheck_Analyzer(r).IsValid() != 0; if (success) { - result = r; + + success = !is_manifold(a) || is_manifold(r); + + if (success) { + result = r; + } } } delete builder; From b05b62f44063fbd423f764fed21f5b3c1f732207 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Mon, 17 Sep 2018 12:02:41 +0200 Subject: [PATCH 06/11] Perform additional check on boolean result output for small edges or distances --- src/ifcgeom/IfcGeomFunctions.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/ifcgeom/IfcGeomFunctions.cpp b/src/ifcgeom/IfcGeomFunctions.cpp index 9f3a06a46a..54421f1b7e 100644 --- a/src/ifcgeom/IfcGeomFunctions.cpp +++ b/src/ifcgeom/IfcGeomFunctions.cpp @@ -3404,7 +3404,15 @@ bool IfcGeom::Kernel::boolean_operation(const TopoDS_Shape& a, const TopTools_Li success = !is_manifold(a) || is_manifold(r); if (success) { - result = r; + + // when there are edges or vertex-edge distances close to the used fuzziness, the + // output is not trusted and the operation is attempted with a higher fuzziness. + double min_len_check = (std::min)(min_edge_length(r), min_vertex_edge_distance(r, getValue(GV_PRECISION))); + success = min_len_check > fuzziness * 10.; + + if (success) { + result = r; + } } } } From 983745b50885a0ff9db2b3f61f0d957f25de042b Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Mon, 17 Sep 2018 12:05:30 +0200 Subject: [PATCH 07/11] More careful face orientation check --- src/ifcgeom/IfcGeomFaces.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ifcgeom/IfcGeomFaces.cpp b/src/ifcgeom/IfcGeomFaces.cpp index b0ea4dd278..d0bf5c8175 100644 --- a/src/ifcgeom/IfcGeomFaces.cpp +++ b/src/ifcgeom/IfcGeomFaces.cpp @@ -339,7 +339,9 @@ bool IfcGeom::Kernel::convert(const IfcSchema::IfcFace* l, TopoDS_Shape& face) { TopoDS_Iterator jt(face, false); for (; jt.More(); jt.Next()) { const TopoDS_Wire& w = TopoDS::Wire(jt.Value()); - if (wire_map.IsBound(w)) { + // tfk: @todo if wire_map contains w, I would assume wire_senses also contains w, + // this is not the case in github issue #405. + if (wire_map.IsBound(w) && wire_senses.IsBound(w)) { const TopTools_ListOfShape& shapes = wire_map.Find(w); TopTools_ListIteratorOfListOfShape it(shapes); for (; it.More(); it.Next()) { From 394a7f14ad58582c540c3728725e006555e9a778 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Mon, 17 Sep 2018 14:11:03 +0200 Subject: [PATCH 08/11] Emit unopened shapes in case of exceptions during subtraction --- src/ifcgeom/IfcGeomFunctions.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/ifcgeom/IfcGeomFunctions.cpp b/src/ifcgeom/IfcGeomFunctions.cpp index 54421f1b7e..a344d03526 100644 --- a/src/ifcgeom/IfcGeomFunctions.cpp +++ b/src/ifcgeom/IfcGeomFunctions.cpp @@ -1316,6 +1316,7 @@ IfcGeom::BRepElement

* IfcGeom::Kernel::create_brep_for_representation_and_pro } IfcGeom::IfcRepresentationShapeItems opened_shapes; + bool caught_error = false; try { #if OCC_VERSION_HEX < 0x60900 const bool faster_booleans = settings.get(IteratorSettings::FASTER_BOOLEANS); @@ -1337,9 +1338,16 @@ IfcGeom::BRepElement

* IfcGeom::Kernel::create_brep_for_representation_and_pro } } catch (const std::exception& e) { Logger::Message(Logger::LOG_ERROR, std::string("Error processing openings for: ") + e.what() + ":", product->entity); + caught_error = true; } catch(...) { Logger::Message(Logger::LOG_ERROR,"Error processing openings for:",product->entity); + caught_error = true; } + + if (caught_error && opened_shapes.size() < shapes.size()) { + opened_shapes = shapes; + } + if (settings.get(IteratorSettings::USE_WORLD_COORDS)) { for ( IfcGeom::IfcRepresentationShapeItems::iterator it = opened_shapes.begin(); it != opened_shapes.end(); ++ it ) { it->prepend(trsf); From 3101ccb83a96a36367ce89a4570cb78fd2af073c Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Mon, 17 Sep 2018 14:42:22 +0200 Subject: [PATCH 09/11] Fixes for degenerate polylines --- src/ifcgeom/IfcGeomWires.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/ifcgeom/IfcGeomWires.cpp b/src/ifcgeom/IfcGeomWires.cpp index 99ad42099e..704e8abbb7 100644 --- a/src/ifcgeom/IfcGeomWires.cpp +++ b/src/ifcgeom/IfcGeomWires.cpp @@ -492,7 +492,7 @@ bool IfcGeom::Kernel::convert(const IfcSchema::IfcPolyline* l, TopoDS_Wire& resu } const double eps = getValue(GV_PRECISION) * 10; - const bool closed_by_proximity = polygon.Length() >= 2 && polygon.First().Distance(polygon.Last()) < eps; + const bool closed_by_proximity = polygon.Length() >= 3 && polygon.First().Distance(polygon.Last()) < eps; if (closed_by_proximity) { // tfk: note 1-based polygon.Remove(polygon.Length()); @@ -500,6 +500,10 @@ bool IfcGeom::Kernel::convert(const IfcSchema::IfcPolyline* l, TopoDS_Wire& resu // Remove points that are too close to one another remove_duplicate_points_from_loop(polygon, closed_by_proximity, eps); + + if (polygon.Length() < 2) { + return false; + } BRepBuilderAPI_MakePolygon w; for (int i = 1; i <= polygon.Length(); ++i) { From a54b7863cd16bdde38828dbd594783d25b6a3220 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Mon, 17 Sep 2018 14:59:15 +0200 Subject: [PATCH 10/11] Fail early on duplicate (non consecutive) points in face loops --- src/ifcgeom/IfcGeomFunctions.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/ifcgeom/IfcGeomFunctions.cpp b/src/ifcgeom/IfcGeomFunctions.cpp index a344d03526..9cc7f89b09 100644 --- a/src/ifcgeom/IfcGeomFunctions.cpp +++ b/src/ifcgeom/IfcGeomFunctions.cpp @@ -2921,6 +2921,15 @@ bool IfcGeom::Kernel::wire_intersections(const TopoDS_Wire& wire, TopTools_ListO tree.add(edge_idx++, exp.Current()); } } + + if (wd->NbEdges() != n) { + // If the number of edges differs, BRepTools_WireExplorer did not + // reach every edge, probably due to loops exactly at vertex locations. + // This is not supported by this algorithm which only elimates loops + // due to edge crossings. + + throw geometry_exception("Invalid loop"); + } bool intersected = false; From 08557c2664b82a9ee4a0979da3ecd1169d16565d Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Mon, 17 Sep 2018 15:21:12 +0200 Subject: [PATCH 11/11] Don't fail on empty bounding box during halfspace processing --- src/ifcgeom/IfcGeomFunctions.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ifcgeom/IfcGeomFunctions.cpp b/src/ifcgeom/IfcGeomFunctions.cpp index 9cc7f89b09..d9de6aa121 100644 --- a/src/ifcgeom/IfcGeomFunctions.cpp +++ b/src/ifcgeom/IfcGeomFunctions.cpp @@ -3127,6 +3127,10 @@ bool IfcGeom::Kernel::fit_halfspace(const TopoDS_Shape& a, const TopoDS_Shape& b Bnd_Box bb; BRepBndLib::Add(a, bb); + if (bb.IsVoid()) { + return false; + } + double xs[2], ys[2], zs[2]; bb.Get(xs[0], ys[0], zs[0], xs[1], ys[1], zs[1]);