remove "brushes" from mapentity_t - it is now local to the ProcessEntity process

"node_t::original_brushes" is now only used during construction and cleared on Prune; original_mapbrushes is used to track the map brushes on nodes post-construction
calculate and cache contents on mapbrush_t

include/qbsp/brush.hh

@@ -23,7 +23,6 @@
 
 #include <qbsp/winding.hh>
 #include <qbsp/qbsp.hh>
-#include <qbsp/map.hh>
 #include <common/aabb.hh>
 #include <optional>
 
@@ -58,10 +57,8 @@ class mapbrush_t;
 struct bspbrush_t
 {
     /**
-     * The brushes in the mapentity_t::brushes vector are considered originals. Brush fragments created during
+     * The brushes in main brush vectors are considered originals. Brush fragments created during
-     * the BrushBSP will have this pointing back to the original brush in mapentity_t::brushes.
+     * the BrushBSP will have this pointing back to the original brush in the list.
-     *
-     * fixme-brushbsp: this is supposed to be a mapbrush_t
      */
     bspbrush_t *original;
     const mapbrush_t *mapbrush;
@@ -81,10 +78,10 @@ struct bspbrush_t
     std::unique_ptr<bspbrush_t> copy_unique() const;
 };
 
+using bspbrush_vector_t = std::vector<std::unique_ptr<bspbrush_t>>;
+
 qplane3d Face_Plane(const face_t *face);
 qplane3d Face_Plane(const side_t *face);
 
 bspbrush_t LoadBrush(const mapentity_t *src, const mapbrush_t *mapbrush, const contentflags_t &contents, const int hullnum);
-contentflags_t Brush_GetContents(const mapbrush_t *mapbrush);
 void CreateBrushWindings(bspbrush_t *brush);
-void FreeBrushes(mapentity_t *ent);

include/qbsp/brushbsp.hh

@@ -23,6 +23,7 @@
 
 #include <qbsp/winding.hh>
 #include <common/qvec.hh>
+#include <qbsp/brush.hh>
 
 #include <atomic>
 #include <list>
@@ -39,6 +40,4 @@ constexpr vec_t EDGE_LENGTH_EPSILON = 0.2;
 
 bool WindingIsTiny(const winding_t &w, double size = EDGE_LENGTH_EPSILON);
 std::unique_ptr<bspbrush_t> BrushFromBounds(const aabb3d &bounds);
-
+std::unique_ptr<tree_t> BrushBSP(mapentity_t *entity, const bspbrush_vector_t &brushes, std::optional<bool> forced_quick_tree);
-// compatibility version
-std::unique_ptr<tree_t> BrushBSP(mapentity_t *entity, std::optional<bool> forced_quick_tree);

include/qbsp/csg.hh

@@ -22,13 +22,13 @@
 #pragma once
 
 #include <common/qvec.hh>
+#include <qbsp/brush.hh>
 
 #include <list>
 #include <memory>
 #include <tuple>
 #include <vector>
 
-struct bspbrush_t;
 struct face_t;
 struct side_t;
 
@@ -37,4 +37,4 @@ std::unique_ptr<face_t> CopyFace(const face_t *in);
 std::tuple<std::unique_ptr<face_t>, std::unique_ptr<face_t>> SplitFace(
     std::unique_ptr<face_t> in, const qplane3d &split);
 void UpdateFaceSphere(face_t *in);
-std::vector<std::unique_ptr<bspbrush_t>> MakeBspBrushList(mapentity_t *entity);
+bspbrush_vector_t MakeBspBrushList(const bspbrush_vector_t &brushes);

include/qbsp/map.hh

@@ -22,6 +22,7 @@
 #pragma once
 
 #include <qbsp/qbsp.hh>
+#include <qbsp/brush.hh>
 
 #include <common/bspfile.hh>
 #include <common/parser.hh>
@@ -80,6 +81,7 @@ public:
     aabb3d bounds {};
     std::optional<uint32_t> outputnumber; /* only set for original brushes */
     parser_source_location line;
+    contentflags_t contents {};
 };
 
 struct lumpdata
@@ -99,15 +101,6 @@ enum class rotation_t
 class mapentity_t
 {
 public:
-#ifdef _MSC_VER
-    // FIXME: this is to allow MSVC to compile
-    mapentity_t() = default;
-    mapentity_t(mapentity_t &&) noexcept = default;
-    mapentity_t(const mapentity_t &) = delete;
-    mapentity_t &operator=(const mapentity_t &) = delete;
-    mapentity_t &operator=(mapentity_t &&) noexcept = default;
-#endif
-
     qvec3d origin{};
     rotation_t rotation;
 
@@ -120,7 +113,6 @@ public:
     entdict_t epairs;
 
     aabb3d bounds;
-    std::vector<std::unique_ptr<bspbrush_t>> brushes;
 
     int firstoutputfacenumber = -1;
     std::optional<size_t> outputmodelnumber = std::nullopt;
@@ -383,7 +375,7 @@ qvec3d FixRotateOrigin(mapentity_t *entity);
 constexpr int HULL_COLLISION = -1;
 
 /* Create BSP brushes from map brushes */
-void Brush_LoadEntity(mapentity_t *entity, const int hullnum);
+void Brush_LoadEntity(mapentity_t *entity, const int hullnum, bspbrush_vector_t &brushes);
 
 std::list<face_t *> CSGFace(
     face_t *srcface, const mapentity_t *srcentity, const bspbrush_t *srcbrush, const node_t *srcnode);

include/qbsp/outside.hh

@@ -22,12 +22,13 @@
 #pragma once
 
 #include <vector>
+#include <qbsp/brush.hh>
 
 class mapentity_t;
 struct node_t;
 struct tree_t;
 
-bool FillOutside(mapentity_t *entity, tree_t *tree, const int hullnum);
+bool FillOutside(mapentity_t *entity, tree_t *tree, const int hullnum, bspbrush_vector_t &brushes);
 std::vector<node_t *> FindOccupiedClusters(node_t *headnode);
 
-void FillBrushEntity(mapentity_t *entity, tree_t *tree, const int hullnum);
+void FillBrushEntity(mapentity_t *entity, tree_t *tree, const int hullnum, bspbrush_vector_t &brushes);

include/qbsp/portals.hh

@@ -79,4 +79,4 @@ std::list<std::unique_ptr<buildportal_t>> MakeHeadnodePortals(tree_t *tree);
 void MakePortalsFromBuildportals(tree_t *tree, std::list<std::unique_ptr<buildportal_t>> buildportals);
 void EmitAreaPortals(node_t *headnode);
 void FloodAreas(mapentity_t *entity, node_t *headnode);
-void MarkVisibleSides(tree_t *tree, mapentity_t *entity);
+void MarkVisibleSides(tree_t *tree, mapentity_t *entity, bspbrush_vector_t &brushes);

include/qbsp/qbsp.hh

@@ -614,6 +614,7 @@ namespace qv
 
 struct bspbrush_t;
 struct side_t;
+class mapbrush_t;
 
 struct bspbrush_t_less
 {
@@ -642,7 +643,6 @@ struct node_t
 
     // information for leafs
     contentflags_t contents; // leaf nodes (0 for decision nodes)
-    std::set<bspbrush_t *, bspbrush_t_less> original_brushes;
     std::vector<face_t *> markfaces; // leaf nodes only, point to node faces
     portal_t *portals;
     int visleafnum; // -1 = solid
@@ -656,6 +656,8 @@ struct node_t
     uint32_t firstleafbrush; // Q2
     uint32_t numleafbrushes;
     int32_t area;
+    std::set<bspbrush_t *, bspbrush_t_less> original_brushes; // FIXME: only valid during construction
+    std::unordered_set<const mapbrush_t *> original_mapbrushes;
 };
 
 void InitQBSP(int argc, const char **argv);

qbsp/brush.cc

@@ -207,16 +207,6 @@ static bool MapBrush_IsHint(const mapbrush_t &brush)
     return false;
 }
 
-/*
-=====================
-FreeBrushes
-=====================
-*/
-void FreeBrushes(mapentity_t *ent)
-{
-    ent->brushes.clear();
-}
-
 #if 0
         if (hullnum <= 0 && Brush_IsHint(*hullbrush)) {
             /* Don't generate hintskip faces */
@@ -229,41 +219,6 @@ void FreeBrushes(mapentity_t *ent)
 
 //============================================================================
 
-contentflags_t Brush_GetContents(const mapbrush_t *mapbrush)
-{
-    bool base_contents_set = false;
-    contentflags_t base_contents = qbsp_options.target_game->create_empty_contents();
-
-    // validate that all of the sides have valid contents
-    for (auto &mapface : mapbrush->faces) {
-        const maptexinfo_t &texinfo = map.mtexinfos.at(mapface.texinfo);
-
-        contentflags_t contents =
-            qbsp_options.target_game->face_get_contents(mapface.texname.data(), texinfo.flags, mapface.contents);
-
-        if (contents.is_empty(qbsp_options.target_game)) {
-            continue;
-        }
-
-        // use the first non-empty as the base contents value
-        if (!base_contents_set) {
-            base_contents_set = true;
-            base_contents = contents;
-        }
-
-        if (!contents.types_equal(base_contents, qbsp_options.target_game)) {
-            logging::print("WARNING: {}: mixed face contents ({} != {})\n",
-                mapface.line, base_contents.to_string(qbsp_options.target_game), contents.to_string(qbsp_options.target_game));
-            break;
-        }
-    }
-
-    // make sure we found a valid type
-    Q_assert(base_contents.is_valid(qbsp_options.target_game, false));
-
-    return base_contents;
-}
-
 /*
 ==================
 CreateBrushWindings
@@ -399,7 +354,7 @@ bspbrush_t LoadBrush(const mapentity_t *src, const mapbrush_t *mapbrush, const c
 
 //=============================================================================
 
-static void Brush_LoadEntity(mapentity_t *dst, const mapentity_t *src, const int hullnum, content_stats_base_t &stats)
+static void Brush_LoadEntity(mapentity_t *dst, const mapentity_t *src, const int hullnum, content_stats_base_t &stats, bspbrush_vector_t &brushes)
 {
     // _omitbrushes 1 just discards all brushes in the entity.
     // could be useful for geometry guides, selective compilation, etc.
@@ -461,7 +416,7 @@ static void Brush_LoadEntity(mapentity_t *dst, const mapentity_t *src, const int
     for (i = 0; i < src->mapbrushes.size(); i++, it++) {
         logging::percent(i, src->mapbrushes.size());
         auto &mapbrush = *it;
-        contentflags_t contents = Brush_GetContents(&mapbrush);
+        contentflags_t contents = mapbrush.contents;
 
         // per-brush settings
         bool detail = false;
@@ -570,7 +525,7 @@ static void Brush_LoadEntity(mapentity_t *dst, const mapentity_t *src, const int
         qbsp_options.target_game->count_contents_in_stats(brush.contents, stats);
 
         dst->bounds += brush.bounds;
-        dst->brushes.push_back(std::make_unique<bspbrush_t>(std::move(brush)));
+        brushes.push_back(std::make_unique<bspbrush_t>(std::move(brush)));
     }
 
     logging::percent(src->mapbrushes.size(), src->mapbrushes.size(), src == map.world_entity());
@@ -584,13 +539,13 @@ hullnum HULL_COLLISION should contain ALL brushes. (used by BSPX_CreateBrushList
 hullnum 0 does not contain clip brushes.
 ============
 */
-void Brush_LoadEntity(mapentity_t *entity, const int hullnum)
+void Brush_LoadEntity(mapentity_t *entity, const int hullnum, bspbrush_vector_t &brushes)
 {
     logging::funcheader();
 
     auto stats = qbsp_options.target_game->create_content_stats();
 
-    Brush_LoadEntity(entity, entity, hullnum, *stats);
+    Brush_LoadEntity(entity, entity, hullnum, *stats, brushes);
 
     /*
      * If this is the world entity, find all func_group and func_detail
@@ -610,7 +565,7 @@ void Brush_LoadEntity(mapentity_t *entity, const int hullnum)
             ProcessAreaPortal(source);
 
             if (IsWorldBrushEntity(source) || IsNonRemoveWorldBrushEntity(source)) {
-                Brush_LoadEntity(entity, source, hullnum, *stats);
+                Brush_LoadEntity(entity, source, hullnum, *stats, brushes);
             }
         }
     }

qbsp/brushbsp.cc

@@ -387,6 +387,7 @@ static void LeafNode(node_t *leafnode, std::vector<std::unique_ptr<bspbrush_t>>
     for (auto &brush : brushes) {
         Q_assert(brush->original != nullptr);
         leafnode->original_brushes.insert(brush->original);
+        leafnode->original_mapbrushes.insert(brush->original->mapbrush);
     }
 
     qbsp_options.target_game->count_contents_in_stats(leafnode->contents, *stats.leafstats);
@@ -1060,7 +1061,7 @@ static void BuildTree_r(node_t *node, std::vector<std::unique_ptr<bspbrush_t>> b
 BrushBSP
 ==================
 */
-static std::unique_ptr<tree_t> BrushBSP(mapentity_t *entity, std::vector<std::unique_ptr<bspbrush_t>> brushlist, std::optional<bool> forced_quick_tree)
+static std::unique_ptr<tree_t> BrushBSP_internal(mapentity_t *entity, std::vector<std::unique_ptr<bspbrush_t>> brushlist, std::optional<bool> forced_quick_tree)
 {
     auto tree = std::make_unique<tree_t>();
 
@@ -1168,7 +1169,7 @@ static std::unique_ptr<tree_t> BrushBSP(mapentity_t *entity, std::vector<std::un
     return tree;
 }
 
-std::unique_ptr<tree_t> BrushBSP(mapentity_t *entity, std::optional<bool> forced_quick_tree)
+std::unique_ptr<tree_t> BrushBSP(mapentity_t *entity, const std::vector<std::unique_ptr<bspbrush_t>> &brushlist, std::optional<bool> forced_quick_tree)
 {
-    return BrushBSP(entity, MakeBspBrushList(entity), forced_quick_tree);
+    return BrushBSP_internal(entity, MakeBspBrushList(brushlist), forced_quick_tree);
 }

qbsp/csg.cc

@@ -121,11 +121,11 @@ std::tuple<std::unique_ptr<face_t>, std::unique_ptr<face_t>> SplitFace(
     return {std::move(new_front), std::move(new_back)};
 }
 
-std::vector<std::unique_ptr<bspbrush_t>> MakeBspBrushList(mapentity_t *entity)
+std::vector<std::unique_ptr<bspbrush_t>> MakeBspBrushList(const bspbrush_vector_t &brushes)
 {
     // set the original pointers
     std::vector<std::unique_ptr<bspbrush_t>> brushcopies;
-    for (const auto &original : entity->brushes) {
+    for (const auto &original : brushes) {
         auto copy = original->copy_unique();
         copy->original = original.get();
         brushcopies.push_back(std::move(copy));

qbsp/map.cc

@@ -1790,6 +1790,41 @@ inline void AddBrushBevels(mapentity_t &e, mapbrush_t &b)
 	}
 }
 
+static contentflags_t Brush_GetContents(const mapbrush_t &mapbrush)
+{
+    bool base_contents_set = false;
+    contentflags_t base_contents = qbsp_options.target_game->create_empty_contents();
+
+    // validate that all of the sides have valid contents
+    for (auto &mapface : mapbrush.faces) {
+        const maptexinfo_t &texinfo = map.mtexinfos.at(mapface.texinfo);
+
+        contentflags_t contents =
+            qbsp_options.target_game->face_get_contents(mapface.texname.data(), texinfo.flags, mapface.contents);
+
+        if (contents.is_empty(qbsp_options.target_game)) {
+            continue;
+        }
+
+        // use the first non-empty as the base contents value
+        if (!base_contents_set) {
+            base_contents_set = true;
+            base_contents = contents;
+        }
+
+        if (!contents.types_equal(base_contents, qbsp_options.target_game)) {
+            logging::print("WARNING: {}: mixed face contents ({} != {})\n",
+                mapface.line, base_contents.to_string(qbsp_options.target_game), contents.to_string(qbsp_options.target_game));
+            break;
+        }
+    }
+
+    // make sure we found a valid type
+    Q_assert(base_contents.is_valid(qbsp_options.target_game, false));
+
+    return base_contents;
+}
+
 static mapbrush_t ParseBrush(parser_t &parser, mapentity_t &entity)
 {
     mapbrush_t brush;
@@ -1864,6 +1899,8 @@ static mapbrush_t ParseBrush(parser_t &parser, mapentity_t &entity)
     }
     // ericw -- end brush primitives
 
+    brush.contents = Brush_GetContents(brush);
+
     return brush;
 }
 
@@ -2112,11 +2149,14 @@ void ProcessAreaPortal(mapentity_t *entity)
     }
 
     for (auto &brush : entity->mapbrushes) {
+        brush.contents.native = Q2_CONTENTS_AREAPORTAL;
+
         for (auto &face : brush.faces) {
-            face.contents.native = Q2_CONTENTS_AREAPORTAL;
+            face.contents.native = brush.contents.native;
             face.texinfo = map.skip_texinfo;
         }
     }
+
     entity->areaportalnum = ++map.numareaportals;
     // set the portal number as "style"
     entity->epairs.set("style", std::to_string(map.numareaportals));
@@ -2235,16 +2275,14 @@ void ProcessMapBrushes()
             // calculate brush bounds
             CalculateBrushBounds(brush);
 
-            const contentflags_t contents = Brush_GetContents(&brush);
-
             // origin brushes are removed, and the origin of the entity is overwritten
             // with its centroid.
-            if (contents.is_origin(qbsp_options.target_game)) {
+            if (brush.contents.is_origin(qbsp_options.target_game)) {
                 if (&entity == map.world_entity()) {
                     logging::print("WARNING: Ignoring origin brush in worldspawn\n");
                 } else if (entity.epairs.has("origin")) {
                     // fixme-brushbsp: entity.line
-                    logging::print("WARNING: Entity at {} has multiple origin brushes\n", entity.mapbrushes.front().faces[0].line);
+                    logging::print("WARNING: Entity at {} has multiple origin brushes\n", entity.mapbrushes.front().line);
                 } else {
                     entity.origin = brush.bounds.centroid();
                     entity.epairs.set("origin", qv::to_string(entity.origin));
@@ -2366,7 +2404,6 @@ void LoadMapFile(void)
 
         // Remove dummy entity inserted above
         assert(!map.entities.back().epairs.size());
-        assert(map.entities.back().brushes.empty());
         map.entities.pop_back();
     }
 
@@ -2396,7 +2433,6 @@ void LoadMapFile(void)
         }
         // Remove dummy entity inserted above
         assert(!map.entities.back().epairs.size());
-        assert(map.entities.back().brushes.empty());
         map.entities.pop_back();
     }
 

qbsp/outside.cc

@@ -349,9 +349,9 @@ std::vector<node_t *> FindOccupiedClusters(node_t *headnode)
 
 //=============================================================================
 
-static void MarkBrushSidesInvisible(mapentity_t *entity)
+static void MarkBrushSidesInvisible(mapentity_t *entity, bspbrush_vector_t &brushes)
 {
-    for (auto &brush : entity->brushes) {
+    for (auto &brush : brushes) {
         for (auto &face : brush->sides) {
             face.visible = false;
         }
@@ -599,7 +599,7 @@ get incorrectly marked as "invisible").
 Special cases: structural fully covered by detail still needs to be marked "visible".
 ===========
 */
-bool FillOutside(mapentity_t *entity, tree_t *tree, const int hullnum)
+bool FillOutside(mapentity_t *entity, tree_t *tree, const int hullnum, bspbrush_vector_t &brushes)
 {
     node_t *node = tree->headnode.get();
 
@@ -701,29 +701,32 @@ bool FillOutside(mapentity_t *entity, tree_t *tree, const int hullnum)
 
     // See missing_face_simple.map for a test case with a brush that straddles between void and non-void
 
-    MarkBrushSidesInvisible(entity);
+    MarkBrushSidesInvisible(entity, brushes);
 
     MarkVisibleBrushSides_R(node);
 
+#if 0
+    // FIXME: move somewhere else
     if (qbsp_options.outsidedebug.value() && (qbsp_options.target_game->get_hull_sizes().size() == 0 || hullnum == 0)) {
         fs::path path = qbsp_options.bsp_path;
         path.replace_extension(".outside.map");
 
         WriteBspBrushMap(path, map.entities[0].brushes);
     }
+#endif
 
     logging::print(logging::flag::STAT, "     {:8} outleafs\n", outleafs);
     return true;
 }
 
-void FillBrushEntity(mapentity_t *entity, tree_t *tree, const int hullnum)
+void FillBrushEntity(mapentity_t *entity, tree_t *tree, const int hullnum, bspbrush_vector_t &brushes)
 {
     logging::funcheader();
 
     // Clear the outside filling state on all nodes
     ClearOccupied_r(tree->headnode.get());
 
-    MarkBrushSidesInvisible(entity);
+    MarkBrushSidesInvisible(entity, brushes);
 
     MarkVisibleBrushSides_R(tree->headnode.get());
 }

qbsp/portals.cc

@@ -912,12 +912,12 @@ MarkVisibleSides
 
 =============
 */
-void MarkVisibleSides(tree_t *tree, mapentity_t *entity)
+void MarkVisibleSides(tree_t *tree, mapentity_t *entity, bspbrush_vector_t &brushes)
 {
     logging::funcheader();
 
     // clear all the visible flags
-    for (auto &brush : entity->brushes) {
+    for (auto &brush : brushes) {
         for (auto &face : brush->sides) {
             face.visible = false;
         }

qbsp/qbsp.cc

@@ -300,19 +300,20 @@ static void ExportBrushList_r(const mapentity_t *entity, node_t *node)
 {
     if (node->is_leaf) {
         if (node->contents.native) {
-            if (node->original_brushes.size()) {
+            if (node->original_mapbrushes.size()) {
-                node->numleafbrushes = node->original_brushes.size();
+                node->numleafbrushes = node->original_mapbrushes.size();
                 brush_state.total_leaf_brushes += node->numleafbrushes;
                 node->firstleafbrush = map.bsp.dleafbrushes.size();
-                for (auto &b : node->original_brushes) {
+                for (auto &b : node->original_mapbrushes) {
 
-                    if (!b->mapbrush->outputnumber.has_value()) {
+                    if (!b->outputnumber.has_value()) {
-                        const_cast<mapbrush_t *>(b->mapbrush)->outputnumber = {static_cast<uint32_t>(map.bsp.dbrushes.size())};
+                        // FIXME
+                        const_cast<mapbrush_t *>(b)->outputnumber = {static_cast<uint32_t>(map.bsp.dbrushes.size())};
 
                         dbrush_t &brush = map.bsp.dbrushes.emplace_back(
                             dbrush_t{static_cast<int32_t>(map.bsp.dbrushsides.size()), 0, b->contents.native});
 
-                        for (auto &side : b->mapbrush->faces) {
+                        for (auto &side : b->faces) {
                             map.bsp.dbrushsides.push_back(
                                 {(uint32_t) ExportMapPlane(side.get_plane()), (int32_t)ExportMapTexinfo(side.texinfo)});
                             brush.numsides++;
@@ -322,7 +323,7 @@ static void ExportBrushList_r(const mapentity_t *entity, node_t *node)
                         brush_state.total_brushes++;
                     }
 
-                    map.bsp.dleafbrushes.push_back(b->mapbrush->outputnumber.value());
+                    map.bsp.dleafbrushes.push_back(b->outputnumber.value());
                 }
             }
         }
@@ -436,20 +437,22 @@ static void ProcessEntity(mapentity_t *entity, const int hullnum)
         entity->epairs.set("_lmscale", std::to_string(qbsp_options.lmscale.value()));
     }
 
-    /*
+    // Init the entity
-     * Init the entity
-     */
-    entity->brushes.clear();
     entity->bounds = {};
 
+    // reserve enough brushes; we would only make less,
+    // never more
+    bspbrush_vector_t brushes;
+    brushes.reserve(entity->mapbrushes.size());
+
     /*
      * Convert the map brushes (planes) into BSP brushes (polygons)
      */
-    Brush_LoadEntity(entity, hullnum);
+    Brush_LoadEntity(entity, hullnum, brushes);
 
     // assign brush file order
-    for (size_t i = 0; i < entity->brushes.size(); ++i) {
+    for (size_t i = 0; i < brushes.size(); ++i) {
-        entity->brushes[i]->file_order = i;
+        brushes[i]->file_order = i;
     }
 
     // entity->brushes = ChopBrushes(entity->brushes);
@@ -465,7 +468,6 @@ static void ProcessEntity(mapentity_t *entity, const int hullnum)
 
     // we're discarding the brush
     if (discarded_trigger) {
-        entity->brushes.clear();
         entity->epairs.set("mins", fmt::to_string(entity->bounds.mins()));
         entity->epairs.set("maxs", fmt::to_string(entity->bounds.maxs()));
         return;
@@ -473,17 +475,17 @@ static void ProcessEntity(mapentity_t *entity, const int hullnum)
 
     std::unique_ptr<tree_t> tree = nullptr;
     if (hullnum > 0) {
-        tree = BrushBSP(entity, true);
+        tree = BrushBSP(entity, brushes, true);
         if (entity == map.world_entity() && !qbsp_options.nofill.value()) {
             // assume non-world bmodels are simple
             MakeTreePortals(tree.get());
-            if (FillOutside(entity, tree.get(), hullnum)) {
+            if (FillOutside(entity, tree.get(), hullnum, brushes)) {
                 // make a really good tree
-                tree = BrushBSP(entity, false);
+                tree = BrushBSP(entity, brushes, false);
 
                 // fill again so PruneNodes works
                 MakeTreePortals(tree.get());
-                FillOutside(entity, tree.get(), hullnum);
+                FillOutside(entity, tree.get(), hullnum, brushes);
                 PruneNodes(tree->headnode.get());
             }
         }
@@ -492,9 +494,9 @@ static void ProcessEntity(mapentity_t *entity, const int hullnum)
         // fixme-brushbsp: return here?
     } else {
         if (qbsp_options.forcegoodtree.value()) {
-            tree = BrushBSP(entity, false);
+            tree = BrushBSP(entity, brushes, false);
         } else {
-            tree = BrushBSP(entity, entity == map.world_entity() ? std::nullopt : std::optional<bool>(false));
+            tree = BrushBSP(entity, brushes, entity == map.world_entity() ? std::nullopt : std::optional<bool>(false));
         }
 
         // build all the portals in the bsp tree
@@ -506,15 +508,15 @@ static void ProcessEntity(mapentity_t *entity, const int hullnum)
             // marks brush sides which are *only* touching void;
             // we can skip using them as BSP splitters on the "really good tree"
             // (effectively expanding those brush sides outwards).
-            if (!qbsp_options.nofill.value() && FillOutside(entity, tree.get(), hullnum)) {
+            if (!qbsp_options.nofill.value() && FillOutside(entity, tree.get(), hullnum, brushes)) {
                 // make a really good tree
-                tree = BrushBSP(entity, false);
+                tree = BrushBSP(entity, brushes, false);
 
                 // make the real portals for vis tracing
                 MakeTreePortals(tree.get());
 
                 // fill again so PruneNodes works
-                FillOutside(entity, tree.get(), hullnum);
+                FillOutside(entity, tree.get(), hullnum, brushes);
             }
 
             // Area portals
@@ -523,15 +525,15 @@ static void ProcessEntity(mapentity_t *entity, const int hullnum)
                 EmitAreaPortals(tree->headnode.get());
             }
         } else {
-            FillBrushEntity(entity, tree.get(), hullnum);
+            FillBrushEntity(entity, tree.get(), hullnum, brushes);
 
             // rebuild BSP now that we've marked invisible brush sides
-            tree = BrushBSP(entity, false);
+            tree = BrushBSP(entity, brushes, false);
         }
 
         MakeTreePortals(tree.get());
 
-        MarkVisibleSides(tree.get(), entity);
+        MarkVisibleSides(tree.get(), entity, brushes);
         MakeFaces(tree->headnode.get());
 
         FreeTreePortals(tree.get());
@@ -566,8 +568,6 @@ static void ProcessEntity(mapentity_t *entity, const int hullnum)
 
         ExportDrawNodes(entity, tree->headnode.get(), entity->firstoutputfacenumber);
     }
-
-    FreeBrushes(entity);
 }
 
 /*
@@ -651,7 +651,7 @@ hull sizes
 */
 
 static void BSPX_Brushes_AddModel(
-    struct bspxbrushes_s *ctx, int modelnum, std::vector<std::unique_ptr<bspbrush_t>> &brushes)
+    struct bspxbrushes_s *ctx, int modelnum, const std::list<mapbrush_t> &brushes)
 {
     std::shared_lock lock(map_planes_lock);
 
@@ -659,7 +659,7 @@ static void BSPX_Brushes_AddModel(
 
     for (auto &b : brushes) {
         permodel.numbrushes++;
-        for (auto &f : b->sides) {
+        for (auto &f : b.faces) {
             /*skip axial*/
             const auto &plane = f.get_plane();
             if (plane.get_type() < plane_type_t::PLANE_ANYX)
@@ -677,7 +677,7 @@ static void BSPX_Brushes_AddModel(
     for (auto &b : brushes) {
         bspxbrushes_perbrush perbrush{};
 
-        for (auto &f : b->sides) {
+        for (auto &f : b.faces) {
             /*skip axial*/
             const auto &plane = f.get_plane();
             if (plane.get_type() < plane_type_t::PLANE_ANYX)
@@ -685,9 +685,11 @@ static void BSPX_Brushes_AddModel(
             perbrush.numfaces++;
         }
 
-        perbrush.bounds = b->bounds;
+        perbrush.bounds = b.bounds;
 
-        switch (b->contents.native) {
+        const auto &contents = b.contents;
+
+        switch (contents.native) {
             // contents should match the engine.
             case CONTENTS_EMPTY: // really an error, but whatever
             case CONTENTS_SOLID: // these are okay
@@ -695,21 +697,21 @@ static void BSPX_Brushes_AddModel(
             case CONTENTS_SLIME:
             case CONTENTS_LAVA:
             case CONTENTS_SKY:
-                if (b->contents.is_clip(qbsp_options.target_game)) {
+                if (contents.is_clip(qbsp_options.target_game)) {
                     perbrush.contents = -8;
                 } else {
-                    perbrush.contents = b->contents.native;
+                    perbrush.contents = contents.native;
                 }
                 break;
             //              case CONTENTS_LADDER:
             //                      perbrush.contents = -16;
             //                      break;
             default: {
-                if (b->contents.is_clip(qbsp_options.target_game)) {
+                if (contents.is_clip(qbsp_options.target_game)) {
                     perbrush.contents = -8;
                 } else {
                     logging::print("WARNING: Unknown contents: {}. Translating to solid.\n",
-                        b->contents.to_string(qbsp_options.target_game));
+                        contents.to_string(qbsp_options.target_game));
                     perbrush.contents = CONTENTS_SOLID;
                 }
                 break;
@@ -718,7 +720,7 @@ static void BSPX_Brushes_AddModel(
 
         str <= perbrush;
 
-        for (auto &f : b->sides) {
+        for (auto &f : b.faces) {
             /*skip axial*/
             const auto &plane = f.get_plane();
             if (plane.get_type() < plane_type_t::PLANE_ANYX)
@@ -755,15 +757,10 @@ static void BSPX_CreateBrushList(void)
             modelnum = std::stoi(mod.substr(1));
         }
 
-        ent->brushes.clear();
+        if (ent->mapbrushes.empty())
-
-        Brush_LoadEntity(ent, HULL_COLLISION);
-
-        if (ent->brushes.empty())
             continue; // non-bmodel entity
 
-        BSPX_Brushes_AddModel(&ctx, modelnum, ent->brushes);
+        BSPX_Brushes_AddModel(&ctx, modelnum, ent->mapbrushes);
-        FreeBrushes(ent);
     }
 
     BSPX_Brushes_Finalize(&ctx);

qbsp/tree.cc

@@ -65,8 +65,10 @@ void FreeTreePortals(tree_t *tree)
 static void ConvertNodeToLeaf(node_t *node, const contentflags_t &contents)
 {
     // merge the children's brush lists
-    node->original_brushes = node->children[0]->original_brushes;
+    node->original_mapbrushes = node->children[0]->original_mapbrushes;
-    node->original_brushes.insert(node->children[1]->original_brushes.begin(), node->children[1]->original_brushes.end());
+    node->original_mapbrushes.insert(node->children[1]->original_mapbrushes.begin(), node->children[1]->original_mapbrushes.end());
+
+    node->original_brushes.clear();
 
     node->is_leaf = true;
 
@@ -84,6 +86,7 @@ static void ConvertNodeToLeaf(node_t *node, const contentflags_t &contents)
 static void PruneNodes_R(node_t *node, std::atomic<int32_t> &count_pruned)
 {
     if (node->is_leaf) {
+        node->original_brushes.clear();
         return;
     }
 
@@ -97,6 +100,8 @@ static void PruneNodes_R(node_t *node, std::atomic<int32_t> &count_pruned)
         // This discards any faces on-node. Should be safe (?)
         ConvertNodeToLeaf(node, qbsp_options.target_game->create_solid_contents());
         ++count_pruned;
+    } else {
+        node->original_brushes.clear();
     }
 
     // DarkPlaces has an assertion that fails if both children are

tests/test_qbsp.cc

@@ -399,7 +399,6 @@ TEST_CASE("duplicatePlanes", "[qbsp]")
 
     mapentity_t worldspawn = LoadMap(mapWithDuplicatePlanes);
     REQUIRE(1 == worldspawn.mapbrushes.size());
-    CHECK(0 == worldspawn.brushes.size());
     CHECK(6 == worldspawn.mapbrushes.front().faces.size());
 
     bspbrush_t brush = LoadBrush(&worldspawn, &worldspawn.mapbrushes.front(), {CONTENTS_SOLID}, 0);