ErrorReporting: set context for STG loading

Ensure the STG absolute path can be propagated to all files triggered
by STG loading, including the delayed files and proxied files. This
allows us to attribute errors to the correct scenery path.

simgear/debug/ErrorReportingCallback.cxx

@@ -79,6 +79,11 @@ void setErrorContextCallback(ContextCallback cb)
 }
 
 ErrorReportContext::ErrorReportContext(const std::string& key, const std::string& value)
+{
+    add(key, value);
+}
+
+void ErrorReportContext::add(const std::string& key, const std::string& value)
 {
     if (static_contextCallback) {
         _keys.push_back(key);

simgear/debug/ErrorReportingCallback.hxx

@@ -49,7 +49,7 @@ enum class LoadFailure {
  translated error messages for the user.
  */
 enum class ErrorCode {
-    MissingShader,
+    LoadEffectsShaders,
     LoadingTexture,
     XMLModelLoad,
     ThreeDModelLoad, // AC3D, OBJ, etc
@@ -60,8 +60,7 @@ enum class ErrorCode {
     XMLLoadCommand,
     AircraftSystems,
     InputDeviceConfig,
-    AITrafficSchedule,
+    AITrafficSchedule
-    AirportDataLoad // ground-net, jetways, etc
 };
 /**
  @brief Define an error-reporting context value, for the duration of this
@@ -80,6 +79,8 @@ public:
      */
     ErrorReportContext(const ContextMap& context = {});
 
+    void add(const std::string& key, const std::string& value);
+
     /**
      @brief allowed delayed add of values
      */

simgear/scene/material/Effect.cxx

@@ -846,7 +846,7 @@ void reload_shaders()
         } else {
             SG_LOG(SG_INPUT, SG_ALERT, "Could not locate shader: " << fileName);
             simgear::reportFailure(simgear::LoadFailure::NotFound,
-                                   simgear::ErrorCode::MissingShader,
+                                   simgear::ErrorCode::LoadEffectsShaders,
                                    "Reload: couldn't find shader:" + sitr->first.first);
         }
     }
@@ -940,7 +940,7 @@ void ShaderProgramBuilder::buildAttribute(Effect* effect, Pass* pass,
         if (fileName.empty())
         {
             SG_LOG(SG_INPUT, SG_ALERT, "Could not locate shader" << shaderName);
-            simgear::reportFailure(simgear::LoadFailure::NotFound, simgear::ErrorCode::MissingShader,
+            simgear::reportFailure(simgear::LoadFailure::NotFound, simgear::ErrorCode::LoadEffectsShaders,
                                    "Couldn't locate shader:" + shaderName, sg_location{shaderName});
 
             throw BuilderException(string("couldn't find shader ") +
@@ -968,7 +968,7 @@ void ShaderProgramBuilder::buildAttribute(Effect* effect, Pass* pass,
             if (loadShaderFromUTF8File(shader, fileName)) {
                 if (!program->addShader(shader.get())) {
                     simgear::reportFailure(simgear::LoadFailure::BadData,
-                                           simgear::ErrorCode::MissingShader,
+                                           simgear::ErrorCode::LoadEffectsShaders,
                                            "Program::addShader failed",
                                            SGPath::fromUtf8(fileName));
                 }
@@ -976,7 +976,7 @@ void ShaderProgramBuilder::buildAttribute(Effect* effect, Pass* pass,
                 shaderMap.insert(ShaderMap::value_type(skey, shader));
             } else {
                 simgear::reportFailure(simgear::LoadFailure::BadData,
-                                       simgear::ErrorCode::MissingShader,
+                                       simgear::ErrorCode::LoadEffectsShaders,
                                        "Failed to read shader source code",
                                        SGPath::fromUtf8(fileName));
             }

simgear/scene/model/ModelRegistry.cxx

@@ -60,12 +60,13 @@
 
 #include <simgear/scene/tgdb/VPBTechnique.hxx>
 
-#include <simgear/structure/exception.hxx>
+#include <simgear/debug/ErrorReportingCallback.hxx>
-#include <simgear/props/props.hxx>
-#include <simgear/props/props_io.hxx>
-#include <simgear/props/condition.hxx>
 #include <simgear/io/sg_file.hxx>
 #include <simgear/misc/lru_cache.hxx>
+#include <simgear/props/condition.hxx>
+#include <simgear/props/props.hxx>
+#include <simgear/props/props_io.hxx>
+#include <simgear/structure/exception.hxx>
 
 #include "BoundingVolumeBuildVisitor.hxx"
 #include "model.hxx"
@@ -293,6 +294,11 @@ ModelRegistry::readImage(const string& fileName,
 
     const SGReaderWriterOptions* sgoptC = dynamic_cast<const SGReaderWriterOptions*>(opt);
 
+    simgear::ErrorReportContext ec;
+    if (sgoptC && sgoptC->getModelData()) {
+        ec.addFromMap(sgoptC->getModelData()->getErrorContext());
+    }
+
     if (cache_active && (!sgoptC || sgoptC->getLoadOriginHint() != SGReaderWriterOptions::LoadOriginHint::ORIGIN_SPLASH_SCREEN)) {
         if (fileExtension != "dds" && fileExtension != "gz") {
 
@@ -716,6 +722,17 @@ ReaderWriter::ReadResult
 ModelRegistry::readNode(const string& fileName,
                         const Options* opt)
 {
+    // propogate error context from the caller
+    simgear::ErrorReportContext ec;
+    auto sgopt = dynamic_cast<const SGReaderWriterOptions*>(opt);
+    if (sgopt) {
+        if (sgopt->getModelData()) {
+            ec.addFromMap(sgopt->getModelData()->getErrorContext());
+        }
+
+        ec.addFromMap(sgopt->getErrorContext());
+    }
+
     ReaderWriter::ReadResult res;
     CallbackMap::iterator iter
         = nodeCallbackMap.find(getFileExtension(fileName));

simgear/scene/model/model.cxx

@@ -24,11 +24,12 @@
 #include <simgear/scene/util/SplicingVisitor.hxx>
 #include <simgear/scene/util/SGReaderWriterOptions.hxx>
 
-#include <simgear/structure/exception.hxx>
+#include <simgear/debug/ErrorReportingCallback.hxx>
-#include <simgear/structure/Singleton.hxx>
+#include <simgear/props/condition.hxx>
 #include <simgear/props/props.hxx>
 #include <simgear/props/props_io.hxx>
-#include <simgear/props/condition.hxx>
+#include <simgear/structure/Singleton.hxx>
+#include <simgear/structure/exception.hxx>
 
 #include "model.hxx"
 
@@ -364,6 +365,8 @@ ref_ptr<Node> instantiateMaterialEffects(osg::Node* modelGroup,
       } else {
         effect = DefaultEffect::instance()->getEffect();
         SG_LOG( SG_TERRAIN, SG_ALERT, "Unable to get effect for " << options->getMaterialName());
+        simgear::reportFailure(simgear::LoadFailure::NotFound, simgear::ErrorCode::LoadEffectsShaders,
+                               "Unable to get effect for material:" + options->getMaterialName());
       }
     } else {
       effect = DefaultEffect::instance()->getEffect();

simgear/scene/model/modellib.cxx

@@ -139,13 +139,6 @@ SGModelLib::loadModel(const string &path,
     opt->setPropertyNode(prop_root ? prop_root: static_propRoot.get());
     opt->setModelData(data);
 
-    // establish the error report context so we can attribute any load
-    // errors correctly
-    simgear::ErrorReportContext ec;
-    if (data) {
-        ec.addFromMap(data->getErrorContext());
-    }
-
     if (load2DPanels) {
        opt->setLoadPanel(static_panelFunc);
     }
@@ -168,11 +161,6 @@ SGModelLib::loadDeferredModel(const string &path, SGPropertyNode *prop_root,
     proxyNode->setLoadingExternalReferenceMode(osg::ProxyNode::DEFER_LOADING_TO_DATABASE_PAGER);
     proxyNode->setFileName(0, path);
 
-    simgear::ErrorReportContext ec;
-    if (data) {
-        ec.addFromMap(data->getErrorContext());
-    }
-
     osg::ref_ptr<SGReaderWriterOptions> opt;
     opt = SGReaderWriterOptions::copyOrCreate(osgDB::Registry::instance()->getOptions());
     opt->getDatabasePathList().push_front( osgDB::getFilePath(path) );
@@ -204,11 +192,6 @@ SGModelLib::loadPagedModel(SGPropertyNode *prop_root, SGModelData *data, SGModel
     unsigned int simple_models = 0;
     osg::PagedLOD *plod = new osg::PagedLOD;
 
-    simgear::ErrorReportContext ec;
-    if (data) {
-        ec.addFromMap(data->getErrorContext());
-    }
-
     osg::ref_ptr<SGReaderWriterOptions> opt;
     opt = SGReaderWriterOptions::copyOrCreate(osgDB::Registry::instance()->getOptions());
     opt->setPropertyNode(prop_root ? prop_root: static_propRoot.get());

simgear/scene/tgdb/ReaderWriterSTG.cxx

@@ -177,7 +177,7 @@ struct ReaderWriterSTG::_ModelBin {
                 proxy->setName("proxyNode");
                 proxy->setLoadingExternalReferenceMode(osg::ProxyNode::DEFER_LOADING_TO_DATABASE_PAGER);
                 proxy->setFileName(0, o._name);
-                proxy->setDatabaseOptions(o._options.get());
+                proxy->setDatabaseOptions(o._options);
 
                 // Give the node some values so the Quadtree builder has
                 // a BoundingBox to work with prior to the model being loaded.
@@ -186,6 +186,7 @@ struct ReaderWriterSTG::_ModelBin {
                 proxy->setCenterMode(osg::ProxyNode::UNION_OF_BOUNDING_SPHERE_AND_USER_DEFINED);
                 node = proxy;
             } else {
+                ErrorReportContext ec("terrain-stg", o._errorLocation.utf8Str());
                 node = osgDB::readRefNodeFile(o._name, o._options.get());
                 if (!node.valid()) {
                     SG_LOG(SG_TERRAIN, SG_ALERT, o._errorLocation << ": Failed to load "
@@ -228,6 +229,8 @@ struct ReaderWriterSTG::_ModelBin {
         virtual osgDB::ReaderWriter::ReadResult
         readNode(const std::string&, const osgDB::Options*)
         {
+            ErrorReportContext ec("terrain-bucket", _bucket.gen_index_str());
+
             STGObjectsQuadtree quadtree((GetModelLODCoord()), (AddModelLOD()));
             quadtree.buildQuadTree(_objectStaticList.begin(), _objectStaticList.end());
             osg::ref_ptr<osg::Group> group = quadtree.getRoot();
@@ -539,6 +542,7 @@ struct ReaderWriterSTG::_ModelBin {
                             opt->setInstantiateEffects(false);
                     _ObjectStatic obj;
 
+                    opt->addErrorContext("terrain-stg", absoluteFileName.utf8Str());
                     obj._errorLocation = absoluteFileName;
                     obj._token = token;
                     obj._name = name;
@@ -582,6 +586,8 @@ struct ReaderWriterSTG::_ModelBin {
                     _ObjectStatic obj;
 
                     opt->setInstantiateEffects(false);
+                    opt->addErrorContext("terrain-stg", absoluteFileName.utf8Str());
+
                     if (SGPath(name).lower_extension() == "ac") {
                       // Generate material/Effects lookups for raw models, i.e.
                       // those not wrapped in an XML while will include Effects
@@ -680,7 +686,7 @@ struct ReaderWriterSTG::_ModelBin {
         std::string terrain_name = string("terrain ").append(bucket.gen_index_str());
         terrainGroup->setName(terrain_name);
 
-        simgear::ErrorReportContext ec{"bucket", bucket.gen_index_str()};
+        simgear::ErrorReportContext ec{"terrain-bucket", bucket.gen_index_str()};
 
         bool vpb_active = SGSceneFeatures::instance()->getVPBActive();
         if (vpb_active) {
@@ -717,6 +723,7 @@ struct ReaderWriterSTG::_ModelBin {
         } else if (_foundBase) {
             for (auto stgObject : _objectList) {
                 osg::ref_ptr<osg::Node> node;
+                simgear::ErrorReportContext ec("terrain-stg", stgObject._errorLocation.utf8Str());
                 node = osgDB::readRefNodeFile(stgObject._name, stgObject._options.get());
 
                 if (!node.valid()) {
@@ -838,11 +845,10 @@ ReaderWriterSTG::readNode(const std::string& fileName, const osgDB::Options* opt
         return ReadResult::FILE_NOT_FOUND;
     }
 
-    osg::ref_ptr<SGReaderWriterOptions> sgOpts(SGReaderWriterOptions::copyOrCreate(options));
+    const auto sgOpts = dynamic_cast<const SGReaderWriterOptions*>(options);
-    if (sgOpts->getSceneryPathSuffixes().empty()) {
+    if (!sgOpts || sgOpts->getSceneryPathSuffixes().empty()) {
         SG_LOG(SG_TERRAIN, SG_ALERT, "Loading tile " << fileName << ", no scenery path suffixes were configured so giving up");
         return ReadResult::FILE_NOT_FOUND;
-
     }
 
     SG_LOG(SG_TERRAIN, SG_INFO, "Loading tile " << fileName);
@@ -869,7 +875,7 @@ ReaderWriterSTG::readNode(const std::string& fileName, const osgDB::Options* opt
         }
 
         for (auto suffix : sgOpts->getSceneryPathSuffixes()) {
-            SGPath p = base / suffix / basePath / fileName;
+            const auto p = base / suffix / basePath / fileName;
             modelBin.read(p, options);
         }
     }

simgear/scene/util/SGReaderWriterOptions.cxx

@@ -17,9 +17,7 @@
 // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 //
 
-#ifdef HAVE_CONFIG_H
+#include <simgear_config.h>
-#  include <simgear_config.h>
-#endif
 
 #include <simgear/scene/util/OsgMath.hxx>
 
@@ -54,4 +52,10 @@ SGReaderWriterOptions::fromPath(const SGPath& path)
     return options;
 }
 
+void SGReaderWriterOptions::addErrorContext(const std::string& key, const std::string& value)
+{
+    _errorContext[key] = value;
 }
+
+
+} // namespace simgear

simgear/scene/util/SGReaderWriterOptions.hxx

@@ -82,22 +82,22 @@ public:
         _LoadOriginHint(ORIGIN_MODEL)
     { }
     SGReaderWriterOptions(const SGReaderWriterOptions& options,
-                          const osg::CopyOp& copyop = osg::CopyOp::SHALLOW_COPY) :
+                          const osg::CopyOp& copyop = osg::CopyOp::SHALLOW_COPY) : osgDB::Options(options, copyop),
-        osgDB::Options(options, copyop),
+                                                                                   _propertyNode(options._propertyNode),
-        _propertyNode(options._propertyNode),
+                                                                                   _materialLib(options._materialLib),
-        _materialLib(options._materialLib),
 #ifdef ENABLE_GDAL
-        _dem(options._dem),
+                                                                                   _dem(options._dem),
 #endif
-        _load_panel(options._load_panel),
+                                                                                   _load_panel(options._load_panel),
-        _model_data(options._model_data),
+                                                                                   _model_data(options._model_data),
-        _instantiateEffects(options._instantiateEffects),
+                                                                                   _instantiateEffects(options._instantiateEffects),
-        _instantiateMaterialEffects(options._instantiateMaterialEffects),
+                                                                                   _instantiateMaterialEffects(options._instantiateMaterialEffects),
-        _materialName(options._materialName),
+                                                                                   _materialName(options._materialName),
-        _sceneryPathSuffixes(options._sceneryPathSuffixes),
+                                                                                   _sceneryPathSuffixes(options._sceneryPathSuffixes),
-        _autoTooltipsMaster(options._autoTooltipsMaster),
+                                                                                   _autoTooltipsMaster(options._autoTooltipsMaster),
-        _autoTooltipsMasterMax(options._autoTooltipsMasterMax),
+                                                                                   _autoTooltipsMasterMax(options._autoTooltipsMasterMax),
-        _LoadOriginHint(ORIGIN_MODEL)
+                                                                                   _LoadOriginHint(ORIGIN_MODEL),
+                                                                                   _errorContext(options._errorContext)
     { }
 
     META_Object(simgear, SGReaderWriterOptions);
@@ -178,6 +178,15 @@ public:
     void setLoadOriginHint(LoadOriginHint _v) const { _LoadOriginHint = _v; } 
     LoadOriginHint getLoadOriginHint() const { return _LoadOriginHint; }
 
+    using ErrorContext = std::map<std::string, std::string>;
+
+    void addErrorContext(const std::string& key, const std::string& value);
+
+    ErrorContext getErrorContext() const
+    {
+        return _errorContext;
+    }
+
 protected:
     virtual ~SGReaderWriterOptions();
 
@@ -199,6 +208,7 @@ private:
     int _autoTooltipsMasterMax;
     SGGeod _geod;
     mutable LoadOriginHint _LoadOriginHint;
+    ErrorContext _errorContext;
 };
 
 }