From 2a53911c52bc6857ef0afd9ea67e460cda51d73c Mon Sep 17 00:00:00 2001 From: Richard Brice <37087370+RickBrice@users.noreply.github.com> Date: Mon, 23 Oct 2023 08:22:03 -0700 Subject: [PATCH] Removed assert, added Logger::Warning and Logger::Error. Made treatment of unexpected data more permissive --- src/ifcgeom/mapping/IfcCurveSegment.cpp | 68 +++++++++++++--------- src/ifcgeom/mapping/IfcLinearPlacement.cpp | 56 +++++++++++------- 2 files changed, 75 insertions(+), 49 deletions(-) diff --git a/src/ifcgeom/mapping/IfcCurveSegment.cpp b/src/ifcgeom/mapping/IfcCurveSegment.cpp index cd3e541246..01458b975c 100644 --- a/src/ifcgeom/mapping/IfcCurveSegment.cpp +++ b/src/ifcgeom/mapping/IfcCurveSegment.cpp @@ -137,11 +137,13 @@ public: m.col(1) = Eigen::Vector4d(0, 1, 0, 0); m.col(2) = Eigen::Vector4d(-dy, 0, dx, 0); m.col(3) = Eigen::Vector4d(0, 0, y, 1.0); // y is an elevation so store it as z + } else if (segment_type == ST_CANT) { + Logger::Warning(std::runtime_error("Use of IfcSpiral for cant is not supported")); } else { - assert(segment_type == ST_CANT); // if it isn't cant, is there a new segment type? - assert(false); // not expecting cant + Logger::Error(std::runtime_error("Unexpected segment type encountered")); } + Eigen::Matrix4d result = transformation_matrix * m; return result; }; @@ -263,7 +265,6 @@ public: auto dx = cos(angle); auto dy = sin(angle); - auto dz = 1.0; auto x = R * dx; auto y = R * dy; @@ -281,11 +282,13 @@ public: m.col(1) = Eigen::Vector4d(0, 1, 0, 0); m.col(2) = Eigen::Vector4d(-dy, 0, dx, 0); m.col(3) = Eigen::Vector4d(0, 0, y, 1.0); // y is an elevation so store it as z + } else if (segment_type == ST_CANT) { + Logger::Warning(std::runtime_error("Use of IfcCircle for cant is not supported")); } else { - assert(segment_type == ST_CANT); // if it isn't cant, is there a new segment type? - assert(false); // not expecting cant + Logger::Error(std::runtime_error("Unexpected segment type encountered")); } + Eigen::Matrix4d result = transformation_matrix * m; return result; }; @@ -313,12 +316,15 @@ public: auto std_compare = [](double u_start, double u, double u_end) {return u_start <= u && u < u_end; }; auto end_compare = [](double u_start, double u, double u_end) {return u_start <= u && u <= (u_end + 0.001); }; - auto iter = p->begin(); + auto begin = p->begin(); + auto iter = begin; auto end = p->end(); auto last = std::prev(end); auto p1 = *(iter++); - assert(p1->Coordinates().size() == 2); // expecting the polyline to be planar - auto u = 0.0; + + if (p1->Coordinates().size() != 2) Logger::Warning("Expected IfcPolyline.Points to be 2D",pl); + + auto u = 0.0; for (; iter != end; iter++) { auto p2 = *iter; @@ -332,10 +338,13 @@ public: auto dx = p2x - p1x; auto dy = p2y - p1y; auto l = sqrt(dx * dx + dy * dy); - if (l == 0.0) + + if (l < mapping_->conversion_settings().getValue(ConversionSettings::GV_PRECISION)) { - // @todo: rb use closeness tolerance instead of absolute 0.0 - throw std::runtime_error("invalid polyline - points must not be coincident"); + std::ostringstream os; + os << "Coincident IfcPolyline.Points are not expected. Skipping point " << std::distance(iter, begin) << std::endl; + Logger::Warning(os.str(), pl); + continue; // go to next point } dx /= l; @@ -360,9 +369,10 @@ public: m.col(1) = Eigen::Vector4d(0, 1, 0, 0); m.col(2) = Eigen::Vector4d(-dy, 0, dx, 0); m.col(3) = Eigen::Vector4d(0, 0, y, 1.0); // y is an elevation so store it as z + } else if (segment_type == ST_CANT) { + Logger::Warning(std::runtime_error("Use of IfcPolyline for cant is not supported")); } else { - assert(segment_type == ST_CANT); // if it isn't cant, is there a new segment type? - assert(false); // not expecting cant + Logger::Error(std::runtime_error("Unexpected segment type encountered")); } return m; @@ -381,10 +391,11 @@ public: return compare(u_start, u, u_end); }); - if (iter == fns.end()) throw std::runtime_error("invalid distance from start"); // this should never happen, but just in case it does + if (iter == fns.end()) throw std::runtime_error("invalid distance from start"); // this should never happen, but just in case it does, throw an exception so the problem gets automatically detected - auto [u_start, u_end, compare] = iter->first; - auto m = (iter->second)(u - u_start); // (u - u_start) is distance from start of this segment of the polyline + const auto& [u_start, u_end, compare] = iter->first; + const auto& fn = iter->second; + Eigen::Matrix4d m = fn(u - u_start); // (u - u_start) is distance from start of this segment of the polyline return m; }; } @@ -420,7 +431,6 @@ public: eval_ = [px, py, dx, dy](double u) { // https://standards.buildingsmart.org/IFC/RELEASE/IFC4_3/HTML/lexical/IfcGradientCurve.htm // the parameter, u, is the parameter of the BaseCurve (u = plan view distance along base curve) - auto x = px + u; // dx and dy are normalized so u needs to be scaled by dy/dx // Consider a 5% uphill grade defined by dr[0] = 1 and dr[1] = 0.05. @@ -438,10 +448,12 @@ public: return m; }; } + else if(segment_type_ == ST_CANT) { + Logger::Warning(std::runtime_error("Use of IfcLine for cant is not supported"), l); + } else { - assert(segment_type_ == ST_CANT); // if it isn't cant, is there a new segment type? - assert(false); // not expecting cant - } + Logger::Error(std::runtime_error("Unexpected segment type encountered"), l); + } } void operator()(IfcSchema::IfcPolynomialCurve* p) { @@ -449,7 +461,7 @@ public: auto coeffX = p->CoefficientsX().get_value_or(std::vector()); auto coeffY = p->CoefficientsY().get_value_or(std::vector()); auto coeffZ = p->CoefficientsZ().get_value_or(std::vector()); - assert(coeffZ.size() == 0); // expecting the curve to by in the XY Plane (ST_HORIZONTAL) or the UZ Plane (ST_VERTICAL) + Logger::Warning("Expected IfcPolynomialCurve.CoefficientsZ to be undefined for alignment geometry", p); auto transformation_matrix = taxonomy::cast(mapping_->map(p->Position()))->ccomponents(); @@ -493,13 +505,13 @@ public: m.col(1) = Eigen::Vector4d(0, 1, 0, 0); m.col(2) = Eigen::Vector4d(dy, 0, dx, 0); m.col(3) = Eigen::Vector4d(0, 0, y, 1.0); // y is an elevation so store it as z - } - else - { - assert(segment_type == ST_CANT); // if it isn't cant, is there a new segment type? - assert(false); // not expecting cant - } - return m; + } else if (segment_type == ST_CANT) { + Logger::Warning(std::runtime_error("Use of IfcPolynomialCurve for cant is not supported")); + } else { + Logger::Error(std::runtime_error("Unexpected segment type encountered")); + } + + return m; }; } diff --git a/src/ifcgeom/mapping/IfcLinearPlacement.cpp b/src/ifcgeom/mapping/IfcLinearPlacement.cpp index 5f0b8fd531..6e205bc3d3 100644 --- a/src/ifcgeom/mapping/IfcLinearPlacement.cpp +++ b/src/ifcgeom/mapping/IfcLinearPlacement.cpp @@ -25,23 +25,15 @@ using namespace ifcopenshell::geometry; taxonomy::ptr mapping::map_impl(const IfcSchema::IfcLinearPlacement* inst) { - // The IFC specification does not provided a description of the optional - // CartesianPosition attribute. It is assumed to be a pre-computed IfcAxis2Placement3D - // to be used by software that don't support IfcLinearPlacement. In this case, - // we will simply use the intended CartesianPosition provided by the IFC model - if (inst->CartesianPosition()) { - return map(inst->CartesianPosition()); - } - - // the following is taken from IfcLocalPlacement and tweaked a little - // assumes that PlacementRelTo is relative to another IfcLinearPlacement - IfcSchema::IfcLinearPlacement* current = (IfcSchema::IfcLinearPlacement*)inst; - auto m4 = taxonomy::make(); - + // IfcLinearPlacement was added in IFC 4.1 but it had an Orientation attribute of type IfcOrientationExpression, which when combined with other + // attributes was similar to IfcAxis2PlacementLinear. IFC 4.1 and IFC 4.2 have been withdrawn so I'm not going to try to implement linear placement for them. + // For this reason the preprocessor skips this code if SCHEMA_HAS_IfcAxis2PlacementLinear is not defined #if defined SCHEMA_HAS_IfcAxis2PlacementLinear - // IfcLinearPlacement was added in IFC 4.1 but it had an Orientation attribute of type IfcOrientationExpression, which when combined with other - // attributes was similar to IfcAxis2PlacementLinear. IFC 4.1 and IFC 4.2 have been withdrawn so I'm not going to try to implement linear placement for them. - // For this reason the preprocessor skips this code if SCHEMA_HAS_IfcAxix2PlacementLinear is not defined + // the following is taken from IfcLocalPlacement and tweaked a little + // assumes that PlacementRelTo is relative to another IfcLinearPlacement + IfcSchema::IfcLinearPlacement* current = (IfcSchema::IfcLinearPlacement*)inst; + auto m4 = taxonomy::make(); + for (;;) { IfcSchema::IfcAxis2PlacementLinear* relplacement = current->RelativePlacement(); @@ -76,12 +68,34 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcLinearPlacement* inst) { break; } } + + // The IFC specification does not provided a description of the optional + // CartesianPosition attribute. It is assumed to be a pre-computed IfcAxis2Placement3D + // to be used by software that don't support IfcLinearPlacement. Check that the + // provided cartesian position is the same as the one determined by linear placement + if (inst->CartesianPosition()) { + auto m_fallback = taxonomy::cast(map(inst->CartesianPosition())); + if (m4 != m_fallback) { + Logger::Warning("IfcLinearPlacement.CartesianPosition is different than the computed placement", inst); + } + } + + // @todo: rb - not sure what this means... it came from IfcLocalPlacement + // m4->components() = offset_and_rotation_ * m4->components(); + + return m4; + +#else + // The IFC specification does not provided a description of the optional + // CartesianPosition attribute. It is assumed to be a pre-computed IfcAxis2Placement3D + // to be used by software that don't support IfcLinearPlacement. In this case, + // we will simply use the intended CartesianPosition provided by the IFC model + if (inst->CartesianPosition()) { + return map(inst->CartesianPosition()); + } else { + Logger::Error("Unsupported"); + } #endif // SCHEMA_HAS_IfcAxis2PlacementLinear - - // @todo: rb - not sure what this means... it came from IfcLocalPlacement - // m4->components() = offset_and_rotation_ * m4->components(); - - return m4; } #endif