Improove bounding volume building in the scenery loading process.

Refactor common code in the BoundingVolumeBuildVisitor.hxx.

Modified Files:
	simgear/scene/model/BoundingVolumeBuildVisitor.hxx
	simgear/scene/model/ModelRegistry.cxx
	simgear/scene/model/ModelRegistry.hxx

simgear/scene/model/BoundingVolumeBuildVisitor.hxx

@@ -23,7 +23,6 @@
 #include <osg/Drawable>
 #include <osg/Geode>
 #include <osg/Group>
-#include <osg/MatrixTransform>
 #include <osg/PagedLOD>
 #include <osg/Transform>
 #include <osg/TriangleFunctor>
@@ -375,8 +374,9 @@ public:
     //     virtual void end() = 0;
     // };
 
-    BoundingVolumeBuildVisitor() :
+    BoundingVolumeBuildVisitor(bool dumpIntoLeafs) :
-        osg::NodeVisitor(osg::NodeVisitor::TRAVERSE_ACTIVE_CHILDREN)
+        osg::NodeVisitor(osg::NodeVisitor::TRAVERSE_ACTIVE_CHILDREN),
+        _dumpIntoLeafs(dumpIntoLeafs)
     {
         setTraversalMask(SG_NODEMASK_TERRAIN_BIT);
     }
@@ -402,57 +402,39 @@ public:
 
     virtual void apply(osg::Geode& geode)
     {
+        if (hasBoundingVolumeTree(geode))
+            return;
+
         const SGMaterial* oldMaterial = pushMaterial(geode.getStateSet());
 
-        if (!hasBoundingVolumeTree(geode))
+        bool flushHere = getNodePath().size() <= 1 || _dumpIntoLeafs;
+        if (flushHere) {
+            // push the current active primitive list
+            PFunctor previousPrimitives;
+            _primitiveFunctor.swap(previousPrimitives);
+
+            // walk the children
             for(unsigned i = 0; i < geode.getNumDrawables(); ++i)
                 fillWith(geode.getDrawable(i));
 
-        // Flush the bounding volume tree if we reached the topmost group
+            // Flush the bounding volume tree if we reached the topmost group
-        if (getNodePath().size() <= 1)
             addBoundingVolumeTreeToNode(geode);
+
+            // pop the current active primitive list
+            _primitiveFunctor.swap(previousPrimitives);
+        } else {
+            for(unsigned i = 0; i < geode.getNumDrawables(); ++i)
+                fillWith(geode.getDrawable(i));
+        }
+
         _primitiveFunctor.setCurrentMaterial(oldMaterial);
     }
 
     virtual void apply(osg::Group& group)
-    {
+    { traverseAndCollect(group); }
-        // Note that we do not need to push the already collected list of
-        // primitives, since we are now in the topmost node ...
-
-        const SGMaterial* oldMaterial = pushMaterial(group.getStateSet());
-
-        if (!hasBoundingVolumeTree(group))
-            traverse(group);
-
-        // Flush the bounding volume tree if we reached the topmost group
-        if (getNodePath().size() <= 1)
-            addBoundingVolumeTreeToNode(group);
-
-        _primitiveFunctor.setCurrentMaterial(oldMaterial);
-    }
 
     virtual void apply(osg::Transform& transform)
-    {
+    { traverseAndDump(transform); }
-        // push the current active primitive list
-        PFunctor previousPrimitives;
-        _primitiveFunctor.swap(previousPrimitives);
-
-        const SGMaterial* oldMaterial = pushMaterial(transform.getStateSet());
-
-        // walk the children
-        if (!hasBoundingVolumeTree(transform))
-            traverse(transform);
-
-        // We know whenever we see a transform, we need to flush the
-        // collected bounding volume tree since these transforms are not
-        // handled by the plain leafs.
-        addBoundingVolumeTreeToNode(transform);
-
-        _primitiveFunctor.setCurrentMaterial(oldMaterial);
-
-        // pop the current active primitive list
-        _primitiveFunctor.swap(previousPrimitives);
-    }
 
     virtual void apply(osg::PagedLOD&)
     {
@@ -463,7 +445,55 @@ public:
     {
         if (camera.getRenderOrder() != osg::Camera::NESTED_RENDER)
             return;
-        apply(static_cast<osg::Transform&>(camera));
+        traverseAndDump(camera);
+    }
+
+    void traverseAndDump(osg::Node& node)
+    {
+        if (hasBoundingVolumeTree(node))
+            return;
+
+        const SGMaterial* oldMaterial = pushMaterial(node.getStateSet());
+
+        // push the current active primitive list
+        PFunctor previousPrimitives;
+        _primitiveFunctor.swap(previousPrimitives);
+
+        // walk the children
+        traverse(node);
+
+        // We know whenever we see a transform, we need to flush the
+        // collected bounding volume tree since these transforms are not
+        // handled by the plain leafs.
+        addBoundingVolumeTreeToNode(node);
+
+        // pop the current active primitive list
+        _primitiveFunctor.swap(previousPrimitives);
+
+        _primitiveFunctor.setCurrentMaterial(oldMaterial);
+    }
+
+    void traverseAndCollect(osg::Node& node)
+    {
+        // Already been here??
+        if (hasBoundingVolumeTree(node))
+            return;
+
+        // Force a flush of the bvtree if we are in the topmost node.
+        if (getNodePath().size() <= 1) {
+            traverseAndDump(node);
+            return;
+        }
+
+        // Note that we do not need to push the already collected list of
+        // primitives, since we are now in the topmost node ...
+
+        const SGMaterial* oldMaterial = pushMaterial(node.getStateSet());
+
+        // walk the children
+        traverse(node);
+
+        _primitiveFunctor.setCurrentMaterial(oldMaterial);
     }
 
     void addBoundingVolumeTreeToNode(osg::Node& node)
@@ -493,6 +523,7 @@ public:
 
 private:
     PFunctor _primitiveFunctor;
+    bool _dumpIntoLeafs;
 };
 
 }

simgear/scene/model/ModelRegistry.cxx

@@ -416,17 +416,10 @@ osg::Node* OptimizeModelPolicy::optimize(osg::Node* node,
 }
 
 osg::Node* DefaultCopyPolicy::copy(osg::Node* model, const string& fileName,
-                    const osgDB::ReaderWriter::Options* opt)
+                                   const osgDB::ReaderWriter::Options* opt)
 {
-    /// Crude hack for the bounding volume sharing problem.
-    /// Better solution this week.
-    /// Note that this does not really build in the case we come here
-    /// the second time for the same node
-    BoundingVolumeBuildVisitor bvBuilder;
-    model->accept(bvBuilder);
-
     // Add an extra reference to the model stored in the database.
-    // That it to avoid expiring the object from the cache even if it is still
+    // That is to avoid expiring the object from the cache even if it is still
     // in use. Note that the object cache will think that a model is unused
     // if the reference count is 1. If we clone all structural nodes here
     // we need that extra reference to the original object
@@ -466,9 +459,34 @@ string OSGSubstitutePolicy::substitute(const string& name,
     return absFileName;
 }
 
+
+void
+BuildLeafBVHPolicy::buildBVH(const std::string& fileName, osg::Node* node)
+{
+    SG_LOG(SG_IO, SG_INFO, "Building leaf attached boundingvolume tree for \""
+           << fileName << "\".");
+    BoundingVolumeBuildVisitor bvBuilder(true);
+    node->accept(bvBuilder);
+}
+
+void
+BuildGroupBVHPolicy::buildBVH(const std::string& fileName, osg::Node* node)
+{
+    SG_LOG(SG_IO, SG_INFO, "Building group attached boundingvolume tree for \""
+           << fileName << "\".");
+    BoundingVolumeBuildVisitor bvBuilder(false);
+    node->accept(bvBuilder);
+}
+
+void
+NoBuildBVHPolicy::buildBVH(const std::string& fileName, osg::Node*)
+{
+    SG_LOG(SG_IO, SG_INFO, "Omitting boundingvolume tree for \""
+           << fileName << "\".");
+}
+
 ModelRegistry::ModelRegistry() :
-    _defaultCallback(new DefaultCallback("")),
+    _defaultCallback(new DefaultCallback(""))
-    _nestingLevel(0)
 {
 }
 
@@ -491,7 +509,6 @@ ModelRegistry::readNode(const string& fileName,
                         const ReaderWriter::Options* opt)
 {
     ScopedLock<ReentrantMutex> lock(readerMutex);
-    ++_nestingLevel;
 
     // XXX Workaround for OSG plugin bug.
     Registry* registry = Registry::instance();
@@ -504,15 +521,6 @@ ModelRegistry::readNode(const string& fileName,
     else
         result = _defaultCallback->readNode(fileName, opt);
 
-    if (0 == --_nestingLevel) {
-        SG_LOG(SG_IO, SG_INFO, "Building boundingvolume tree for \""
-               << fileName << "\".");
-        BoundingVolumeBuildVisitor bvBuilder;
-        result.getNode()->accept(bvBuilder);
-    } else {
-        SG_LOG(SG_IO, SG_INFO, "Defering boundingvolume tree built for \""
-               << fileName << "\" to parent.");
-    }
     return result;
 }
 
@@ -600,7 +608,8 @@ struct ACProcessPolicy {
 
 typedef ModelRegistryCallback<ACProcessPolicy, DefaultCachePolicy,
                               ACOptimizePolicy, DefaultCopyPolicy,
-                              OSGSubstitutePolicy> ACCallback;
+                              OSGSubstitutePolicy, BuildLeafBVHPolicy>
+ACCallback;
 
 namespace
 {

simgear/scene/model/ModelRegistry.hxx

@@ -61,13 +61,13 @@ namespace simgear
 // readNode function is specified as a template with a bunch of
 // pluggable (and predefined) policies.
 template <typename ProcessPolicy, typename CachePolicy, typename OptimizePolicy,
-          typename CopyPolicy, typename SubstitutePolicy>
+          typename CopyPolicy, typename SubstitutePolicy, typename BVHPolicy>
 class ModelRegistryCallback : public osgDB::Registry::ReadFileCallback {
 public:
     ModelRegistryCallback(const std::string& extension) :
         _processPolicy(extension), _cachePolicy(extension),
         _optimizePolicy(extension), _copyPolicy(extension),
-        _substitutePolicy(extension)
+        _substitutePolicy(extension), _bvhPolicy(extension)
     {
     }
     virtual osgDB::ReaderWriter::ReadResult
@@ -97,11 +97,12 @@ public:
                 optimizedNode = _optimizePolicy.optimize(processedNode.get(),
                                                          fileName, opt);
             }
+            _bvhPolicy.buildBVH(fileName, optimizedNode.get());
             _cachePolicy.addToCache(fileName, optimizedNode.get());
         }
-        return ReaderWriter::ReadResult(_copyPolicy.copy(optimizedNode.get(),
+        osg::ref_ptr<osg::Node> copyNode;
-                                                         fileName,
+        copyNode = _copyPolicy.copy(optimizedNode.get(), fileName, opt);
-                                                         opt));
+        return ReaderWriter::ReadResult(copyNode);
     }
 protected:
     static osgDB::ReaderWriter::ReadResult
@@ -121,6 +122,7 @@ protected:
     OptimizePolicy _optimizePolicy;
     CopyPolicy _copyPolicy;
     SubstitutePolicy _substitutePolicy;
+    BVHPolicy _bvhPolicy;
     virtual ~ModelRegistryCallback() {}
 };
 
@@ -196,9 +198,26 @@ struct NoSubstitutePolicy {
         return std::string();
     }
 };
+
+struct BuildLeafBVHPolicy {
+    BuildLeafBVHPolicy(const std::string& extension) {}
+    void buildBVH(const std::string& fileName, osg::Node* node);
+};
+
+struct BuildGroupBVHPolicy {
+    BuildGroupBVHPolicy(const std::string& extension) {}
+    void buildBVH(const std::string& fileName, osg::Node* node);
+};
+
+struct NoBuildBVHPolicy {
+    NoBuildBVHPolicy(const std::string& extension) {}
+    void buildBVH(const std::string& fileName, osg::Node* node);
+};
+
 typedef ModelRegistryCallback<DefaultProcessPolicy, DefaultCachePolicy,
                               OptimizeModelPolicy, DefaultCopyPolicy,
-                              OSGSubstitutePolicy> DefaultCallback;
+                              OSGSubstitutePolicy, BuildLeafBVHPolicy>
+DefaultCallback;
 
 // The manager for the callbacks
 class ModelRegistry : public osgDB::Registry::ReadFileCallback,
@@ -227,14 +246,14 @@ protected:
     // Protect against simultaneous calls from main thread (MP models)
     // and pager thread.
     OpenThreads::ReentrantMutex readerMutex;
-    unsigned _nestingLevel;
 };
 
 // Callback that only loads the file without any caching or
 // postprocessing.
 typedef ModelRegistryCallback<DefaultProcessPolicy, NoCachePolicy,
                               NoOptimizePolicy, NoCopyPolicy,
-                              NoSubstitutePolicy> LoadOnlyCallback;
+                              NoSubstitutePolicy, BuildLeafBVHPolicy>
+LoadOnlyCallback;
 
 // Proxy for registering extension-based callbacks