Performance|Renderer|Refactor: Retain constness in APIs; avoid redund…

…ant memory allocs

It is important not to accidentally lost constness of object
references, particularly when dealing with Qt containers. Non-const
access to containers may force Qt to detach the data into a separate
private instance.

However, the most impactful change is the removal of the QBitArray
in Rend_RadioBspLeafEdges(). Allocating and freeing the array in
complex maps is much slower than simply calculating dot products
for the planes as we go.

doomsday/client/include/render/lightdecoration.h

@@ -72,7 +72,7 @@ class LightDecoration : public Decoration, public Lumobj::Source
      *
      * @see Decoration::setSurface(), Decoration::hasSurface()
      */
-    Lumobj *generateLumobj();
+    Lumobj *generateLumobj() const;
 };
 
 #endif // DENG_CLIENT_RENDER_LIGHTDECORATION_H

doomsday/client/include/render/lumobj.h

@@ -99,7 +99,7 @@ class Lumobj : public de::MapObject
      *
      * @param newSource  New source to attribute. Use @c 0 to clear.
      */
-    void setSource(Source *newSource);
+    void setSource(Source const *newSource);
 
     /**
      * Returns the light color/intensity of the lumobj.

doomsday/client/include/render/rend_fakeradio.h

@@ -145,7 +145,7 @@ void Rend_RadioWallSection(de::WallEdge const &leftEdge, de::WallEdge const &rig
  * Render FakeRadio for the given BSP leaf. Draws all shadow geometry linked to the
  * BspLeaf, that has not already been rendered.
  */
-void Rend_RadioBspLeafEdges(BspLeaf &bspLeaf);
+void Rend_RadioBspLeafEdges(BspLeaf const &bspLeaf);
 
 /**
  * Render the shadow poly vertices, for debug.

doomsday/client/include/render/shadowedge.h

@@ -35,7 +35,9 @@ class HEdge;
 class ShadowEdge
 {
 public:
-    ShadowEdge(HEdge &leftMostHEdge, int edge);
+    ShadowEdge();
+
+    void init(HEdge const &leftMostHEdge, int edge);
 
     void prepare(int planeIndex);
 

doomsday/client/include/world/line.h

@@ -532,7 +532,7 @@ class Line : public de::MapElement
          *
          * @param newCount  New shadow vis count.
          */
-        void setShadowVisCount(int newCount);
+        void setShadowVisCount(int newCount) const;
 
 #ifdef __CLIENT__
 

doomsday/client/include/world/sector.h

@@ -116,7 +116,10 @@ class Sector : public de::MapElement
         /**
          * Returns the parent sector of the cluster.
          */
-        Sector &sector() const;
+        Sector const &sector() const;
+
+        /// @copydoc sector()
+        Sector &sector();
 
         /**
          * Returns the identified @em physical plane of the parent sector. Note
@@ -125,43 +128,61 @@ class Sector : public de::MapElement
          *
          * @param planeIndex  Index of the plane to return.
          */
-        Plane &plane(int planeIndex) const;
+        Plane const &plane(int planeIndex) const;
+
+        /// @copydoc plane()
+        Plane &plane(int planeIndex);
 
         /**
          * Returns the sector plane which defines the @em physical floor of the
          * cluster.
          * @see hasSector(), plane()
          */
-        inline Plane &floor() const { return plane(Sector::Floor); }
+        inline Plane const &floor() const { return plane(Sector::Floor); }
+
+        /// @copydoc floor()
+        inline Plane &floor() { return plane(Sector::Floor); }
 
         /**
          * Returns the sector plane which defines the @em physical ceiling of
          * the cluster.
          * @see hasSector(), plane()
          */
-        inline Plane &ceiling() const { return plane(Sector::Ceiling); }
+        inline Plane const &ceiling() const { return plane(Sector::Ceiling); }
+
+        /// @copydoc ceiling()
+        inline Plane &ceiling() { return plane(Sector::Ceiling); }
 
         /**
          * Returns the identified @em visual sector plane for the cluster (which
          * may or may not be the same as the physical plane).
          *
          * @param planeIndex  Index of the plane to return.
          */
-        Plane &visPlane(int planeIndex) const;
+        Plane const &visPlane(int planeIndex) const;
+
+        /// @copydoc visPlane()
+        Plane &visPlane(int planeIndex);
 
         /**
          * Returns the sector plane which defines the @em visual floor of the
          * cluster.
          * @see hasSector(), floor()
          */
-        inline Plane &visFloor() const   { return visPlane(Sector::Floor); }
+        inline Plane const &visFloor() const { return visPlane(Sector::Floor); }
+
+        /// @copydoc visFloor()
+        inline Plane &visFloor() { return visPlane(Sector::Floor); }
 
         /**
          * Returns the sector plane which defines the @em visual ceiling of the
          * cluster.
          * @see hasSector(), ceiling()
          */
-        inline Plane &visCeiling() const { return visPlane(Sector::Ceiling); }
+        inline Plane const &visCeiling() const { return visPlane(Sector::Ceiling); }
+
+        /// @copydoc visCeiling()
+        inline Plane &visCeiling() { return visPlane(Sector::Ceiling); }
 
         /**
          * Returns the total number of @em visual planes in the cluster.
@@ -637,7 +658,7 @@ class SectorClusterCirculator
      * cluster to face boundary, or when navigating the so-called "one-ring" of
      * a vertex.
      */
-    static de::HEdge &findBackNeighbor(de::HEdge &hedge, de::ClockDirection direction)
+    static de::HEdge &findBackNeighbor(de::HEdge const &hedge, de::ClockDirection direction)
     {
         return getNeighbor(hedge, direction, getCluster(hedge)).twin();
     }
@@ -648,16 +669,16 @@ class SectorClusterCirculator
      *
      * @param direction  Relative direction of the desired neighbor.
      */
-    de::HEdge &neighbor(de::ClockDirection direction) {
+    de::HEdge const &neighbor(de::ClockDirection direction) {
         _current = &getNeighbor(*_current, direction, _cluster);
         return *_current;
     }
 
     /// Returns the next half-edge (clockwise) and advances the circulator.
-    inline de::HEdge &next() { return neighbor(de::Clockwise); }
+    inline de::HEdge const &next() { return neighbor(de::Clockwise); }
 
     /// Returns the previous half-edge (anticlockwise) and advances the circulator.
-    inline de::HEdge &previous() { return neighbor(de::Anticlockwise); }
+    inline de::HEdge const &previous() { return neighbor(de::Anticlockwise); }
 
     /// Advance to the next half-edge (clockwise).
     inline SectorClusterCirculator &operator ++ () {
@@ -688,7 +709,7 @@ class SectorClusterCirculator
     }
 
     /// Returns the current half-edge of a non-empty sequence.
-    de::HEdge &operator * () const {
+    de::HEdge const &operator * () const {
         if(!_current)
         {
             /// @throw NullError Attempted to dereference a "null" circulator.
@@ -699,17 +720,17 @@ class SectorClusterCirculator
 
     /// Returns a pointer to the current half-edge (might be @c NULL, meaning the
     /// circulator references an empty sequence).
-    de::HEdge *operator -> () const { return _current; }
+    de::HEdge const *operator -> () const { return _current; }
 
 private:
-    static SectorCluster *getCluster(de::HEdge &hedge);
+    static SectorCluster *getCluster(de::HEdge const &hedge);
 
-    static de::HEdge &getNeighbor(de::HEdge &hedge, de::ClockDirection direction,
-                                  SectorCluster *cluster = 0);
+    static de::HEdge const &getNeighbor(de::HEdge const &hedge, de::ClockDirection direction,
+                                        SectorCluster const *cluster = 0);
 
-    de::HEdge *_hedge;
-    de::HEdge *_current;
-    SectorCluster *_cluster;
+    de::HEdge const *_hedge;
+    de::HEdge const *_current;
+    SectorCluster const *_cluster;
 };
 
 #endif // DENG_WORLD_SECTOR_H

doomsday/client/src/render/lightdecoration.cpp

@@ -70,7 +70,7 @@ static float checkLightLevel(float lightlevel, float min, float max)
     return de::clamp(0.f, (lightlevel - min) / float(max - min), 1.f);
 }
 
-Lumobj *LightDecoration::generateLumobj()
+Lumobj *LightDecoration::generateLumobj() const
 {
     // Decorations with zero color intensity produce no light.
     if(source().color == Vector3f(0, 0, 0))

doomsday/client/src/render/lumobj.cpp

@@ -41,7 +41,7 @@ float Lumobj::Source::occlusion(Vector3d const &eye) const
 
 DENG2_PIMPL_NOREF(Lumobj)
 {
-    Source *source;     ///< Source of the lumobj (if any, not owned).
+    Source const *source;///< Source of the lumobj (if any, not owned).
     double maxDistance; ///< Used when rendering to limit the number drawn lumobjs.
     Vector3f color;     ///< Light color/intensity.
     double radius;      ///< Radius in map space units.
@@ -92,7 +92,7 @@ Lumobj::Lumobj(Lumobj const &other)
     : MapObject(other.origin()), d(new Instance(*other.d))
 {}
 
-void Lumobj::setSource(Source *newSource)
+void Lumobj::setSource(Source const *newSource)
 {
     d->source = newSource;
 }

doomsday/client/src/render/rend_fakeradio.cpp

@@ -1259,7 +1259,7 @@ static void writeShadowSection2(ShadowEdge const &leftEdge, ShadowEdge const &ri
                          0, 4, rvertices, rcolors);
 }
 
-static void writeShadowSection(int planeIndex, LineSide &side, float shadowDark)
+static void writeShadowSection(int planeIndex, LineSide const &side, float shadowDark)
 {
     DENG_ASSERT(side.hasSections());
     DENG_ASSERT(!side.line().definesPolyobj());
@@ -1268,9 +1268,9 @@ static void writeShadowSection(int planeIndex, LineSide &side, float shadowDark)
 
     if(!side.leftHEdge()) return;
 
-    HEdge *leftHEdge   = side.leftHEdge();
-    Plane const &plane = side.sector().plane(planeIndex);
-    Surface const *suf = &plane.surface();
+    HEdge const *leftHEdge = side.leftHEdge();
+    Plane const &plane     = side.sector().plane(planeIndex);
+    Surface const *suf     = &plane.surface();
 
     // Surfaces with a missing material don't shadow.
     if(!suf->hasMaterial()) return;
@@ -1284,12 +1284,15 @@ static void writeShadowSection(int planeIndex, LineSide &side, float shadowDark)
     /// @todo Encapsulate this logic in ShadowEdge -ds
     if(!leftHEdge->hasFace()) return;
 
-    BspLeaf &frontLeaf = leftHEdge->face().mapElementAs<BspLeaf>();
+    BspLeaf const &frontLeaf = leftHEdge->face().mapElementAs<BspLeaf>();
     if(!frontLeaf.hasCluster() || !frontLeaf.cluster().hasWorldVolume())
         return;
 
-    ShadowEdge leftEdge(*leftHEdge, Line::From);
-    ShadowEdge rightEdge(*leftHEdge, Line::To);
+    static ShadowEdge leftEdge; // this function is called often; keep these around
+    static ShadowEdge rightEdge;
+
+    leftEdge.init(*leftHEdge, Line::From);
+    rightEdge.init(*leftHEdge, Line::To);
 
     leftEdge.prepare(planeIndex);
     rightEdge.prepare(planeIndex);
@@ -1303,14 +1306,14 @@ static void writeShadowSection(int planeIndex, LineSide &side, float shadowDark)
  * @attention Do not use the global radio state in here, as @a bspLeaf can be
  * part of any sector, not the one chosen for wall rendering.
  */
-void Rend_RadioBspLeafEdges(BspLeaf &bspLeaf)
+void Rend_RadioBspLeafEdges(BspLeaf const &bspLeaf)
 {
     if(!rendFakeRadio) return;
     if(levelFullBright) return;
 
     if(bspLeaf.shadowLines().isEmpty()) return;
 
-    SectorCluster &cluster = bspLeaf.cluster();
+    SectorCluster const &cluster = bspLeaf.cluster();
     float sectorlight = cluster.sector().lightLevel();
 
     // Determine the shadow properties.
@@ -1321,28 +1324,11 @@ void Rend_RadioBspLeafEdges(BspLeaf &bspLeaf)
     // Any need to continue?
     if(shadowDark < .0001f) return;
 
-    bool workToDo = false;
-
-    // Flag planes in the cluster which face the viewer.
-    QBitArray planesVisible(cluster.visPlaneCount());
-    Vector3f eyeToSurface(Vector2d(vOrigin[VX], vOrigin[VZ]) - bspLeaf.poly().center());
-    for(int pln = 0; pln < cluster.visPlaneCount(); ++pln)
-    {
-        Plane const &plane = cluster.visPlane(pln);
-
-        eyeToSurface.z = vOrigin[VY] - plane.heightSmoothed();
-        if(eyeToSurface.dot(plane.surface().normal()) >= 0)
-        {
-            planesVisible.setBit(pln);
-            workToDo = true;
-        }
-    }
-
-    if(!workToDo) return;
+    Vector3f const eyeToSurface(Vector2d(vOrigin[VX], vOrigin[VZ]) - bspLeaf.poly().center());
 
     // We need to check all the shadow lines linked to this BspLeaf for
     // the purpose of fakeradio shadowing.
-    foreach(LineSide *side, bspLeaf.shadowLines())
+    foreach(LineSide const *side, bspLeaf.shadowLines())
     {
         // Already rendered during the current frame? We only want to
         // render each shadow once per frame.
@@ -1352,7 +1338,9 @@ void Rend_RadioBspLeafEdges(BspLeaf &bspLeaf)
 
             for(int pln = 0; pln < cluster.visPlaneCount(); ++pln)
             {
-                if(planesVisible.testBit(pln))
+                Plane const &plane = cluster.visPlane(pln);
+                if(Vector3f(eyeToSurface, vOrigin[VY] - plane.heightSmoothed())
+                        .dot(plane.surface().normal()) >= 0)
                 {
                     writeShadowSection(pln, *side, shadowDark);
                 }

doomsday/client/src/render/shadowedge.cpp

@@ -38,26 +38,27 @@ namespace de {
 
 DENG2_PIMPL_NOREF(ShadowEdge)
 {
-    HEdge *leftMostHEdge;
+    HEdge const *leftMostHEdge;
     int edge;
 
     Vector3d inner;
     Vector3d outer;
     float sectorOpenness;
     float openness;
-
-    Instance(HEdge &leftMostHEdge, int edge)
-        : leftMostHEdge(&leftMostHEdge),
-          edge(edge),
-          sectorOpenness(0),
-          openness(0)
-    {}
 };
 
-ShadowEdge::ShadowEdge(HEdge &leftMostHEdge, int edge)
-    : d(new Instance(leftMostHEdge, edge))
+ShadowEdge::ShadowEdge() : d(new Instance)
 {}
 
+void ShadowEdge::init(HEdge const &leftMostHEdge, int edge)
+{
+    d->leftMostHEdge = &leftMostHEdge;
+    d->edge          = edge;
+
+    d->inner = d->outer = Vector3d();
+    d->sectorOpenness = d->openness = 0;
+}
+
 /**
  * Returns a value in the range of 0..2, representing how 'open' the edge is.
  *

doomsday/client/src/render/walledge.cpp

@@ -499,15 +499,15 @@ DENG2_PIMPL(WallEdge), public IHPlane
     {
         ClockDirection const direction = edge? Clockwise : Anticlockwise;
 
-        HEdge *hedge = wallHEdge;
+        HEdge const *hedge = wallHEdge;
         while((hedge = &SectorClusterCirculator::findBackNeighbor(*hedge, direction)) != wallHEdge)
         {
             // Stop if there is no back cluster.
-            BspLeaf *backLeaf = hedge->hasFace()? &hedge->face().mapElementAs<BspLeaf>() : 0;
+            BspLeaf const *backLeaf = hedge->hasFace()? &hedge->face().mapElementAs<BspLeaf>() : 0;
             if(!backLeaf || !backLeaf->hasCluster())
                 break;
 
-            SectorCluster &cluster = backLeaf->cluster();
+            SectorCluster const &cluster = backLeaf->cluster();
             if(cluster.hasWorldVolume())
             {
                 for(int i = 0; i < cluster.visPlaneCount(); ++i)

doomsday/client/src/world/line.cpp

@@ -666,7 +666,7 @@ int Line::Side::shadowVisCount() const
     return d->shadowVisCount;
 }
 
-void Line::Side::setShadowVisCount(int newCount)
+void Line::Side::setShadowVisCount(int newCount) const
 {
     d->shadowVisCount = newCount;
 }

doomsday/client/src/world/map.cpp

@@ -1109,11 +1109,11 @@ DENG2_OBSERVES(bsp::Partitioner, UnclosedSectorFound)
         }
     }
 
-    void generateLumobjs(QList<Decoration *> const &decorList)
+    void generateLumobjs(QList<Decoration *> const &decorList) const
     {
-        foreach(Decoration *decor, decorList)
+        foreach(Decoration const *decor, decorList)
         {
-            if(LightDecoration *decorLight = decor->maybeAs<LightDecoration>())
+            if(LightDecoration const *decorLight = decor->maybeAs<LightDecoration>())
             {
                 QScopedPointer<Lumobj>lum(decorLight->generateLumobj());
                 if(!lum.isNull())