shaders: improve GLSL diagnostics and source attribution

src/shaders/SoGLSLShaderDiagnostics.h

@@ -0,0 +1,64 @@
+#ifndef COIN_SOGLSLSHADERDIAGNOSTICS_H
+#define COIN_SOGLSLSHADERDIAGNOSTICS_H
+
+// Private helpers shared by the GLSL shader and program implementations.
+
+#include <Inventor/SbString.h>
+
+#include "shaders/SoGLShaderObject.h"
+#include "glue/glp.h"
+#include "glue/glslp.h"
+
+#include <vector>
+
+static inline const char *
+soglsl_stage_name(const SoGLShaderObject::ShaderType type)
+{
+  switch (type) {
+  case SoGLShaderObject::VERTEX:
+    return "vertex shader";
+  case SoGLShaderObject::FRAGMENT:
+    return "fragment shader";
+  case SoGLShaderObject::GEOMETRY:
+    return "geometry shader";
+  default:
+    return "shader";
+  }
+}
+
+static inline SbString
+soglsl_get_info_log(const cc_glglue * glue,
+                    const GLuint handle,
+                    const SbBool program)
+{
+  GLint length = 0;
+  if (program) {
+    cc_glglue_glGetGLSLProgramiv(glue, handle, GL_INFO_LOG_LENGTH, &length);
+  }
+  else {
+    cc_glglue_glGetShaderiv(glue, handle, GL_INFO_LOG_LENGTH, &length);
+  }
+
+  if (length <= 1) return SbString();
+
+  std::vector<COIN_GLchar> infoLog(static_cast<size_t>(length), '\0');
+  GLsizei charsWritten = 0;
+  if (program) {
+    cc_glglue_glGetProgramInfoLog(glue, handle, length, &charsWritten,
+                                  infoLog.data());
+  }
+  else {
+    cc_glglue_glGetShaderInfoLog(glue, handle, length, &charsWritten,
+                                 infoLog.data());
+  }
+
+  if (charsWritten >= 0 && charsWritten < length) {
+    infoLog[static_cast<size_t>(charsWritten)] = '\0';
+  }
+  else {
+    infoLog.back() = '\0';
+  }
+  return SbString(infoLog.data());
+}
+
+#endif /* ! COIN_SOGLSLSHADERDIAGNOSTICS_H */

src/shaders/SoGLSLShaderObject.cpp

@@ -48,12 +48,11 @@
 #include "glue/glp.h"
 #include "glue/glslp.h"
 #include "rendering/SoGL.h"
+#include "shaders/SoGLSLShaderDiagnostics.h"
 #include "shaders/SoGLSLShaderParameter.h"
 
 static int32_t soglshaderobject_idcounter = 1;
 
-// *************************************************************************
-
 SoGLSLShaderObject::SoGLSLShaderObject(const uint32_t cachecontext)
   : SoGLShaderObject(cachecontext)
 {
@@ -131,7 +130,9 @@ SoGLSLShaderObject::load(const char* srcStr)
                           GL_COMPILE_STATUS, &flag);
   SoGLSLShaderObject::printInfoLog(this->GLContext(),
                                    this->shaderHandle,
-                                   this->getShaderType());
+                                   this->getShaderType(),
+                                   this->sourceHint,
+                                   !flag);
 
   if (!flag) {
     this->shaderHandle = 0;
@@ -195,28 +196,35 @@ SoGLSLShaderObject::isAttached(void) const
 void
 SoGLSLShaderObject::printInfoLog(const cc_glglue * g,
                                  COIN_GLhandle handle,
-                                 int objType)
+                                 const ShaderType shaderType,
+                                 const SbString & sourceHint,
+                                 const SbBool failed)
 {
-  GLint length = 0;
-
-  cc_glglue_glGetShaderiv(g, (GLuint) handle, GL_INFO_LOG_LENGTH, &length);
-
-  if (length > 1) {
-    COIN_GLchar *infoLog = new COIN_GLchar[length];
-    GLsizei charsWritten = 0;
-    cc_glglue_glGetShaderInfoLog(g, (GLuint) handle, length, &charsWritten,
-                                 (char *) infoLog);
-    SbString s("GLSL");
-    switch (objType) {
-    case 0: s += "vertexShader "; break;
-    case 1: s += "fragmentShader "; break;
-    case 2: s += "geometryShader "; break;
-    default: ;// do nothing
+  const char * sourceName = sourceHint.getLength() > 0 ?
+    sourceHint.getString() : "<unnamed>";
+  const SbString infoLog = soglsl_get_info_log(g, (GLuint) handle, FALSE);
+
+  if (infoLog.getLength() > 0) {
+    if (failed) {
+      SoDebugError::postWarning("SoGLSLShaderObject::printInfoLog",
+                                "%s '%s' failed to compile: %s",
+                                soglsl_stage_name(shaderType),
+                                sourceName,
+                                infoLog.getString());
     }
-    SoDebugError::postInfo("SoGLSLShaderObject::printInfoLog",
-                           "%s log: '%s'",
-                           s.getString(), infoLog);
-    delete [] infoLog;
+    else {
+      SoDebugError::postInfo("SoGLSLShaderObject::printInfoLog",
+                             "%s '%s' log: %s",
+                             soglsl_stage_name(shaderType),
+                             sourceName,
+                             infoLog.getString());
+    }
+  }
+  else if (failed) {
+    SoDebugError::postWarning("SoGLSLShaderObject::printInfoLog",
+                              "%s '%s' failed to compile with no compiler log",
+                              soglsl_stage_name(shaderType),
+                              sourceName);
   }
 }
 

src/shaders/SoGLSLShaderObject.h

@@ -68,11 +68,16 @@ class SoGLSLShaderObject : public SoGLShaderObject
 
   // source should be the name of the calling function
   static SbBool didOpenGLErrorOccur(const SbString & source);
-  static void printInfoLog(const cc_glglue * g, COIN_GLhandle handle, int objType);
 
   void updateCoinParameter(SoState * state, const SbName & name, SoShaderParameter * param, const int value) override;
 
 private:
+  static void printInfoLog(const cc_glglue * g,
+                           COIN_GLhandle handle,
+                           const ShaderType shaderType,
+                           const SbString & sourceHint,
+                           const SbBool failed);
+
   COIN_GLhandle programHandle;
   COIN_GLhandle shaderHandle;
   SbBool isattached;

src/shaders/SoGLSLShaderProgram.cpp

@@ -36,11 +36,34 @@
 #include <Inventor/misc/SoContextHandler.h>
 
 #include "shaders/SoGLSLShaderObject.h"
+#include "shaders/SoGLSLShaderDiagnostics.h"
+#include <Inventor/errors/SoDebugError.h>
 #include "glue/glp.h"
 #include "glue/glslp.h"
 
 // *************************************************************************
 
+static void
+soglshaderprogram_append_source_description(
+  SbString & result, const SoGLSLShaderObject * shader)
+{
+  if (shader == NULL || !shader->isActive() ||
+      shader->sourceHint.getLength() == 0) return;
+
+  SbString sourceDescription(soglsl_stage_name(shader->getShaderType()));
+  sourceDescription += "=";
+  sourceDescription += shader->sourceHint;
+
+  if (result.getLength() > 0) result += ", ";
+  result += sourceDescription;
+  if (result.getLength() > 256) {
+    result = result.getSubString(0, 252);
+    result += "...";
+  }
+}
+
+// *************************************************************************
+
 // FIXME: no checking is done to see whether "shader objects" (as for
 // GL_ARB_shader_objects) are actually supported or not. 20050124 mortene.
 
@@ -116,7 +139,8 @@ SoGLSLShaderProgram::enable(const cc_glglue * g)
     cc_glglue_glUseProgram(g, (GLuint) programhandle);
 
     if (SoGLSLShaderObject::didOpenGLErrorOccur("SoGLSLShaderProgram::enable")) {
-      SoGLSLShaderObject::printInfoLog(g, programhandle, 0);
+      SoGLSLShaderProgram::printInfoLog(g, programhandle,
+                                        this->getSourceDescription(), FALSE);
     }
   }
 }
@@ -129,22 +153,16 @@ SoGLSLShaderProgram::disable(const cc_glglue * g)
   }
 }
 
-#if defined(SOURCE_HINT)
 SbString
-SoGLSLShaderProgram::getSourceHint(void) const
+SoGLSLShaderProgram::getSourceDescription(void) const
 {
   SbString result;
-  for (int i=0; i<this->shaderObjects.size(); i++) {
+  for (int i=0; i<this->shaderObjects.getLength(); i++) {
     SoGLSLShaderObject *shader = this->shaderObjects[i];
-    if (shader && shader->isActive()) {
-      SbString str = shader->sourceHint;
-      if (str.getLength() > 0) str += " ";
-      result += str;
-    }
+    soglshaderprogram_append_source_description(result, shader);
   }
   return result;
 }
-#endif
 
 void
 SoGLSLShaderProgram::ensureLinking(const cc_glglue * g)
@@ -168,6 +186,7 @@ SoGLSLShaderProgram::ensureLinking(const cc_glglue * g)
   if (cnt > 0) {
     int i;
     GLint didLink = 0;
+    const SbString sourceDescription = this->getSourceDescription();
 
     for (i = 0; i < cnt; i++) {
       this->shaderObjects[i]->attach(programHandle);
@@ -183,15 +202,47 @@ SoGLSLShaderProgram::ensureLinking(const cc_glglue * g)
     cc_glglue_glLinkProgram(g, (GLuint) programHandle);
     cc_glglue_glGetGLSLProgramiv(g, (GLuint) programHandle, GL_LINK_STATUS, &didLink);
 
-    if (SoGLSLShaderObject::didOpenGLErrorOccur("SoGLSLShaderProgram::ensureLinking")) {
-      SoGLSLShaderObject::printInfoLog(g, programHandle, 0);
+    if (SoGLSLShaderObject::didOpenGLErrorOccur("SoGLSLShaderProgram::ensureLinking")
+      || !didLink) {
+      printInfoLog(g, programHandle, sourceDescription, !didLink);
     }
 
     this->isExecutable = didLink;
     this->neededlinking = TRUE;
   }
 }
 
+void
+SoGLSLShaderProgram::printInfoLog(const cc_glglue * g,
+                                  COIN_GLhandle handle,
+                                  const SbString & sourceDescription,
+                                  const SbBool failed)
+{
+  const char * sourceName = sourceDescription.getLength() > 0 ?
+    sourceDescription.getString() : "<unnamed>";
+  const SbString infoLog = soglsl_get_info_log(g, (GLuint) handle, TRUE);
+
+  if (infoLog.getLength() > 0) {
+    if (failed) {
+      SoDebugError::postWarning("SoGLSLShaderProgram::printInfoLog",
+                                "program [%s] failed to link: %s",
+                                sourceName,
+                                infoLog.getString());
+    }
+    else {
+      SoDebugError::postInfo("SoGLSLShaderProgram::printInfoLog",
+                             "program [%s] log: %s",
+                             sourceName,
+                             infoLog.getString());
+    }
+  }
+  else if (failed) {
+    SoDebugError::postWarning("SoGLSLShaderProgram::printInfoLog",
+                              "program [%s] failed to link with no linker log",
+                              sourceName);
+  }
+}
+
 int
 SoGLSLShaderProgram::indexOfShaderObject(SoGLSLShaderObject *shaderObject)
 {

src/shaders/SoGLSLShaderProgram.h

@@ -39,6 +39,7 @@
 
 // *************************************************************************
 
+#include <Inventor/SbString.h>
 #include <Inventor/lists/SbList.h>
 
 #include "misc/SbHash.h"
@@ -63,11 +64,8 @@ class SoGLSLShaderProgram
   void addProgramParameter(int mode, int value);
   void removeProgramParameters(void);
 
-#if defined(SOURCE_HINT)
-  SbString getSourceHint(void) const;
-#endif
+  SbString getSourceDescription(void) const;
 
-public:
   SoGLSLShaderProgram(void);
   ~SoGLSLShaderProgram();
 
@@ -82,6 +80,10 @@ class SoGLSLShaderProgram
   SbBool isExecutable;
   SbBool neededlinking;
 
+  static void printInfoLog(const cc_glglue * g,
+                           COIN_GLhandle handle,
+                           const SbString & sourceDescription,
+                           const SbBool failed);
   int indexOfShaderObject(SoGLSLShaderObject * shaderObject);
   void ensureLinking(const cc_glglue * g);
   void ensureProgramHandle(const cc_glglue * g);

src/shaders/SoGLShaderObject.h

@@ -85,9 +85,7 @@ class SoGLShaderObject
   void setParametersDirty(SbBool flag);
   SbBool getParametersDirty(void) const;
 
-#if defined(SOURCE_HINT)
-  SbString sourceHint; // either the file name or the first line of source code
-#endif
+  SbString sourceHint; // diagnostic source name used in shader compile/link logs
 
 protected:
   const cc_glglue * glctx;