Merge pull request #656 from w23/stream-E332

Things done during stream E332

- [x] comment on no light under emissive acid
- [x] normalmaps for water, fix #655 
- [x] add docs regarding regression tests

ref/vk/NOTES.md

@@ -808,4 +808,92 @@ Seems to be the preferred option: less geometry overall. Do not need to generate
 worlmodel has them.
 However: glass brushes still do have back surfaces. How to deal with those? Doesn't seem to break anything for now.
 
+# 2023-11-17 E332
+## Automated testing
+Q:
+- How to run? On which hardware?
+    - Steam Deck as a target HW.
+- How to run from GH actions CI?
+- Do we need a headless build? It does require engine changes.
+- How to enable/integrate testing into the build system?
 
+### Unit tests
+Some things can be covered by unit tests. Things that are independent of the engine.
+Currently somewhat covered:
+- alolcator
+- urmom
+
+Possibly coverable:
+- Water tesselation
+- Math stuff (tangents, etc)
+- Parts of light clusters
+- Studio model:
+    - cache
+    - geometry generation
+- sebastian + meatpipe
+- brush loading
+
+Things that potentially coverable, but depend on the engine:
+- Loading patches, mapents, rad files, etc. -- Depend on engine file loading and parsing
+- Texture loading
+- studio model loading
+
+### Regression testing
+Check that:
+- internal structures have expected values. I.e. that all expected entity/material/... patches are applied for a given map:
+    - Internal lists of brush models has expected items.
+    - Each brush model has expected number of surfaces, geometries, water models, etc.
+    - Each brush surface has expected number of vertices (with expected values like position, normal, uvs, ...) expected textures/materials, etc.
+    - There's an expected number of light sources with expected properties (vertices, etc)
+- the desired image is rendered.
+- internal state remains valid/expected during game/demo play
+- performance remains within expected bounds
+
+#### Internal structures verification
+We can make a thing that dumps internal structures we care about into a file. This is done once for a "golden" state.
+This state is now what we need to compare against.
+Then for a test run we do the same thing: we just dump internal state into another file. And then we just `diff -u` this file
+with a golden file. If there are any differences, then it's a failure, and the diff file highlight which things have changed.
+This way there's no immediate need to write a deserialization -- just comparing text files is enough.
+Serialization step is expected to be reasonably simple.
+Possible concerns:
+- Text file should be somewhat structured so that context for found differences is easily reconstructible.
+- Some things are not expected to match exactly. There can be floating point differences, etc.
+- Some things can be order independent. Serializator should have a way to make a stable order for them.
+
+Possible serialization implementation:
+- Similar to R_SPEEDS, provide a way to register structures to be dumped. Pass a function that dumps these structures by `const void*`
+- This function can further pass sub-structures for serialization.
+- Pass array of types/structs for serialization. Possibly with a sort function for stable ordering.
+- Pass basic types for serialization, possibly with precision hints.
+- Pass strings for serialization.
+
+What should be the format? Simple text format is bad when we have arbitrary strings.
+Something json-like (not necessarily valid json) might be good enough.
+
+Updates to code/patches/materials might need changes to the golden state.
+Q: materials are tracked in a different repo, need to have a way to synchronize with golden state.
+We can track golden state also in a different repo, have it link to PBR repo as a submodule (or, better, just a link to a commit).
+And it can itself be a submodule for xash3d-fwgs-rt.
+
+#### Comparing rendering results
+Need to make a screenshot at desired location. Can be the first frame of a playdemo, or just a save file.
+Then need a way to compare images with given error tolerance. OR we can make everything (that we can) completely reproducible.
+E.g. fix all random seeds with known values for testing.
+Should it be a special mode, e.g. run with a save/demo, make a first screenshot and exit?
+Can we do multiple screenshots during a timedemo? Concern here is that it might be not stable enough (e.g. random particles, etc).
+
+#### Validating internal state
+Basically, lots of expensive TESTING-ONLY asserts everywhere.
+Would probably benefit from extensive context collector. `proval.h`
+
+#### Performance tracking
+Release build (i.e. w/o expensive asserts, state validation, dumping or anything) with an ability to dump profiling data.
+Run a short 1-2min timedemo, collect ALL performance stats for the entire lifetime and ALL frames:
+- all memory allocations
+- all custom metrics
+- all cpu and gpu scopes, the entire timeline
+This is then dumped into a file.
+Then there's a piece of software that analyzes these dumps. It can check for a few basic metrics (e.g. frame percentiles,
+amount and count of memory allocations, etc.) and compare them against known bounds. Going way too fast or too slow is a failure.
+The same software could do more analysis, e.g. producing graphs and statistics for all other metrics.

ref/vk/TODO.md

@@ -1,3 +1,9 @@
+# 2023-11-17 E332
+- [-] backside emissive water polygons:
+    - adding them makes things worse in other parts of the level
+- [x] water normalmap support -- added missing tangents
+- [x] discuss integration test strategies
+
 # 2023-11-16 E331
 - [x] Emissive waters
     - [x] add emissive water surface to polygon lights
@@ -6,7 +12,6 @@
 - [x] dynamic UVs
     - [x] update UVs for conveyors
     - [ ] pls don't aggravate validation on changelevel -- cannot reproduce
-- [ ] discuss integration test strategies
 
 # 2023-11-14 E330
 - [x] culling worldmodel waters

ref/vk/shaders/ray_primary_hit.glsl

@@ -106,7 +106,6 @@ void primaryRayHit(rayQueryEXT rq, inout RayPayloadPrimary payload) {
 	payload.base_color_a *= color;
 	payload.emissive.rgb *= color.rgb;
 
-
 	if (ubo.ubo.debug_display_only == DEBUG_DISPLAY_DISABLED) {
 		// Nop
 	} else if (ubo.ubo.debug_display_only == DEBUG_DISPLAY_SURFHASH) {

ref/vk/vk_brush.c

@@ -322,8 +322,12 @@ static void brushComputeWaterPolys( compute_water_polys_t args ) {
 			poly_vertices[i].normal[1] = 0;
 			poly_vertices[i].normal[2] = 0;
 
+			poly_vertices[i].tangent[0] = 0;
+			poly_vertices[i].tangent[1] = 0;
+			poly_vertices[i].tangent[2] = 0;
+
 			if (i > 1) {
-				vec3_t e0, e1, normal;
+				vec3_t e0, e1, normal, tangent;
 				VectorSubtract( poly_vertices[i - 1].pos, poly_vertices[0].pos, e0 );
 				VectorSubtract( poly_vertices[i].pos, poly_vertices[0].pos, e1 );
 				CrossProduct( e1, e0, normal );
@@ -332,6 +336,13 @@ static void brushComputeWaterPolys( compute_water_polys_t args ) {
 				VectorAdd(normal, poly_vertices[i].normal, poly_vertices[i].normal);
 				VectorAdd(normal, poly_vertices[i - 1].normal, poly_vertices[i - 1].normal);
 
+				computeTangentE(tangent, e0, e1,
+					poly_vertices[0].gl_tc, poly_vertices[i-1].gl_tc, poly_vertices[i].gl_tc);
+
+				VectorAdd(tangent, poly_vertices[0].tangent, poly_vertices[0].tangent);
+				VectorAdd(tangent, poly_vertices[i].tangent, poly_vertices[i].tangent);
+				VectorAdd(tangent, poly_vertices[i - 1].tangent, poly_vertices[i - 1].tangent);
+
 				args.dst_indices[indices++] = (uint16_t)(vertices);
 				args.dst_indices[indices++] = (uint16_t)(vertices + i - 1);
 				args.dst_indices[indices++] = (uint16_t)(vertices + i);
@@ -345,6 +356,7 @@ static void brushComputeWaterPolys( compute_water_polys_t args ) {
 
 		for( int i = 0; i < p->numverts; i++ ) {
 			VectorNormalize(poly_vertices[i].normal);
+			VectorNormalize(poly_vertices[i].tangent);
 #if 0
 			//const float dot = DotProduct(poly_vertices[i].normal, args.warp->plane->normal);
 			//if (dot < 0.) {
@@ -1732,6 +1744,7 @@ void R_VkBrushModelCollectEmissiveSurfaces( const struct model_s *mod, qboolean
 		int surface_index;
 		const msurface_t *surf;
 		vec3_t emissive;
+		qboolean is_water;
 	} emissive_surface_t;
 	emissive_surface_t emissive_surfaces[MAX_SURFACE_LIGHTS];
 	int geom_indices[MAX_SURFACE_LIGHTS];
@@ -1741,8 +1754,9 @@ void R_VkBrushModelCollectEmissiveSurfaces( const struct model_s *mod, qboolean
 	for( int i = 0; i < mod->nummodelsurfaces; ++i) {
 		const int surface_index = mod->firstmodelsurface + i;
 		const msurface_t *surf = mod->surfaces + surface_index;
+		const brush_surface_type_e type = getSurfaceType(surf, surface_index, is_worldmodel);
 
-		switch (getSurfaceType(surf, surface_index, is_worldmodel)) {
+		switch (type) {
 		case BrushSurface_Regular:
 		case BrushSurface_Animated:
 		case BrushSurface_Water:
@@ -1776,6 +1790,7 @@ void R_VkBrushModelCollectEmissiveSurfaces( const struct model_s *mod, qboolean
 		surface->model_surface_index = i;
 		surface->surface_index = surface_index;
 		surface->surf = surf;
+		surface->is_water = type == BrushSurface_Water;
 		VectorCopy(emissive, surface->emissive);
 	}
 
@@ -1816,6 +1831,21 @@ void R_VkBrushModelCollectEmissiveSurfaces( const struct model_s *mod, qboolean
 		// Non-static ones will be applied later when the model is actually rendered
 		if (is_static) {
 			RT_LightAddPolygon(&polylight);
+
+			/* TODO figure out when this is needed.
+			 * This is needed in cases where we can dive into emissive acid, which should illuminate what's under it
+			 * Likely, this is not a correct fix, though, see https://github.com/w23/xash3d-fwgs/issues/56
+			if (s->is_water) {
+				// Add backside for water
+				for (int i = 0; i < polylight.num_vertices; ++i) {
+					vec3_t tmp;
+					VectorCopy(polylight.vertices[i], tmp);
+					VectorCopy(polylight.vertices[polylight.num_vertices-1-i], polylight.vertices[i]);
+					VectorCopy(tmp, polylight.vertices[polylight.num_vertices-1-i]);
+					RT_LightAddPolygon(&polylight);
+				}
+			}
+			*/
 		} else {
 			bmodel->render_model.dynamic_polylights[i] = polylight;
 		}

ref/vk/vk_math.c

@@ -267,12 +267,9 @@ void Matrix4x4_ConcatScale3( matrix4x4 out, float x, float y, float z )
 	Matrix4x4_Concat( out, base, temp );
 }
 
-void computeTangent(vec3_t out_tangent, const vec3_t v0, const vec3_t v1, const vec3_t v2, const vec2_t uv0, const vec2_t uv1, const vec2_t uv2) {
-	vec3_t e1, e2;
+void computeTangentE(vec3_t out_tangent, const vec3_t e1, const vec3_t e2, const vec2_t uv0, const vec2_t uv1, const vec2_t uv2) {
 	vec2_t duv1, duv2;
 
-	VectorSubtract(v1, v0, e1);
-	VectorSubtract(v2, v0, e2);
 	Vector2Subtract(uv1, uv0, duv1);
 	Vector2Subtract(uv2, uv0, duv2);
 
@@ -287,6 +284,15 @@ void computeTangent(vec3_t out_tangent, const vec3_t v0, const vec3_t v1, const
 	out_tangent[2] = f * (duv2[1] * e1[2] - duv1[1] * e2[2]);
 }
 
+void computeTangent(vec3_t out_tangent, const vec3_t v0, const vec3_t v1, const vec3_t v2, const vec2_t uv0, const vec2_t uv1, const vec2_t uv2) {
+	vec3_t e1, e2;
+
+	VectorSubtract(v1, v0, e1);
+	VectorSubtract(v2, v0, e2);
+
+	computeTangentE(out_tangent, e1, e2, uv0, uv1, uv2);
+}
+
 void Matrix4x4_CreateFromVectors(matrix4x4 out, const vec3_t right, const vec3_t up, const vec3_t z, const vec3_t translate) {
 	out[0][0] = right[0];
 	out[1][0] = right[1];

ref/vk/vk_math.h

@@ -21,6 +21,7 @@ void Matrix4x4_CreateProjection(matrix4x4 out, float xMax, float xMin, float yMa
 void Matrix4x4_CreateOrtho(matrix4x4 m, float xLeft, float xRight, float yBottom, float yTop, float zNear, float zFar);
 void Matrix4x4_CreateModelview( matrix4x4 out );
 
+void computeTangentE(vec3_t out_tangent, const vec3_t e1, const vec3_t e2, const vec2_t uv0, const vec2_t uv1, const vec2_t uv2);
 void computeTangent(vec3_t out_tangent, const vec3_t v0, const vec3_t v1, const vec3_t v2, const vec2_t uv0, const vec2_t uv1, const vec2_t uv2);
 
 void Matrix4x4_CreateFromVectors(matrix4x4 out, const vec3_t right, const vec3_t up, const vec3_t z, const vec3_t translate);