qbsp: sealing: in hull1/2 treat onnode point entities as being in empty space

qbsp/outside.cc

@@ -48,13 +48,18 @@ static bool LeafSealsMap(const node_t *node)
 PointInLeaf
 
 If the point is exactly on a node plane, prefer to return the
-opaque leaf.
+one that seals the map if `prefer_sealing` is true (otherwise
+prefer the one that doesn't seal).
 
 This avoids spurious leaks if a point entity is on the outside
 of the map (exactly on a brush faces) - happens in base1.map.
+
+However, in Q1 hull1/hull2, it makes more sense to prefer the empty
+leaf, so an info_player_start 24 units off a floor causes the
+room to not get filled in as solid.
 ===========
 */
-static node_t *PointInLeaf(node_t *node, const qvec3d &point)
+static node_t *PointInLeaf(node_t *node, const qvec3d &point, bool prefer_sealing)
 {
     if (node->is_leaf) {
         return node;
@@ -64,18 +69,17 @@ static node_t *PointInLeaf(node_t *node, const qvec3d &point)
 
     if (dist > 0) {
         // point is on the front of the node plane
-        return PointInLeaf(node->children[0], point);
+        return PointInLeaf(node->children[0], point, prefer_sealing);
     } else if (dist < 0) {
         // point is on the back of the node plane
-        return PointInLeaf(node->children[1], point);
+        return PointInLeaf(node->children[1], point, prefer_sealing);
     } else {
         // point is exactly on the node plane
 
-        node_t *front = PointInLeaf(node->children[0], point);
+        node_t *front = PointInLeaf(node->children[0], point, prefer_sealing);
-        node_t *back = PointInLeaf(node->children[1], point);
+        node_t *back = PointInLeaf(node->children[1], point, prefer_sealing);
 
-        // prefer the opaque one
+        if (prefer_sealing == LeafSealsMap(front)) {
-        if (LeafSealsMap(front)) {
             return front;
         }
         return back;
@@ -324,7 +328,7 @@ FindOccupiedLeafs
 sets node->occupant
 ==================
 */
-static void MarkOccupiedClusters(node_t *headnode)
+static void MarkOccupiedClusters(node_t *headnode, hull_index_t hullnum)
 {
     for (int i = 1; i < map.entities.size(); i++) {
         mapentity_t &entity = map.entities.at(i);
@@ -339,7 +343,8 @@ static void MarkOccupiedClusters(node_t *headnode)
         }
 
         /* find the leaf it's in. Skip opqaue leafs */
-        node_t *cluster = PointInLeaf(headnode, entity.origin);
+        bool prefer_sealing = !hullnum.has_value() || hullnum.value() == 0;
+        node_t *cluster = PointInLeaf(headnode, entity.origin, prefer_sealing);
 
         if (LeafSealsMap(cluster)) {
             continue;
@@ -630,7 +635,7 @@ bool FillOutside(tree_t &tree, hull_index_t hullnum, bspbrush_t::container &brus
     ClearOccupied_r(node);
 
     // Sets leaf->occupant
-    MarkOccupiedClusters(node);
+    MarkOccupiedClusters(node, hullnum);
     const std::vector<node_t *> occupied_clusters = FindOccupiedClusters(node);
 
     for (auto *occupied_cluster : occupied_clusters) {

testmaps/q1_sealing_hull1_onnode.map

@@ -0,0 +1,66 @@
+// Game: Quake
+// Format: Standard
+// entity 0
+{
+"classname" "worldspawn"
+"wad" "deprecated/free_wad.wad"
+// brush 0
+{
+( -304 32 16 ) ( -304 256 16 ) ( -304 32 192 ) bolt9 0 0 0 1 1
+( -304 32 192 ) ( -288 32 192 ) ( -304 32 16 ) bolt9 0 0 0 1 1
+( -304 32 16 ) ( -288 32 16 ) ( -304 256 16 ) bolt9 0 0 0 1 1
+( -304 256 192 ) ( -288 256 192 ) ( -304 32 192 ) bolt9 0 0 0 1 1
+( -304 256 16 ) ( -288 256 16 ) ( -304 256 192 ) bolt9 0 0 0 1 1
+( -288 32 16 ) ( -288 32 192 ) ( -288 256 16 ) bolt9 0 0 0 1 1
+}
+// brush 1
+{
+( -288 256 192 ) ( -288 240 192 ) ( -288 256 16 ) bolt9 0 0 0 1 1
+( -64 240 192 ) ( -64 240 16 ) ( -288 240 192 ) bolt9 0 0 0 1 1
+( -288 256 16 ) ( -288 240 16 ) ( -64 256 16 ) bolt9 0 0 0 1 1
+( -64 256 192 ) ( -64 240 192 ) ( -288 256 192 ) bolt9 0 0 0 1 1
+( -64 256 192 ) ( -288 256 192 ) ( -64 256 16 ) bolt9 0 0 0 1 1
+( 224 256 16 ) ( 224 240 16 ) ( 224 256 192 ) bolt9 0 0 0 1 1
+}
+// brush 2
+{
+( -288 32 16 ) ( -288 48 16 ) ( -288 32 192 ) bolt9 0 0 0 1 1
+( -64 32 16 ) ( -288 32 16 ) ( -64 32 192 ) bolt9 0 0 0 1 1
+( -64 32 16 ) ( -64 48 16 ) ( -288 32 16 ) bolt9 0 0 0 1 1
+( -288 32 192 ) ( -288 48 192 ) ( -64 32 192 ) bolt9 0 0 0 1 1
+( -288 48 16 ) ( -64 48 16 ) ( -288 48 192 ) bolt9 0 0 0 1 1
+( 224 32 192 ) ( 224 48 192 ) ( 224 32 16 ) bolt9 0 0 0 1 1
+}
+// brush 3
+{
+( -288 48 192 ) ( -288 48 176 ) ( -288 240 192 ) bolt9 0 0 0 1 1
+( -64 48 192 ) ( -64 48 176 ) ( -288 48 192 ) bolt9 0 0 0 1 1
+( -64 240 176 ) ( -288 240 176 ) ( -64 48 176 ) bolt9 0 0 0 1 1
+( -64 240 192 ) ( -64 48 192 ) ( -288 240 192 ) bolt9 0 0 0 1 1
+( -288 240 192 ) ( -288 240 176 ) ( -64 240 192 ) bolt9 0 0 0 1 1
+( 224 240 192 ) ( 224 240 176 ) ( 224 48 192 ) bolt9 0 0 0 1 1
+}
+// brush 4
+{
+( -288 240 16 ) ( -288 240 32 ) ( -288 48 16 ) bolt9 0 0 0 1 1
+( -288 48 16 ) ( -288 48 32 ) ( -64 48 16 ) bolt9 0 0 0 1 1
+( -288 240 16 ) ( -288 48 16 ) ( -64 240 16 ) bolt9 0 0 0 1 1
+( -288 48 32 ) ( -288 240 32 ) ( -64 48 32 ) bolt9 0 0 0 1 1
+( -64 240 16 ) ( -64 240 32 ) ( -288 240 16 ) bolt9 0 0 0 1 1
+( 224 48 16 ) ( 224 48 32 ) ( 224 240 16 ) bolt9 0 0 0 1 1
+}
+// brush 5
+{
+( 208 48 32 ) ( 208 49 32 ) ( 208 48 33 ) bolt9 0 0 0 1 1
+( 208 48 32 ) ( 208 48 33 ) ( 209 48 32 ) bolt9 0 0 0 1 1
+( 208 48 32 ) ( 209 48 32 ) ( 208 49 32 ) bolt9 0 0 0 1 1
+( 224 240 192 ) ( 224 241 192 ) ( 225 240 192 ) bolt9 0 0 0 1 1
+( 224 240 40 ) ( 225 240 40 ) ( 224 240 41 ) bolt9 0 0 0 1 1
+( 224 240 40 ) ( 224 240 41 ) ( 224 241 40 ) bolt9 0 0 0 1 1
+}
+}
+// entity 1
+{
+"classname" "info_player_start"
+"origin" "-192 132 56"
+}

tests/test_qbsp.cc

@@ -1211,6 +1211,25 @@ TEST_CASE("qbsp_sealing_point_entity_on_outside" * doctest::test_suite("testmaps
     REQUIRE(prt.has_value());
 }
 
+TEST_CASE("q1_sealing_hull1_onnode" * doctest::test_suite("testmaps_q1"))
+{
+    const auto [bsp, bspx, prt] = LoadTestmapQ1("q1_sealing_hull1_onnode.map");
+
+    const auto player_start_pos = qvec3d(-192, 132, 56);
+
+    INFO("hull0 is empty at the player start");
+    CHECK(CONTENTS_EMPTY == BSP_FindContentsAtPoint(&bsp, 0, &bsp.dmodels[0], player_start_pos));
+
+    INFO("hull1/2 are empty just above the player start");
+    CHECK(CONTENTS_EMPTY == BSP_FindContentsAtPoint(&bsp, 1, &bsp.dmodels[0], player_start_pos + qvec3d(0, 0, 1)));
+    CHECK(CONTENTS_EMPTY == BSP_FindContentsAtPoint(&bsp, 2, &bsp.dmodels[0], player_start_pos + qvec3d(0, 0, 1)));
+
+    INFO("hull0/1/2 are solid in the void");
+    CHECK(CONTENTS_SOLID == BSP_FindContentsAtPoint(&bsp, 0, &bsp.dmodels[0], player_start_pos + qvec3d(0, 0, 1000)));
+    CHECK(CONTENTS_SOLID == BSP_FindContentsAtPoint(&bsp, 1, &bsp.dmodels[0], player_start_pos + qvec3d(0, 0, 1000)));
+    CHECK(CONTENTS_SOLID == BSP_FindContentsAtPoint(&bsp, 2, &bsp.dmodels[0], player_start_pos + qvec3d(0, 0, 1000)));
+}
+
 TEST_CASE("q1_0125unit_faces" * doctest::test_suite("testmaps_q1") * doctest::may_fail())
 {
     const auto [bsp, bspx, prt] = LoadTestmapQ1("q1_0125unit_faces.map");