From 0ad0c4aecbf088c59199f203ff386e04ddc6dc7e Mon Sep 17 00:00:00 2001 From: danij Date: Sat, 6 Apr 2013 12:41:56 +0100 Subject: [PATCH] Refactor|GameMap: Take ownership of the editable Vertexes immediately As hardening of the vertexes is now unnecessary and because we can simply take ownership of the BspBuilder's vertexes as and when the BSP build completes - there is no longer a need to defer acquiring the "primary" vertexes from the EditableMap. Also removed the now unnecessary hardening of Polyobjs. --- doomsday/client/include/edit_map.h | 2 - doomsday/client/include/map/bsp/partitioner.h | 3 +- doomsday/client/include/map/bspbuilder.h | 3 +- doomsday/client/include/map/polyobj.h | 5 + doomsday/client/src/edit_map.cpp | 155 ++++++++---------- doomsday/client/src/map/bsp/partitioner.cpp | 37 ++--- doomsday/client/src/map/bspbuilder.cpp | 9 +- doomsday/client/src/map/gamemap.cpp | 50 ++---- 8 files changed, 108 insertions(+), 156 deletions(-) diff --git a/doomsday/client/include/edit_map.h b/doomsday/client/include/edit_map.h index 2defd3cba1..c07e37cf94 100644 --- a/doomsday/client/include/edit_map.h +++ b/doomsday/client/include/edit_map.h @@ -28,8 +28,6 @@ DENG_EXTERN_C int bspFactor; void MPE_Register(); -QList &MPE_EditableVertexes(); - GameMap *MPE_GetLastBuiltMap(); bool MPE_GetLastBuiltMapResult(); diff --git a/doomsday/client/include/map/bsp/partitioner.h b/doomsday/client/include/map/bsp/partitioner.h index b585f13d1e..7ebe36d7ac 100644 --- a/doomsday/client/include/map/bsp/partitioner.h +++ b/doomsday/client/include/map/bsp/partitioner.h @@ -49,8 +49,7 @@ static const coord_t ANG_EPSILON = (1.0 / 1024.0); class Partitioner { public: - Partitioner(GameMap &map, QList const &editableVertexes, - int splitCostFactor = 7); + Partitioner(GameMap &map, int splitCostFactor = 7); ~Partitioner(); /** diff --git a/doomsday/client/include/map/bspbuilder.h b/doomsday/client/include/map/bspbuilder.h index 41f5db05c5..1fe0bee8df 100644 --- a/doomsday/client/include/map/bspbuilder.h +++ b/doomsday/client/include/map/bspbuilder.h @@ -47,8 +47,7 @@ class BspBuilder * @param map GameMap for which to construct a BSP object tree. * @param splitCostFactor Cost factor attributed to splitting an existing half-edge. */ - BspBuilder(GameMap &map, QList const &editableVertexes, - int splitCostFactor = DEFAULT_PARTITION_COST_HEDGESPLIT); + BspBuilder(GameMap &map, int splitCostFactor = DEFAULT_PARTITION_COST_HEDGESPLIT); /** * Set the cost factor associated with splitting an existing half-edge. diff --git a/doomsday/client/include/map/polyobj.h b/doomsday/client/include/map/polyobj.h index 2b7b349c04..041cb300f4 100644 --- a/doomsday/client/include/map/polyobj.h +++ b/doomsday/client/include/map/polyobj.h @@ -61,6 +61,11 @@ typedef struct polyobj_s // Does nothing about the user data section. ~polyobj_s() { + foreach(LineDef *line, lines()) + { + delete line->front()._leftHEdge; + } + delete static_cast(_lines); delete static_cast(_uniqueVertexes); } diff --git a/doomsday/client/src/edit_map.cpp b/doomsday/client/src/edit_map.cpp index 77040b737e..9f28d0aee7 100644 --- a/doomsday/client/src/edit_map.cpp +++ b/doomsday/client/src/edit_map.cpp @@ -145,24 +145,26 @@ class EditableMap return po; } - void clearPolyobjs() - { - foreach(Polyobj *po, polyobjs) - { - po->~Polyobj(); - M_Free(po); - } - polyobjs.clear(); - } - void clear() { + qDeleteAll(vertexes); vertexes.clear(); + + qDeleteAll(lines); lines.clear(); + + qDeleteAll(sideDefs); sideDefs.clear(); + + qDeleteAll(sectors); sectors.clear(); - clearPolyobjs(); + foreach(Polyobj *po, polyobjs) + { + po->~Polyobj(); + M_Free(po); + } + polyobjs.clear(); } #if 0 @@ -403,11 +405,6 @@ void MPE_Register() C_VAR_INT("bsp-factor", &bspFactor, CVF_NO_MAX, 0, 0); } -QList &MPE_EditableVertexes() -{ - return editMap.vertexes; -} - /** * Material name references specified during map conversion are recorded in * this dictionary. A dictionary is used to avoid repeatedly resolving the same @@ -807,66 +804,6 @@ static void buildVertexLineOwnerRings() } } -static void hardenPolyobjs(GameMap &dest, EditableMap &e_map) -{ - if(!e_map.polyobjs.count()) - return; - - DENG2_ASSERT(dest._polyobjs.isEmpty()); -#ifdef DENG2_QT_4_7_OR_NEWER - dest._polyobjs.reserve(e_map.polyobjs.count()); -#endif - - foreach(Polyobj *srcP, e_map.polyobjs) - { - void *region = M_Calloc(POLYOBJ_SIZE); - Polyobj *destP = new (region) Polyobj; - - destP->idx = dest._polyobjs.count(); // 0-based index. - destP->crush = srcP->crush; - destP->tag = srcP->tag; - destP->seqType = srcP->seqType; - destP->origin[VX] = srcP->origin[VX]; - destP->origin[VY] = srcP->origin[VY]; - - // Create a hedge for each line of this polyobj. - /// @todo fixme: Polyobj has ownership, must free it. - HEdge *hedges = new HEdge[srcP->lineCount()]; - - DENG_ASSERT(static_cast(destP->_lines)->isEmpty()); -#ifdef DENG2_QT_4_7_OR_NEWER - static_cast(destP->_lines)->reserve(srcP->lineCount()); -#endif - uint n = 0; - foreach(LineDef *line, srcP->lines()) - { - // This line belongs to a polyobj. - line->_inFlags |= LF_POLYOBJ; - - HEdge *hedge = &hedges[n]; - hedge->_v[0] = &line->v1(); - hedge->_v[1] = &line->v2(); - hedge->_line = line; - hedge->_length = line->length(); - hedge->_sector = line->frontSectorPtr(); - hedge->_twin = 0; - hedge->_bspLeaf = 0; - - line->front()._leftHEdge = - line->front()._rightHEdge = hedge; - - static_cast(destP->_lines)->append(line); - - n++; - } - - destP->buildUniqueVertexes(); - - // Add this polyobj to the global list. - dest._polyobjs.append(destP); - } -} - #if 0 /* Currently unused. */ /** * @return The "lowest" vertex (normally the left-most, but if the line is vertical, @@ -1025,19 +962,30 @@ boolean MPE_End() pruneMapElements(editMap, PRUNE_ALL); #endif + buildVertexLineOwnerRings(); + /* * Acquire ownership of the map elements from the editable map. */ map->entityDatabase = editMap.entityDatabase; // Take ownership. + DENG2_ASSERT(map->_vertexes.isEmpty()); +#ifdef DENG2_QT_4_7_OR_NEWER + map->_vertexes.reserve(editMap.vertexes.count()); +#endif + while(!editMap.vertexes.isEmpty()) + { + map->_vertexes.append(editMap.vertexes.takeFirst()); + } + // Collate sectors: DENG2_ASSERT(map->_sectors.isEmpty()); #ifdef DENG2_QT_4_7_OR_NEWER map->_sectors.reserve(editMap.sectors.count()); #endif - foreach(Sector *sector, editMap.sectors) + while(!editMap.sectors.isEmpty()) { - map->_sectors.append(sector); // Take ownership. + map->_sectors.append(editMap.sectors.takeFirst()); } // Collate sidedefs: @@ -1045,9 +993,9 @@ boolean MPE_End() #ifdef DENG2_QT_4_7_OR_NEWER map->_sideDefs.reserve(editMap.sideDefs.count()); #endif - foreach(SideDef *sideDef, editMap.sideDefs) + while(!editMap.sideDefs.isEmpty()) { - map->_sideDefs.append(sideDef); // Take ownership. + map->_sideDefs.append(editMap.sideDefs.takeFirst()); } // Collate lines: @@ -1055,9 +1003,10 @@ boolean MPE_End() #ifdef DENG2_QT_4_7_OR_NEWER map->_lines.reserve(editMap.lines.count()); #endif - foreach(LineDef *line, editMap.lines) + while(!editMap.lines.isEmpty()) { - map->_lines.append(line); // Take ownership. + map->_lines.append(editMap.lines.takeFirst()); + LineDef *line = map->_lines.back(); line->updateSlopeType(); line->updateAABox(); @@ -1067,10 +1016,46 @@ boolean MPE_End() int( line->_direction[VX] )); } - buildVertexLineOwnerRings(); + // Collate polyobjs: + DENG2_ASSERT(map->_polyobjs.isEmpty()); +#ifdef DENG2_QT_4_7_OR_NEWER + map->_lines.reserve(editMap.polyobjs.count()); +#endif + while(!editMap.polyobjs.isEmpty()) + { + map->_polyobjs.append(editMap.polyobjs.takeFirst()); + Polyobj *polyobj = map->_polyobjs.back(); + + polyobj->idx = map->_polyobjs.count(); // 0-based index. + + // Create a hedge for each line of this polyobj. + foreach(LineDef *line, polyobj->lines()) + { + HEdge *hedge = new HEdge; // Polyobj has ownership. + + hedge->_v[0] = &line->v1(); + hedge->_v[1] = &line->v2(); + hedge->_line = line; + hedge->_length = line->length(); + hedge->_sector = line->frontSectorPtr(); + hedge->_twin = 0; + hedge->_bspLeaf = 0; + + line->front()._leftHEdge = + line->front()._rightHEdge = hedge; + } - hardenPolyobjs(*map, editMap); - editMap.clearPolyobjs(); + polyobj->buildUniqueVertexes(); + uint n = 0; + foreach(Vertex *vertex, polyobj->uniqueVertexes()) + { + // The originalPts are relative to the polyobj origin. + vec2d_t const &vertexOrigin = vertex->origin(); + polyobj->originalPts[n].origin[VX] = vertexOrigin[VX] - polyobj->origin[VX]; + polyobj->originalPts[n].origin[VY] = vertexOrigin[VY] - polyobj->origin[VY]; + n++; + } + } /* * Build blockmaps. diff --git a/doomsday/client/src/map/bsp/partitioner.cpp b/doomsday/client/src/map/bsp/partitioner.cpp index ebf7e77212..1e84d9edf7 100644 --- a/doomsday/client/src/map/bsp/partitioner.cpp +++ b/doomsday/client/src/map/bsp/partitioner.cpp @@ -100,10 +100,9 @@ struct Partitioner::Instance /// Current map which we are building BSP data for. GameMap *map; - AABoxd mapBounds; + /// Original counts - /// @todo Refactor me away: - QList const &editableVertexes; + AABoxd mapBounds; /// Running totals of constructed BSP data objects. uint numNodes; @@ -187,10 +186,9 @@ struct Partitioner::Instance /// @c true = a BSP for the current map has been built successfully. bool builtOK; - Instance(GameMap *_map, QList const &_editableVertexes, int _splitCostFactor) + Instance(GameMap *_map, int _splitCostFactor) : splitCostFactor(_splitCostFactor), map(_map), - editableVertexes(_editableVertexes), numNodes(0), numLeafs(0), numHEdges(0), numVertexes(0), rootNode(0), treeNodeMap(), partition(), partitionLine(0), unclosedSectors(), unclosedBspLeafs(), migrantHEdges(), @@ -387,11 +385,11 @@ struct Partitioner::Instance mapBounds = findMapBounds(map); // Initialize vertex info for the initial set of vertexes. - vertexInfos.resize(editableVertexes.count()); + vertexInfos.resize(map->vertexCount()); // Count the total number of one and two-sided line owners for each vertex. // (Used in the process of locating window effect lines.) - foreach(Vertex *vtx, editableVertexes) + foreach(Vertex *vtx, map->vertexes()) { VertexInfo &vtxInfo = vertexInfo(*vtx); vtx->countLineOwners(&vtxInfo.oneSidedOwnerCount, &vtxInfo.twoSidedOwnerCount); @@ -2004,7 +2002,9 @@ struct Partitioner::Instance Vertex *newVertex(const_pvec2d_t origin) { Vertex *vtx = new Vertex; - vtx->_buildData.index = editableVertexes.count() + uint(vertexes.size() + 1); // 1-based index, 0 = NIL. + /// @todo We do not have authorization to specify this index. -ds + /// (This job should be done post BSP build.) + vtx->_buildData.index = map->vertexCount() + uint(vertexes.size() + 1); // 1-based index, 0 = NIL. vertexes.push_back(vtx); // There is now one more Vertex. @@ -2032,9 +2032,9 @@ struct Partitioner::Instance void clearAllHEdgeTips() { - foreach(Vertex *vertex, editableVertexes) + for(uint i = 0; i < vertexInfos.size(); ++i) { - clearHEdgeTipsByVertex(vertex); + vertexInfos[i].clearHEdgeTips(); } } @@ -2200,13 +2200,12 @@ struct Partitioner::Instance { case DMU_VERTEX: { Vertex *vtx = dmuOb->castTo(); - if(vtx->_buildData.index > 0 && vtx->_buildData.index > editableVertexes.count()) + /// @todo optimize: Poor performance O(n). + for(uint i = 0; i < vertexes.size(); ++i) { - // Is this object owned? - int idx = vtx->_buildData.index - 1 - editableVertexes.count(); - if((unsigned) idx < vertexes.size() && vertexes[idx]) + if(vertexes[i] == vtx) { - vertexes[idx] = 0; + vertexes[i] = 0; // There is now one fewer Vertex. numVertexes -= 1; return true; @@ -2399,8 +2398,8 @@ struct Partitioner::Instance } }; -Partitioner::Partitioner(GameMap &map, QList const &editableVertexes, int splitCostFactor) - : d(new Instance(&map, editableVertexes, splitCostFactor)) +Partitioner::Partitioner(GameMap &map, int splitCostFactor) + : d(new Instance(&map, splitCostFactor)) { d->initForMap(); } @@ -2460,8 +2459,8 @@ void Partitioner::release(MapElement *mapElement) if(!d->release(mapElement)) { LOG_AS("Partitioner::release"); - LOG_DEBUG("Attempted to release an unknown/unowned object %p.") - << de::dintptr(mapElement); + LOG_DEBUG("Attempted to release an unknown/unowned %s %p.") + << DMU_Str(mapElement->type()) << de::dintptr(mapElement); } } diff --git a/doomsday/client/src/map/bspbuilder.cpp b/doomsday/client/src/map/bspbuilder.cpp index cf356e705d..0e3f7fbdab 100644 --- a/doomsday/client/src/map/bspbuilder.cpp +++ b/doomsday/client/src/map/bspbuilder.cpp @@ -31,14 +31,11 @@ DENG2_PIMPL_NOREF(BspBuilder) /// The space partitioner. Partitioner partitioner; - Instance(GameMap &map, QList const &editableVertexes) - : partitioner(map, editableVertexes) - {} + Instance(GameMap &map) : partitioner(map) {} }; -BspBuilder::BspBuilder(GameMap &map, QList const &editableVertexes, - int splitCostFactor) - : d(new Instance(map, editableVertexes)) +BspBuilder::BspBuilder(GameMap &map, int splitCostFactor) + : d(new Instance(map)) { d->partitioner.setSplitCostFactor(splitCostFactor); } diff --git a/doomsday/client/src/map/gamemap.cpp b/doomsday/client/src/map/gamemap.cpp index 480fcca432..7632425720 100644 --- a/doomsday/client/src/map/gamemap.cpp +++ b/doomsday/client/src/map/gamemap.cpp @@ -82,28 +82,15 @@ DENG2_PIMPL(GameMap) std::memset(&traceLine, 0, sizeof(traceLine)); } - void collateVertexes(BspBuilder &builder, - QList const &editableVertexes) + void collateVertexes(BspBuilder &builder) { uint bspVertexCount = builder.numVertexes(); - - DENG2_ASSERT(self._vertexes.isEmpty()); -#ifdef DENG2_QT_4_7_OR_NEWER - self._vertexes.reserve(editableVertexes.count() + bspVertexCount); -#endif - - uint n = 0; - foreach(Vertex *vertex, editableVertexes) - { - self._vertexes.append(vertex); - ++n; - } - - for(uint i = 0; i < bspVertexCount; ++i, ++n) + for(uint i = 0; i < bspVertexCount; ++i) { + // Take ownership of this Vertex. Vertex *vertex = &builder.vertex(i); - builder.take(vertex); + self._vertexes.append(vertex); } } @@ -215,22 +202,6 @@ DENG2_PIMPL(GameMap) } } - void finishPolyobjs() - { - foreach(Polyobj *po, self._polyobjs) - { - uint n = 0; - foreach(Vertex *vertex, po->uniqueVertexes()) - { - // The originalPts are relative to the polyobj origin. - vec2d_t const &vertexOrigin = vertex->origin(); - po->originalPts[n].origin[VX] = vertexOrigin[VX] - po->origin[VX]; - po->originalPts[n].origin[VY] = vertexOrigin[VY] - po->origin[VY]; - n++; - } - } - } - void buildSectorLineLists() { LOG_VERBOSE("Building Sector line lists..."); @@ -357,7 +328,7 @@ DENG2_PIMPL(GameMap) } } - + /// @todo Relocate this work to R_SetupMap() -ds void finishPlanes() { foreach(Sector *sector, self._sectors) @@ -569,10 +540,8 @@ bool GameMap::buildBsp() LOG_INFO("Building BSP using tunable split factor of %d...") << bspFactor; - Vertexes &editableVertexes = MPE_EditableVertexes(); - // Instantiate and configure a new BSP builder. - BspBuilder nodeBuilder(*this, editableVertexes, bspFactor); + BspBuilder nodeBuilder(*this, bspFactor); // Build the BSP. bool builtOK = nodeBuilder.buildBsp(); @@ -601,12 +570,12 @@ bool GameMap::buildBsp() * Take ownership of all the built map data elements. */ #ifdef DENG2_QT_4_7_OR_NEWER + _vertexes.reserve(_vertexes.count() + nodeBuilder.numVertexes()); d->hedges.reserve(nodeBuilder.numHEdges()); d->bspNodes.reserve(nodeBuilder.numNodes()); d->bspLeafs.reserve(nodeBuilder.numLeafs()); #endif - - d->collateVertexes(nodeBuilder, editableVertexes); + d->collateVertexes(nodeBuilder); BspTreeNode *rootNode = nodeBuilder.root(); d->bspRoot = rootNode->userData(); // We'll formally take ownership shortly... @@ -663,7 +632,6 @@ bool GameMap::buildBsp() void GameMap::finishMapElements() { d->finishLines(); - d->finishPolyobjs(); d->finishSectors(); d->finishPlanes(); } @@ -943,6 +911,8 @@ void GameMap::initPolyobjs() po->bspLeaf = bspLeaf; } + /// @todo Is this still necessary? -ds + /// (This data is updated automatically when moving/rotating). po->updateAABox(); po->updateSurfaceTangents();