From 3e55c5126c75bb41a19ece7a8f8d3509326d5e2f Mon Sep 17 00:00:00 2001
From: Petru Conduraru <petru@bimvoice.com>
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 <noreply@anthropic.com>
---
 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 c3f7ff7219..c965db542a 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 = [&](int idx) -> const taxonomy::point3::ptr& {
+		if (pn_index) {
+			if (idx < 1 || idx > (int)pn_index->size()) {
+				throw IfcParse::IfcException("IfcPolygonalFaceSet PnIndex out of bounds for index " + boost::lexical_cast<std::string>(idx));
+			}
+			idx = (*pn_index)[idx - 1];
+		}
+		if (idx < 1 || idx > max_index) {
+			throw IfcParse::IfcException("IfcPolygonalFaceSet index out of bounds for index " + boost::lexical_cast<std::string>(idx));
+		}
+		return points[idx - 1];
+	};
+
 	auto shell = taxonomy::make<taxonomy::shell>();
-	
+
 	for (auto& f : *polygonal_faces) {
 		auto fa = taxonomy::make<taxonomy::face>();
 		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<int>::const_iterator jt = indices.begin(); jt != indices.end(); ++jt) {
-				if (*jt < 1 || *jt > max_index) {
-					throw IfcParse::IfcException("IfcPolygonalFaceSet index out of bounds for index " + boost::lexical_cast<std::string>(*jt));
-				}
-				auto current = points[(*jt) - 1];
+				auto current = resolve(*jt);
 				if (jt != indices.begin()) {
 					loop->children.push_back(taxonomy::make<taxonomy::edge>(previous, current));
 				}
 				previous = current;
 			}
 			if (!indices.empty()) {
-				auto current = points[indices.front() - 1];
+				auto current = resolve(indices.front());
 				loop->children.push_back(taxonomy::make<taxonomy::edge>(previous, current));
 			}
 		}
@@ -77,17 +91,14 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcPolygonalFaceSet* inst) {
 					loop->external = false;
 
 					for (std::vector<int>::const_iterator jt = li.begin(); jt != li.end(); ++jt) {
-						if (*jt < 1 || *jt > max_index) {
-							throw IfcParse::IfcException("IfcPolygonalFaceSet index out of bounds for index " + boost::lexical_cast<std::string>(*jt));
-						}
-						auto current = points[(*jt) - 1];
+						auto current = resolve(*jt);
 						if (jt != li.begin()) {
 							loop->children.push_back(taxonomy::make<taxonomy::edge>(previous, current));
 						}
 						previous = current;
 					}
 					if (!li.empty()) {
-						auto current = points[li.front() - 1];
+						auto current = resolve(li.front());
 						loop->children.push_back(taxonomy::make<taxonomy::edge>(previous, current));
 					}
 				}
diff --git a/src/ifcgeom/mapping/IfcTriangulatedFaceSet.cpp b/src/ifcgeom/mapping/IfcTriangulatedFaceSet.cpp
index 776ea59605..f234c3bd3f 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 = [&](int idx) -> const taxonomy::point3::ptr& {
+		if (pn_index) {
+			if (idx < 1 || idx > (int)pn_index->size()) {
+				throw IfcParse::IfcException("IfcTriangulatedFaceSet PnIndex out of bounds for index " + boost::lexical_cast<std::string>(idx));
+			}
+			idx = (*pn_index)[idx - 1];
+		}
+		if (idx < 1 || idx > max_index) {
+			throw IfcParse::IfcException("IfcTriangulatedFaceSet index out of bounds for index " + boost::lexical_cast<std::string>(idx));
+		}
+		return points[idx - 1];
+	};
+
 	auto shell = taxonomy::make<taxonomy::shell>();
 
 	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<int>::const_iterator jt = indices.begin(); jt != indices.end(); ++jt) {
-				if (*jt < 1 || *jt > max_index) {
-					throw IfcParse::IfcException("IfcTriangulatedFaceSet index out of bounds for index " + boost::lexical_cast<std::string>(*jt));
-				}
-				const taxonomy::point3::ptr& current = points[(*jt) - 1];
+				const taxonomy::point3::ptr& current = resolve(*jt);
 				if (jt == indices.begin()) {
 					first = current;
 				} else {