From 1dbafbb674df89dc8847db050f962e35c9e10587 Mon Sep 17 00:00:00 2001 From: danij Date: Tue, 2 Dec 2014 22:19:32 +0000 Subject: [PATCH] Refactor|World|Sector: Implement Sector-linked MapElement iterations with C++11 lambdas --- doomsday/client/include/world/sector.h | 91 +++++++-------- doomsday/client/src/dd_main.cpp | 7 +- doomsday/client/src/render/rend_main.cpp | 19 +-- .../client/src/resource/resourcesystem.cpp | 11 +- doomsday/client/src/world/api_map.cpp | 16 +-- doomsday/client/src/world/map.cpp | 94 +++++++++------ doomsday/client/src/world/sector.cpp | 109 +++++++++++------- doomsday/client/src/world/worldsystem.cpp | 7 +- 8 files changed, 196 insertions(+), 158 deletions(-) diff --git a/doomsday/client/include/world/sector.h b/doomsday/client/include/world/sector.h index c7c72f29ad..8abcaed917 100644 --- a/doomsday/client/include/world/sector.h +++ b/doomsday/client/include/world/sector.h @@ -21,7 +21,7 @@ #ifndef DENG_WORLD_SECTOR_H #define DENG_WORLD_SECTOR_H -#include +#include #ifdef __CLIENT__ # include #endif @@ -56,12 +56,6 @@ class Sector : public de::MapElement /// Notified whenever a light color change occurs. DENG2_DEFINE_AUDIENCE(LightColorChange, void sectorLightColorChanged(Sector §or)) - /* - * Linked-element lists: - */ - typedef QList Planes; - typedef QList Sides; - // Plane identifiers: enum { Floor, Ceiling }; @@ -76,81 +70,82 @@ class Sector : public de::MapElement de::Vector3f const &lightColor = de::Vector3f(1, 1, 1)); /** - * Returns the sector plane with the specified @a planeIndex. + * Returns @c true if at least one Plane in the sector is sky-masked. + * + * @see Surface::hasSkyMaskedMaterial() */ - Plane &plane(int planeIndex); - - /// @copydoc plane() - Plane const &plane(int planeIndex) const; - bool hasSkyMaskedPlane() const; /** - * Returns the floor plane of the sector. + * Returns the total number of planes in/owned by the sector. */ - inline Plane &floor() { return plane(Floor); } + int planeCount() const; - /// @copydoc floor() - inline Plane const &floor() const { return plane(Floor); } + /** + * Lookup a Plane by it's sector-unique @a planeIndex. + */ + Plane &plane(int planeIndex); + Plane const &plane(int planeIndex) const; /** - * Returns the ceiling plane of the sector. + * Returns the @em floor Plane of the sector. */ - inline Plane &ceiling() { return plane(Ceiling); } + inline Plane &floor() { return plane(Floor); } + inline Plane const &floor() const { return plane(Floor); } - /// @copydoc ceiling() + /** + * Returns the @em ceiling Plane of the sector. + */ + inline Plane &ceiling() { return plane(Ceiling); } inline Plane const &ceiling() const { return plane(Ceiling); } - Plane *addPlane(de::Vector3f const &normal, coord_t height); - /** - * Provides access to the list of planes in/owned by the sector, for efficient - * traversal. + * Add a new Plane to the sector. + * + * @param normal World space normal for the new plane. + * @param height World space Z axis coordinate for the new plane. */ - Planes const &planes() const; + Plane *addPlane(de::Vector3f const &normal, coord_t height); /** - * Returns the total number of planes in/owned by the sector. + * Iterate through the Planes of the sector. + * + * @param func Callback to make for each Plane. */ - inline int planeCount() const { return planes().count(); } + de::LoopResult forAllPlanes(std::function func) const; /** * Convenient accessor method for returning the surface of the specified * plane of the sector. */ - inline Surface &planeSurface(int planeIndex) { return plane(planeIndex).surface(); } - - /// @copydoc planeSurface() + inline Surface &planeSurface(int planeIndex) { return plane(planeIndex).surface(); } inline Surface const &planeSurface(int planeIndex) const { return plane(planeIndex).surface(); } /** * Convenient accessor method for returning the surface of the floor plane * of the sector. */ - inline Surface &floorSurface() { return floor().surface(); } - - /// @copydoc floorSurface() + inline Surface &floorSurface() { return floor().surface(); } inline Surface const &floorSurface() const { return floor().surface(); } /** * Convenient accessor method for returning the surface of the ceiling plane * of the sector. */ - inline Surface &ceilingSurface() { return ceiling().surface(); } - - /// @copydoc ceilingSurface() + inline Surface &ceilingSurface() { return ceiling().surface(); } inline Surface const &ceilingSurface() const { return ceiling().surface(); } /** - * Provides access to the list of line sides which reference the sector, - * for efficient traversal. + * Returns the total number of Line::Sides which reference the sector. */ - Sides const &sides() const; + int sideCount() const; /** - * Returns the total number of line sides which reference the sector. + * Iterate through the Line::Sides of the sector. + * + * @param func Callback to make for each Line::Side. */ - inline int sideCount() const { return sides().count(); } + de::LoopResult forAllSides(std::function func) const; /** * (Re)Build the side list for the sector. @@ -171,9 +166,7 @@ class Sector : public de::MapElement * sector are linked to this, forming a chain which can be traversed using * the 'next' pointer of the emitter's thinker_t. */ - SoundEmitter &soundEmitter(); - - /// @copydoc soundEmitter() + SoundEmitter &soundEmitter(); SoundEmitter const &soundEmitter() const; /** @@ -230,9 +223,9 @@ class Sector : public de::MapElement /** * Unlink the mobj from the list of mobjs "in" the sector. * - * @param mobj Mobj to be unlinked. + * @param mob Mobj to be unlinked. */ - void unlink(struct mobj_s *mobj); + void unlink(struct mobj_s *mob); /** * Link the mobj to the head of the list of mobjs "in" the sector. Note that @@ -240,9 +233,9 @@ class Sector : public de::MapElement * the sector is determined by interpreting the BSP leaf as a half-space and * not a closed convex subspace (@ref de::Map::link()). * - * @param mobj Mobj to be linked. + * @param mob Mobj to be linked. */ - void link(struct mobj_s *mobj); + void link(struct mobj_s *mob); /** * Returns the @em validCount of the sector. Used by some legacy iteration @@ -285,4 +278,4 @@ class Sector : public de::MapElement DENG2_PRIVATE(d) }; -#endif // DENG_WORLD_SECTOR_H +#endif // DENG_WORLD_SECTOR_H diff --git a/doomsday/client/src/dd_main.cpp b/doomsday/client/src/dd_main.cpp index 7eabebce76..d65f6b3679 100644 --- a/doomsday/client/src/dd_main.cpp +++ b/doomsday/client/src/dd_main.cpp @@ -3325,10 +3325,11 @@ DENG_EXTERN_C void R_SetupMap(int mode, int flags) /// @todo Refactor away. map.forAllSectors([] (Sector §or) { - for(LineSide *side : sector.sides()) + sector.forAllSides([] (LineSide &side) { - side->fixMissingMaterials(); - } + side.fixMissingMaterials(); + return LoopContinue; + }); return LoopContinue; }); #endif diff --git a/doomsday/client/src/render/rend_main.cpp b/doomsday/client/src/render/rend_main.cpp index e09bdcc0ac..abd41c4d76 100644 --- a/doomsday/client/src/render/rend_main.cpp +++ b/doomsday/client/src/render/rend_main.cpp @@ -4802,25 +4802,26 @@ static void drawSoundEmitters(Map &map) }); } - if(devSoundEmitters & (SOF_SECTOR|SOF_PLANE)) + if(devSoundEmitters & (SOF_SECTOR | SOF_PLANE)) { - map.forAllSectors([] (Sector &sec) + map.forAllSectors([] (Sector §or) { if(devSoundEmitters & SOF_PLANE) { - for(Plane *plane : sec.planes()) + sector.forAllPlanes([] (Plane &plane) { - drawSoundEmitter(plane->soundEmitter(), + drawSoundEmitter(plane.soundEmitter(), String("Sector #%1 (pln:%2)") - .arg(sec.indexInMap()) - .arg(plane->indexInSector())); - } + .arg(plane.sector().indexInMap()) + .arg(plane.indexInSector())); + return LoopContinue; + }); } if(devSoundEmitters & SOF_SECTOR) { - drawSoundEmitter(sec.soundEmitter(), - String("Sector #%1").arg(sec.indexInMap())); + drawSoundEmitter(sector.soundEmitter(), + String("Sector #%1").arg(sector.indexInMap())); } return LoopContinue; }); diff --git a/doomsday/client/src/resource/resourcesystem.cpp b/doomsday/client/src/resource/resourcesystem.cpp index f0b9fd733e..418d3aadba 100644 --- a/doomsday/client/src/resource/resourcesystem.cpp +++ b/doomsday/client/src/resource/resourcesystem.cpp @@ -3974,12 +3974,15 @@ void ResourceSystem::cacheForCurrentMap() { // Skip sectors with no line sides as their planes will never be drawn. if(sector.sideCount()) - for(Plane *plane : sector.planes()) { - if(plane->surface().hasMaterial()) + sector.forAllPlanes([this, &spec] (Plane &plane) { - cache(plane->surface().material(), spec); - } + if(plane.surface().hasMaterial()) + { + cache(plane.surface().material(), spec); + } + return LoopContinue; + }); } return LoopContinue; }); diff --git a/doomsday/client/src/world/api_map.cpp b/doomsday/client/src/world/api_map.cpp index b84530fd48..22ae43eb58 100644 --- a/doomsday/client/src/world/api_map.cpp +++ b/doomsday/client/src/world/api_map.cpp @@ -396,20 +396,16 @@ int P_Iteratep(void *elPtr, uint prop, int (*callback) (void *p, void *ctx), voi switch(prop) { case DMU_LINE: - for(LineSide *side : sector.sides()) + return sector.forAllSides([&callback, &context] (LineSide &side) { - if(int result = callback(&side->line(), context)) - return result; - } - return false; // Continue iteration + return callback(&side.line(), context); + }); case DMU_PLANE: - for(Plane *plane : sector.planes()) + return sector.forAllPlanes([&callback, &context] (Plane &plane) { - if(int result = callback(plane, context)) - return result; - } - return false; // Continue iteration + return callback(&plane, context); + }); default: throw Error("P_Iteratep", QString("Property %1 unknown/not vector").arg(DMU_Str(prop))); diff --git a/doomsday/client/src/world/map.cpp b/doomsday/client/src/world/map.cpp index 1a68e07c0a..d3400832e6 100644 --- a/doomsday/client/src/world/map.cpp +++ b/doomsday/client/src/world/map.cpp @@ -1045,12 +1045,12 @@ DENG2_PIMPL(Map) */ Polyobj *polyobjBySoundEmitter(SoundEmitter const &soundEmitter) const { - foreach(Polyobj *polyobj, polyobjs) + for(Polyobj *polyobj : polyobjs) { if(&soundEmitter == &polyobj->soundEmitter()) return polyobj; } - return 0; + return nullptr; // Not found. } /** @@ -1062,12 +1062,12 @@ DENG2_PIMPL(Map) */ Sector *sectorBySoundEmitter(SoundEmitter const &soundEmitter) const { - foreach(Sector *sector, sectors) + for(Sector *sector : sectors) { if(&soundEmitter == §or->soundEmitter()) return sector; } - return 0; // Not found. + return nullptr; // Not found. } /** @@ -1079,15 +1079,21 @@ DENG2_PIMPL(Map) */ Plane *planeBySoundEmitter(SoundEmitter const &soundEmitter) const { - foreach(Sector *sector, sectors) - foreach(Plane *plane, sector->planes()) + Plane *found = nullptr; // Not found. + for(Sector *sector : sectors) { - if(&soundEmitter == &plane->soundEmitter()) + LoopResult located = sector->forAllPlanes([&soundEmitter, &found] (Plane &plane) { - return plane; - } + if(&soundEmitter == &plane.soundEmitter()) + { + found = &plane; + return LoopAbort; + } + return LoopContinue; + }); + if(located) break; } - return 0; // Not found. + return found; } /** @@ -1100,7 +1106,7 @@ DENG2_PIMPL(Map) Surface *surfaceBySoundEmitter(SoundEmitter const &soundEmitter) const { // Perhaps a wall surface? - foreach(Line *line, lines) + for(Line *line : lines) for(int i = 0; i < 2; ++i) { LineSide &side = line->side(i); @@ -1120,7 +1126,7 @@ DENG2_PIMPL(Map) } } - return 0; // Not found. + return nullptr; // Not found. } #ifdef __CLIENT__ @@ -1141,7 +1147,7 @@ DENG2_PIMPL(Map) if(resetNextViewer) { // Reset the plane height trackers. - foreach(Plane *plane, trackedPlanes) + for(Plane *plane : trackedPlanes) { plane->resetSmoothedHeight(); } @@ -1176,7 +1182,7 @@ DENG2_PIMPL(Map) if(resetNextViewer) { // Reset the surface material origin trackers. - foreach(Surface *surface, scrollingSurfaces) + for(Surface *surface : scrollingSurfaces) { surface->resetSmoothedMaterialOrigin(); } @@ -1906,7 +1912,7 @@ void Map::buildMaterialLists() { d->surfaceDecorator().reset(); - foreach(Line *line, d->lines) + for(Line *line : d->lines) for(int i = 0; i < 2; ++i) { LineSide &side = line->side(i); @@ -1917,15 +1923,16 @@ void Map::buildMaterialLists() linkInMaterialLists(&side.bottom()); } - foreach(Sector *sector, d->sectors) + for(Sector *sector : d->sectors) { // Skip sectors with no lines as their planes will never be drawn. if(!sector->sideCount()) continue; - foreach(Plane *plane, sector->planes()) + sector->forAllPlanes([this] (Plane &plane) { - linkInMaterialLists(&plane->surface()); - } + linkInMaterialLists(&plane.surface()); + return LoopContinue; + }); } } @@ -2514,9 +2521,9 @@ LoopResult Map::forAllMobjsTouchingSector(Sector §or, std::functionlineNodes.nodes; - for(LineSide *side : sector.sides()) + sector.forAllSides([this, &linkStore, &ln] (LineSide &side) { - nodeindex_t root = d->lineLinks[side->line().indexInMap()]; + nodeindex_t root = d->lineLinks[side.line().indexInMap()]; for(nodeindex_t nix = ln[root].next; nix != root; nix = ln[nix].next) { mobj_t *mob = (mobj_t *)(ln[nix].ptr); @@ -2526,7 +2533,8 @@ LoopResult Map::forAllMobjsTouchingSector(Sector §or, std::functionsectors) + for(Sector *sector : d->sectors) { if(!sector->sideCount()) continue; @@ -2802,19 +2810,20 @@ void Map::initSkyFix() // Update for middle materials on lines which intersect the // floor and/or ceiling on the front (i.e., sector) side. - foreach(LineSide *side, sector->sides()) + sector->forAllSides([this, &skyCeil, &skyFloor] (LineSide &side) { - if(!side->hasSections()) continue; - if(!side->middle().hasMaterial()) continue; + if(!side.hasSections()) return LoopContinue; + if(!side.middle().hasMaterial()) return LoopContinue; // There must be a sector on both sides. - if(!side->hasSector() || !side->back().hasSector()) continue; + if(!side.hasSector() || !side.back().hasSector()) + return LoopContinue; // Possibility of degenerate BSP leaf. - if(!side->leftHEdge()) continue; + if(!side.leftHEdge()) return LoopContinue; - WallEdge edge(WallSpec::fromMapSide(*side, LineSide::Middle), - *side->leftHEdge(), Line::From); + WallEdge edge(WallSpec::fromMapSide(side, LineSide::Middle), + *side.leftHEdge(), Line::From); if(edge.isValid() && edge.top().z() > edge.bottom().z()) { @@ -2830,7 +2839,8 @@ void Map::initSkyFix() d->skyFloorHeight = edge.bottom().z() + edge.materialOrigin().y; } } - } + return LoopContinue; + }); } LOGDEV_MAP_VERBOSE("Completed in %.2f seconds") << begunAt.since(); @@ -3090,9 +3100,12 @@ void Map::update() // Update all surfaces. for(Sector *sector : d->sectors) - for(Plane *plane : sector->planes()) { - plane->surface().markForDecorationUpdate(); + sector->forAllPlanes([] (Plane &plane) + { + plane.surface().markForDecorationUpdate(); + return LoopContinue; + }); } for(Line *line : d->lines) @@ -3206,10 +3219,14 @@ void Map::worldSystemFrameBegins(bool resetNextViewer) d->generateLumobjs(side.bottom()); d->generateLumobjs(side.top()); } + for(Sector *sector : d->sectors) - for(Plane *plane : sector->planes()) { - d->generateLumobjs(plane->surface()); + sector->forAllPlanes([this] (Plane &plane) + { + d->generateLumobjs(plane.surface()); + return LoopContinue; + }); } } @@ -3924,9 +3941,12 @@ bool Map::endEditing() // Finish planes. for(Sector *sector : d->sectors) - for(Plane *plane : sector->planes()) { - plane->updateSoundEmitterOrigin(); + sector->forAllPlanes([] (Plane &plane) + { + plane.updateSoundEmitterOrigin(); + return LoopContinue; + }); } // We can now initialize the convex subspace blockmap. diff --git a/doomsday/client/src/world/sector.cpp b/doomsday/client/src/world/sector.cpp index 2ad093a4a5..6f0768c6bc 100644 --- a/doomsday/client/src/world/sector.cpp +++ b/doomsday/client/src/world/sector.cpp @@ -20,6 +20,7 @@ #include "world/sector.h" +#include #include #include #include @@ -41,8 +42,12 @@ DENG2_PIMPL(Sector) ThinkerT emitter; ///< Head of the sound emitter chain. + typedef QList Planes; Planes planes; ///< All owned planes. + + typedef QList Sides; Sides sides; ///< All referencing line sides (not owned). + mobj_t *mobjList = nullptr; ///< All mobjs "in" the sector (not owned). float lightLevel = 0; ///< Ambient light level. @@ -98,12 +103,12 @@ DENG2_PIMPL(Sector) // point of the sector geometry is now known. if(haveGeometry) { - emitter->origin[VX] = (aaBox.minX + aaBox.maxX) / 2; - emitter->origin[VY] = (aaBox.minY + aaBox.maxY) / 2; + emitter->origin[0] = (aaBox.minX + aaBox.maxX) / 2; + emitter->origin[1] = (aaBox.minY + aaBox.maxY) / 2; } else { - emitter->origin[VX] = emitter->origin[VY] = 0; + emitter->origin[0] = emitter->origin[1] = 0; } } @@ -138,7 +143,7 @@ DENG2_PIMPL(Sector) void planeHeightChanged(Plane & /*plane*/) { // Update the z-height origin of our sound emitter right away. - emitter->origin[VZ] = (self.floor().height() + self.ceiling().height()) / 2; + emitter->origin[2] = (self.floor().height() + self.ceiling().height()) / 2; #ifdef __CLIENT__ // A plane move means we must re-apply missing material fixes. @@ -271,12 +276,22 @@ void Sector::setValidCount(int newValidCount) d->validCount = newValidCount; } -Plane &Sector::plane(int planeIndex) +bool Sector::hasSkyMaskedPlane() const +{ + for(Plane *plane : d->planes) + { + if(plane->surface().hasSkyMaskedMaterial()) + return true; + } + return false; +} + +int Sector::planeCount() const { - return const_cast(const_cast(*this).plane(planeIndex)); + return d->planes.count(); } -Plane const &Sector::plane(int planeIndex) const +Plane &Sector::plane(int planeIndex) { if(planeIndex >= 0 && planeIndex < d->planes.count()) { @@ -286,19 +301,57 @@ Plane const &Sector::plane(int planeIndex) const throw MissingPlaneError("Sector::plane", QString("Missing plane %1").arg(planeIndex)); } -bool Sector::hasSkyMaskedPlane() const +Plane const &Sector::plane(int planeIndex) const +{ + return const_cast(this)->plane(planeIndex); +} + +Plane *Sector::addPlane(Vector3f const &normal, coord_t height) +{ + Plane *plane = new Plane(*this, normal, height); + + plane->setIndexInSector(d->planes.count()); + d->planes.append(plane); + + if(plane->isSectorFloor() || plane->isSectorCeiling()) + { + // We want notification of height changes so that we can update sound + // emitter origins of dependent surfaces. + plane->audienceForHeightChange() += d; + } + + // Once both floor and ceiling are known we can determine the z-height origin + // of our sound emitter. + /// @todo fixme: Assume planes are defined in order. + if(planeCount() == 2) + { + d->emitter->origin[2] = (floor().height() + ceiling().height()) / 2; + } + + return plane; +} + +LoopResult Sector::forAllPlanes(std::function func) const { for(Plane *plane : d->planes) { - if(plane->surface().hasSkyMaskedMaterial()) - return true; + if(auto result = func(*plane)) return result; } - return false; + return LoopContinue; +} + +int Sector::sideCount() const +{ + return d->sides.count(); } -Sector::Sides const &Sector::sides() const +LoopResult Sector::forAllSides(std::function func) const { - return d->sides; + for(LineSide *side : d->sides) + { + if(auto result = func(*side)) return result; + } + return LoopContinue; } void Sector::buildSides() @@ -337,36 +390,6 @@ void Sector::buildSides() }); } -Plane *Sector::addPlane(Vector3f const &normal, coord_t height) -{ - Plane *plane = new Plane(*this, normal, height); - - plane->setIndexInSector(d->planes.count()); - d->planes.append(plane); - - if(plane->isSectorFloor() || plane->isSectorCeiling()) - { - // We want notification of height changes so that we can update sound - // emitter origins of dependent surfaces. - plane->audienceForHeightChange() += d; - } - - // Once both floor and ceiling are known we can determine the z-height origin - // of our sound emitter. - /// @todo fixme: Assume planes are defined in order. - if(planeCount() == 2) - { - d->emitter->origin[VZ] = (floor().height() + ceiling().height()) / 2; - } - - return plane; -} - -Sector::Planes const &Sector::planes() const -{ - return d->planes; -} - static void linkSoundEmitter(SoundEmitter &root, SoundEmitter &newEmitter) { // The sector's base is always root of the chain, so link the other after it. diff --git a/doomsday/client/src/world/worldsystem.cpp b/doomsday/client/src/world/worldsystem.cpp index cd503f8aaf..d5b8200189 100644 --- a/doomsday/client/src/world/worldsystem.cpp +++ b/doomsday/client/src/world/worldsystem.cpp @@ -574,10 +574,11 @@ DENG2_PIMPL(WorldSystem) /// @todo Refactor away: map->forAllSectors([] (Sector §or) { - for(LineSide *side : sector.sides()) + sector.forAllSides([] (LineSide &side) { - side->fixMissingMaterials(); - } + side.fixMissingMaterials(); + return LoopContinue; + }); return LoopContinue; }); #endif