diff --git a/filament/backend/include/private/backend/Program.h b/filament/backend/include/private/backend/Program.h index 6f19badd51..568f9b5657 100644 --- a/filament/backend/include/private/backend/Program.h +++ b/filament/backend/include/private/backend/Program.h @@ -94,9 +94,8 @@ public: using ShaderBlob = utils::FixedCapacityVector; using ShaderSource = std::array; - ShaderSource const& getShadersSource() const noexcept { - return mShadersSource; - } + ShaderSource const& getShadersSource() const noexcept { return mShadersSource; } + ShaderSource& getShadersSource() noexcept { return mShadersSource; } UniformBlockInfo const& getUniformBlockInfo() const noexcept { return mUniformBlocks; } UniformBlockInfo& getUniformBlockInfo() noexcept { return mUniformBlocks; } diff --git a/filament/backend/src/opengl/OpenGLProgram.cpp b/filament/backend/src/opengl/OpenGLProgram.cpp index 1eb3f67588..5290b3223f 100644 --- a/filament/backend/src/opengl/OpenGLProgram.cpp +++ b/filament/backend/src/opengl/OpenGLProgram.cpp @@ -35,7 +35,7 @@ using namespace backend; static void logCompilationError(utils::io::ostream& out, backend::Program::Shader shaderType, const char* name, - GLuint shaderId) noexcept; + GLuint shaderId, CString sourceCode) noexcept; static void logProgramLinkError(utils::io::ostream& out, const char* name, GLuint program) noexcept; @@ -47,10 +47,12 @@ OpenGLProgram::OpenGLProgram(OpenGLContext& context, Program&& programBuilder) n auto samplerGroupInfo = std::move(programBuilder.getSamplerGroupInfo()); auto uniformBlockInfo = std::move(programBuilder.getUniformBlockInfo()); + auto shaderSource = std::move(programBuilder.getShadersSource()); + std::array shaderSourceCode; // this cannot fail because we check compilation status after linking the program // shaders[] is filled with id of shader stages present. - OpenGLProgram::compileShaders(context, programBuilder.getShadersSource(), gl.shaders); + OpenGLProgram::compileShaders(context, std::move(shaderSource), gl.shaders, shaderSourceCode); // link the program, this also cannot fail because status is checked later. // TODO: defer this until beginRenderPass() @@ -60,7 +62,7 @@ OpenGLProgram::OpenGLProgram(OpenGLContext& context, Program&& programBuilder) n // in case of error. // TODO: defer this until first use bool success = OpenGLProgram::checkProgramStatus(name.c_str_safe(), - gl.program, gl.shaders); + gl.program, gl.shaders, std::move(shaderSourceCode)); // Failing to compile a program can't be fatal, because this will happen a lot in // the material tools. We need to have a better way to handle these errors and @@ -98,8 +100,9 @@ OpenGLProgram::~OpenGLProgram() noexcept { * This always returns the GL shader IDs or zero a shader stage is not present. */ void OpenGLProgram::compileShaders(OpenGLContext& context, - Program::ShaderSource const& shadersSource, - GLuint shaderIds[Program::SHADER_TYPE_COUNT]) noexcept { + Program::ShaderSource shadersSource, + GLuint shaderIds[Program::SHADER_TYPE_COUNT], + std::array& outShaderSourceCode) noexcept { // build all shaders UTILS_NOUNROLL @@ -189,8 +192,10 @@ highp uint packHalf2x16(vec2 v) { const GLint length = (GLint)shaderView.length(); glShaderSource(shaderId, 1, &source, &length); glCompileShader(shaderId); +#ifndef NDEBUG + outShaderSourceCode[i] = { source, static_cast(length) }; +#endif } - shaderIds[i] = shaderId; } } @@ -217,7 +222,8 @@ GLuint OpenGLProgram::linkProgram(const GLuint shaderIds[Program::SHADER_TYPE_CO * Returns true on success. */ bool OpenGLProgram::checkProgramStatus(const char* name, - GLuint& program, GLuint shaderIds[backend::Program::SHADER_TYPE_COUNT]) noexcept { + GLuint& program, GLuint shaderIds[backend::Program::SHADER_TYPE_COUNT], + std::array&& shaderSourceCode) noexcept { GLint status; glGetProgramiv(program, GL_LINK_STATUS, &status); @@ -232,7 +238,7 @@ bool OpenGLProgram::checkProgramStatus(const char* name, const GLuint shader = shaderIds[i]; glGetShaderiv(shader, GL_COMPILE_STATUS, &status); if (status != GL_TRUE) { - logCompilationError(slog.e, type, name, shader); + logCompilationError(slog.e, type, name, shader, std::move(shaderSourceCode[i])); } glDetachShader(program, shader); glDeleteShader(shader); @@ -389,7 +395,7 @@ void OpenGLProgram::updateSamplers(OpenGLDriver* gld) noexcept { UTILS_NOINLINE void logCompilationError(io::ostream& out, Program::Shader shaderType, - const char* name, GLuint shaderId) noexcept { + const char* name, GLuint shaderId, CString sourceCode) noexcept { auto to_string = [](Program::Shader type) -> const char* { switch (type) { @@ -398,21 +404,50 @@ void logCompilationError(io::ostream& out, Program::Shader shaderType, } }; - char error[1024]; - glGetShaderInfoLog(shaderId, sizeof(error), nullptr, error); + { // scope for the temporary string storage + GLint length = 0; + glGetShaderiv(shaderId, GL_INFO_LOG_LENGTH, &length); - out << "Compilation error in " << to_string(shaderType) << " shader \"" << name << "\":\n" - << "\"" << error << "\"" - << io::endl; + CString infoLog(length); + glGetShaderInfoLog(shaderId, length, nullptr, infoLog.data()); + + out << "Compilation error in " << to_string(shaderType) << " shader \"" << name << "\":\n" + << "\"" << infoLog.c_str() << "\"" + << io::endl; + } + +#ifndef NDEBUG + std::string_view shader{ sourceCode.data(), sourceCode.size() }; + size_t lc = 1; + size_t start = 0; + std::string line; + while (true) { + size_t end = shader.find('\n', start); + if (end == std::string::npos) { + line = shader.substr(start); + } else { + line = shader.substr(start, end - start); + } + out << lc++ << ": " << line.c_str() << '\n'; + if (end == std::string::npos) { + break; + } + start = end + 1; + } + out << io::endl; +#endif } UTILS_NOINLINE void logProgramLinkError(io::ostream& out, char const* name, GLuint program) noexcept { - char error[1024]; - glGetProgramInfoLog(program, sizeof(error), nullptr, error); + GLint length = 0; + glGetProgramiv(program, GL_INFO_LOG_LENGTH, &length); + + CString infoLog(length); + glGetProgramInfoLog(program, length, nullptr, infoLog.data()); out << "Link error in \"" << name << "\":\n" - << "\"" << error << "\"" + << "\"" << infoLog.c_str() << "\"" << io::endl; } diff --git a/filament/backend/src/opengl/OpenGLProgram.h b/filament/backend/src/opengl/OpenGLProgram.h index 02360817f3..e67dcc4b48 100644 --- a/filament/backend/src/opengl/OpenGLProgram.h +++ b/filament/backend/src/opengl/OpenGLProgram.h @@ -72,13 +72,15 @@ private: static constexpr uint8_t FRAGMENT_SHADER_BIT = uint8_t(1) << size_t(backend::Program::Shader::FRAGMENT); static void compileShaders(OpenGLContext& context, - const backend::Program::ShaderSource& shadersSource, - GLuint shaderIds[backend::Program::SHADER_TYPE_COUNT]) noexcept; + backend::Program::ShaderSource shadersSource, + GLuint shaderIds[backend::Program::SHADER_TYPE_COUNT], + std::array& outShaderSourceCode) noexcept; static GLuint linkProgram(const GLuint shaderIds[backend::Program::SHADER_TYPE_COUNT]) noexcept; static bool checkProgramStatus(const char* name, - GLuint& program, GLuint shaderIds[backend::Program::SHADER_TYPE_COUNT]) noexcept; + GLuint& program, GLuint shaderIds[backend::Program::SHADER_TYPE_COUNT], + std::array&& shaderSourceCode) noexcept; void initializeProgramState(OpenGLContext& context, GLuint program, backend::Program::UniformBlockInfo const& uniformBlockInfo, diff --git a/libs/utils/include/utils/CString.h b/libs/utils/include/utils/CString.h index b76dfcb5e4..da809d11cf 100644 --- a/libs/utils/include/utils/CString.h +++ b/libs/utils/include/utils/CString.h @@ -206,6 +206,11 @@ public: // inside the string (i.e. it can contain nulls or non-ASCII encodings). CString(const char* cstr, size_t length); + // Allocates memory for a string of size length plus space for the null terminating character. + // Also initializes the memory to 0. This constructor can be used to hold arbitrary data + // inside the string. + explicit CString(size_t length); + // Allocates memory and copies traditional C string content. Unlike the above constructor, this // does not alllow embedded nulls. This is explicit because this operation is costly. explicit CString(const char* cstr); diff --git a/libs/utils/src/CString.cpp b/libs/utils/src/CString.cpp index e108545e31..a33bd14e77 100644 --- a/libs/utils/src/CString.cpp +++ b/libs/utils/src/CString.cpp @@ -45,6 +45,16 @@ CString::CString(const char* cstr, size_t length) { } } +CString::CString(size_t length) { + if (length) { + Data* p = (Data*)malloc(sizeof(Data) + length + 1); + p->length = (size_type)length; + mCStr = (value_type*)(p + 1); + std::fill_n(mCStr, length, 0); + mCStr[length] = '\0'; + } +} + CString::CString(const char* cstr) : CString(cstr, size_type(cstr ? strlen(cstr) : 0)) { }