From fa890456f608476940b5fdfa10585e1f4a83a118 Mon Sep 17 00:00:00 2001
From: Eric Wasylishen <ewasylishen@gmail.com>
Date: Sat, 27 May 2023 23:42:33 -0600
Subject: [PATCH] qbsp: don't merge faces across sky boundaries

---
 qbsp/merge.cc              |   8 ++-
 testmaps/q1_sky_window.map | 104 +++++++++++++++++++++++++++++++++++++
 tests/test_qbsp.cc         |  14 +++++
 3 files changed, 124 insertions(+), 2 deletions(-)
 create mode 100644 testmaps/q1_sky_window.map

diff --git a/qbsp/merge.cc b/qbsp/merge.cc
index 0549ee6a..cac37c90 100644
--- a/qbsp/merge.cc
+++ b/qbsp/merge.cc
@@ -72,12 +72,16 @@ static std::unique_ptr<face_t> TryMerge(const face_t *f1, const face_t *f2)
         f1->original_side->lmshift != f2->original_side->lmshift)
         return NULL;
 
-    // Q1: don't merge across water boundaries; ezQuake/nQuake water caustics will leak onto
-    // above-water faces.
     // TODO: make this configurable?
     if (qbsp_options.target_game->id != GAME_QUAKE_II) {
+        // Q1: don't merge across water boundaries; ezQuake/nQuake water caustics will leak onto
+        // above-water faces.
         if (f1->contents[0].is_liquid(qbsp_options.target_game) != f2->contents[0].is_liquid(qbsp_options.target_game))
             return nullptr;
+
+        // Q1: don't merge across sky boundary - we delete faces inside sky
+        if (f1->contents[0].is_sky(qbsp_options.target_game) != f2->contents[0].is_sky(qbsp_options.target_game))
+            return nullptr;
     }
 
     // find a common edge
diff --git a/testmaps/q1_sky_window.map b/testmaps/q1_sky_window.map
new file mode 100644
index 00000000..79b412ab
--- /dev/null
+++ b/testmaps/q1_sky_window.map
@@ -0,0 +1,104 @@
+// Game: Quake
+// Format: Valve
+// entity 0
+{
+"mapversion" "220"
+"classname" "worldspawn"
+"wad" "deprecated/free_wad.wad"
+// brush 0
+{
+( -208 -288 256 ) ( -208 -288 160 ) ( -208 -224 256 ) brown_brick [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( -176 -288 256 ) ( -176 -288 160 ) ( -208 -288 256 ) brown_brick [ 1 0 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( -208 -224 160 ) ( -208 -288 160 ) ( -176 -224 160 ) brown_brick [ 0 1 0 0 ] [ 1 0 0 -112 ] 270 1 1
+( -208 -288 256 ) ( -208 -224 256 ) ( -176 -288 256 ) brown_brick [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( -208 -224 256 ) ( -208 -224 160 ) ( -176 -224 256 ) brown_brick [ 1 0 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( -176 -224 256 ) ( -176 -224 160 ) ( -176 -288 256 ) brown_brick [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1
+}
+// brush 1
+{
+( -208 -448 -224 ) ( -208 -288 -224 ) ( -208 -448 256 ) brown_brick [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( -176 -432 -224 ) ( -208 -432 -224 ) ( -176 -432 256 ) brown_brick [ 1 0 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( -176 -448 -64 ) ( -176 -288 -64 ) ( -208 -448 -64 ) brown_brick [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( -208 -448 256 ) ( -208 -288 256 ) ( -176 -448 256 ) brown_brick [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( -176 -288 256 ) ( -208 -288 256 ) ( -176 -288 -224 ) brown_brick [ 1 0 0 112 ] [ 0 0 -1 0 ] 0 -1 1
+( -176 -448 256 ) ( -176 -288 256 ) ( -176 -448 -224 ) brown_brick [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1
+}
+// brush 2
+{
+( -208 128 336 ) ( -208 -224 336 ) ( -208 128 -224 ) brown_brick [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( -176 -224 -224 ) ( -208 -224 -224 ) ( -176 -224 336 ) brown_brick [ 1 0 0 -112 ] [ 0 0 -1 0 ] 0 1 1
+( -208 128 -64 ) ( -208 -224 -64 ) ( -176 128 -64 ) brown_brick [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( -176 128 256 ) ( -176 -224 256 ) ( -208 128 256 ) brown_brick [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( -176 112 336 ) ( -208 112 336 ) ( -176 112 -224 ) brown_brick [ 1 0 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( -176 128 -224 ) ( -176 -224 -224 ) ( -176 128 336 ) brown_brick [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1
+}
+// brush 3
+{
+( -208 -288 160 ) ( -208 -288 -32 ) ( -208 -224 160 ) sky3 [ 0 0 -1.0000000000000002 -16 ] [ 0 -1.0000000000000002 0 0 ] 0 1 1
+( -208 -288 160 ) ( -194 -288 160 ) ( -208 -288 -32 ) sky3 [ 1.0000000000000002 0 0 0 ] [ 0 0 1.0000000000000002 -32 ] 0 1 1
+( -208 -224 -32 ) ( -208 -288 -32 ) ( -194 -224 -32 ) sky3 [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( -208 -288 160 ) ( -208 -224 160 ) ( -194 -288 160 ) sky3 [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( -208 -224 -32 ) ( -194 -224 -32 ) ( -208 -224 160 ) sky3 [ 1.0000000000000002 0 0 0 ] [ 0 0 -1.0000000000000002 -32 ] 0 1 1
+( -192 -224 160 ) ( -192 -224 -32 ) ( -192 -288 160 ) sky3 [ 0 0 1.0000000000000002 62 ] [ 0 -1.0000000000000002 0 0 ] 0 1 1
+}
+// brush 4
+{
+( -208 -288 -224 ) ( -208 -224 -224 ) ( -208 -288 -32 ) brown_brick [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( -176 -288 -224 ) ( -208 -288 -224 ) ( -176 -288 -32 ) brown_brick [ 1 0 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( -208 -224 -64 ) ( -208 -288 -64 ) ( -176 -224 -64 ) brown_brick [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( -208 -288 -32 ) ( -208 -224 -32 ) ( -176 -288 -32 ) brown_brick [ 0 1 0 0 ] [ 1 0 0 112 ] 270 1 -1
+( -176 -224 -224 ) ( -176 -224 -32 ) ( -208 -224 -224 ) brown_brick [ 1 0 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( -176 -224 -224 ) ( -176 -288 -224 ) ( -176 -224 -32 ) brown_brick [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1
+}
+// brush 5
+{
+( -176 -448 -224 ) ( -176 -288 -224 ) ( -176 -448 256 ) brown_brick [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( -144 -448 -224 ) ( -176 -448 -224 ) ( -144 -448 256 ) brown_brick [ 1 0 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( -144 -448 -64 ) ( -144 -288 -64 ) ( -176 -448 -64 ) brown_brick [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( -176 -448 256 ) ( -176 -288 256 ) ( -144 -448 256 ) brown_brick [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( -144 -432 256 ) ( -176 -432 256 ) ( -144 -432 -224 ) brown_brick [ 1 0 0 0 ] [ 0 0 -1 0 ] 0 -1 1
+( 192 -448 256 ) ( 192 -288 256 ) ( 192 -448 -224 ) brown_brick [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1
+}
+// brush 6
+{
+( -176 112 -224 ) ( -176 272 -224 ) ( -176 112 256 ) brown_brick [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( -144 112 -224 ) ( -176 112 -224 ) ( -144 112 256 ) brown_brick [ 1 0 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( -144 112 -64 ) ( -144 272 -64 ) ( -176 112 -64 ) brown_brick [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( -176 112 256 ) ( -176 272 256 ) ( -144 112 256 ) brown_brick [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( -144 128 256 ) ( -176 128 256 ) ( -144 128 -224 ) brown_brick [ 1 0 0 0 ] [ 0 0 -1 0 ] 0 -1 1
+( 192 112 256 ) ( 192 272 256 ) ( 192 112 -224 ) brown_brick [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1
+}
+// brush 7
+{
+( 192 -80 336 ) ( 192 -432 336 ) ( 192 -80 -224 ) brown_brick [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( 224 -432 -224 ) ( 192 -432 -224 ) ( 224 -432 336 ) brown_brick [ 1 0 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( 192 -80 -64 ) ( 192 -432 -64 ) ( 224 -80 -64 ) brown_brick [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( 224 -80 256 ) ( 224 -432 256 ) ( 192 -80 256 ) brown_brick [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( 224 112 336 ) ( 192 112 336 ) ( 224 112 -224 ) brown_brick [ 1 0 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( 224 -80 -224 ) ( 224 -432 -224 ) ( 224 -80 336 ) brown_brick [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1
+}
+// brush 8
+{
+( -176 -432 256 ) ( -176 -431 256 ) ( -176 -432 257 ) grate [ 0 -1 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( -176 -432 256 ) ( -176 -432 257 ) ( -175 -432 256 ) grate [ 1 0 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( -176 -432 256 ) ( -175 -432 256 ) ( -176 -431 256 ) grate [ -1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( 192 112 272 ) ( 192 113 272 ) ( 193 112 272 ) grate [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( 192 112 272 ) ( 193 112 272 ) ( 192 112 273 ) grate [ -1 0 0 0 ] [ 0 0 -1 0 ] 0 1 1
+( 192 112 272 ) ( 192 112 273 ) ( 192 113 272 ) grate [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1
+}
+// brush 9
+{
+( -176 -432 -80 ) ( -176 -431 -80 ) ( -176 -432 -79 ) grass [ 0 -1 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( -176 -432 -80 ) ( -176 -432 -79 ) ( -175 -432 -80 ) grass [ 1 0 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( -176 -432 -80 ) ( -175 -432 -80 ) ( -176 -431 -80 ) grass [ -1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( 192 112 -64 ) ( 192 113 -64 ) ( 193 112 -64 ) grass [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( 192 112 -64 ) ( 193 112 -64 ) ( 192 112 -63 ) grass [ -1 0 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( 192 112 -64 ) ( 192 112 -63 ) ( 192 113 -64 ) grass [ 0 1 0 0 ] [ 0 0 -1 32 ] 0 1 1
+}
+}
+// entity 1
+{
+"classname" "info_player_start"
+"origin" "-96 -304 -40"
+"angle" "90"
+}
diff --git a/tests/test_qbsp.cc b/tests/test_qbsp.cc
index 5e943cba..3d9d58dd 100644
--- a/tests/test_qbsp.cc
+++ b/tests/test_qbsp.cc
@@ -1802,3 +1802,17 @@ TEST_CASE("q1_hull1_fail" * doctest::may_fail())
         CHECK(CONTENTS_SOLID == BSP_FindContentsAtPoint(&bsp, 1, &bsp.dmodels[0], qvec3d{0, 0, 0}));
     }
 }
+
+TEST_CASE("q1_sky_window")
+{
+    INFO("faces partially covered by sky were getting wrongly merged and deleted");
+    const auto [bsp, bspx, prt] = LoadTestmap("q1_sky_window.map");
+
+    {
+        INFO("faces around window");
+        CHECK(BSP_FindFaceAtPoint(&bsp, &bsp.dmodels[0], qvec3d(-184, -252, -32))); // bottom
+        CHECK(BSP_FindFaceAtPoint(&bsp, &bsp.dmodels[0], qvec3d(-184, -252, 160))); // top
+        CHECK(BSP_FindFaceAtPoint(&bsp, &bsp.dmodels[0], qvec3d(-184, -288, 60))); // left
+        CHECK(BSP_FindFaceAtPoint(&bsp, &bsp.dmodels[0], qvec3d(-184, -224, 60))); // right
+    }
+}
\ No newline at end of file