Refactor|World|Map: Minor Map cleanup refactorings

doomsday/client/include/world/map.h

@@ -706,35 +706,58 @@ class Map
     bool isValidClPolyobj(int i);
 
     /**
-     * Returns the set of decorated surfaces for the map.
+     * Link the given @a surface in all material lists and surface sets which
+     * the map maintains to improve performance. Only surfaces attributed to
+     * the map will be linked (alien surfaces are ignored).
+     *
+     * @param surface  The surface to be linked.
      */
-    SurfaceSet /*const*/ &decoratedSurfaces();
+    void linkInMaterialLists(Surface *surface);
 
     /**
-     * Returns the set of glowing surfaces for the map.
+     * Unlink the given @a surface in all material lists and surface sets which
+     * the map maintains to improve performance.
+     *
+     * @note The material currently attributed to the surface does not matter
+     * for unlinking purposes and the surface will be unlinked from all lists
+     * regardless.
+     *
+     * @param surface  The surface to be unlinked.
      */
-    SurfaceSet /*const*/ &glowingSurfaces();
+    void unlinkInMaterialLists(Surface *surface);
 
     /**
-     * $smoothmatoffset: Roll the surface material offset tracker buffers.
+     * Provides a set of all surfaces linked to the material lists for which
+     * decorations are defined.
      */
-    void updateScrollingSurfaces();
+    SurfaceSet const &decoratedSurfaces();
+
+    /**
+     * Provides a set of all surfaces linked to the material lists for which
+     * glow animations are defined.
+     */
+    SurfaceSet const &glowingSurfaces();
 
     /**
      * Returns the set of scrolling surfaces for the map.
      */
     SurfaceSet /*const*/ &scrollingSurfaces();
 
     /**
-     * $smoothplane: Roll the height tracker buffers.
+     * $smoothmatoffset: Roll the surface material offset tracker buffers.
      */
-    void updateTrackedPlanes();
+    void updateScrollingSurfaces();
 
     /**
      * Returns the set of tracked planes for the map.
      */
     PlaneSet /*const*/ &trackedPlanes();
 
+    /**
+     * $smoothplane: Roll the height tracker buffers.
+     */
+    void updateTrackedPlanes();
+
     /**
      * Returns @c true iff a LightGrid has been initialized for the map.
      *
@@ -790,7 +813,11 @@ class Map
      */
     void initSkyFix();
 
-    void buildSurfaceLists();
+    /**
+     * Rebuild the surface material lists. To be called when a full update is
+     * necessary.
+     */
+    void buildMaterialLists();
 
     /**
      * Initializes bias lighting for the map. New light sources are initialized

doomsday/client/src/world/map.cpp

@@ -1115,21 +1115,6 @@ DENG2_OBSERVES(bsp::Partitioner, UnclosedSectorFound)
         }
     }
 
-    void addSurfaceToLists(Surface &suf)
-    {
-        if(!suf.hasMaterial()) return;
-
-        if(suf.material().hasGlow())
-        {
-            glowingSurfaces.insert(&suf);
-        }
-
-        if(suf.material().isDecorated())
-        {
-            decoratedSurfaces.insert(&suf);
-        }
-    }
-
     /**
      * $smoothplane: interpolate the visual offset of planes.
      */
@@ -1301,44 +1286,6 @@ Map::BspLeafs const &Map::bspLeafs() const
 
 #ifdef __CLIENT__
 
-Map::SurfaceSet &Map::decoratedSurfaces()
-{
-    return d->decoratedSurfaces;
-}
-
-Map::SurfaceSet &Map::glowingSurfaces()
-{
-    return d->glowingSurfaces;
-}
-
-void Map::buildSurfaceLists()
-{
-    d->decoratedSurfaces.clear();
-    d->glowingSurfaces.clear();
-
-    foreach(Line *line, d->lines)
-    for(int i = 0; i < 2; ++i)
-    {
-        Line::Side &side = line->side(i);
-        if(!side.hasSections()) continue;
-
-        d->addSurfaceToLists(side.middle());
-        d->addSurfaceToLists(side.top());
-        d->addSurfaceToLists(side.bottom());
-    }
-
-    foreach(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())
-        {
-            d->addSurfaceToLists(plane->surface());
-        }
-    }
-}
-
 bool Map::hasLightGrid()
 {
     return !d->lightGrid.isNull();
@@ -1396,8 +1343,96 @@ void Map::initBias()
     LOG_INFO(String("Completed in %1 seconds.").arg(begunAt.since(), 0, 'g', 2));
 }
 
+void Map::unlinkInMaterialLists(Surface *surface)
+{
+    if(!surface) return;
+
+    d->decoratedSurfaces.remove(surface);
+    d->glowingSurfaces.remove(surface);
+}
+
+void Map::linkInMaterialLists(Surface *surface)
+{
+    if(!surface) return;
+
+    // Only surfaces with a material will be linked.
+    if(!surface->hasMaterial()) return;
+
+    // Ignore surfaces not currently attributed to the map.
+    if(&surface->map() != this)
+    {
+        qDebug() << "Ignoring alien surface" << de::dintptr(surface) << "in Map::unlinkInMaterialLists";
+        return;
+    }
+
+    Material &material = surface->material();
+    if(material.hasGlow())
+    {
+        d->glowingSurfaces.insert(surface);
+    }
+    if(material.isDecorated())
+    {
+        d->decoratedSurfaces.insert(surface);
+    }
+}
+
+void Map::buildMaterialLists()
+{
+    d->decoratedSurfaces.clear();
+    d->glowingSurfaces.clear();
+
+    foreach(Line *line, d->lines)
+    for(int i = 0; i < 2; ++i)
+    {
+        Line::Side &side = line->side(i);
+        if(!side.hasSections()) continue;
+
+        linkInMaterialLists(&side.middle());
+        linkInMaterialLists(&side.top());
+        linkInMaterialLists(&side.bottom());
+    }
+
+    foreach(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())
+        {
+            linkInMaterialLists(&plane->surface());
+        }
+    }
+}
+
+Map::SurfaceSet const &Map::decoratedSurfaces()
+{
+    return d->decoratedSurfaces;
+}
+
+Map::SurfaceSet const &Map::glowingSurfaces()
+{
+    return d->glowingSurfaces;
+}
 #endif // __CLIENT__
 
+void Map::updateSurfacesOnMaterialChange(Material &material)
+{
+    if(ddMapSetup) return;
+
+#ifdef __CLIENT__
+    if(material.isDecorated())
+    {
+        foreach(Surface *surface, d->decoratedSurfaces)
+        {
+            if(&material == surface->materialPtr())
+            {
+                surface->markAsNeedingDecorationUpdate();
+            }
+        }
+    }
+#endif
+}
+
 Uri const &Map::uri() const
 {
     return d->uri;
@@ -2594,21 +2629,6 @@ BspLeaf &Map::bspLeafAt_FixedPrecision(Vector2d const &point) const
     return *bspElement->as<BspLeaf>();
 }
 
-void Map::updateSurfacesOnMaterialChange(Material &material)
-{
-    if(ddMapSetup) return;
-
-#ifdef __CLIENT__
-    foreach(Surface *surface, d->decoratedSurfaces)
-    {
-        if(&material == surface->materialPtr())
-        {
-            surface->markAsNeedingDecorationUpdate();
-        }
-    }
-#endif
-}
-
 #ifdef __CLIENT__
 
 void Map::updateScrollingSurfaces()
@@ -2960,8 +2980,8 @@ void Map::update()
         line->front().middle().markAsNeedingDecorationUpdate();
     }
 
-    // Rebuild the surface lists.
-    buildSurfaceLists();
+    // Rebuild the surface material lists.
+    buildMaterialLists();
 
 #endif // __CLIENT__
 

doomsday/client/src/world/plane.cpp

@@ -94,13 +94,7 @@ DENG2_PIMPL(Plane)
         /// @todo Map should observe Deletion.
         map.scrollingSurfaces().remove(&surface);
 
-        // If this plane's surface is in the glowing list, remove it.
-        /// @todo Map should observe Deletion.
-        map.glowingSurfaces().remove(&surface);
-
-        // If this plane's surface is in the decorated list, remove it.
-        /// @todo Map should observe Deletion.
-        map.decoratedSurfaces().remove(&surface);
+        map.unlinkInMaterialLists(&surface);
 
 #endif // __CLIENT__
     }
@@ -129,6 +123,9 @@ DENG2_PIMPL(Plane)
 
 #ifdef __CLIENT__
             // We need the decorations updated.
+            /// @todo optimize: Translation on the world up axis would be a
+            /// trivial operation to perform, which, would not require plotting
+            /// decorations again. This frequent case should be designed for.
             surface.markAsNeedingDecorationUpdate();
 #endif
         }

doomsday/client/src/world/surface.cpp

@@ -88,20 +88,20 @@ DENG2_PIMPL(Surface)
           flags(0)
     {}
 
-    /// @todo Refactor away -ds
+#ifdef DENG_DEBUG
     inline bool isSideMiddle()
     {
         return self.parent().type() == DMU_SIDE &&
                &self == &self.parent().as<Line::Side>()->middle();
     }
 
-    /// @todo Refactor away -ds
     inline bool isSectorExtraPlane()
     {
         if(self.parent().type() != DMU_PLANE) return false;
         Plane const &plane = *self.parent().as<Plane>();
         return !(plane.isSectorFloor() || plane.isSectorCeiling());
     }
+#endif
 
     void notifyNormalChanged(Vector3f const &oldNormal)
     {
@@ -299,44 +299,30 @@ bool Surface::setMaterial(Material *newMaterial, bool isMissingFix)
             d->materialIsMissingFix = false;
         }
 
+        d->material = newMaterial;
+
 #ifdef __CLIENT__
         if(!ddMapSetup)
         {
-            // If this plane's surface is in the decorated list, remove it.
-            map().decoratedSurfaces().remove(this);
-
-            // If this plane's surface is in the glowing list, remove it.
-            map().glowingSurfaces().remove(this);
+            map().unlinkInMaterialLists(this);
 
-            if(newMaterial)
+            if(d->material)
             {
-                if(newMaterial->hasGlow())
-                {
-                    map().glowingSurfaces().insert(this);
-                }
-
-                if(newMaterial->isDecorated())
-                {
-                    map().decoratedSurfaces().insert(this);
-                }
+                map().linkInMaterialLists(this);
 
                 if(parent().type() == DMU_PLANE)
                 {
-                    de::Uri uri = newMaterial->manifest().composeUri();
+                    de::Uri uri = d->material->manifest().composeUri();
                     ded_ptcgen_t const *def = Def_GetGenerator(reinterpret_cast<uri_s *>(&uri));
                     P_SpawnPlaneParticleGen(def, parent().as<Plane>());
                 }
 
             }
         }
-#endif // __CLIENT__
-
-        d->material = newMaterial;
 
-#ifdef __CLIENT__
         /// @todo Replace with a de::Observer-based mechanism.
         _decorationData.needsUpdate = true;
-#endif
+#endif // __CLIENT__
     }
     return true;
 }

doomsday/client/src/world/world.cpp

@@ -624,7 +624,7 @@ DENG2_PIMPL(World)
 #ifdef __CLIENT__
         map->initLightGrid();
         map->initSkyFix();
-        map->buildSurfaceLists();
+        map->buildMaterialLists();
         P_MapSpawnPlaneParticleGens();
 
         Time begunPrecacheAt;