Merge branch 'brushbsp' of https://github.com/ericwa/ericw-tools into brushbsp

common/bspfile.cc

@@ -598,7 +598,7 @@ public:
         return contents_are_solid(contents) || contents_are_sky(contents);
     }
 
-    contentflags_t contents_remap_for_export(const contentflags_t &contents) const override
+    contentflags_t contents_remap_for_export(const contentflags_t &contents, remap_type_t type) const override
     {
         /*
          * This is for func_detail_wall.. we want to write a solid leaf that has faces,
@@ -1267,21 +1267,31 @@ struct gamedef_q2_t : public gamedef_t
         return contents_are_solid(contents) || contents_are_sky(contents);
     }
 
-    contentflags_t contents_remap_for_export(const contentflags_t &contents) const override
+    contentflags_t contents_remap_for_export(const contentflags_t &contents, remap_type_t type) const override
     {
         // HACK: borrowing Q2_CONTENTS_MONSTER for func_detail_wall
         if (contents.native & Q2_CONTENTS_MONSTER) {
             return {Q2_CONTENTS_SOLID};
         }
+        // Solid wipes out any other contents
+        // Previously, this was done in LeafNode but we've changed to detail-solid being
+        // non-sealing.
+        if (type == remap_type_t::leaf) {
+            if (contents.native & Q2_CONTENTS_SOLID) {
+                return {Q2_CONTENTS_SOLID};
+            }
+        }
 
         return contents;
     }
 
     contentflags_t combine_contents(const contentflags_t &a, const contentflags_t &b) const override
     {
+#if 0
         if ((a.native & Q2_CONTENTS_SOLID) || (b.native & Q2_CONTENTS_SOLID)) {
             return {Q2_CONTENTS_SOLID};
         }
+#endif
 
         contentflags_t result;
         result.native = a.native | b.native;

common/prtfile.cc

@@ -186,7 +186,7 @@ void WriteDebugPortals(const std::vector<polylib::winding_t> &portals, fs::path
 {
     size_t portal_count = portals.size();
 
-    std::ofstream portal_file(name, std::ios_base::binary | std::ios_base::out);
+    std::ofstream portal_file(name, std::ios_base::out);
     if (!portal_file)
         FError("Failed to open {}: {}", name, strerror(errno));
 

include/common/bspfile.hh

@@ -317,7 +317,12 @@ struct gamedef_t
     virtual bool portal_can_see_through(
         const contentflags_t &contents0, const contentflags_t &contents1, bool transwater, bool transsky) const = 0;
     virtual bool contents_seals_map(const contentflags_t &contents) const = 0;
-    virtual contentflags_t contents_remap_for_export(const contentflags_t &contents) const = 0;
+    enum class remap_type_t
+    {
+        brush,
+        leaf
+    };
+    virtual contentflags_t contents_remap_for_export(const contentflags_t &contents, remap_type_t type) const = 0;
     virtual contentflags_t combine_contents(const contentflags_t &a, const contentflags_t &b) const = 0;
     // for a portal with contents from `a` to `b`, returns what type of face should be rendered facing `a` and `b`
     virtual contentflags_t portal_visible_contents(const contentflags_t &a, const contentflags_t &b) const = 0;

qbsp/brushbsp.cc

@@ -949,16 +949,20 @@ static side_t *SelectSplitPlane(
     side_t *bestside = nullptr;
     int bestvalue = -99999;
 
-    // the search order goes: visible-structural, visible-detail,
+    // the search order goes: (changed from q2 tools - see q2_detail_leak_test.map for the issue
-    // nonvisible-structural, nonvisible-detail.
+    // with the vanilla q2 tools method):
+    //
+    //   0. visible-structural
+    //   1. nonvisible-structural,
+    //   2. visible-detail
+    //   3. nonvisible-detail.
+    //
     // If any valid plane is available in a pass, no further
     // passes will be tried.
     constexpr int numpasses = 4;
     for (int pass = 0; pass < numpasses; pass++) {
         for (auto &brush : brushes) {
-            if ((pass & 1) && !brush->contents.is_any_detail(qbsp_options.target_game))
+            if ((pass >= 2) != brush->contents.is_any_detail(qbsp_options.target_game))
-                continue;
-            if (!(pass & 1) && brush->contents.is_any_detail(qbsp_options.target_game))
                 continue;
             for (auto &side : brush->sides) {
                 if (side.bevel)
@@ -971,8 +975,8 @@ static side_t *SelectSplitPlane(
                     continue; // we allready have metrics for this plane
                 if (side.get_texinfo().flags.is_hintskip)
                     continue; // skip surfaces are never chosen
-                if (side.is_visible() ^ (pass < 2))
+                if (side.is_visible() != (pass == 0 || pass == 2))
-                    continue; // only check visible faces on first pass
+                    continue; // only check visible faces on pass 0/2
 
                 size_t positive_planenum = side.planenum & ~1;
                 const qbsp_plane_t &plane = side.get_positive_plane(); // always use positive facing plane
@@ -1046,7 +1050,7 @@ static side_t *SelectSplitPlane(
         // if we found a good plane, don't bother trying any
         // other passes
         if (bestside) {
-            if (pass > 0)
+            if (pass >= 2)
                 node->detail_separator = true; // not needed for vis
             break;
         }

qbsp/portals.cc

@@ -30,6 +30,7 @@
 #include <qbsp/tree.hh>
 #include <common/log.hh>
 #include <atomic>
+#include <common/prtfile.hh>
 
 #include "tbb/task_group.h"
 #include "common/vectorutils.hh"
@@ -792,6 +793,8 @@ struct visible_faces_stats_t : logging::stat_tracker_t
 {
     stat &sides_not_found = register_stat("sides not found (use -verbose to display)", false, true);
     stat &sides_visible = register_stat("sides visible");
+
+    std::vector<polylib::winding_t> missing_portal_sides;
 };
 
 /*
@@ -898,6 +901,7 @@ static void FindPortalSide(portal_t *p, visible_faces_stats_t &stats)
     if (!bestside[0] && !bestside[1]) {
         stats.sides_not_found++;
         logging::print(logging::flag::VERBOSE, "couldn't find portal side at {}\n", p->winding.center());
+        stats.missing_portal_sides.push_back(p->winding.clone());
     }
 
     p->sidefound = true;
@@ -959,4 +963,10 @@ void MarkVisibleSides(tree_t &tree, bspbrush_t::container &brushes)
     visible_faces_stats_t stats;
     // set visible flags on the sides that are used by portals
     MarkVisibleSides_r(tree.headnode, stats);
+
+    if (!stats.missing_portal_sides.empty()) {
+        fs::path name = qbsp_options.bsp_path;
+        name.replace_extension("missing_portal_sides.prt");
+        WriteDebugPortals(stats.missing_portal_sides, name);
+    }
 }

qbsp/prtfile.cc

@@ -173,7 +173,7 @@ static void NumberLeafs_r(node_t *node, portal_state_t &state, int cluster)
         return;
     }
 
-    if (node->contents.is_solid(qbsp_options.target_game)) {
+    if (node->contents.is_any_solid(qbsp_options.target_game)) {
         /* solid block, viewpoint never inside */
         node->visleafnum = -1;
         node->viscluster = -1;

qbsp/qbsp.cc

@@ -819,7 +819,9 @@ static void ExportBrushList_r(const mapentity_t &entity, node_t *node, brush_lis
                         dbrush_t &brush = map.bsp.dbrushes.emplace_back(
                             dbrush_t{.firstside = static_cast<int32_t>(map.bsp.dbrushsides.size()),
                                 .numsides = 0,
-                                .contents = qbsp_options.target_game->contents_remap_for_export(b->contents).native});
+                                .contents = qbsp_options.target_game
+                                                ->contents_remap_for_export(b->contents, gamedef_t::remap_type_t::brush)
+                                                .native});
 
                         for (auto &side : b->mapbrush->faces) {
                             map.bsp.dbrushsides.push_back(

qbsp/writebsp.cc

@@ -150,7 +150,8 @@ static void ExportLeaf(node_t *node)
 {
     mleaf_t &dleaf = map.bsp.dleafs.emplace_back();
 
-    const contentflags_t remapped = qbsp_options.target_game->contents_remap_for_export(node->contents);
+    const contentflags_t remapped =
+        qbsp_options.target_game->contents_remap_for_export(node->contents, gamedef_t::remap_type_t::leaf);
 
     if (!remapped.is_valid(qbsp_options.target_game, false)) {
         FError("Internal error: On leaf {}, tried to save invalid contents type {}", map.bsp.dleafs.size() - 1,

tests/test_common.cc

@@ -166,7 +166,8 @@ TEST_SUITE("common")
             for (const auto &c : test_contents) {
                 // solid is treated specially in Q2 and wipes out any other content
                 // flags when combined
-                auto combined = game_q2->combine_contents(solid, c);
+                auto combined = game_q2->contents_remap_for_export(
+                    game_q2->combine_contents(solid, c), gamedef_t::remap_type_t::leaf);
 
                 CHECK(combined.native == Q2_CONTENTS_SOLID);
                 CHECK(!combined.game_data.has_value());

tests/test_qbsp_q2.cc

@@ -500,12 +500,9 @@ TEST_CASE("q2_seal_empty_rooms" * doctest::test_suite("testmaps_q2"))
     CHECK(prt->portalleafs == 1);
 }
 
-/**
+TEST_CASE("q2_detail_non_sealing" * doctest::test_suite("testmaps_q2"))
- * Detail seals in Q2
- **/
-TEST_CASE("q2_detail_seals" * doctest::test_suite("testmaps_q2"))
 {
-    const auto [bsp, bspx, prt] = LoadTestmapQ2("q2_detail_seals.map");
+    const auto [bsp, bspx, prt] = LoadTestmapQ2("q2_detail_non_sealing.map");
 
     CHECK(GAME_QUAKE_II == bsp.loadversion->game->id);
 
@@ -514,7 +511,7 @@ TEST_CASE("q2_detail_seals" * doctest::test_suite("testmaps_q2"))
 
     // check leaf contents
     CHECK(Q2_CONTENTS_EMPTY == BSP_FindLeafAtPoint(&bsp, &bsp.dmodels[0], in_start_room)->contents);
-    CHECK(Q2_CONTENTS_SOLID == BSP_FindLeafAtPoint(&bsp, &bsp.dmodels[0], in_void)->contents);
+    CHECK(Q2_CONTENTS_EMPTY == BSP_FindLeafAtPoint(&bsp, &bsp.dmodels[0], in_void)->contents);
 }
 
 /**