qbsp: only reuse edges once for software renderer compat

Issue reported by Izhido
ericwa · Aug 5, 2024 · 3f1659d · 3f1659d
1 parent 0b2395d
commit 3f1659d
Show file tree

Hide file tree

Showing 5 changed files with 127 additions and 3 deletions.
diff --git a/include/qbsp/map.hh b/include/qbsp/map.hh
@@ -171,6 +171,12 @@ struct hashedge_t
      * the face that edge v1 -> v2 belongs to
      */
     const face_t *face;
+
+    /**
+     * Has v2 -> v1 been referenced by another face yet, by using -edge_index?
+     * This is only allowed to happen once (software renderer limitation).
+     */
+    bool has_been_reused;
 };
 
 struct mapdata_t

diff --git a/qbsp/faces.cc b/qbsp/faces.cc
@@ -150,11 +150,16 @@ inline int64_t GetEdge(const size_t &v1, const size_t &v2, const face_t *face, e
     if (!qbsp_options.noedgereuse.value()) {
         // search for existing edges
         if (auto it = map.hashedges.find(std::make_pair(v2, v1)); it != map.hashedges.end()) {
-            const hashedge_t &existing = it->second;
+            hashedge_t &existing = it->second;
             // this content check is required for software renderers
             // (see q1_liquid_software test case)
             if (existing.face->contents.front.equals(qbsp_options.target_game, face->contents.front)) {
-                return -existing.edge_index;
+                // only reusing an edge once is a separate limitation of software renderers
+                // (see q1_edge_sharing_software.map test case)
+                if (!existing.has_been_reused) {
+                    existing.has_been_reused = true;
+                    return -existing.edge_index;
+                }
             }
         }
     }

diff --git a/qbsp/map.cc b/qbsp/map.cc
@@ -168,7 +168,8 @@ void mapdata_t::add_hash_vector(const qvec3d &point, const size_t &num)
 
 void mapdata_t::add_hash_edge(size_t v1, size_t v2, int64_t edge_index, const face_t *face)
 {
-    hashedges.emplace(std::make_pair(v1, v2), hashedge_t{.v1 = v1, .v2 = v2, .edge_index = edge_index, .face = face});
+    hashedges.emplace(std::make_pair(v1, v2), hashedge_t{.v1 = v1, .v2 = v2, .edge_index = edge_index, .face = face,
+                                                         .has_been_reused = false});
 }
 
 const std::optional<img::texture_meta> &mapdata_t::load_image_meta(const std::string_view &name)

diff --git a/testmaps/q1_edge_sharing_software.map b/testmaps/q1_edge_sharing_software.map
@@ -0,0 +1,92 @@
+// Game: Quake
+// Format: Valve
+// entity 0
+{
+"mapversion" "220"
+"classname" "worldspawn"
+"wad" "deprecated/free_wad.wad;deprecated/hintskip.wad"
+// brush 0
+{
+( 184 112 152 ) ( 184 -432 152 ) ( 184 112 -80 ) skip [ 0 1 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( 192 -432 -80 ) ( 184 -432 -80 ) ( 192 -432 152 ) skip [ 1 0 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( 192 112 -80 ) ( 184 112 -80 ) ( 192 -432 -80 ) skip [ -1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( 192 -432 152 ) ( 184 -432 152 ) ( 192 112 152 ) skip [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( 192 112 152 ) ( 184 112 152 ) ( 192 112 -80 ) skip [ -1 0 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( 192 -432 152 ) ( 192 112 152 ) ( 192 -432 -80 ) skip [ 0 1 0 0 ] [ 0 0 -1 32 ] 0 1 1
+}
+// brush 1
+{
+( -176 -432 -80 ) ( -176 112 -80 ) ( -176 -432 152 ) skip [ 0 -1 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( -176 -432 152 ) ( -168 -432 152 ) ( -176 -432 -80 ) skip [ 1 0 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( -176 -432 -80 ) ( -168 -432 -80 ) ( -176 112 -80 ) skip [ -1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( -176 112 152 ) ( -168 112 152 ) ( -176 -432 152 ) skip [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( -176 112 -80 ) ( -168 112 -80 ) ( -176 112 152 ) skip [ -1 0 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( -168 -432 -80 ) ( -168 -432 152 ) ( -168 112 -80 ) skip [ 0 -1 0 0 ] [ 0 0 -1 32 ] 0 1 1
+}
+// brush 2
+{
+( -168 112 152 ) ( -168 104 152 ) ( -168 112 -80 ) skip [ 0 -1 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( 184 104 152 ) ( 184 104 -80 ) ( -168 104 152 ) skip [ -1 0 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( -168 112 -80 ) ( -168 104 -80 ) ( 184 112 -80 ) skip [ -1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( 184 112 152 ) ( 184 104 152 ) ( -168 112 152 ) skip [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( 184 112 152 ) ( -168 112 152 ) ( 184 112 -80 ) skip [ -1 0 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( 184 112 -80 ) ( 184 104 -80 ) ( 184 112 152 ) skip [ 0 1 0 0 ] [ 0 0 -1 32 ] 0 1 1
+}
+// brush 3
+{
+( -168 -432 -80 ) ( -168 -424 -80 ) ( -168 -432 152 ) skip [ 0 -1 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( 184 -432 -80 ) ( -168 -432 -80 ) ( 184 -432 152 ) skip [ 1 0 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( 184 -432 -80 ) ( 184 -424 -80 ) ( -168 -432 -80 ) skip [ -1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( -168 -432 152 ) ( -168 -424 152 ) ( 184 -432 152 ) skip [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( -168 -424 -80 ) ( 184 -424 -80 ) ( -168 -424 152 ) skip [ 1 0 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( 184 -432 152 ) ( 184 -424 152 ) ( 184 -432 -80 ) skip [ 0 1 0 0 ] [ 0 0 -1 32 ] 0 1 1
+}
+// brush 4
+{
+( -168 -424 152 ) ( -168 -424 144 ) ( -168 104 152 ) skip [ 0 -1 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( 184 -424 152 ) ( 184 -424 144 ) ( -168 -424 152 ) skip [ 1 0 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( 184 104 144 ) ( -168 104 144 ) ( 184 -424 144 ) skip [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( 184 104 152 ) ( 184 -424 152 ) ( -168 104 152 ) skip [ 1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( -168 104 152 ) ( -168 104 144 ) ( 184 104 152 ) skip [ -1 0 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( 184 104 152 ) ( 184 104 144 ) ( 184 -424 152 ) skip [ 0 1 0 0 ] [ 0 0 -1 32 ] 0 1 1
+}
+// brush 5
+{
+( -168 104 -80 ) ( -168 104 -72 ) ( -168 -424 -80 ) skip [ 0 -1 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( -168 -424 -80 ) ( -168 -424 -72 ) ( 184 -424 -80 ) skip [ 1 0 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( -168 104 -80 ) ( -168 -424 -80 ) ( 184 104 -80 ) skip [ -1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( -168 -424 -72 ) ( -168 104 -72 ) ( 184 -424 -72 ) skip [ -1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( 184 104 -80 ) ( 184 104 -72 ) ( -168 104 -80 ) skip [ -1 0 0 0 ] [ 0 0 -1 32 ] 0 1 1
+( 184 -424 -80 ) ( 184 -424 -72 ) ( 184 104 -80 ) skip [ 0 1 0 0 ] [ 0 0 -1 32 ] 0 1 1
+}
+// brush 6
+{
+( -80 16 -16 ) ( -112 -64 -16 ) ( -112 -64 -64 ) bolt12 [ 0 1.0000000000000002 0 20 ] [ 0 0 -1.0000000000000002 -8 ] 0 1 1
+( -112 -64 -16 ) ( -64 -64 -48 ) ( -64 -64 -64 ) bolt12 [ -1.0000000000000002 0 0 0 ] [ 0 0 -1.0000000000000002 -8 ] 0 1 1
+( -112 -64 -64 ) ( -64 -64 -64 ) ( -80 16 -64 ) bolt12 [ -1.0000000000000002 0 0 0 ] [ 0 1.0000000000000002 0 42.13333 ] 0 1 1
+( -80 16 -16 ) ( -64 -64 -48 ) ( -112 -64 -16 ) bolt16 [ -1 0 0 0 ] [ 0 -1 0 0 ] 0 1 1
+( -64 -64 -64 ) ( -64 -64 -48 ) ( -80 16 -16 ) bolt12 [ 0 0 1.0000000000000002 32 ] [ 0 -1.0000000000000002 0 0 ] 0 1 1
+}
+// brush 7
+{
+( -64 -64 -48 ) ( 0 -48 0 ) ( 0 -96 0 ) bolt16 [ -3.061616997868383e-16 -1 0 -16 ] [ 1 -3.061616997868383e-16 0 -15.999998 ] 321.38824 1 1
+( 0 -96 -64 ) ( -64 -64 -64 ) ( -64 -64 -48 ) bolt12 [ -1.0000000000000002 0 0 0 ] [ 0 0 -1.0000000000000002 -8 ] 0 1 1
+( 0 -48 -64 ) ( 0 -48 0 ) ( -64 -64 -48 ) bolt12 [ 0 0 1.0000000000000002 32 ] [ 1.0000000000000002 -3.0616169978683836e-16 0 -16 ] 0 1 1
+( 0 -96 -64 ) ( 0 -48 -64 ) ( -64 -64 -64 ) bolt12 [ -1.0000000000000002 0 0 0 ] [ 0 1.0000000000000002 0 10.133331 ] 0 1 1
+( 0 -96 0 ) ( 0 -48 0 ) ( 0 -48 -64 ) bolt12 [ -3.0616169978683836e-16 -1.0000000000000002 0 -16 ] [ 0 0 -1.0000000000000002 -8 ] 0 1 1
+}
+// brush 8
+{
+( -64 -16 -16 ) ( -64 -64 -48 ) ( -64 -64 -64 ) bolt12 [ 1.8369701987210302e-16 1.0000000000000002 0 0 ] [ 0 0 -1.0000000000000002 -8 ] 0 1 1
+( -64 -16 -16 ) ( 16 -16 -16 ) ( -64 -64 -48 ) bolt16 [ 1.8369701987210297e-16 1 0 0 ] [ -1 1.8369701987210297e-16 0 0 ] 90 1 1
+( -64 -64 -64 ) ( 16 -16 -64 ) ( -64 -16 -64 ) bolt12 [ -1.0000000000000002 0 0 0 ] [ 0 1.0000000000000002 0 60.266663 ] 0 1 1
+( 16 -16 -64 ) ( 16 -16 -16 ) ( -64 -16 -16 ) bolt12 [ -1.0000000000000002 0 0 0 ] [ 0 0 -1.0000000000000002 -8 ] 0 1 1
+( 16 -16 -64 ) ( -64 -64 -64 ) ( -64 -64 -48 ) bolt12 [ 0 0 1.0000000000000002 32 ] [ -1.0000000000000002 1.8369701987210302e-16 0 0 ] 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
@@ -2060,6 +2060,26 @@ TEST(qbspQ1, liquidSoftware)
     }
 }
 
+TEST(qbspQ1, edgeSharingSoftware)
+{
+    SCOPED_TRACE("the software renderer only allows a given edge to be reused at most once, as the backwards version (negative index)");
+    const auto [bsp, bspx, prt] = LoadTestmap("q1_edge_sharing_software.map");
+
+    std::map<int, std::vector<const mface_t *>> signed_edge_faces;
+    for (auto &face : bsp.dfaces) {
+        for (int i = face.firstedge; i < (face.firstedge + face.numedges); ++i) {
+            // may be negative
+            const int edge = bsp.dsurfedges.at(i);
+
+            signed_edge_faces[edge].push_back(&face);
+        }
+    }
+
+    for (auto &[edge, faces] : signed_edge_faces) {
+        EXPECT_EQ(1, faces.size());
+    }
+}
+
 TEST(qbspQ1, missingTexture)
 {
     const auto [bsp, bspx, prt] = LoadTestmap("q1_missing_texture.map");