From 5e30d83a431b7c4db2997158c44c4df2f0a2abd7 Mon Sep 17 00:00:00 2001
From: James Turner <zakalawe@mac.com>
Date: Fri, 30 Apr 2021 13:47:27 +0100
Subject: [PATCH] Error-reporting: improve reports for STGs and effects

When an effect defines no shader sources, special case this error to
avoid confusing result from SGProgram.

Add error-context for readNode STG loading, so failures inside an
STG can be attributed
---
 simgear/scene/material/Effect.cxx      |  8 ++++++++
 simgear/scene/tgdb/ReaderWriterSTG.cxx |  7 ++++++-
 simgear/scene/util/SGProgram.cxx       | 21 +++++++++++----------
 3 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/simgear/scene/material/Effect.cxx b/simgear/scene/material/Effect.cxx
index 5c942455..d0b648ef 100644
--- a/simgear/scene/material/Effect.cxx
+++ b/simgear/scene/material/Effect.cxx
@@ -989,6 +989,14 @@ void ShaderProgramBuilder::buildAttribute(Effect* effect, Pass* pass,
             }
         }
     }
+
+    if (sgprogram->getNumShaders() == 0) {
+        simgear::reportFailure(simgear::LoadFailure::BadData,
+                               simgear::ErrorCode::LoadEffectsShaders,
+                               "No shader source code defined for effect",
+                               effect->filePath());
+    }
+
     for (const auto& key : prgKey.attributes) {
         program->addBindAttribLocation(key.first, key.second);
     }
diff --git a/simgear/scene/tgdb/ReaderWriterSTG.cxx b/simgear/scene/tgdb/ReaderWriterSTG.cxx
index aad3f372..34e00b41 100644
--- a/simgear/scene/tgdb/ReaderWriterSTG.cxx
+++ b/simgear/scene/tgdb/ReaderWriterSTG.cxx
@@ -690,7 +690,9 @@ struct ReaderWriterSTG::_ModelBin {
                             }
                             callback(token,name, SGGeod::fromDegM(obj._lon, obj._lat, obj._elev), obj._hdg,restofline);
                         } else {
-                            SG_LOG( SG_TERRAIN, SG_ALERT, absoluteFileName << ": Unknown token '" << token << "'" );
+                            // SG_LOG( SG_TERRAIN, SG_ALERT, absoluteFileName << ": Unknown token '" << token << "'" );
+                            simgear::reportFailure(simgear::LoadFailure::Misconfigured, simgear::ErrorCode::BTGLoad,
+                                                   "Unknown STG token:" + token, absoluteFileName);
                         }
                     }
                 }
@@ -866,6 +868,7 @@ ReaderWriterSTG::readNode(const std::string& fileName, const osgDB::Options* opt
 {
     _ModelBin modelBin;
     SGBucket bucket(bucketIndexFromFileName(fileName));
+    simgear::ErrorReportContext ec("terrain-bucket", bucket.gen_index_str());
 
     // We treat 123.stg different than ./123.stg.
     // The difference is that ./123.stg as well as any absolute path
@@ -873,6 +876,7 @@ ReaderWriterSTG::readNode(const std::string& fileName, const osgDB::Options* opt
     // In contrast 123.stg uses the search paths to load a set of stg
     // files spread across the scenery directories.
     if (osgDB::getSimpleFileName(fileName) != fileName) {
+        simgear::ErrorReportContext ec("terrain-stg", fileName);
         if (!modelBin.read(fileName, options))
             return ReadResult::FILE_NOT_FOUND;
     }
@@ -913,6 +917,7 @@ ReaderWriterSTG::readNode(const std::string& fileName, const osgDB::Options* opt
 
         for (auto suffix : sgOpts->getSceneryPathSuffixes()) {
             const auto p = base / suffix / basePath / fileName;
+            simgear::ErrorReportContext ec("terrain-stg", p.utf8Str());
             modelBin.read(p, options);
         }
     }
diff --git a/simgear/scene/util/SGProgram.cxx b/simgear/scene/util/SGProgram.cxx
index 419493c6..ddf2fc59 100644
--- a/simgear/scene/util/SGProgram.cxx
+++ b/simgear/scene/util/SGProgram.cxx
@@ -52,19 +52,20 @@ void SGProgram::apply(osg::State& state) const
         std::string infoLog;
         _checkState = FailedToApply;
         getPCP(state)->getInfoLog(infoLog);
+        if (!infoLog.empty()) {
+            // log all the shader source file names, to help in debugging link errors
+            std::ostringstream os;
+            for (int i = 0; i < getNumShaders(); ++i) {
+                const auto shader = getShader(i);
+                os << "\t" << shader->getFileName() << "\n";
+            }
 
-        // log all the shader source file names, to help in debugging link errors
-        std::ostringstream os;
-        for (int i = 0; i < getNumShaders(); ++i) {
-            const auto shader = getShader(i);
-            os << "\t" << shader->getFileName() << "\n";
+            simgear::reportFailure(simgear::LoadFailure::BadData, simgear::ErrorCode::LoadEffectsShaders,
+                                   "Shader program errors: " + infoLog +
+                                       "\n\nShader sources:\n" + os.str(),
+                                   _effectFilePath);
         }
 
-        simgear::reportFailure(simgear::LoadFailure::BadData, simgear::ErrorCode::LoadEffectsShaders,
-                               "Shader program errors: " + infoLog +
-                                   "\n\nShader sources:\n" + os.str(),
-                               _effectFilePath);
-
         for (int i = 0; i < getNumShaders(); ++i) {
             const auto shader = getShader(i);
             auto pcs = shader->getPCS(state);