diff --git a/src/shaders/SoGLSLShaderDiagnostics.h b/src/shaders/SoGLSLShaderDiagnostics.h new file mode 100644 index 00000000000..bc244908ea6 --- /dev/null +++ b/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 + +#include "shaders/SoGLShaderObject.h" +#include "glue/glp.h" +#include "glue/glslp.h" + +#include + +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 infoLog(static_cast(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(charsWritten)] = '\0'; + } + else { + infoLog.back() = '\0'; + } + return SbString(infoLog.data()); +} + +#endif /* ! COIN_SOGLSLSHADERDIAGNOSTICS_H */ diff --git a/src/shaders/SoGLSLShaderObject.cpp b/src/shaders/SoGLSLShaderObject.cpp index e9ac6522fb7..5f24998688a 100644 --- a/src/shaders/SoGLSLShaderObject.cpp +++ b/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() : ""; + 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); } } diff --git a/src/shaders/SoGLSLShaderObject.h b/src/shaders/SoGLSLShaderObject.h index 1eb6ccb3ebd..7c25a14de3f 100644 --- a/src/shaders/SoGLSLShaderObject.h +++ b/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; diff --git a/src/shaders/SoGLSLShaderProgram.cpp b/src/shaders/SoGLSLShaderProgram.cpp index eaab8bf13d7..3898f919470 100644 --- a/src/shaders/SoGLSLShaderProgram.cpp +++ b/src/shaders/SoGLSLShaderProgram.cpp @@ -36,11 +36,34 @@ #include #include "shaders/SoGLSLShaderObject.h" +#include "shaders/SoGLSLShaderDiagnostics.h" +#include #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; ishaderObjects.size(); i++) { + for (int i=0; ishaderObjects.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,8 +202,9 @@ 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; @@ -192,6 +212,37 @@ SoGLSLShaderProgram::ensureLinking(const cc_glglue * g) } } +void +SoGLSLShaderProgram::printInfoLog(const cc_glglue * g, + COIN_GLhandle handle, + const SbString & sourceDescription, + const SbBool failed) +{ + const char * sourceName = sourceDescription.getLength() > 0 ? + sourceDescription.getString() : ""; + 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) { diff --git a/src/shaders/SoGLSLShaderProgram.h b/src/shaders/SoGLSLShaderProgram.h index 144362c82d0..7f170aeacfa 100644 --- a/src/shaders/SoGLSLShaderProgram.h +++ b/src/shaders/SoGLSLShaderProgram.h @@ -39,6 +39,7 @@ // ************************************************************************* +#include #include #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); diff --git a/src/shaders/SoGLShaderObject.h b/src/shaders/SoGLShaderObject.h index 266ada66af2..cbeaf0d2a1e 100644 --- a/src/shaders/SoGLShaderObject.h +++ b/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; diff --git a/src/shaders/SoShaderObject.cpp b/src/shaders/SoShaderObject.cpp index e2e91363d36..18008c3b641 100644 --- a/src/shaders/SoShaderObject.cpp +++ b/src/shaders/SoShaderObject.cpp @@ -115,6 +115,7 @@ #include #include #include +#include #include #include #include @@ -130,6 +131,41 @@ #include "shaders/SoGLSLShaderObject.h" #include "shaders/SoGLShaderProgram.h" +#include + +// ************************************************************************* + +static SbString +soshaderobject_inline_source_preview(const SbString & source) +{ + const char * cursor = source.getString(); + if (cursor == NULL) return SbString(""); + + while (*cursor != '\0') { + const char * text = cursor; + while (*text == ' ' || *text == '\t') ++text; + + const char * end = text; + while (*end != '\0' && *end != '\n' && *end != '\r') ++end; + while (end > text && (end[-1] == ' ' || end[-1] == '\t')) --end; + + // #version is useful to the compiler but not as a source identity. + if (end != text && std::strncmp(text, "#version", 8) != 0) { + SbString preview(text, 0, int(end - text) - 1); + if (preview.getLength() > 80) { + preview = preview.getSubString(0, 79); + preview += "..."; + } + return preview; + } + + cursor = end; + while (*cursor == '\n' || *cursor == '\r') ++cursor; + } + + return SbString(""); +} + // ************************************************************************* class SoShaderObjectP @@ -198,6 +234,7 @@ class SoShaderObjectP SoShaderObject * owner; SoShaderObject::SourceType cachedSourceType; + SbString resolvedSourceName; SbString cachedSourceProgram; SbBool didSetSearchDirectories; SbBool shouldload; @@ -221,9 +258,7 @@ class SoShaderObjectP SbBool isSupported(SoShaderObject::SourceType sourceType, const cc_glglue * glue); -#if defined(SOURCE_HINT) SbString getSourceHint(void) const; -#endif }; #define PRIVATE(obj) ((obj)->pimpl) @@ -466,9 +501,7 @@ SoShaderObjectP::render(SoState * state) } -#if defined(SOURCE_HINT) shaderobject->sourceHint = getSourceHint(); -#endif shaderobject->load(this->cachedSourceProgram.getString()); this->setGLShaderObject(shaderobject, cachecontext); } @@ -540,6 +573,7 @@ SoShaderObjectP::readSource(void) SoShaderObject::SourceType srcType = (SoShaderObject::SourceType)this->owner->sourceType.getValue(); + this->resolvedSourceName.makeEmpty(); this->cachedSourceProgram.makeEmpty(); if (this->owner->sourceProgram.isDefault()) @@ -577,6 +611,7 @@ SoShaderObjectP::readSource(void) size_t readlen = fread(srcstr, 1, length, f); if (readlen == (size_t) length) { srcstr[length] = '\0'; + this->resolvedSourceName = fileName; this->cachedSourceProgram = srcstr; readok = TRUE; } @@ -772,19 +807,19 @@ SoShaderObjectP::containStateMatrixParameters(void) const return FALSE; } -#if defined(SOURCE_HINT) SbString SoShaderObjectP::getSourceHint(void) const { - SoShaderObject::SourceType srcType = - (SoShaderObject::SourceType)this->owner->sourceType.getValue(); + if (this->resolvedSourceName.getLength() > 0) { + return this->resolvedSourceName; + } - if (srcType == SoShaderObject::FILENAME) + if (this->cachedSourceType == SoShaderObject::FILENAME) { return this->owner->sourceProgram.getValue(); - else - return ""; // FIXME: should return first line of shader source code + } + + return soshaderobject_inline_source_preview(this->cachedSourceProgram); } -#endif void SoShaderObjectP::sensorCB(void *data, SoSensor *sensor) diff --git a/testsuite/CMakeLists.txt b/testsuite/CMakeLists.txt index 1a953262616..fd05aa6cf2c 100644 --- a/testsuite/CMakeLists.txt +++ b/testsuite/CMakeLists.txt @@ -145,6 +145,7 @@ endif() if(COIN_BUILD_GL_TESTS_EFFECTIVE) coin_add_gl_test(GLSLRuntimeTest GLSLRuntimeTest.cpp) + coin_add_gl_test(GLSLDiagnosticsTest GLSLDiagnosticsTest.cpp) endif() # Many warnings are generated from test macros on macOS with Xcode. diff --git a/testsuite/GLSLDiagnosticsTest.cpp b/testsuite/GLSLDiagnosticsTest.cpp new file mode 100644 index 00000000000..b1f1892d7e1 --- /dev/null +++ b/testsuite/GLSLDiagnosticsTest.cpp @@ -0,0 +1,157 @@ +#include +#include + +#include "glue/glp.h" +#include "shaders/SoGLSLShaderObject.h" +#include "shaders/SoGLSLShaderProgram.h" +#include "support/GLTestContext.h" + +#include +#include +#include + +namespace { + +int skip(const char * reason) +{ + std::cout << "SKIP: " << reason << std::endl; + return 77; +} + +bool check(bool condition, const char * message) +{ + if (!condition) std::cerr << "FAIL: " << message << std::endl; + return condition; +} + +struct DiagnosticCapture { + int warnings = 0; + int infos = 0; + std::vector messages; +}; + +void capture_diagnostic(const SoError * error, void * data) +{ + DiagnosticCapture * capture = static_cast(data); + if (!error->isOfType(SoDebugError::getClassTypeId())) return; + + const SoDebugError * debug = static_cast(error); + if (debug->getSeverity() == SoDebugError::WARNING) ++capture->warnings; + if (debug->getSeverity() == SoDebugError::INFO) ++capture->infos; + capture->messages.push_back(debug->getDebugString().getString()); +} + +std::string messages_as_text(const DiagnosticCapture & capture) +{ + std::string result; + for (std::vector::const_iterator it = capture.messages.begin(); + it != capture.messages.end(); ++it) { + if (!result.empty()) result += "\n"; + result += *it; + } + return result; +} + +bool contains(const DiagnosticCapture & capture, const char * text) +{ + return messages_as_text(capture).find(text) != std::string::npos; +} + +} // namespace + +int main() +{ + SoDB::init(); + + GLTestContextConfig config; + config.profile = GLTestProfile::Core; + config.major = 3; + config.minor = 3; + config.width = 16; + config.height = 16; + GLTestContext context; + if (!context.initialize(config)) { + SoDB::finish(); + return skip("core OpenGL test context is unavailable"); + } + if (!context.makeCurrent()) { + SoDB::finish(); + return skip("core OpenGL test context could not be made current"); + } + + const cc_glglue * glue = cc_glglue_instance(context.contextId()); + if (glue == NULL || glue->contextid == 0) { + SoDB::finish(); + return skip("GL glue instance is unavailable"); + } + + SoErrorCB * previousCallback = SoDebugError::getHandlerCallback(); + void * previousData = SoDebugError::getHandlerData(); + int result = 1; + + do { + DiagnosticCapture compileCapture; + SoDebugError::setHandlerCallback(capture_diagnostic, &compileCapture); + + SoGLSLShaderObject brokenShader(glue->contextid); + brokenShader.setShaderType(SoGLShaderObject::VERTEX); + brokenShader.sourceHint = "broken.vert"; + brokenShader.load("#version 330 core\n" + "void main() { this is not valid GLSL; }\n"); + + if (!check(!brokenShader.isLoaded(), + "invalid shader unexpectedly compiled")) break; + if (!check(compileCapture.warnings > 0, + "shader compilation failure was not reported as a warning")) break; + if (!check(contains(compileCapture, "vertex shader"), + "shader diagnostic omitted its stage")) break; + if (!check(contains(compileCapture, "broken.vert"), + "shader diagnostic omitted its source identity")) break; + if (!check(contains(compileCapture, "failed to compile"), + "shader diagnostic omitted its failure description")) break; + + DiagnosticCapture linkCapture; + SoDebugError::setHandlerCallback(capture_diagnostic, &linkCapture); + + SoGLSLShaderObject vertexShader(glue->contextid); + vertexShader.setShaderType(SoGLShaderObject::VERTEX); + vertexShader.sourceHint = "link.vert"; + vertexShader.load("#version 330 core\n" + "out vec3 varying_color;\n" + "void main() { varying_color = vec3(1.0); " + "gl_Position = vec4(0.0); }\n"); + + SoGLSLShaderObject fragmentShader(glue->contextid); + fragmentShader.setShaderType(SoGLShaderObject::FRAGMENT); + fragmentShader.sourceHint = "link.frag"; + fragmentShader.load("#version 330 core\n" + "in vec4 varying_color;\n" + "out vec4 color;\n" + "void main() { color = varying_color; }\n"); + + if (!check(vertexShader.isLoaded() && fragmentShader.isLoaded(), + "link test shaders failed to compile")) break; + + { + SoGLSLShaderProgram program; + program.addShaderObject(&vertexShader); + program.addShaderObject(&fragmentShader); + program.enable(glue); + } + + if (!check(linkCapture.warnings > 0, + "program link failure was not reported as a warning")) break; + if (!check(contains(linkCapture, "failed to link"), + "program diagnostic omitted its failure description")) break; + if (!check(contains(linkCapture, "vertex shader=link.vert"), + "program diagnostic omitted the vertex source stage")) break; + if (!check(contains(linkCapture, "fragment shader=link.frag"), + "program diagnostic omitted the fragment source stage")) break; + + result = 0; + } while (false); + + SoDebugError::setHandlerCallback(previousCallback, previousData); + SoDB::finish(); + return result; +}