From 802ce5ad230cdd01c395a37a449d66a9478ee0bd Mon Sep 17 00:00:00 2001 From: James Turner Date: Thu, 25 Feb 2021 22:00:58 +0000 Subject: [PATCH] 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 | 5 +++ simgear/debug/ErrorReportingCallback.hxx | 7 ++-- simgear/scene/material/Effect.cxx | 8 ++-- simgear/scene/model/ModelRegistry.cxx | 25 ++++++++++-- simgear/scene/model/model.cxx | 9 +++-- simgear/scene/model/modellib.cxx | 17 --------- simgear/scene/tgdb/ReaderWriterSTG.cxx | 18 ++++++--- simgear/scene/util/SGReaderWriterOptions.cxx | 10 +++-- simgear/scene/util/SGReaderWriterOptions.hxx | 40 ++++++++++++-------- 9 files changed, 84 insertions(+), 55 deletions(-) diff --git a/simgear/debug/ErrorReportingCallback.cxx b/simgear/debug/ErrorReportingCallback.cxx index d735a149..361f6c0a 100644 --- a/simgear/debug/ErrorReportingCallback.cxx +++ b/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); diff --git a/simgear/debug/ErrorReportingCallback.hxx b/simgear/debug/ErrorReportingCallback.hxx index c4885ff3..164d7f95 100644 --- a/simgear/debug/ErrorReportingCallback.hxx +++ b/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, - AirportDataLoad // ground-net, jetways, etc + AITrafficSchedule }; /** @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 */ diff --git a/simgear/scene/material/Effect.cxx b/simgear/scene/material/Effect.cxx index 76daaaca..1918b219 100644 --- a/simgear/scene/material/Effect.cxx +++ b/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)); } diff --git a/simgear/scene/model/ModelRegistry.cxx b/simgear/scene/model/ModelRegistry.cxx index 39ede8cb..42db5dfd 100644 --- a/simgear/scene/model/ModelRegistry.cxx +++ b/simgear/scene/model/ModelRegistry.cxx @@ -60,12 +60,13 @@ #include -#include -#include -#include -#include +#include #include #include +#include +#include +#include +#include #include "BoundingVolumeBuildVisitor.hxx" #include "model.hxx" @@ -293,6 +294,11 @@ ModelRegistry::readImage(const string& fileName, const SGReaderWriterOptions* sgoptC = dynamic_cast(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(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)); diff --git a/simgear/scene/model/model.cxx b/simgear/scene/model/model.cxx index 79a97ac5..7ca4d858 100644 --- a/simgear/scene/model/model.cxx +++ b/simgear/scene/model/model.cxx @@ -24,11 +24,12 @@ #include #include -#include -#include +#include +#include #include #include -#include +#include +#include #include "model.hxx" @@ -364,6 +365,8 @@ ref_ptr 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(); diff --git a/simgear/scene/model/modellib.cxx b/simgear/scene/model/modellib.cxx index 2c545acf..35dfbfea 100644 --- a/simgear/scene/model/modellib.cxx +++ b/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 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 opt; opt = SGReaderWriterOptions::copyOrCreate(osgDB::Registry::instance()->getOptions()); opt->setPropertyNode(prop_root ? prop_root: static_propRoot.get()); diff --git a/simgear/scene/tgdb/ReaderWriterSTG.cxx b/simgear/scene/tgdb/ReaderWriterSTG.cxx index a5e16a53..496635ca 100644 --- a/simgear/scene/tgdb/ReaderWriterSTG.cxx +++ b/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 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 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 sgOpts(SGReaderWriterOptions::copyOrCreate(options)); - if (sgOpts->getSceneryPathSuffixes().empty()) { + const auto sgOpts = dynamic_cast(options); + 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); } } diff --git a/simgear/scene/util/SGReaderWriterOptions.cxx b/simgear/scene/util/SGReaderWriterOptions.cxx index 8937bd42..9be951df 100644 --- a/simgear/scene/util/SGReaderWriterOptions.cxx +++ b/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 -#endif +#include #include @@ -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 diff --git a/simgear/scene/util/SGReaderWriterOptions.hxx b/simgear/scene/util/SGReaderWriterOptions.hxx index dfbe86b7..9122c83a 100644 --- a/simgear/scene/util/SGReaderWriterOptions.hxx +++ b/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) : - osgDB::Options(options, copyop), - _propertyNode(options._propertyNode), - _materialLib(options._materialLib), + const osg::CopyOp& copyop = osg::CopyOp::SHALLOW_COPY) : osgDB::Options(options, copyop), + _propertyNode(options._propertyNode), + _materialLib(options._materialLib), #ifdef ENABLE_GDAL - _dem(options._dem), + _dem(options._dem), #endif - _load_panel(options._load_panel), - _model_data(options._model_data), - _instantiateEffects(options._instantiateEffects), - _instantiateMaterialEffects(options._instantiateMaterialEffects), - _materialName(options._materialName), - _sceneryPathSuffixes(options._sceneryPathSuffixes), - _autoTooltipsMaster(options._autoTooltipsMaster), - _autoTooltipsMasterMax(options._autoTooltipsMasterMax), - _LoadOriginHint(ORIGIN_MODEL) + _load_panel(options._load_panel), + _model_data(options._model_data), + _instantiateEffects(options._instantiateEffects), + _instantiateMaterialEffects(options._instantiateMaterialEffects), + _materialName(options._materialName), + _sceneryPathSuffixes(options._sceneryPathSuffixes), + _autoTooltipsMaster(options._autoTooltipsMaster), + _autoTooltipsMasterMax(options._autoTooltipsMasterMax), + _LoadOriginHint(ORIGIN_MODEL), + _errorContext(options._errorContext) { } META_Object(simgear, SGReaderWriterOptions); @@ -176,7 +176,16 @@ public: // any texture that is used in a shader, as these often have special values // encoded into the channels that aren't suitable for conversion. void setLoadOriginHint(LoadOriginHint _v) const { _LoadOriginHint = _v; } - LoadOriginHint getLoadOriginHint() const { return _LoadOriginHint; } + LoadOriginHint getLoadOriginHint() const { return _LoadOriginHint; } + + using ErrorContext = std::map; + + 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; }; }