qbsp: fix func_detail_fence frustum culling issue,

reported by hemebond. Fixes q1_detail_fence2.map.

Author: ericwa

qbsp/faces.cc

@@ -407,6 +407,24 @@ static node_t *FixupMarkFaces_ProcessCluster_FindStorageLeaf(node_t *node)
     return node;
 }
 
+static const aabb3d BoundFaces(const std::set<face_t *> &marfaces_to_add)
+{
+    aabb3d result;
+    for (const face_t *face : marfaces_to_add) {
+        for (const qvec3d &pt : face->w) {
+            result += pt;
+        }
+    }
+    return result;
+}
+
+static void AddBoundsToNode(node_t *start, const aabb3d &bounds_to_add)
+{
+    for (node_t *node = start; node; node = node->parent) {
+        node->bounds += bounds_to_add;
+    }
+}
+
 static void FixupMarkFaces_AddFacesToLeaf(node_t *node, const std::set<face_t *> &marfaces_to_add)
 {
     Q_assert(FixupMarkFaces_IsUsableLeaf(node));
@@ -421,6 +439,16 @@ static void FixupMarkFaces_AddFacesToLeaf(node_t *node, const std::set<face_t *>
 
     auto new_markfaces = std::vector<face_t *>(current_markfaces.begin(), current_markfaces.end());
     leafdata->markfaces = new_markfaces;
+
+    // expand the bounds of `node` (plus all ancestors) to encompass `markfaces_to_add`, to prevent frustum culling
+    // of the storage leaf when the markfaces_to_add are in view (fixes q1_detail_fence2.map test case).
+    //
+    // This does mean we "lie" about the node bounds. Alternative to this would be adding the marfaces to _every_
+    // eligible leaf in the storage clusters, which would potentially waste a ton of marksurfaces
+    aabb3d added_bounds = BoundFaces(marfaces_to_add);
+    //logging::print("old storage node bounds: {}, want to add faces bounds: {}\n", node->bounds, added_bounds);
+
+    AddBoundsToNode(node, added_bounds);
 }
 
 /**

testmaps/q1_detail_fence.map

@@ -285,3 +285,13 @@
 "origin" "120 -72 104"
 "wait" "0.3"
 }
+// entity 6
+{
+"classname" "ambient_thunder"
+"origin" "-128 -320 64"
+}
+// entity 7
+{
+"classname" "ambient_suck_wind"
+"origin" "224 16 192"
+}

tests/test_qbsp.cc

@@ -1499,6 +1499,10 @@ TEST(testmapsQ1, detailFence)
     EXPECT_EQ(player_start_leaf->contents, CONTENTS_EMPTY);
     auto markfaces = Leaf_Markfaces(&bsp, player_start_leaf);
     EXPECT_THAT(markfaces, testing::Contains(back_of_pillar_face));
+    // check that its bounds were extended to encompass the faces inside the func_detail_fence -
+    // they're using the player start leaf as "storage".
+    EXPECT_EQ(qvec3f(-128, -320, 64), player_start_leaf->mins);
+    EXPECT_EQ(qvec3f(224, 16, 192), player_start_leaf->maxs);
 
     // check the cubby off to the side - it _shouldn't_ have got the back_of_pillar_face added to its marksurfaces
     // (make sure the flood fill in FixupDetailFence() isn't propagating them excessively)
@@ -1509,6 +1513,41 @@ TEST(testmapsQ1, detailFence)
     EXPECT_THAT(cubby_leaf_markfaces, testing::Not(testing::Contains(back_of_pillar_face)));
 }
 
+TEST(testmapsQ1, detailFence2)
+{
+    const auto [bsp, bspx, prt] = LoadTestmapQ1("q1_detail_fence2.map");
+
+    // similar to above.. this one repro's an issue where the "faces inside the func_detail_fence volume" get
+    // added to a leaf that can get frustum culled, so we expand the bounds.
+
+    const aabb3f func_detail_fence_box = aabb3f({-80, -192, -48}, {-16, -48, 64});
+
+    // grab a random face inside the detail_fence
+    const auto in_fence_pos = qvec3d(-80, -104, 8);
+    auto *in_fence_face = BSP_FindFaceAtPoint(&bsp, &bsp.dmodels[0], in_fence_pos);
+
+    // find the "storage leaf", that is an empty leaf that has in_fence_face in its marksurfaces
+    const mleaf_t *storage_leaf = nullptr;
+    for (const mleaf_t &l : bsp.dleafs) {
+        auto markfaces = Leaf_Markfaces(&bsp, &l);
+        if (l.contents != CONTENTS_EMPTY) {
+            continue;
+        }
+        if (std::ranges::find(markfaces, in_fence_face) != markfaces.end()) {
+            storage_leaf = &l;
+            break;
+        }
+    }
+
+    ASSERT_NE(nullptr, storage_leaf);
+
+    const aabb3f storage_leaf_bounds = aabb3f(storage_leaf->mins, storage_leaf->maxs);
+
+    SCOPED_TRACE(testing::Message() << "storage_leaf_bounds: " << storage_leaf_bounds
+                                    << " func_detail_fence_box: " << func_detail_fence_box);
+    ASSERT_TRUE(storage_leaf_bounds.contains(func_detail_fence_box));
+}
+
 TEST(testmapsQ1, detailFenceWithoutExtendedContents)
 {
     const auto [bsp, bspx, prt] = LoadTestmapQ1("q1_detail_fence.map", {"-noextendedcontentflags"});