From 9f80b4b9ed43db91035ed3ddbf9bad4c40c9cf9d Mon Sep 17 00:00:00 2001
From: Dan Goodliffe <dan@randomdan.homeip.net>
Date: Wed, 18 Dec 2024 14:55:33 +0000
Subject: Don't make arbitrary changes to mesh for triangle boundaries

Making these arbitrary changes can lead to inverted adjacent faces,
instead just:
  a) use the near node where it is, or
  b) create the edge split along its length without lateral movement

Same principal as previous commit, addresses issues where tracing would
fail seemingly at random and throws on error now.
---
 game/geoData.cpp      | 66 +++++++++++++++++++++++++++------------------------
 test/test-geoData.cpp |  2 +-
 2 files changed, 36 insertions(+), 32 deletions(-)

diff --git a/game/geoData.cpp b/game/geoData.cpp
index 8d7e18a..643b24b 100644
--- a/game/geoData.cpp
+++ b/game/geoData.cpp
@@ -497,40 +497,44 @@ GeoData::setHeights(const std::span<const GlobalPosition3D> triangleStrip, const
 
 	// Cut along each edge of triangleStrip AB, AC, BC, BD, CD, CE etc
 	std::map<VertexHandle, const Triangle<3> *> boundaryTriangles;
-	auto doBoundaryPart = [this, &boundaryTriangles, &newVerts, &vertexDistFrom, &opts, &addVertexForNormalUpdate](
+	auto doBoundaryPart = [this, &boundaryTriangles, &vertexDistFrom, &opts, &addVertexForNormalUpdate](
 								  VertexHandle start, VertexHandle end, const Triangle<3> & triangle) {
 		boundaryTriangles.emplace(start, &triangle);
 		const auto endPoint = point(end);
-		while (!std::ranges::contains(vv_range(start), end)
-				&& std::ranges::any_of(voh_range(start), [&](const auto & outHalf) {
-					   const auto next = next_halfedge_handle(outHalf);
-					   const auto startPoint = point(start);
-					   const auto nexts = std::array {from_vertex_handle(next), to_vertex_handle(next)};
-					   const auto nextPoints = nexts | std::views::transform([this](const auto v) {
-						   return std::make_pair(v, this->point(v));
-					   });
-					   if (linesCross(startPoint, endPoint, nextPoints.front().second, nextPoints.back().second)) {
-						   if (const auto intersection = linesIntersectAt(startPoint.xy(), endPoint.xy(),
-									   nextPoints.front().second.xy(), nextPoints.back().second.xy())) {
-							   if (const auto nextDist
-									   = std::ranges::min(nexts | std::views::transform(vertexDistFrom(*intersection)),
-											   {}, &std::pair<VertexHandle, float>::second);
-									   nextDist.second < opts.nearNodeTolerance
-									   && !boundaryTriangles.contains(nextDist.first)
-									   && !std::ranges::contains(newVerts, nextDist.first)) {
-								   start = nextDist.first;
-								   point(start) = positionOnTriangle(*intersection, triangle);
-							   }
-							   else {
-								   start = split(edge_handle(next), positionOnTriangle(*intersection, triangle));
-							   }
-							   addVertexForNormalUpdate(start);
-							   boundaryTriangles.emplace(start, &triangle);
-							   return true;
-						   }
-					   }
-					   return false;
-				   })) { }
+		while (!std::ranges::contains(vv_range(start), end)) {
+			const auto startPoint = point(start);
+			if (std::ranges::none_of(voh_range(start), [&](const auto & outHalf) {
+					const auto next = next_halfedge_handle(outHalf);
+					const auto nexts = std::array {from_vertex_handle(next), to_vertex_handle(next)};
+					const auto nextPoints = nexts | std::views::transform([this](const auto v) {
+						return std::make_pair(v, this->point(v));
+					});
+					if (linesCross(startPoint, endPoint, nextPoints.front().second, nextPoints.back().second)) {
+						if (const auto intersection = linesIntersectAt(startPoint.xy(), endPoint.xy(),
+									nextPoints.front().second.xy(), nextPoints.back().second.xy())) {
+							const auto newPosition = positionOnTriangle(*intersection, triangle);
+							if (const auto nextDist
+									= std::ranges::min(nexts | std::views::transform(vertexDistFrom(*intersection)), {},
+											&std::pair<VertexHandle, float>::second);
+									nextDist.second < opts.nearNodeTolerance) {
+								start = nextDist.first;
+								return true;
+							}
+							else {
+								start = split(edge_handle(next), newPosition);
+							}
+							addVertexForNormalUpdate(start);
+							boundaryTriangles.emplace(start, &triangle);
+							return true;
+						}
+						throw std::runtime_error("Crossing lines don't intersect");
+					}
+					return false;
+				})) {
+				throw std::runtime_error(
+						std::format("Could not navigate to ({}, {}, {})", endPoint.x, endPoint.y, endPoint.z));
+			}
+		}
 	};
 	auto doBoundary = [&doBoundaryPart, triangle = strip.begin()](const auto & verts) mutable {
 		const auto & [a, b, c] = verts;
diff --git a/test/test-geoData.cpp b/test/test-geoData.cpp
index 1ca050d..5998789 100644
--- a/test/test-geoData.cpp
+++ b/test/test-geoData.cpp
@@ -269,6 +269,6 @@ BOOST_DATA_TEST_CASE(
 	auto gd = std::make_shared<GeoData>(GeoData::createFlat({0, 0}, {1000000, 1000000}, 100));
 	for (const auto & strip : points) {
 		BOOST_REQUIRE_GE(strip.size(), 3);
-		BOOST_CHECK_NO_THROW(gd->setHeights(strip, {.surface = surface}));
+		BOOST_CHECK_NO_THROW(gd->setHeights(strip, {.surface = surface, .nearNodeTolerance = 50}));
 	}
 }
-- 
cgit v1.2.3