diff --git a/common/mathlib.cc b/common/mathlib.cc index 04afb174..11383a72 100644 --- a/common/mathlib.cc +++ b/common/mathlib.cc @@ -413,8 +413,14 @@ std::pair, std::vector> GLM_ClipPoly(const std::vect auto clipped = w.clip({plane.xyz(), plane[3]}); - return make_pair( - clipped[0].value_or(winding_t{}).glm_winding_points(), clipped[1].value_or(winding_t{}).glm_winding_points()); + std::pair, std::vector> result; + if (clipped[0]) { + result.first = clipped[0]->glm_winding_points(); + } + if (clipped[1]) { + result.second = clipped[1]->glm_winding_points(); + } + return result; } std::vector GLM_ShrinkPoly(const std::vector &poly, const float amount) diff --git a/include/common/polylib.hh b/include/common/polylib.hh index 7cf2e94d..fffac7c4 100644 --- a/include/common/polylib.hh +++ b/include/common/polylib.hh @@ -648,19 +648,14 @@ public: // initializer list constructor inline winding_base_t(std::initializer_list l) : storage(l.begin(), l.end()) { } - // copy constructor; uses optimized method of copying - // data over. - inline winding_base_t(const winding_base_t ©) : storage(copy.storage) { } + // copy constructor; we require copying to be done with clone() to avoid performance bugs + inline winding_base_t(const winding_base_t ©) = delete; // move constructor inline winding_base_t(winding_base_t &&move) noexcept : storage(std::move(move.storage)) { } // assignment copy - inline winding_base_t &operator=(const winding_base_t ©) - { - storage = copy.storage; - return *this; - } + inline winding_base_t &operator=(const winding_base_t ©) = delete; // assignment move inline winding_base_t &operator=(winding_base_t &&move) noexcept @@ -745,6 +740,14 @@ public: // non-storage functions + // explicit copying function + winding_base_t clone() const + { + winding_base_t result; + result.storage = storage; + return result; + } + vec_t area() const { vec_t total = 0; @@ -996,12 +999,12 @@ public: std::array counts = calc_sides(plane, dists, sides, on_epsilon); if (keepon && !counts[SIDE_FRONT] && !counts[SIDE_BACK]) - return {*this, std::nullopt}; + return {this->clone(), std::nullopt}; if (!counts[SIDE_FRONT]) - return {std::nullopt, *this}; + return {std::nullopt, this->clone()}; else if (!counts[SIDE_BACK]) - return {*this, std::nullopt}; + return {this->clone(), std::nullopt}; twosided results{}; @@ -1108,7 +1111,7 @@ public: result.push_back(mid); } - return std::move(result); + return result; } @@ -1250,7 +1253,7 @@ public: winding_base_t translate(const qvec3d &offset) const { - winding_base_t result(*this); + winding_base_t result = this->clone(); for (qvec3d &p : result) { p += offset; diff --git a/include/qbsp/brush.hh b/include/qbsp/brush.hh index cf92b32a..760696a2 100644 --- a/include/qbsp/brush.hh +++ b/include/qbsp/brush.hh @@ -45,6 +45,9 @@ struct side_t bool tested; + side_t clone_non_winding_data() const; + side_t clone() const; + bool is_visible() const; const maptexinfo_t &get_texinfo() const; const qbsp_plane_t &get_plane() const; @@ -86,6 +89,8 @@ struct bspbrush_t bool update_bounds(bool warn_on_failures); ptr copy_unique() const; + + bspbrush_t clone() const; }; std::optional LoadBrush(const mapentity_t *src, mapbrush_t *mapbrush, const contentflags_t &contents, const int hullnum); diff --git a/include/qbsp/portals.hh b/include/qbsp/portals.hh index 73510cc8..1e68460b 100644 --- a/include/qbsp/portals.hh +++ b/include/qbsp/portals.hh @@ -37,7 +37,7 @@ struct portal_t node_t *onnode; // nullptr = portal to the outside of the world (one of six sides of a box) node_t *nodes[2]; // [0] = front side of planenum portal_t *next[2]; // [0] = next portal in nodes[0]'s list of portals - std::unique_ptr winding; + winding_t winding; bool sidefound; // false if ->side hasn't been checked side_t *sides[2]; // [0] = the brush side visible on nodes[0] - it could come from a brush in nodes[1]. NULL = @@ -51,7 +51,7 @@ struct buildportal_t qbsp_plane_t plane; node_t *onnode = nullptr; // nullptr = portal to the outside of the world (one of six sides of a box) node_t *nodes[2] = {nullptr, nullptr}; // [0] = front side of planenum - std::unique_ptr winding; + winding_t winding; inline void set_nodes(node_t *front, node_t *back) { nodes[0] = front; diff --git a/include/vis/vis.hh b/include/vis/vis.hh index 05f2166c..3c24505a 100644 --- a/include/vis/vis.hh +++ b/include/vis/vis.hh @@ -61,10 +61,10 @@ struct winding_t : polylib::winding_base_t #include +side_t side_t::clone_non_winding_data() const +{ + side_t result; + result.planenum = this->planenum; + result.texinfo = this->texinfo; + result.onnode = this->onnode; + result.bevel = this->bevel; + result.source = this->source; + result.tested = this->tested; + return result; +} + +side_t side_t::clone() const +{ + side_t result = clone_non_winding_data(); + result.w = this->w.clone(); + return result; +} + bool side_t::is_visible() const { return source && source->visible; @@ -50,7 +69,31 @@ const qbsp_plane_t &side_t::get_positive_plane() const bspbrush_t::ptr bspbrush_t::copy_unique() const { - return bspbrush_t::make_ptr(*this); + return bspbrush_t::make_ptr(this->clone()); +} + +bspbrush_t bspbrush_t::clone() const +{ + bspbrush_t result; + + result.original_ptr = this->original_ptr; + result.mapbrush = this->mapbrush; + + result.bounds = this->bounds; + result.side = this->side; + result.testside = this->testside; + + result.sides.reserve(this->sides.size()); + for (auto &side : this->sides) { + result.sides.push_back(side.clone()); + } + + result.contents = this->contents; + + result.sphere_origin = this->sphere_origin; + result.sphere_radius = this->sphere_radius; + + return result; } /* @@ -238,7 +281,7 @@ bool CreateBrushWindings(bspbrush_t *brush) } } - side->w = *w; + side->w = std::move(*w); } else { side->w.clear(); } diff --git a/qbsp/brushbsp.cc b/qbsp/brushbsp.cc index e63f402b..7b871fc0 100644 --- a/qbsp/brushbsp.cc +++ b/qbsp/brushbsp.cc @@ -481,7 +481,7 @@ static twosided SplitBrush(bspbrush_t::ptr brush, size_t planen logging::print("WARNING: huge winding\n"); } - winding_t midwinding = *w; + winding_t &midwinding = *w; // split it for real @@ -512,7 +512,7 @@ static twosided SplitBrush(bspbrush_t::ptr brush, size_t planen #endif // add the clipped face to result[j] - side_t faceCopy = face; + side_t faceCopy = face.clone_non_winding_data(); faceCopy.w = std::move(*cw[j]); // fixme-brushbsp: configure any settings on the faceCopy? @@ -583,7 +583,11 @@ static twosided SplitBrush(bspbrush_t::ptr brush, size_t planen cs.onnode = true; // fixme-brushbsp: configure any other settings on the face? - cs.w = brushOnFront ? midwinding.flip() : midwinding; + if (brushOnFront) { + cs.w = midwinding.flip(); + } else { + cs.w = std::move(midwinding); + } result[i]->sides.push_back(std::move(cs)); } diff --git a/qbsp/csg.cc b/qbsp/csg.cc index ff71ca30..e824d85e 100644 --- a/qbsp/csg.cc +++ b/qbsp/csg.cc @@ -65,7 +65,7 @@ std::unique_ptr NewFaceFromFace(const face_t *in) std::unique_ptr CopyFace(const face_t *in) { auto temp = NewFaceFromFace(in); - temp->w = in->w; + temp->w = in->w.clone(); return temp; } diff --git a/qbsp/faces.cc b/qbsp/faces.cc index e2e26aac..be6161fd 100644 --- a/qbsp/faces.cc +++ b/qbsp/faces.cc @@ -504,9 +504,9 @@ static std::unique_ptr FaceFromPortal(portal_t *p, bool pside) #endif if (pside) { - f->w = p->winding->flip(); + f->w = p->winding.flip(); } else { - f->w = *p->winding; + f->w = p->winding.clone(); } f->contents = p->nodes[pside]->contents; diff --git a/qbsp/map.cc b/qbsp/map.cc index 117454ff..8fe2c19f 100644 --- a/qbsp/map.cc +++ b/qbsp/map.cc @@ -2233,10 +2233,11 @@ inline void CalculateBrushBounds(mapbrush_t &ob) } if (w) { - ob.faces[i].winding = w.value(); + // calc bounds before moving from w for (auto &p : w.value()) { ob.bounds += p; } + ob.faces[i].winding = std::move(w.value()); } } diff --git a/qbsp/outside.cc b/qbsp/outside.cc index 68925cc1..2cf92b2f 100644 --- a/qbsp/outside.cc +++ b/qbsp/outside.cc @@ -272,7 +272,7 @@ static void WriteLeakLine(const mapentity_t *leakentity, const std::vectororigin; for (portal_t *portal : leakline) { - qvec3d currpt = portal->winding->center(); + qvec3d currpt = portal->winding.center(); // draw dots from prevpt to currpt WriteLeakTrail(ptsfile, prevpt, currpt); diff --git a/qbsp/portals.cc b/qbsp/portals.cc index 20c6835d..bff68b47 100644 --- a/qbsp/portals.cc +++ b/qbsp/portals.cc @@ -158,7 +158,7 @@ std::list> MakeHeadnodePortals(tree_t *tree) } bool side = p->plane.set_plane(pl, true); - p->winding = std::make_unique(BaseWindingForPlane(pl)); + p->winding = BaseWindingForPlane(pl); if (side) { p->set_nodes(&tree->outside_node, tree->headnode); } else { @@ -168,7 +168,7 @@ std::list> MakeHeadnodePortals(tree_t *tree) // clip the basewindings by all the other planes for (i = 0; i < 6; i++) { - winding_t &w = *portals[i]->winding.get(); + winding_t &w = portals[i]->winding; for (j = 0; j < 6; j++) { if (j == i) @@ -266,7 +266,7 @@ std::unique_ptr MakeNodePortal(node_t *node, const std::list(); new_portal->plane = node->get_plane(); new_portal->onnode = node; - new_portal->winding = std::make_unique(*w); + new_portal->winding = std::move(*w); new_portal->set_nodes(node->children[0], node->children[1]); return new_portal; @@ -304,7 +304,7 @@ twosided>> SplitNodePortals(const node_ // // cut the portal into two portals, one on each side of the cut plane // - auto [frontwinding, backwinding] = p->winding->clip(plane, SPLIT_WINDING_EPSILON, true); + auto [frontwinding, backwinding] = p->winding.clip(plane, SPLIT_WINDING_EPSILON, true); if (frontwinding && WindingIsTiny(*frontwinding)) { frontwinding = {}; @@ -345,8 +345,8 @@ twosided>> SplitNodePortals(const node_ new_portal->onnode = p->onnode; new_portal->nodes[0] = p->nodes[0]; new_portal->nodes[1] = p->nodes[1]; - new_portal->winding = std::make_unique(*backwinding); - p->winding = std::make_unique(*frontwinding); + new_portal->winding = std::move(*backwinding); + p->winding = std::move(*frontwinding); if (side == SIDE_FRONT) { p->set_nodes(f, other_node); @@ -392,7 +392,7 @@ void CalcNodeBounds(node_t *node) for (portal_t *p = node->portals; p;) { int s = (p->nodes[1] == node); - for (auto &point : *p->winding) { + for (auto &point : p->winding) { node->bounds += point; } p = p->next[s]; diff --git a/qbsp/prtfile.cc b/qbsp/prtfile.cc index 033c414c..42adefd0 100644 --- a/qbsp/prtfile.cc +++ b/qbsp/prtfile.cc @@ -69,7 +69,7 @@ static void WritePortals_r(node_t *node, std::ofstream &portalFile, bool cluster if (!Portal_VisFlood(p)) continue; - w = p->winding.get(); + w = &p->winding; front = clusters ? p->nodes[0]->viscluster : p->nodes[0]->visleafnum; back = clusters ? p->nodes[1]->viscluster : p->nodes[1]->visleafnum; diff --git a/tests/test.cc b/tests/test.cc index c0eec9b6..ba32fbf7 100644 --- a/tests/test.cc +++ b/tests/test.cc @@ -410,7 +410,7 @@ TEST_CASE("winding iterators", "[winding_base_t]") // check that constructors work { - polylib::winding_base_t> winding_other(winding); + polylib::winding_base_t> winding_other(winding.begin(), winding.end()); { auto it = winding_other.begin();