From 43d849232bb7829734c41be6e780c1c24c4fe003 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 | 27 ++++++++++++--- simgear/scene/model/model.cxx | 9 +++-- simgear/scene/model/modellib.cxx | 17 --------- simgear/scene/tgdb/ReaderWriterSTG.cxx | 36 +++++++++++--------- simgear/scene/util/SGReaderWriterOptions.cxx | 10 ++++-- simgear/scene/util/SGReaderWriterOptions.hxx | 24 +++++++++---- 9 files changed, 86 insertions(+), 57 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 14ba1dd1..cd32ff89 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); } } @@ -944,7 +944,7 @@ void ShaderProgramBuilder::buildAttribute(Effect* effect, Pass* pass, string fileName = SGModelLib::findDataFile(shaderName, options); if (fileName.empty()) { - 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 ") + @@ -972,7 +972,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)); } @@ -980,7 +980,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 8080e888..5b137149 100644 --- a/simgear/scene/model/ModelRegistry.cxx +++ b/simgear/scene/model/ModelRegistry.cxx @@ -56,12 +56,15 @@ #include #include -#include -#include -#include -#include + +#include + #include #include +#include +#include +#include +#include #include "BoundingVolumeBuildVisitor.hxx" #include "model.hxx" @@ -289,6 +292,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") { @@ -714,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 81f4e9e0..f22fddd6 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" @@ -380,6 +381,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 9511f83c..5b4de2f4 100644 --- a/simgear/scene/model/modellib.cxx +++ b/simgear/scene/model/modellib.cxx @@ -141,13 +141,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); } @@ -167,11 +160,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) ); @@ -203,11 +191,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 6d7db879..90e5a69e 100644 --- a/simgear/scene/tgdb/ReaderWriterSTG.cxx +++ b/simgear/scene/tgdb/ReaderWriterSTG.cxx @@ -156,7 +156,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. @@ -165,6 +165,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()); #if OSG_VERSION_LESS_THAN(3,4,0) node = osgDB::readNodeFile(o._name, o._options.get()); #else @@ -211,6 +212,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(); @@ -467,15 +470,15 @@ struct ReaderWriterSTG::_ModelBin { else opt->setInstantiateEffects(false); _ObjectStatic obj; - - obj._errorLocation = absoluteFileName; - obj._token = token; - obj._name = name; - obj._agl = (token == "OBJECT_STATIC_AGL"); - obj._proxy = true; - in >> obj._lon >> obj._lat >> obj._elev >> obj._hdg >> obj._pitch >> obj._roll; - obj._range = range; - obj._options = opt; + opt->addErrorContext("terrain-stg", absoluteFileName.utf8Str()); + obj._errorLocation = absoluteFileName; + obj._token = token; + obj._name = name; + obj._agl = (token == "OBJECT_STATIC_AGL"); + obj._proxy = true; + in >> obj._lon >> obj._lat >> obj._elev >> obj._hdg >> obj._pitch >> obj._roll; + obj._range = range; + obj._options = opt; checkInsideBucket(absoluteFileName, obj._lon, obj._lat); _objectStaticList.push_back(obj); } else if (token == "OBJECT_SHARED" || token == "OBJECT_SHARED_AGL") { @@ -511,6 +514,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 @@ -596,10 +601,11 @@ 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()}; if (_foundBase) { osg::ref_ptr node; + #if OSG_VERSION_LESS_THAN(3,4,0) node = osgDB::readNodeFile(stgObject._name, stgObject._options.get()); #else @@ -624,7 +630,6 @@ struct ReaderWriterSTG::_ModelBin { "Warning: failed to generate ocean tile!" ); } } - for (std::list<_ObjectStatic>::iterator i = _objectStaticList.begin(); i != _objectStaticList.end(); ++i) { if (!i->_agl) continue; @@ -720,11 +725,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); @@ -749,7 +753,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 50d21a01..23129e96 100644 --- a/simgear/scene/util/SGReaderWriterOptions.hxx +++ b/simgear/scene/util/SGReaderWriterOptions.hxx @@ -76,12 +76,11 @@ 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), @@ -89,7 +88,8 @@ public: _instantiateMaterialEffects(options._instantiateMaterialEffects), _materialName(options._materialName), _sceneryPathSuffixes(options._sceneryPathSuffixes), - _LoadOriginHint(ORIGIN_MODEL) + _LoadOriginHint(ORIGIN_MODEL), + _errorContext(options._errorContext) { } META_Object(simgear, SGReaderWriterOptions); @@ -158,7 +158,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(); @@ -179,6 +188,7 @@ private: string_list _sceneryPathSuffixes; SGGeod _geod; mutable LoadOriginHint _LoadOriginHint; + ErrorContext _errorContext; }; }