adopt parent node bounds if a leaf is unbounded now too

move midsplitbrushes into same group as blocksize
always return a split rather than erroring
list out which method is used of the three

include/qbsp/qbsp.hh

@@ -284,7 +284,7 @@ public:
     setting_invertible_bool oldaxis{this, "oldaxis", true, &debugging_group,
         "uses alternate texture alignment which was default in tyrutils-ericw v0.15.1 and older"};
     setting_bool forcegoodtree{
-        this, "forcegoodtree", false, &debugging_group, "force use of expensive processing for SolidBSP stage"};
+        this, "forcegoodtree", false, &debugging_group, "force use of expensive processing for BrushBSP stage"};
     setting_scalar midsplitsurffraction{this, "midsplitsurffraction", 0.f, 0.f, 1.f, &debugging_group,
         "if 0 (default), use `maxnodesize` for deciding when to switch to midsplit bsp heuristic.\nif 0 < midsplitSurfFraction <= 1, switch to midsplit if the node contains more than this fraction of the model's\ntotal surfaces. Try 0.15 to 0.5. Works better than maxNodeSize for maps with a 3D skybox (e.g. +-128K unit maps)"};
     setting_int32 maxnodesize{this, "maxnodesize", 1024, &debugging_group,
@@ -352,7 +352,7 @@ public:
     // since the max world size in Q3 is {-65536, -65536, -65536, 65536, 65536, 65536}. should we dynamically change this?
     // should we automatically turn this on if the world gets too big but leave it off for smaller worlds?
     setting_blocksize blocksize{this, "blocksize", { 0, 0, 0 }, &common_format_group, "from q3map2; split the world by x/y/z sized chunks, speeding up split decisions"};
-    setting_int32 midsplitbrushes{this, "midsplitbrushes", 128, &debugging_group, "switch to cheaper partitioning if a node contains this many brushes"};
+    setting_int32 midsplitbrushes{this, "midsplitbrushes", 128, &common_format_group, "switch to cheaper partitioning if a node contains this many brushes"};
 
     void setParameters(int argc, const char **argv) override
     {

qbsp/brushbsp.cc

@@ -54,8 +54,12 @@ struct bspstats_t
     std::atomic<int> c_nodes;
     // number of nodes created by splitting on a side_t which had !visible
     std::atomic<int> c_nonvis;
+    // total number of nodes created by qbsp3 method
+    std::atomic<int> c_qbsp3;
     // total number of nodes created by block splitting
     std::atomic<int> c_blocksplit;
+    // total number of nodes created by midsplit
+    std::atomic<int> c_midsplit;
     // total number of leafs
     std::atomic<int> c_leafs;
 };
@@ -745,13 +749,7 @@ static std::optional<qbsp_plane_t> ChooseMidPlaneFromList(const std::vector<std:
     }
 
     // prefer the axial split
-    auto bestsurface = bestaxialplane ? bestaxialplane : bestanyplane;
-
-    if (!bestsurface) {
-        FError("No valid planes in surface list");
-    }
-
-    return bestsurface;
+    return bestaxialplane ? bestaxialplane : bestanyplane;
 }
 
 
@@ -794,8 +792,9 @@ static std::optional<qbsp_plane_t> SelectSplitPlane(const std::vector<std::uniqu
 		}
 	}
 
-    if (brushes.size() >= qbsp_options.midsplitbrushes.value()) {
+    if (brushes.size() >= qbsp_options.midsplitbrushes.value() || use_mid_split) {
         if (auto mid_plane = ChooseMidPlaneFromList(brushes, node->bounds)) {
+            stats.c_midsplit++;
 
             for (auto &b : brushes) {
                 b->side = TestBrushToPlanenum(*b, mid_plane.value(), nullptr, nullptr, nullptr);
@@ -929,6 +928,8 @@ static std::optional<qbsp_plane_t> SelectSplitPlane(const std::vector<std::uniqu
         stats.c_nonvis++;
     }
 
+    stats.c_qbsp3++;
+
     return bestside->plane;
 }
 
@@ -1124,6 +1125,8 @@ static std::unique_ptr<tree_t> BrushBSP(mapentity_t *entity, std::vector<std::un
     logging::print(logging::flag::STAT, "     {:8} visible nodes\n", stats.c_nodes - stats.c_nonvis);
     logging::print(logging::flag::STAT, "     {:8} nonvis nodes\n", stats.c_nonvis);
     logging::print(logging::flag::STAT, "     {:8} block split nodes\n", stats.c_blocksplit);
+    logging::print(logging::flag::STAT, "     {:8} expensive split nodes\n", stats.c_qbsp3);
+    logging::print(logging::flag::STAT, "     {:8} midsplit nodes\n", stats.c_midsplit);
     logging::print(logging::flag::STAT, "     {:8} leafs\n", stats.c_leafs);
     qbsp_options.target_game->print_content_stats(*stats.leafstats, "leafs");
 

qbsp/portals.cc

@@ -389,18 +389,17 @@ static void CalcTreeBounds_r(tree_t *tree, node_t *node)
 {
     if (node->is_leaf) {
         CalcNodeBounds(node);
-        return;
+    } else {
+        tbb::task_group g;
+        g.run([&]() { CalcTreeBounds_r(tree, node->children[0].get()); });
+        g.run([&]() { CalcTreeBounds_r(tree, node->children[1].get()); });
+        g.wait();
+
+        node->bounds = node->children[0]->bounds + node->children[1]->bounds;
     }
 
-    tbb::task_group g;
-    g.run([&]() { CalcTreeBounds_r(tree, node->children[0].get()); });
-    g.run([&]() { CalcTreeBounds_r(tree, node->children[1].get()); });
-    g.wait();
-
-    node->bounds = node->children[0]->bounds + node->children[1]->bounds;
-
     if (node->bounds.mins()[0] >= node->bounds.maxs()[0]) {
-        logging::print("WARNING: node without a volume\n");
+        logging::print("WARNING: {} without a volume\n", node->is_leaf ? "leaf" : "node");
 
         // fixme-brushbsp: added this to work around leafs with no portals showing up in "qbspfeatures.map" among other
         // test maps. Not sure if correct or there's another underlying problem.
@@ -409,7 +408,7 @@ static void CalcTreeBounds_r(tree_t *tree, node_t *node)
 
     for (auto &v : node->bounds.mins()) {
         if (fabs(v) > qbsp_options.worldextent.value()) {
-            logging::print("WARNING: node with unbounded volume\n");
+            logging::print("WARNING: {} with unbounded volume\n", node->is_leaf ? "leaf" : "node");
             break;
         }
     }