From 956881bb99dbc54fb2382cad8e8226fe6b75f87d Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Fri, 10 Jul 2026 16:28:56 +0300 Subject: [PATCH] ifcgeom: honour PnIndex in triangulated and polygonal face sets (#3434) IfcTriangulatedFaceSet and IfcPolygonalFaceSet used CoordIndex values to index Coordinates.CoordList directly, ignoring the optional PnIndex attribute. When PnIndex is present it remaps point references, so a CoordIndex value i must resolve as CoordList[PnIndex[i-1]-1] (both 1-based). Without the indirection any model carrying a PnIndex was built from the wrong points. Add a resolve() helper in both mappings that applies the PnIndex indirection when present and is a plain bounds-checked lookup otherwise, with bounds checks at both index levels. When PnIndex is absent the behavior is unchanged. Co-Authored-By: Claude Opus 4.8 (cherry picked from commit 3e55c5126c75bb41a19ece7a8f8d3509326d5e2f) --- src/ifcgeom/mapping/IfcPolygonalFaceSet.cpp | 33 ++++++++++++------- .../mapping/IfcTriangulatedFaceSet.cpp | 22 ++++++++++--- 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/src/ifcgeom/mapping/IfcPolygonalFaceSet.cpp b/src/ifcgeom/mapping/IfcPolygonalFaceSet.cpp index eb38834ee9..45f0186c32 100644 --- a/src/ifcgeom/mapping/IfcPolygonalFaceSet.cpp +++ b/src/ifcgeom/mapping/IfcPolygonalFaceSet.cpp @@ -39,8 +39,25 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcPolygonalFaceSet& inst) { int max_index = (int)points.size(); + // When the optional PnIndex is present, CoordIndex values do not index into + // CoordList directly but into PnIndex, which in turn remaps to CoordList. + // Both index levels are 1-based per the IFC specification. + auto pn_index = inst.PnIndex(); + auto resolve = [&](int64_t idx) -> const taxonomy::point3::ptr& { + if (pn_index) { + if (idx < 1 || idx > (int64_t)pn_index->size()) { + throw ifcopenshell::exception("IfcPolygonalFaceSet PnIndex out of bounds for index " + boost::lexical_cast(idx)); + } + idx = (*pn_index)[idx - 1]; + } + if (idx < 1 || idx > max_index) { + throw ifcopenshell::exception("IfcPolygonalFaceSet index out of bounds for index " + boost::lexical_cast(idx)); + } + return points[idx - 1]; + }; + auto shell = taxonomy::make(); - + for (auto& f : polygonal_faces) { auto fa = taxonomy::make(); shell->children.push_back(fa); @@ -52,17 +69,14 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcPolygonalFaceSet& inst) { auto indices = f.CoordIndex(); taxonomy::point3::ptr previous; for (std::vector::const_iterator jt = indices.begin(); jt != indices.end(); ++jt) { - if (*jt < 1 || *jt > max_index) { - throw ifcopenshell::exception("IfcPolygonalFaceSet index out of bounds for index " + boost::lexical_cast(*jt)); - } - auto current = points[(*jt) - 1]; + auto current = resolve(*jt); if (jt != indices.begin()) { loop->children.push_back(taxonomy::make(previous, current)); } previous = current; } if (!indices.empty()) { - auto current = points[indices.front() - 1]; + auto current = resolve(indices.front()); loop->children.push_back(taxonomy::make(previous, current)); } } @@ -77,17 +91,14 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcPolygonalFaceSet& inst) { loop->external = false; for (std::vector::const_iterator jt = li.begin(); jt != li.end(); ++jt) { - if (*jt < 1 || *jt > max_index) { - throw ifcopenshell::exception("IfcPolygonalFaceSet index out of bounds for index " + boost::lexical_cast(*jt)); - } - auto current = points[(*jt) - 1]; + auto current = resolve(*jt); if (jt != li.begin()) { loop->children.push_back(taxonomy::make(previous, current)); } previous = current; } if (!li.empty()) { - auto current = points[li.front() - 1]; + auto current = resolve(li.front()); loop->children.push_back(taxonomy::make(previous, current)); } } diff --git a/src/ifcgeom/mapping/IfcTriangulatedFaceSet.cpp b/src/ifcgeom/mapping/IfcTriangulatedFaceSet.cpp index ce4f2fb7a6..ced46cd2cc 100644 --- a/src/ifcgeom/mapping/IfcTriangulatedFaceSet.cpp +++ b/src/ifcgeom/mapping/IfcTriangulatedFaceSet.cpp @@ -39,6 +39,23 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcTriangulatedFaceSet& inst) { int max_index = (int)points.size(); + // When the optional PnIndex is present, CoordIndex values do not index into + // CoordList directly but into PnIndex, which in turn remaps to CoordList. + // Both index levels are 1-based per the IFC specification. + auto pn_index = inst->PnIndex(); + auto resolve = [&](int64_t idx) -> const taxonomy::point3::ptr& { + if (pn_index) { + if (idx < 1 || idx > (int64_t)pn_index->size()) { + throw ifcopenshell::exception("IfcTriangulatedFaceSet PnIndex out of bounds for index " + boost::lexical_cast(idx)); + } + idx = (*pn_index)[idx - 1]; + } + if (idx < 1 || idx > max_index) { + throw ifcopenshell::exception("IfcTriangulatedFaceSet index out of bounds for index " + boost::lexical_cast(idx)); + } + return points[idx - 1]; + }; + auto shell = taxonomy::make(); for (auto& indices : indices_list) { @@ -51,10 +68,7 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcTriangulatedFaceSet& inst) { loop->external = true; taxonomy::point3::ptr first, previous; for (std::vector::const_iterator jt = indices.begin(); jt != indices.end(); ++jt) { - if (*jt < 1 || *jt > max_index) { - throw ifcopenshell::exception("IfcTriangulatedFaceSet index out of bounds for index " + boost::lexical_cast(*jt)); - } - const taxonomy::point3::ptr& current = points[(*jt) - 1]; + const taxonomy::point3::ptr& current = resolve(*jt); if (jt == indices.begin()) { first = current; } else {