qbsp: redesign _chop and _chop_order

- add docs and tests

docs/qbsp.rst

@@ -173,11 +173,12 @@ Options
 
 .. option:: -q2bsp
 
-   Target Quake II's BSP format.
+   Target Quake II and the vanilla Q2BSP format, automatically switching to Qbism format
+   if necessary (unless :option:`-noallowupgrade` is specified.)
 
 .. option:: -qbism
 
-   Target Qbism's extended Quake II BSP format.
+   Target Quake II and use Qbism's extended Quake II BSP format.
 
 .. option:: -q2rtx
 
@@ -810,6 +811,21 @@ Model Entity Keys
    player view is inside the bmodel, they will still see the faces.
    (e.g. for func_water, or func_illusionary)
 
+.. bmodel-key:: "_chop_order" "n"
+
+   Customize the brush order, which affects which brush "wins" in the CSG phase when there are multiple overlapping
+   brushes, since most .map editors don't directly expose the brush order.
+
+   Defaults to 0, brushes with higher values (equivalent to appearing later in the .map file) will clip away lower
+   valued brushes.
+
+.. bmodel-key:: "_chop" "n"
+
+   Set to 0 to prevent these brushes from being chopped.
+
+   .. deprecated:: 2.0.0
+      Prefer the more flexible :bmodel-key:`_chop_order` instead.
+
 Other Special-Purpose Entities
 ------------------------------
 

include/qbsp/map.hh

@@ -100,8 +100,9 @@ public:
     int16_t lmshift = 0; /* lightmap scaling (qu/lightmap pixel), passed to the light util */
     mapentity_t *func_areaportal = nullptr;
     bool is_hint = false; // whether we are a hint brush or not (at least one side is "hint" or SURF_HINT)
-    bool no_chop = false; // don't chop this
     int32_t chop_index = 0; // chopping order; higher numbers chop lower numbers
+
+    std::tuple<int32_t, std::optional<size_t>> sort_key() const;
 };
 
 enum class rotation_t

qbsp/brushbsp.cc

@@ -1348,6 +1348,9 @@ Returns true if b1 is allowed to bite b2
 */
 inline bool BrushGE(const bspbrush_t &b1, const bspbrush_t &b2)
 {
+    if (b1.mapbrush->sort_key() < b2.mapbrush->sort_key())
+        return false;
+
     // detail brushes never bite structural brushes
     if ((b1.contents.is_any_detail(qbsp_options.target_game)) &&
         !(b2.contents.is_any_detail(qbsp_options.target_game))) {
@@ -1467,17 +1470,9 @@ newlist:
 
         auto &b1 = *b1_it;
 
-        if (b1->mapbrush->no_chop) {
-            continue;
-        }
-
         for (auto b2_it = next; b2_it != list.end(); b2_it++) {
             auto &b2 = *b2_it;
 
-            if (b2->mapbrush->no_chop) {
-                continue;
-            }
-
             if (BrushesDisjoint(*b1, *b2)) {
                 continue;
             }

qbsp/map.cc

@@ -995,6 +995,11 @@ const qbsp_plane_t &mapface_t::get_positive_plane() const
     return map.get_plane(planenum & ~1);
 }
 
+std::tuple<int32_t, std::optional<size_t>> mapbrush_t::sort_key() const
+{
+    return {chop_index, line.line_number};
+}
+
 static std::optional<mapface_t> ParseBrushFace(
     const mapfile::brush_side_t &input_side, const mapbrush_t &brush, const mapentity_t &entity, texture_def_issues_t &issue_stats)
 {
@@ -2132,10 +2137,10 @@ void ProcessMapBrushes()
                 brush.func_areaportal = areaportal;
                 brush.is_hint = MapBrush_IsHint(brush);
 
-                // _chop signals that a brush does not partake in the BSP chopping phase.
-                // this allows brushes embedded in others to be retained.
+                // "_chop" "0" is a deprecated way of saying "don't let this brush get chopped by others", i.e.
+                // move it to the end of the brush list.
                 if (entity.epairs.has("_chop") && !entity.epairs.get_int("_chop")) {
-                    brush.no_chop = true;
+                    brush.chop_index = 1;
                 }
 
                 // brushes are sorted by their _chop_order; higher numbered brushes

qbsp/qbsp.cc

@@ -1062,16 +1062,13 @@ static void ProcessEntity(mapentity_t &entity, hull_index_t hullnum)
     logging::print(
         logging::flag::STAT, "INFO: calculating BSP for {} brushes with {} sides\n", brushes.size(), num_sides);
 
+    // sort by ascending (chop_index, line_number) pair
+    std::ranges::sort(brushes, {}, [](const bspbrush_t::ptr &a) -> std::tuple<int32_t, std::optional<size_t>> {
+        return a->mapbrush->sort_key();
+    });
+
     // always chop the other hulls to reduce brush tests
     if (qbsp_options.chop.value() || hullnum.value_or(0)) {
-        std::sort(brushes.begin(), brushes.end(), [](const bspbrush_t::ptr &a, const bspbrush_t::ptr &b) -> bool {
-            if (a->mapbrush->chop_index == b->mapbrush->chop_index) {
-                return a->mapbrush->line.line_number < b->mapbrush->line.line_number;
-            }
-
-            return a->mapbrush->chop_index < b->mapbrush->chop_index;
-        });
-
         ChopBrushes(brushes, qbsp_options.chopfragment.value());
     }
 

testmaps/q2_chop_order_0.map

@@ -0,0 +1,40 @@
+// Game: Quake 2
+// Format: Quake2 (Valve)
+// entity 0
+{
+"mapversion" "220"
+"classname" "worldspawn"
+"_tb_textures" "textures/e1u1"
+}
+// entity 1
+{
+"classname" "info_player_start"
+"origin" "-32 32 24"
+}
+// entity 2
+{
+"classname" "func_group"
+"_chop_order" "0"
+// brush 0
+{
+( -32 32 0 ) ( -32 -32 0 ) ( -32 -32 -64 ) e1u1/+0btshoot2 [ 0 -1 0 29 ] [ 0 0 -1 0 ] 0 1 1 0 0 0
+( -32 -32 0 ) ( 32 -32 0 ) ( 32 -32 -64 ) e1u1/+0btshoot2 [ 1 0 0 -3 ] [ 0 0 -1 0 ] 0 1 1 0 0 0
+( 32 -32 -64 ) ( 32 32 -64 ) ( -32 32 -64 ) e1u1/+0btshoot2 [ -1 0 0 3 ] [ 0 -1 0 29 ] 0 1 1 0 0 0
+( -32 32 0 ) ( 32 32 0 ) ( 32 -32 0 ) e1u1/+0btshoot2 [ 1 0 0 -3 ] [ 0 -1 0 29 ] 0 1 1 0 0 0
+( 32 32 -64 ) ( 32 32 0 ) ( -32 32 0 ) e1u1/+0btshoot2 [ -1 0 0 3 ] [ 0 0 -1 0 ] 0 1 1 0 0 0
+( 32 -32 0 ) ( 32 32 0 ) ( 32 32 -64 ) e1u1/+0btshoot2 [ 0 1 0 -29 ] [ 0 0 -1 0 ] 0 1 1 0 0 0
+}
+}
+// entity 3
+{
+"classname" "func_group"
+// brush 0
+{
+( -64 64 0 ) ( -64 -64 0 ) ( -64 -64 -32 ) e1u1/ggrat4_2 [ 0 -1 0 0 ] [ 0 0 -1 -112 ] 0 1 1 0 0 0
+( -64 -64 0 ) ( 64 -64 0 ) ( 64 -64 -32 ) e1u1/ggrat4_2 [ 1 0 0 -96 ] [ 0 0 -1 -112 ] 0 1 1 0 0 0
+( 64 -64 -96 ) ( 64 64 -96 ) ( -64 64 -96 ) e1u1/ggrat4_2 [ 1 0 0 -96 ] [ 0 -1 0 0 ] 0 1 1 0 0 0
+( -64 64 0 ) ( 64 64 0 ) ( 64 -64 0 ) e1u1/ggrat4_2 [ 1 0 0 -96 ] [ 0 -1 0 0 ] 0 1 1 0 0 0
+( 64 64 -32 ) ( 64 64 0 ) ( -64 64 0 ) e1u1/ggrat4_2 [ -1 0 0 96 ] [ 0 0 -1 -112 ] 0 1 1 0 0 0
+( 64 -64 0 ) ( 64 64 0 ) ( 64 64 -32 ) e1u1/ggrat4_2 [ 0 1 0 0 ] [ 0 0 -1 -112 ] 0 1 1 0 0 0
+}
+}

testmaps/q2_chop_order_1.map

@@ -0,0 +1,40 @@
+// Game: Quake 2
+// Format: Quake2 (Valve)
+// entity 0
+{
+"mapversion" "220"
+"classname" "worldspawn"
+"_tb_textures" "textures/e1u1"
+}
+// entity 1
+{
+"classname" "info_player_start"
+"origin" "-32 32 24"
+}
+// entity 2
+{
+"classname" "func_group"
+"_chop_order" "1"
+// brush 0
+{
+( -32 32 0 ) ( -32 -32 0 ) ( -32 -32 -64 ) e1u1/+0btshoot2 [ 0 -1 0 29 ] [ 0 0 -1 0 ] 0 1 1 0 0 0
+( -32 -32 0 ) ( 32 -32 0 ) ( 32 -32 -64 ) e1u1/+0btshoot2 [ 1 0 0 -3 ] [ 0 0 -1 0 ] 0 1 1 0 0 0
+( 32 -32 -64 ) ( 32 32 -64 ) ( -32 32 -64 ) e1u1/+0btshoot2 [ -1 0 0 3 ] [ 0 -1 0 29 ] 0 1 1 0 0 0
+( -32 32 0 ) ( 32 32 0 ) ( 32 -32 0 ) e1u1/+0btshoot2 [ 1 0 0 -3 ] [ 0 -1 0 29 ] 0 1 1 0 0 0
+( 32 32 -64 ) ( 32 32 0 ) ( -32 32 0 ) e1u1/+0btshoot2 [ -1 0 0 3 ] [ 0 0 -1 0 ] 0 1 1 0 0 0
+( 32 -32 0 ) ( 32 32 0 ) ( 32 32 -64 ) e1u1/+0btshoot2 [ 0 1 0 -29 ] [ 0 0 -1 0 ] 0 1 1 0 0 0
+}
+}
+// entity 3
+{
+"classname" "func_group"
+// brush 0
+{
+( -64 64 0 ) ( -64 -64 0 ) ( -64 -64 -32 ) e1u1/ggrat4_2 [ 0 -1 0 0 ] [ 0 0 -1 -112 ] 0 1 1 0 0 0
+( -64 -64 0 ) ( 64 -64 0 ) ( 64 -64 -32 ) e1u1/ggrat4_2 [ 1 0 0 -96 ] [ 0 0 -1 -112 ] 0 1 1 0 0 0
+( 64 -64 -96 ) ( 64 64 -96 ) ( -64 64 -96 ) e1u1/ggrat4_2 [ 1 0 0 -96 ] [ 0 -1 0 0 ] 0 1 1 0 0 0
+( -64 64 0 ) ( 64 64 0 ) ( 64 -64 0 ) e1u1/ggrat4_2 [ 1 0 0 -96 ] [ 0 -1 0 0 ] 0 1 1 0 0 0
+( 64 64 -32 ) ( 64 64 0 ) ( -64 64 0 ) e1u1/ggrat4_2 [ -1 0 0 96 ] [ 0 0 -1 -112 ] 0 1 1 0 0 0
+( 64 -64 0 ) ( 64 64 0 ) ( 64 64 -32 ) e1u1/ggrat4_2 [ 0 1 0 0 ] [ 0 0 -1 -112 ] 0 1 1 0 0 0
+}
+}

tests/test_qbsp_q2.cc

@@ -1105,3 +1105,17 @@ TEST(ltfaceQ2, noclipfacesNodraw)
     EXPECT_EQ(Face_TextureNameView(&bsp, up_faces[0]), "e1u1/water1_8");
     EXPECT_EQ(Face_TextureNameView(&bsp, down_faces[0]), "e1u1/water1_8");
 }
+
+TEST(testmapsQ2, chopOrder0) {
+    const auto [bsp, bspx, prt] = LoadTestmapQ2("q2_chop_order_0.map");
+
+    EXPECT_VECTORS_UNOREDERED_EQUAL(TexNames(bsp, BSP_FindFacesAtPoint(&bsp, &bsp.dmodels[0], {0, 0, 0})),
+        std::vector<std::string>({"e1u1/ggrat4_2"}));
+}
+
+TEST(testmapsQ2, chopOrder1) {
+    const auto [bsp, bspx, prt] = LoadTestmapQ2("q2_chop_order_1.map");
+
+    EXPECT_VECTORS_UNOREDERED_EQUAL(TexNames(bsp, BSP_FindFacesAtPoint(&bsp, &bsp.dmodels[0], {0, 0, 0})),
+        std::vector<std::string>({"e1u1/+0btshoot2"}));
+}