From 978f456083956c758fea227b30f14f1a16cb1737 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rados=C5=82aw=20Piliszek?= <radek@piliszek.it>
Date: Tue, 12 Nov 2024 22:28:55 +0100
Subject: [PATCH] Clip-filter all BSP faces early

Cull-filtering was weak and late. We can do better by clip-filtering
all faces early. This reduces the number of faces that other
code needs to process.

Similarly, move the face orientation check early too.
---
 src/Engine/Graphics/BspRenderer.cpp           | 77 ++++++++++---------
 .../Graphics/Renderer/OpenGLRenderer.cpp      | 35 ---------
 2 files changed, 40 insertions(+), 72 deletions(-)

diff --git a/src/Engine/Graphics/BspRenderer.cpp b/src/Engine/Graphics/BspRenderer.cpp
index 00d490ae1125..71e101d5a627 100644
--- a/src/Engine/Graphics/BspRenderer.cpp
+++ b/src/Engine/Graphics/BspRenderer.cpp
@@ -15,25 +15,57 @@ void BspRenderer::AddFace(const int node_id, const int uFaceID) {
     assert(uFaceID > -1 && uFaceID < pIndoor->pFaces.size() && "please report with a nearby save file");
 
     BLVFace *pFace = &pIndoor->pFaces[uFaceID];
+    pFace->uAttributes |= FACE_SeenByParty;
+
+    // NOTE(yoctozepto): the below happens, e.g., on various stairs
+    // TODO(yoctozepto): might be nice to check if the vertices actually form a plane and not a line;
+    //                   this could be done when loading the location and filtering out such broken faces
+    if (pFace->uNumVertices < 3) {
+        return;  // nothing to render
+    }
+
+    const BspRenderer_ViewportNode *currentNode = &nodes[node_id];
+
+    // check if any triangle of the face can be seen
+
+    static RenderVertexSoft originalFaceVertices[64];
+    static RenderVertexSoft clippedFaceVertices[64];
+
+    // TODO(yoctozepto): are face vertices consecutive? are face vertices shared/overlapping?
+    for (unsigned k = 0; k < pFace->uNumVertices; ++k) {
+        originalFaceVertices[k].vWorldPosition.x = pIndoor->pVertices[pFace->pVertexIDs[k]].x;
+        originalFaceVertices[k].vWorldPosition.y = pIndoor->pVertices[pFace->pVertexIDs[k]].y;
+        originalFaceVertices[k].vWorldPosition.z = pIndoor->pVertices[pFace->pVertexIDs[k]].z;
+    }
+
+    unsigned int pNewNumVertices = pFace->uNumVertices;
+
+    // TODO(yoctozepto): original vertices could have been just Vec3f
+    // clip to current viewing node frustum
+    pCamera3D->ClipFaceToFrustum(
+        originalFaceVertices,
+        &pNewNumVertices,
+        clippedFaceVertices,
+        currentNode->ViewportNodeFrustum.data());
+
+    if (!pNewNumVertices) {
+        return;  // no triangle of face in view
+    }
 
     if (!pFace->isPortal()) {
         // NOTE(yoctozepto): the below happens, e.g., on various stairs
         if (pFace->Invisible())
             return;  // nothing to render
-        // NOTE(yoctozepto): the below happens, e.g., on various stairs
-        // TODO(yoctozepto): might be nice to check if the vertices actually form a plane and not a line
-        if (pFace->uNumVertices < 3) {
-            return;  // nothing to render
-        }
+
         // TODO(yoctozepto): does the below happen?
         // TODO(yoctozepto, pskelton): we should probably try to handle these faces as they are otherwise marked as visible (see also OpenGLRenderer)
         if (!pFace->GetTexture()) {
             return;  // nothing to render
         }
 
-        // TODO(yoctozepto): experiment with clipping also regular faces to frustum and checking their facing:
-        //                   this would optimise the amount of faces that the other code has to process (OpenGLRenderer, Vis picking);
-        //                   remember to handle minimap with it
+        if (!pCamera3D->is_face_faced_to_cameraBLV(pFace)) {
+            return;  // we don't see the face, no need to render
+        }
 
         assert(num_faces < 1500 && "please report with a nearby save file");
 
@@ -45,10 +77,6 @@ void BspRenderer::AddFace(const int node_id, const int uFaceID) {
         return;
     }
 
-    // TODO(yoctozepto): similarly to regular faces, maybe check if the portal vertices form a proper plane
-
-    const BspRenderer_ViewportNode *currentNode = &nodes[node_id];
-
     // portals are invisible faces marking the transition between sectors
 
     // NOTE(yoctozepto): this is a quick test for a sure loop,
@@ -79,31 +107,6 @@ void BspRenderer::AddFace(const int node_id, const int uFaceID) {
         return;
     }
 
-    // check if portal is visible on screen
-
-    static RenderVertexSoft originalFaceVertices[64];
-    static RenderVertexSoft clippedFaceVertices[64];
-
-    for (unsigned k = 0; k < pFace->uNumVertices; ++k) {
-        originalFaceVertices[k].vWorldPosition.x = pIndoor->pVertices[pFace->pVertexIDs[k]].x;
-        originalFaceVertices[k].vWorldPosition.y = pIndoor->pVertices[pFace->pVertexIDs[k]].y;
-        originalFaceVertices[k].vWorldPosition.z = pIndoor->pVertices[pFace->pVertexIDs[k]].z;
-    }
-
-    unsigned int pNewNumVertices = pFace->uNumVertices;
-
-    // TODO(yoctozepto): original vertices could have been just Vec3f
-    // clip to current viewing node frustum
-    pCamera3D->ClipFaceToFrustum(
-        originalFaceVertices,
-        &pNewNumVertices,
-        clippedFaceVertices,
-        currentNode->ViewportNodeFrustum.data());
-
-    if (!pNewNumVertices) {
-        return;  // no triangle of face in view
-    }
-
     assert(num_nodes < 150 && "please report with a nearby save file");
 
     // start to construct the new node
diff --git a/src/Engine/Graphics/Renderer/OpenGLRenderer.cpp b/src/Engine/Graphics/Renderer/OpenGLRenderer.cpp
index 8ac911f963a2..0f5f8d1bce52 100644
--- a/src/Engine/Graphics/Renderer/OpenGLRenderer.cpp
+++ b/src/Engine/Graphics/Renderer/OpenGLRenderer.cpp
@@ -4059,42 +4059,7 @@ void OpenGLRenderer::DrawIndoorFaces() {
 
             for (unsigned i = 0; i < pBspRenderer->num_faces; ++i) {
                 int uFaceID = pBspRenderer->faces[i].uFaceID;
-                if (uFaceID >= pIndoor->pFaces.size())
-                    continue;
                 BLVFace *face = &pIndoor->pFaces[uFaceID];
-                face->uAttributes |= FACE_SeenByParty;
-
-                Planef *portalfrustumnorm = pBspRenderer->nodes[pBspRenderer->faces[i].uNodeID].ViewportNodeFrustum.data();
-                // TODO(yoctozepto): just have this as a global constant instead of random vars/4 around
-                unsigned int uNumFrustums = 4;
-
-                // unsigned ColourMask;  // ebx@25
-                unsigned int uNumVerticesa;  // [sp+24h] [bp-4h]@17
-                // int LightLevel;                     // [sp+34h] [bp+Ch]@25
-
-                static RenderVertexSoft static_vertices_buff_in[64];  // buff in
-                static RenderVertexSoft static_vertices_calc_out[64];  // buff out - calc portal shape
-
-                // check face is towards camera
-                if (!pCamera3D->is_face_faced_to_cameraBLV(face)) {
-                    continue;
-                }
-
-                uNumVerticesa = face->uNumVertices;
-
-                // copy to buff in
-                for (unsigned i = 0; i < face->uNumVertices; ++i) {
-                    static_vertices_buff_in[i].vWorldPosition.x = pIndoor->pVertices[face->pVertexIDs[i]].x;
-                    static_vertices_buff_in[i].vWorldPosition.y = pIndoor->pVertices[face->pVertexIDs[i]].y;
-                    static_vertices_buff_in[i].vWorldPosition.z = pIndoor->pVertices[face->pVertexIDs[i]].z;
-                    static_vertices_buff_in[i].u = (signed short)face->pVertexUIDs[i];
-                    static_vertices_buff_in[i].v = (signed short)face->pVertexVIDs[i];
-                }
-
-                // check if this face is visible through current portal node
-                if (!pCamera3D->CullFaceToFrustum(static_vertices_buff_in, &uNumVerticesa, static_vertices_calc_out, portalfrustumnorm, 4)) {
-                    continue;
-                }
 
                 float skymodtimex{};
                 float skymodtimey{};