Fix dmap vertex-allocation crash on heavily split maps

.github/workflows/commit-validation.yml

@@ -143,6 +143,7 @@ jobs:
             tools/tests/demo_playback.py \
             tools/tests/docs_link_integrity.py \
             tools/tests/dmap_render_geometry_lifecycle.py \
+            tools/tests/dmap_vertex_sharing.py \
             tools/tests/fatal_error_startup_safety.py \
             tools/tests/android_build.py \
             tools/tests/android_touch_input.py \
@@ -521,6 +522,7 @@ jobs:
           python tools/tests/demo_playback.py
           python tools/tests/docs_link_integrity.py
           python tools/tests/dmap_render_geometry_lifecycle.py
+          python tools/tests/dmap_vertex_sharing.py
           python tools/tests/fatal_error_startup_safety.py
           python tools/tests/android_build.py
           python tools/tests/android_touch_input.py
@@ -833,7 +835,7 @@ jobs:
     name: Windows x64 Commit Validation
     runs-on: windows-2025
     needs: script-smoke
-    timeout-minutes: 60
+    timeout-minutes: 150
     env:
       OPENQ4_GAMELIBS_REPO: ${{ github.workspace }}
 

.github/workflows/push-verification.yml

@@ -143,6 +143,7 @@ jobs:
             tools/tests/demo_playback.py \
             tools/tests/docs_link_integrity.py \
             tools/tests/dmap_render_geometry_lifecycle.py \
+            tools/tests/dmap_vertex_sharing.py \
             tools/tests/fatal_error_startup_safety.py \
             tools/tests/android_build.py \
             tools/tests/android_touch_input.py \
@@ -521,6 +522,7 @@ jobs:
           python tools/tests/demo_playback.py
           python tools/tests/docs_link_integrity.py
           python tools/tests/dmap_render_geometry_lifecycle.py
+          python tools/tests/dmap_vertex_sharing.py
           python tools/tests/fatal_error_startup_safety.py
           python tools/tests/android_build.py
           python tools/tests/android_touch_input.py

docs/dev/dmap-compilation.md

@@ -0,0 +1,77 @@
+# dmap surface storage
+
+The `hub2.map` attachment in [issue #181](https://github.com/themuffinator/openQ4/issues/181)
+exposed a crash in `ShareMapTriVerts` while writing the compiled `.proc` file.
+The supplied dump records a request for 36,635,340 vertex slots, or
+2,344,661,760 bytes with the 64-byte vertex layout. This exceeds the legacy
+heap's signed 32-bit allocation limit. The index allocation succeeds, but the
+vertex allocation returns null and the old writer immediately writes through it.
+
+Surface sharing now keeps references to the first occurrence of each unique
+vertex and allocates the final vertex buffer only after sharing is complete.
+Index storage still includes every input triangle. The implementation checks
+index-count multiplication, rejected geometry allocations and non-finite
+positions, freeing the surface before reporting a compiler error.
+
+A spatial hash replaces the linear scan through all preceding vertices.
+Neighboring cells are searched across epsilon boundaries, and the earliest
+matching vertex wins. This preserves the original position/UV tolerances,
+normal matching, vertex order and index order, including texture seams and
+hard edges. It does not discard overlapping triangles or change triangulation.
+
+T-junction repair and output surface assembly also retain the tail of each
+accumulated triangle list.
+Appending each new fragment no longer traverses every previously appended
+fragment. This preserves list order and avoids quadratic list assembly on
+heavily split surfaces. A sample of the original `hub2` compile was inside
+`MergeTriLists`, called by `FixGlobalTjunctions`; the same rescan was measured
+later in `WriteOutputSurfaces`.
+
+## Regression coverage
+
+`python tools/tests/dmap_vertex_sharing.py` compiles the production sharing
+implementation against a counted, fallible geometry allocator. It compares
+results with the original linear matching rule, including positive and negative
+cell boundaries, seams, smoothed/hard normals, ambiguous matches and varied
+geometry. A 14,000-triangle surface with three unique vertices reproduces the
+allocation-limit condition at a small scale: the old reservation exceeds the
+256 KiB test limit, while the shared vertices and all 42,000 indexes fit.
+Rejected index/vertex allocations, integer overflow and invalid positions also
+exercise the error and cleanup paths.
+
+Both GitHub validation workflows and the local validation runner include the
+test. Use `--compiler cl` in the Windows developer environment, or
+`--sanitize` with Clang/GCC on Linux. The existing
+`dmap_render_geometry_lifecycle.py` and `aas_compiler_contract.py` checks cover
+the surrounding allocator lifetime and compiler wiring.
+
+The reporter's map and crash dump are investigation inputs only and are not
+redistributed in the repository. Local qualification evidence belongs under
+`.tmp/issue-181/`.
+
+## Supplied-map qualification (14 October 2026)
+
+The Windows x64 optimized build completed `dmap game/hub2` against installed
+stock assets and exited normally. Geometry took 503 seconds, collision 3 seconds,
+and navigation 9 seconds. The run produced the `.proc`, `.cm` and all five AAS
+files. The largest surface contains 36,635,340 indexes but only 21,559 shared
+vertices: its final vertex allocation is 1,379,776 bytes rather than the old
+2,344,661,760-byte reservation. Windows Clang/MSVC native checks and Linux
+Clang/GCC address/undefined-behavior sanitizer checks pass. Running the original
+sharing implementation in the same regression fixture reproduces the null write.
+
+Compilation success does **not** qualify this map for gameplay. Loading the
+result reaches `R_ParseModel: bad surface counts`: its largest surface exceeds
+the renderer's existing 16,777,216-index limit. The load also reports
+`MAX_SIL_EDGES`. Those runtime limits and the source map's duplicate-plane,
+node-without-volume and backwards-triangle diagnostics remain follow-up work;
+the compiler fix does not relax renderer validation or rewrite map geometry.
+Keep issue #181 open to track this distinction.
+
+A separate staged OpenGL single-player smoke loads stock `game/airdefense1`,
+enters gameplay, writes an engine screenshot and exits normally after three
+seconds. This confirms stock-map startup, not playability of the rebuilt `hub2`.
+
+The reporter's log additionally rejects the `hidef2kbc72` mod manifest's required
+openQ4 version `1`; it expects a `major.minor.patch` value. This is separate from
+the compiler allocation failure and was excluded from the stock-asset compile.

docs/dev/release-completion.md

@@ -2,6 +2,23 @@
 
 ## Post-0.14.000 corrections
 
+- [x] Qualify the `hub2` map compiler crash fix from
+  [#181](https://github.com/themuffinator/openQ4/issues/181). Allocate shared
+  vertices by their actual count, retain matching/order behavior, reject failed
+  allocations safely, and avoid repeated triangle-list rescans during T-junction
+  repair and output assembly. Windows MSVC and Clang native regression tests and
+  Linux sanitizer checks pass. The supplied map completes geometry, collision
+  and navigation compilation, with a 503-second geometry pass and normal exit.
+  Gameplay remains blocked by its oversized surface exceeding the renderer's
+  existing index limit; keep #181 open for that follow-up.
+  See [compiler storage and regression coverage](dmap-compilation.md).
+
+Candidate release wording: Large, heavily split maps such as `hub2` compile
+without crashing on an oversized temporary vertex allocation. Surface sharing
+and T-junction list assembly also do less repeated work. Rebuild affected maps
+with the updated engine. The supplied `hub2` map still exceeds a separate runtime
+surface limit and is not yet qualified for gameplay.
+
 - [x] Reduce repeated hidden HUD animation checks without changing animation
   phase, appearance or reveal behavior. Eight optimized native UI suites pass.
   Matched 720p gameplay comparisons in both condition orders add 2.60-4.32%

src/tools/compilers/dmap/output.cpp

@@ -31,6 +31,12 @@ If you have questions concerning this license or the applicable additional terms
 
 #include "dmap.h"
 
+#include <array>
+#include <cmath>
+#include <functional>
+#include <unordered_map>
+#include <vector>
+
 //=================================================================================
 
 
@@ -181,6 +187,22 @@ static bool MatchVert( const idDrawVert *a, const idDrawVert *b ) {
 	return true;
 }
 
+// Unit-sized spatial buckets keep the epsilon search local. Search both sides
+// of a bucket boundary and retain the earliest matching vertex, just as the
+// original linear search did. Float keys avoid narrowing large map coordinates
+// to an integer; floor() still makes every key an integral position.
+typedef std::array<float, 3> mapTriVertexCell_t;
+
+struct mapTriVertexCellHash_t {
+	size_t operator()( const mapTriVertexCell_t &cell ) const {
+		size_t hash = 0;
+		for ( int i = 0; i < 3; i++ ) {
+			hash ^= std::hash<float>()( cell[i] ) + size_t( 0x9e3779b9u ) + ( hash << 6 ) + ( hash >> 2 );
+		}
+		return hash;
+	}
+};
+
 /*
 ====================
 ShareMapTriVerts
@@ -189,47 +211,85 @@ Converts independent triangles to shared vertex triangles
 ====================
 */
 srfTriangles_t	*ShareMapTriVerts( const mapTri_t *tris ) {
-	const mapTri_t	*step;
-	int			count;
-	int			i, j;
-	int			numVerts;
-	int			numIndexes;
-	srfTriangles_t	*uTri;
-
-	// unique the vertexes
-	count = CountTriList( tris );
-
-	uTri = R_AllocStaticTriSurf();
-	R_AllocStaticTriSurfVerts( uTri, count * 3 );
-	R_AllocStaticTriSurfIndexes( uTri, count * 3 );
-
-	numVerts = 0;
-	numIndexes = 0;
-
-	for ( step = tris ; step ; step = step->next ) {
-		for ( i = 0 ; i < 3 ; i++ ) {
-			const idDrawVert	*dv;
+	const int count = CountTriList( tris );
+	if ( count < 0 || count > idMath::INT_MAX / 3 ) {
+		common->Error( "ShareMapTriVerts: too many triangles (%i)", count );
+		return NULL;
+	}
 
-			dv = &step->v[i];
+	srfTriangles_t *uTri = R_AllocStaticTriSurf();
+	const int numIndexes = count * 3;
+	R_AllocStaticTriSurfIndexes( uTri, numIndexes );
+	if ( numIndexes != 0 && uTri->indexes == NULL ) {
+		R_FreeStaticTriSurf( uTri );
+		common->Error( "ShareMapTriVerts: could not allocate %i indexes", numIndexes );
+		return NULL;
+	}
+
+	// Keep references to the first occurrence of each vertex until sharing is
+	// complete. Reserving three full vertices per triangle can exceed the heap's
+	// single-allocation limit even when the shared surface is small (#181).
+	std::vector<const idDrawVert *> uniqueVerts;
+	std::unordered_map<mapTriVertexCell_t, std::vector<int>, mapTriVertexCellHash_t> cells;
+	int index = 0;
+	for ( const mapTri_t *step = tris; step; step = step->next ) {
+		for ( int i = 0; i < 3; i++ ) {
+			const idDrawVert *dv = &step->v[i];
+			mapTriVertexCell_t cell;
+			float neighbors[3][2];
+			int numNeighbors[3];
+			for ( int axis = 0; axis < 3; axis++ ) {
+				if ( !std::isfinite( dv->xyz[axis] ) ) {
+					R_FreeStaticTriSurf( uTri );
+					common->Error( "ShareMapTriVerts: non-finite vertex position" );
+					return NULL;
+				}
+				cell[axis] = float( std::floor( double( dv->xyz[axis] ) ) );
+				// Leave room for rounding in MatchVert's float subtraction.
+				neighbors[axis][0] = float( std::floor( double( dv->xyz[axis] ) - 2 * XYZ_EPSILON ) );
+				neighbors[axis][1] = float( std::floor( double( dv->xyz[axis] ) + 2 * XYZ_EPSILON ) );
+				numNeighbors[axis] = neighbors[axis][0] == neighbors[axis][1] ? 1 : 2;
+			}
 
-			// search for a match
-			for ( j = 0 ; j < numVerts ; j++ ) {
-				if ( MatchVert( &uTri->verts[j], dv ) ) {
-					break;
+			int match = int( uniqueVerts.size() );
+			for ( int x = 0; x < numNeighbors[0]; x++ ) {
+				for ( int y = 0; y < numNeighbors[1]; y++ ) {
+					for ( int z = 0; z < numNeighbors[2]; z++ ) {
+						const mapTriVertexCell_t neighbor = { neighbors[0][x], neighbors[1][y], neighbors[2][z] };
+						const auto found = cells.find( neighbor );
+						if ( found == cells.end() ) {
+							continue;
+						}
+						for ( int candidate : found->second ) {
+							if ( candidate < match && MatchVert( uniqueVerts[candidate], dv ) ) {
+								match = candidate;
+								break;
+							}
+						}
+					}
 				}
 			}
-			if ( j == numVerts ) {
-				numVerts++;
-				uTri->verts[j].xyz = dv->xyz;
-				uTri->verts[j].normal = dv->normal;
-				uTri->verts[j].st[0] = dv->st[0];
-				uTri->verts[j].st[1] = dv->st[1];
+			if ( match == int( uniqueVerts.size() ) ) {
+				uniqueVerts.push_back( dv );
+				cells[cell].push_back( match );
 			}
-
-			uTri->indexes[numIndexes++] = j;
+			uTri->indexes[index++] = match;
 		}
 	}
 
+	const int numVerts = int( uniqueVerts.size() );
+	R_AllocStaticTriSurfVerts( uTri, numVerts );
+	if ( numVerts != 0 && uTri->verts == NULL ) {
+		R_FreeStaticTriSurf( uTri );
+		common->Error( "ShareMapTriVerts: could not allocate %i shared vertices", numVerts );
+		return NULL;
+	}
+	for ( int i = 0; i < numVerts; i++ ) {
+		uTri->verts[i].Clear();
+		uTri->verts[i].xyz = uniqueVerts[i]->xyz;
+		uTri->verts[i].normal = uniqueVerts[i]->normal;
+		uTri->verts[i].st = uniqueVerts[i]->st;
+	}
 	uTri->numVerts = numVerts;
 	uTri->numIndexes = numIndexes;
 
@@ -435,6 +495,7 @@ typedef struct interactionTris_s {
 		// vertexes because they had different planes, texture coordinates, or lights.
 		// Different mergeGroups will stay in separate surfaces.
 		ambient = NULL;
+		mapTri_t **ambientTail = &ambient;
 
 		// each light that illuminates any of the groups in the surface will
 		// get its own list of indexes out of the original surface
@@ -450,7 +511,10 @@ typedef struct interactionTris_s {
 
 			// copy it out to the ambient list
 			copy = CopyTriList( groupStep->triList );
-			ambient = MergeTriLists( ambient, copy );
+			*ambientTail = copy;
+			while ( *ambientTail ) {
+				ambientTail = &(*ambientTail)->next;
+			}
 			groupStep->surfaceEmited = true;
 
 			// duplicate it into an interaction for each groupLight

src/tools/compilers/dmap/tritjunction.cpp

@@ -494,9 +494,15 @@ void	FixAreaGroupsTjunctions( optimizeGroup_t *groupList ) {
 		}
 
 		newList = NULL;
+		mapTri_t **tail = &newList;
 		for ( tri = group->triList ; tri ; tri = tri->next ) {
 			fixed = FixTriangleAgainstHash( tri );
-			newList = MergeTriLists( newList, fixed );
+			// Large split surfaces must not rescan the accumulated list for
+			// every input triangle. Preserve the same output order with a tail.
+			*tail = fixed;
+			while ( *tail ) {
+				tail = &(*tail)->next;
+			}
 		}
 		FreeTriList( group->triList );
 		group->triList = newList;
@@ -648,9 +654,13 @@ void	FixGlobalTjunctions( uEntity_t *e ) {
 			}
 
 			mapTri_t *newList = NULL;
+			mapTri_t **tail = &newList;
 			for ( mapTri_t *tri = group->triList ; tri ; tri = tri->next ) {
 				mapTri_t *fixed = FixTriangleAgainstHash( tri );
-				newList = MergeTriLists( newList, fixed );
+				*tail = fixed;
+				while ( *tail ) {
+					tail = &(*tail)->next;
+				}
 			}
 			FreeTriList( group->triList );
 			group->triList = newList;