From 2dcedf9f3901dac39067d6bf6aedad9b4ead28df Mon Sep 17 00:00:00 2001 From: samcake Date: Tue, 30 Jan 2018 12:52:05 -0800 Subject: [PATCH 01/13] cherry picking Tony's fix for shader compilation time not taking soo long and adding better feedback from shader compilation --- interface/src/Application.cpp | 23 ++- libraries/gl/src/gl/GLShaders.cpp | 161 +++++++++++------- libraries/gl/src/gl/GLShaders.h | 6 +- libraries/gpu-gl/src/gpu/gl/GLBackend.h | 4 +- .../gpu-gl/src/gpu/gl/GLBackendShader.cpp | 44 +++-- libraries/gpu-gl/src/gpu/gl/GLShader.cpp | 8 +- libraries/gpu-gl/src/gpu/gl/GLShader.h | 4 +- libraries/gpu-gles/src/gpu/gl/GLBackend.h | 4 +- .../gpu-gles/src/gpu/gl/GLBackendShader.cpp | 65 ++++++- libraries/gpu-gles/src/gpu/gl/GLShader.cpp | 8 +- libraries/gpu-gles/src/gpu/gl/GLShader.h | 4 +- libraries/gpu/src/gpu/Context.cpp | 4 +- libraries/gpu/src/gpu/Context.h | 4 +- libraries/gpu/src/gpu/Shader.cpp | 11 +- libraries/gpu/src/gpu/Shader.h | 29 +++- libraries/render/src/render/ShapePipeline.cpp | 52 +++--- 16 files changed, 297 insertions(+), 134 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 64e9feea02..f1d0378792 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -396,6 +396,7 @@ public: setObjectName("Deadlock Watchdog"); // Give the heartbeat an initial value _heartbeat = usecTimestampNow(); + _paused = false; connect(qApp, &QCoreApplication::aboutToQuit, [this] { _quit = true; }); @@ -413,11 +414,20 @@ public: *crashTrigger = 0xDEAD10CC; } + static void pause() { + _paused = true; + } + + static void resume() { + _paused = false; + updateHeartbeat(); + } + void run() override { while (!_quit) { QThread::sleep(HEARTBEAT_UPDATE_INTERVAL_SECS); // Don't do heartbeat detection under nsight - if (nsightActive()) { + if (nsightActive() || _paused) { continue; } uint64_t lastHeartbeat = _heartbeat; // sample atomic _heartbeat, because we could context switch away and have it updated on us @@ -473,6 +483,7 @@ public: } } + static std::atomic _paused; static std::atomic _heartbeat; static std::atomic _maxElapsed; static std::atomic _maxElapsedAverage; @@ -481,6 +492,7 @@ public: bool _quit { false }; }; +std::atomic DeadlockWatchdogThread::_paused; std::atomic DeadlockWatchdogThread::_heartbeat; std::atomic DeadlockWatchdogThread::_maxElapsed; std::atomic DeadlockWatchdogThread::_maxElapsedAverage; @@ -2269,6 +2281,11 @@ void Application::initializeGL() { initDisplay(); qCDebug(interfaceapp, "Initialized Display."); +#ifdef Q_OS_OSX + // FIXME: on mac os the shaders take up to 1 minute to compile, so we pause the deadlock watchdog thread. + +DeadlockWatchdogThread::pause(); +#endif + // Set up the render engine render::CullFunctor cullFunctor = LODManager::shouldRender; static const QString RENDER_FORWARD = "HIFI_RENDER_FORWARD"; @@ -2283,6 +2300,10 @@ void Application::initializeGL() { // Now that OpenGL is initialized, we are sure we have a valid context and can create the various pipeline shaders with success. DependencyManager::get()->initializeShapePipelines(); +#ifdef Q_OS_OSX + DeadlockWatchdogThread::resume(); +#endif + _offscreenContext = new OffscreenGLCanvas(); _offscreenContext->setObjectName("MainThreadContext"); _offscreenContext->create(_glWidget->qglContext()); diff --git a/libraries/gl/src/gl/GLShaders.cpp b/libraries/gl/src/gl/GLShaders.cpp index 017c92b71c..ecd6fe3323 100644 --- a/libraries/gl/src/gl/GLShaders.cpp +++ b/libraries/gl/src/gl/GLShaders.cpp @@ -6,9 +6,9 @@ namespace gl { #ifdef SEPARATE_PROGRAM - bool compileShader(GLenum shaderDomain, const std::string& shaderSource, const std::string& defines, GLuint &shaderObject, GLuint &programObject, std::string& error) { + bool compileShader(GLenum shaderDomain, const std::string& shaderSource, const std::string& defines, GLuint &shaderObject, GLuint &programObject, std::string& message) { #else - bool compileShader(GLenum shaderDomain, const std::string& shaderSource, const std::string& defines, GLuint &shaderObject, std::string& error) { + bool compileShader(GLenum shaderDomain, const std::string& shaderSource, const std::string& defines, GLuint &shaderObject, std::string& message) { #endif if (shaderSource.empty()) { qCDebug(glLogging) << "GLShader::compileShader - no GLSL shader source code ? so failed to create"; @@ -34,52 +34,57 @@ namespace gl { GLint compiled = 0; glGetShaderiv(glshader, GL_COMPILE_STATUS, &compiled); - // if compilation fails - if (!compiled) { - - // save the source code to a temp file so we can debug easily - /* - std::ofstream filestream; - filestream.open("debugshader.glsl"); - if (filestream.is_open()) { - filestream << srcstr[0]; - filestream << srcstr[1]; - filestream.close(); - } - */ - - GLint infoLength = 0; - glGetShaderiv(glshader, GL_INFO_LOG_LENGTH, &infoLength); + GLint infoLength = 0; + glGetShaderiv(glshader, GL_INFO_LOG_LENGTH, &infoLength); + if ((infoLength > 0) || !compiled) { char* temp = new char[infoLength]; glGetShaderInfoLog(glshader, infoLength, NULL, temp); + message = std::string(temp); - /* - filestream.open("debugshader.glsl.info.txt"); - if (filestream.is_open()) { - filestream << std::string(temp); - filestream.close(); - } - */ - - qCCritical(glLogging) << "GLShader::compileShader - failed to compile the gl shader object:"; - int lineNumber = 0; - for (auto s : srcstr) { - QString str(s); - QStringList lines = str.split("\n"); - for (auto& line : lines) { - qCCritical(glLogging).noquote() << QString("%1: %2").arg(lineNumber++, 5, 10, QChar('0')).arg(line); + // if compilation fails + if (!compiled) { + // save the source code to a temp file so we can debug easily + /* + std::ofstream filestream; + filestream.open("debugshader.glsl"); + if (filestream.is_open()) { + filestream << srcstr[0]; + filestream << srcstr[1]; + filestream.close(); } + */ + + /* + filestream.open("debugshader.glsl.info.txt"); + if (filestream.is_open()) { + filestream << std::string(temp); + filestream.close(); + } + */ + + qCCritical(glLogging) << "GLShader::compileShader - failed to compile the gl shader object:"; + int lineNumber = 0; + for (auto s : srcstr) { + QString str(s); + QStringList lines = str.split("\n"); + for (auto& line : lines) { + qCCritical(glLogging).noquote() << QString("%1: %2").arg(lineNumber++, 5, 10, QChar('0')).arg(line); + } + } + qCCritical(glLogging) << "GLShader::compileShader - errors:"; + qCCritical(glLogging) << temp; + + delete[] temp; + glDeleteShader(glshader); + return false; } - qCCritical(glLogging) << "GLShader::compileShader - errors:"; - qCCritical(glLogging) << temp; - error = std::string(temp); + // Compilation success + qCWarning(glLogging) << "GLShader::compileShader - Success:"; + qCWarning(glLogging) << temp; delete[] temp; - - glDeleteShader(glshader); - return false; } #ifdef SEPARATE_PROGRAM @@ -137,7 +142,7 @@ namespace gl { return true; } -GLuint compileProgram(const std::vector& glshaders, std::string& error) { +GLuint compileProgram(const std::vector& glshaders, std::string& message, std::vector& binary) { // A brand new program: GLuint glprogram = glCreateProgram(); if (!glprogram) { @@ -157,39 +162,65 @@ GLuint compileProgram(const std::vector& glshaders, std::string& error) GLint linked = 0; glGetProgramiv(glprogram, GL_LINK_STATUS, &linked); - if (!linked) { - /* - // save the source code to a temp file so we can debug easily - std::ofstream filestream; - filestream.open("debugshader.glsl"); - if (filestream.is_open()) { - filestream << shaderSource->source; - filestream.close(); - } - */ - - GLint infoLength = 0; - glGetProgramiv(glprogram, GL_INFO_LOG_LENGTH, &infoLength); + GLint infoLength = 0; + glGetProgramiv(glprogram, GL_INFO_LOG_LENGTH, &infoLength); + if ((infoLength > 0) || !linked) { char* temp = new char[infoLength]; glGetProgramInfoLog(glprogram, infoLength, NULL, temp); - qCDebug(glLogging) << "GLShader::compileProgram - failed to LINK the gl program object :"; - qCDebug(glLogging) << temp; + message = std::string(temp); - error = std::string(temp); - delete[] temp; + if (!linked) { + /* + // save the source code to a temp file so we can debug easily + std::ofstream filestream; + filestream.open("debugshader.glsl"); + if (filestream.is_open()) { + filestream << shaderSource->source; + filestream.close(); + } + */ - /* - filestream.open("debugshader.glsl.info.txt"); - if (filestream.is_open()) { - filestream << std::string(temp); - filestream.close(); + qCDebug(glLogging) << "GLShader::compileProgram - failed to LINK the gl program object :"; + qCDebug(glLogging) << temp; + + delete[] temp; + + /* + filestream.open("debugshader.glsl.info.txt"); + if (filestream.is_open()) { + filestream << std::string(temp); + filestream.close(); + } + */ + + glDeleteProgram(glprogram); + return 0; + } else { + qCDebug(glLogging) << "GLShader::compileProgram - success:"; + qCDebug(glLogging) << temp; + delete[] temp; } - */ + } - glDeleteProgram(glprogram); - return 0; + // If linked get the binaries + if (linked) { + GLint binaryLength = 0; + glGetProgramiv(glprogram, GL_PROGRAM_BINARY_LENGTH, &binaryLength); + + if (binaryLength > 0) { + GLint numBinFormats = 0; + glGetIntegerv(GL_NUM_PROGRAM_BINARY_FORMATS, &numBinFormats); + if (numBinFormats > 0) { + binary.resize(binaryLength); + std::vector binFormats(numBinFormats); + glGetIntegerv(GL_NUM_PROGRAM_BINARY_FORMATS, binFormats.data()); + + GLenum programBinFormat; + glGetProgramBinary(glprogram, binaryLength, NULL, &programBinFormat, binary.data()); + } + } } return glprogram; diff --git a/libraries/gl/src/gl/GLShaders.h b/libraries/gl/src/gl/GLShaders.h index a6213fd280..c5262b6b61 100644 --- a/libraries/gl/src/gl/GLShaders.h +++ b/libraries/gl/src/gl/GLShaders.h @@ -17,12 +17,12 @@ namespace gl { #ifdef SEPARATE_PROGRAM - bool compileShader(GLenum shaderDomain, const std::string& shaderSource, const std::string& defines, GLuint &shaderObject, GLuint &programObject, std::string& error); + bool compileShader(GLenum shaderDomain, const std::string& shaderSource, const std::string& defines, GLuint &shaderObject, GLuint &programObject, std::string& message); #else - bool compileShader(GLenum shaderDomain, const std::string& shaderSource, const std::string& defines, GLuint &shaderObject, std::string& error); + bool compileShader(GLenum shaderDomain, const std::string& shaderSource, const std::string& defines, GLuint &shaderObject, std::string& message); #endif - GLuint compileProgram(const std::vector& glshaders, std::string& error); + GLuint compileProgram(const std::vector& glshaders, std::string& message, , std::vector& binary); } diff --git a/libraries/gpu-gl/src/gpu/gl/GLBackend.h b/libraries/gpu-gl/src/gpu/gl/GLBackend.h index 5558d3ada1..004bfe1c85 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLBackend.h +++ b/libraries/gpu-gl/src/gpu/gl/GLBackend.h @@ -64,7 +64,7 @@ protected: explicit GLBackend(bool syncCache); GLBackend(); public: - static bool makeProgram(Shader& shader, const Shader::BindingSet& slotBindings = Shader::BindingSet()); + static bool makeProgram(Shader& shader, const Shader::BindingSet& slotBindings = Shader::BindingSet(), Shader::CompilationHandler handler = nullptr); virtual ~GLBackend(); @@ -424,7 +424,7 @@ protected: // Backend dependant compilation of the shader virtual GLShader* compileBackendProgram(const Shader& program); - virtual GLShader* compileBackendShader(const Shader& shader); + virtual GLShader* compileBackendShader(const Shader& shader, Shader::CompilationHandler handler); virtual std::string getBackendShaderHeader() const; virtual void makeProgramBindings(ShaderObject& shaderObject); class ElementResource { diff --git a/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp b/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp index 9adfd550ef..84170dbffd 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp +++ b/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp @@ -56,28 +56,43 @@ static const std::array VERSION_DEFINES { { stereoVersion } }; -GLShader* GLBackend::compileBackendShader(const Shader& shader) { +GLShader* GLBackend::compileBackendShader(const Shader& shader, Shader::CompilationHandler handler) { // Any GLSLprogram ? normally yes... const std::string& shaderSource = shader.getSource().getCode(); GLenum shaderDomain = SHADER_DOMAINS[shader.getType()]; GLShader::ShaderObjects shaderObjects; + Shader::CompilationLogs compilationLogs(GLShader::NumVersions); for (int version = 0; version < GLShader::NumVersions; version++) { auto& shaderObject = shaderObjects[version]; std::string shaderDefines = getBackendShaderHeader() + "\n" + DOMAIN_DEFINES[shader.getType()] + "\n" + VERSION_DEFINES[version]; - std::string error; + if (handler) { + bool retest = true; + std::string currentSrc = shaderSource; + while (retest) { + bool result = ::gl::compileShader(shaderDomain, currentSrc, shaderDefines, shaderObject.glshader, compilationLogs[version].message); + compilationLogs[version].compiled = result; + if (!result) { + std::string newSrc; + retest = handler(shader, currentSrc, compilationLogs[version], newSrc); + currentSrc = newSrc; + } else { + retest = false; + } + } + } else { + compilationLogs[version].compiled = ::gl::compileShader(shaderDomain, shaderSource, shaderDefines, shaderObject.glshader, compilationLogs[version].message); + } -#ifdef SEPARATE_PROGRAM - bool result = ::gl::compileShader(shaderDomain, shaderSource, shaderDefines, shaderObject.glshader, shaderObject.glprogram, error); -#else - bool result = ::gl::compileShader(shaderDomain, shaderSource, shaderDefines, shaderObject.glshader, error); -#endif - if (!result) { - qCWarning(gpugllogging) << "GLBackend::compileBackendProgram - Shader didn't compile:\n" << error.c_str(); + if (!compilationLogs[version].compiled) { + qCWarning(gpugllogging) << "GLBackend::compileBackendProgram - Shader didn't compile:\n" << compilationLogs[version].message.c_str(); + shader.setCompilationLogs(compilationLogs); return nullptr; } } + // Compilation feedback + shader.setCompilationLogs(compilationLogs); // So far so good, the shader is created successfully GLShader* object = new GLShader(this->shared_from_this()); @@ -93,6 +108,8 @@ GLShader* GLBackend::compileBackendProgram(const Shader& program) { GLShader::ShaderObjects programObjects; + Shader::CompilationLogs compilationLogs(GLShader::NumVersions); + for (int version = 0; version < GLShader::NumVersions; version++) { auto& programObject = programObjects[version]; @@ -104,14 +121,15 @@ GLShader* GLBackend::compileBackendProgram(const Shader& program) { shaderGLObjects.push_back(object->_shaderObjects[version].glshader); } else { qCWarning(gpugllogging) << "GLBackend::compileBackendProgram - One of the shaders of the program is not compiled?"; + program.setCompilationLogs(compilationLogs); return nullptr; } } - std::string error; - GLuint glprogram = ::gl::compileProgram(shaderGLObjects, error); + GLuint glprogram = ::gl::compileProgram(shaderGLObjects, compilationLogs[version].message, compilationLogs[version].binary); if (glprogram == 0) { - qCWarning(gpugllogging) << "GLBackend::compileBackendProgram - Program didn't link:\n" << error.c_str(); + qCWarning(gpugllogging) << "GLBackend::compileBackendProgram - Program didn't link:\n" << compilationLogs[version].message.c_str(); + program.setCompilationLogs(compilationLogs); return nullptr; } @@ -119,6 +137,8 @@ GLShader* GLBackend::compileBackendProgram(const Shader& program) { makeProgramBindings(programObject); } + // Compilation feedback + program.setCompilationLogs(compilationLogs); // So far so good, the program versions have all been created successfully GLShader* object = new GLShader(this->shared_from_this()); diff --git a/libraries/gpu-gl/src/gpu/gl/GLShader.cpp b/libraries/gpu-gl/src/gpu/gl/GLShader.cpp index 7ed9121978..a626376e27 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLShader.cpp +++ b/libraries/gpu-gl/src/gpu/gl/GLShader.cpp @@ -30,7 +30,7 @@ GLShader::~GLShader() { } } -GLShader* GLShader::sync(GLBackend& backend, const Shader& shader) { +GLShader* GLShader::sync(GLBackend& backend, const Shader& shader, Shader::CompilationHandler handler) { GLShader* object = Backend::getGPUObject(shader); // If GPU object already created then good @@ -45,7 +45,7 @@ GLShader* GLShader::sync(GLBackend& backend, const Shader& shader) { Backend::setGPUObject(shader, object); } } else if (shader.isDomain()) { - GLShader* tempObject = backend.compileBackendShader(shader); + GLShader* tempObject = backend.compileBackendShader(shader, handler); if (tempObject) { object = tempObject; Backend::setGPUObject(shader, object); @@ -56,10 +56,10 @@ GLShader* GLShader::sync(GLBackend& backend, const Shader& shader) { return object; } -bool GLShader::makeProgram(GLBackend& backend, Shader& shader, const Shader::BindingSet& slotBindings) { +bool GLShader::makeProgram(GLBackend& backend, Shader& shader, const Shader::BindingSet& slotBindings, Shader::CompilationHandler handler) { // First make sure the Shader has been compiled - GLShader* object = sync(backend, shader); + GLShader* object = sync(backend, shader, handler); if (!object) { return false; } diff --git a/libraries/gpu-gl/src/gpu/gl/GLShader.h b/libraries/gpu-gl/src/gpu/gl/GLShader.h index dcf2dc330d..42d63f8dfb 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLShader.h +++ b/libraries/gpu-gl/src/gpu/gl/GLShader.h @@ -21,8 +21,8 @@ struct ShaderObject { class GLShader : public GPUObject { public: - static GLShader* sync(GLBackend& backend, const Shader& shader); - static bool makeProgram(GLBackend& backend, Shader& shader, const Shader::BindingSet& slotBindings); + static GLShader* sync(GLBackend& backend, const Shader& shader, Shader::CompilationHandler handler = nullptr); + static bool makeProgram(GLBackend& backend, Shader& shader, const Shader::BindingSet& slotBindings, Shader::CompilationHandler handler = nullptr); enum Version { Mono = 0, diff --git a/libraries/gpu-gles/src/gpu/gl/GLBackend.h b/libraries/gpu-gles/src/gpu/gl/GLBackend.h index ea06b6b672..bb8e15bad3 100644 --- a/libraries/gpu-gles/src/gpu/gl/GLBackend.h +++ b/libraries/gpu-gles/src/gpu/gl/GLBackend.h @@ -61,7 +61,7 @@ protected: explicit GLBackend(bool syncCache); GLBackend(); public: - static bool makeProgram(Shader& shader, const Shader::BindingSet& slotBindings = Shader::BindingSet()); + static bool makeProgram(Shader& shader, const Shader::BindingSet& slotBindings = Shader::BindingSet(), Shader::CompilationHandler handler = nullptr); virtual ~GLBackend(); @@ -421,7 +421,7 @@ protected: // Backend dependant compilation of the shader virtual GLShader* compileBackendProgram(const Shader& program); - virtual GLShader* compileBackendShader(const Shader& shader); + virtual GLShader* compileBackendShader(const Shader& shader, Shader::CompilationHandler handler); virtual std::string getBackendShaderHeader() const; virtual void makeProgramBindings(ShaderObject& shaderObject); class ElementResource { diff --git a/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp b/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp index fd44ad462f..26dbb12cf7 100644 --- a/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp +++ b/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp @@ -56,11 +56,12 @@ static const std::array VERSION_DEFINES { { stereoVersion } }; -GLShader* GLBackend::compileBackendShader(const Shader& shader) { +GLShader* GLBackend::compileBackendShader(const Shader& shader, Shader::CompilationHandler handler) { // Any GLSLprogram ? normally yes... const std::string& shaderSource = shader.getSource().getCode(); GLenum shaderDomain = SHADER_DOMAINS[shader.getType()]; GLShader::ShaderObjects shaderObjects; + Shader::CompilationLogs compilationLogs(GLShader::NumVersions); for (int version = 0; version < GLShader::NumVersions; version++) { auto& shaderObject = shaderObjects[version]; @@ -90,6 +91,55 @@ GLShader* GLBackend::compileBackendShader(const Shader& shader) { return object; } +GLShader* GLBackend::compileBackendShader(const Shader& shader, Shader::CompilationHandler handler) { + // Any GLSLprogram ? normally yes... + const std::string& shaderSource = shader.getSource().getCode(); + GLenum shaderDomain = SHADER_DOMAINS[shader.getType()]; + GLShader::ShaderObjects shaderObjects; + Shader::CompilationLogs compilationLogs(GLShader::NumVersions); + + for (int version = 0; version < GLShader::NumVersions; version++) { + auto& shaderObject = shaderObjects[version]; + + std::string shaderDefines = getBackendShaderHeader() + "\n" + DOMAIN_DEFINES[shader.getType()] + "\n" + VERSION_DEFINES[version] + + "\n#extension GL_EXT_texture_buffer : enable" + + "\nprecision lowp float; // check precision 2" + + "\nprecision lowp samplerBuffer;" + + "\nprecision lowp sampler2DShadow;"; + if (handler) { + bool retest = true; + std::string currentSrc = shaderSource; + while (retest) { + bool result = ::gl::compileShader(shaderDomain, currentSrc, shaderDefines, shaderObject.glshader, compilationLogs[version].message); + compilationLogs[version].compiled = result; + if (!result) { + std::string newSrc; + retest = handler(shader, currentSrc, compilationLogs[version], newSrc); + currentSrc = newSrc; + } else { + retest = false; + } + } + } else { + compilationLogs[version].compiled = ::gl::compileShader(shaderDomain, shaderSource, shaderDefines, shaderObject.glshader, compilationLogs[version].message); + } + + if (!compilationLogs[version].compiled) { + qCWarning(gpugllogging) << "GLBackend::compileBackendProgram - Shader didn't compile:\n" << compilationLogs[version].message.c_str(); + shader.setCompilationLogs(compilationLogs); + return nullptr; + } + } + // Compilation feedback + shader.setCompilationLogs(compilationLogs); + + // So far so good, the shader is created successfully + GLShader* object = new GLShader(this->shared_from_this()); + object->_shaderObjects = shaderObjects; + + return object; +} + GLShader* GLBackend::compileBackendProgram(const Shader& program) { if (!program.isProgram()) { return nullptr; @@ -97,25 +147,28 @@ GLShader* GLBackend::compileBackendProgram(const Shader& program) { GLShader::ShaderObjects programObjects; + Shader::CompilationLogs compilationLogs(GLShader::NumVersions); + for (int version = 0; version < GLShader::NumVersions; version++) { auto& programObject = programObjects[version]; // Let's go through every shaders and make sure they are ready to go - std::vector shaderGLObjects; + std::vector< GLuint > shaderGLObjects; for (auto subShader : program.getShaders()) { auto object = GLShader::sync((*this), *subShader); if (object) { shaderGLObjects.push_back(object->_shaderObjects[version].glshader); } else { qCWarning(gpugllogging) << "GLBackend::compileBackendProgram - One of the shaders of the program is not compiled?"; + program.setCompilationLogs(compilationLogs); return nullptr; } } - std::string error; - GLuint glprogram = ::gl::compileProgram(shaderGLObjects, error); + GLuint glprogram = ::gl::compileProgram(shaderGLObjects, compilationLogs[version].message, compilationLogs[version].binary); if (glprogram == 0) { - qCWarning(gpugllogging) << "GLBackend::compileBackendProgram - Program didn't link:\n" << error.c_str(); + qCWarning(gpugllogging) << "GLBackend::compileBackendProgram - Program didn't link:\n" << compilationLogs[version].message.c_str(); + program.setCompilationLogs(compilationLogs); return nullptr; } @@ -123,6 +176,8 @@ GLShader* GLBackend::compileBackendProgram(const Shader& program) { makeProgramBindings(programObject); } + // Compilation feedback + program.setCompilationLogs(compilationLogs); // So far so good, the program versions have all been created successfully GLShader* object = new GLShader(this->shared_from_this()); diff --git a/libraries/gpu-gles/src/gpu/gl/GLShader.cpp b/libraries/gpu-gles/src/gpu/gl/GLShader.cpp index 7ed9121978..a626376e27 100644 --- a/libraries/gpu-gles/src/gpu/gl/GLShader.cpp +++ b/libraries/gpu-gles/src/gpu/gl/GLShader.cpp @@ -30,7 +30,7 @@ GLShader::~GLShader() { } } -GLShader* GLShader::sync(GLBackend& backend, const Shader& shader) { +GLShader* GLShader::sync(GLBackend& backend, const Shader& shader, Shader::CompilationHandler handler) { GLShader* object = Backend::getGPUObject(shader); // If GPU object already created then good @@ -45,7 +45,7 @@ GLShader* GLShader::sync(GLBackend& backend, const Shader& shader) { Backend::setGPUObject(shader, object); } } else if (shader.isDomain()) { - GLShader* tempObject = backend.compileBackendShader(shader); + GLShader* tempObject = backend.compileBackendShader(shader, handler); if (tempObject) { object = tempObject; Backend::setGPUObject(shader, object); @@ -56,10 +56,10 @@ GLShader* GLShader::sync(GLBackend& backend, const Shader& shader) { return object; } -bool GLShader::makeProgram(GLBackend& backend, Shader& shader, const Shader::BindingSet& slotBindings) { +bool GLShader::makeProgram(GLBackend& backend, Shader& shader, const Shader::BindingSet& slotBindings, Shader::CompilationHandler handler) { // First make sure the Shader has been compiled - GLShader* object = sync(backend, shader); + GLShader* object = sync(backend, shader, handler); if (!object) { return false; } diff --git a/libraries/gpu-gles/src/gpu/gl/GLShader.h b/libraries/gpu-gles/src/gpu/gl/GLShader.h index dcf2dc330d..42d63f8dfb 100644 --- a/libraries/gpu-gles/src/gpu/gl/GLShader.h +++ b/libraries/gpu-gles/src/gpu/gl/GLShader.h @@ -21,8 +21,8 @@ struct ShaderObject { class GLShader : public GPUObject { public: - static GLShader* sync(GLBackend& backend, const Shader& shader); - static bool makeProgram(GLBackend& backend, Shader& shader, const Shader::BindingSet& slotBindings); + static GLShader* sync(GLBackend& backend, const Shader& shader, Shader::CompilationHandler handler = nullptr); + static bool makeProgram(GLBackend& backend, Shader& shader, const Shader::BindingSet& slotBindings, Shader::CompilationHandler handler = nullptr); enum Version { Mono = 0, diff --git a/libraries/gpu/src/gpu/Context.cpp b/libraries/gpu/src/gpu/Context.cpp index 24128524da..2399d3ddc3 100644 --- a/libraries/gpu/src/gpu/Context.cpp +++ b/libraries/gpu/src/gpu/Context.cpp @@ -127,7 +127,7 @@ void Context::executeFrame(const FramePointer& frame) const { _frameStats.evalDelta(beginStats, endStats); } -bool Context::makeProgram(Shader& shader, const Shader::BindingSet& bindings) { +bool Context::makeProgram(Shader& shader, const Shader::BindingSet& bindings, Shader::CompilationHandler handler) { // If we're running in another DLL context, we need to fetch the program callback out of the application // FIXME find a way to do this without reliance on Qt app properties if (!_makeProgramCallback) { @@ -135,7 +135,7 @@ bool Context::makeProgram(Shader& shader, const Shader::BindingSet& bindings) { _makeProgramCallback = reinterpret_cast(rawCallback); } if (shader.isProgram() && _makeProgramCallback) { - return _makeProgramCallback(shader, bindings); + return _makeProgramCallback(shader, bindings, handler); } return false; } diff --git a/libraries/gpu/src/gpu/Context.h b/libraries/gpu/src/gpu/Context.h index 7b7575e9ed..7d04463609 100644 --- a/libraries/gpu/src/gpu/Context.h +++ b/libraries/gpu/src/gpu/Context.h @@ -143,7 +143,7 @@ class Context { public: using Size = Resource::Size; typedef BackendPointer (*CreateBackend)(); - typedef bool (*MakeProgram)(Shader& shader, const Shader::BindingSet& bindings); + typedef bool (*MakeProgram)(Shader& shader, const Shader::BindingSet& bindings, Shader::CompilationHandler handler); // This one call must happen before any context is created or used (Shader::MakeProgram) in order to setup the Backend and any singleton data needed @@ -262,7 +262,7 @@ protected: // makeProgramShader(...) make a program shader ready to be used in a Batch. // It compiles the sub shaders, link them and defines the Slots and their bindings. // If the shader passed is not a program, nothing happens. - static bool makeProgram(Shader& shader, const Shader::BindingSet& bindings); + static bool makeProgram(Shader& shader, const Shader::BindingSet& bindings, Shader::CompilationHandler handler = nullptr); static CreateBackend _createBackendCallback; static MakeProgram _makeProgramCallback; diff --git a/libraries/gpu/src/gpu/Shader.cpp b/libraries/gpu/src/gpu/Shader.cpp index 398a269f3f..35c9ad8ae2 100755 --- a/libraries/gpu/src/gpu/Shader.cpp +++ b/libraries/gpu/src/gpu/Shader.cpp @@ -82,9 +82,16 @@ void Shader::defineSlots(const SlotSet& uniforms, const SlotSet& uniformBuffers, _outputs = outputs; } -bool Shader::makeProgram(Shader& shader, const Shader::BindingSet& bindings) { +bool Shader::makeProgram(Shader& shader, const Shader::BindingSet& bindings, CompilationHandler handler) { if (shader.isProgram()) { - return Context::makeProgram(shader, bindings); + return Context::makeProgram(shader, bindings, handler); } return false; } + +void Shader::setCompilationLogs(const CompilationLogs& logs) const { + _compilationLogs.clear(); + for (const auto& log : logs) { + _compilationLogs.emplace_back(CompilationLog(log)); + } +} diff --git a/libraries/gpu/src/gpu/Shader.h b/libraries/gpu/src/gpu/Shader.h index 181c9b5e78..33449fe656 100755 --- a/libraries/gpu/src/gpu/Shader.h +++ b/libraries/gpu/src/gpu/Shader.h @@ -44,6 +44,19 @@ public: Language _lang = GLSL; }; + struct CompilationLog { + std::string message; + std::vector binary; + bool compiled{ false }; + + CompilationLog() {} + CompilationLog(const CompilationLog& src) : + message(src.message), + binary(src.binary), + compiled(src.compiled) {} + }; + using CompilationLogs = std::vector; + static const int32 INVALID_LOCATION = -1; class Slot { @@ -155,6 +168,8 @@ public: const SlotSet& inputs, const SlotSet& outputs); + typedef bool(*CompilationHandler)(const Shader& shader, const std::string& src, CompilationLog& log, std::string& newSrc); + // makeProgram(...) make a program shader ready to be used in a Batch. // It compiles the sub shaders, link them and defines the Slots and their bindings. // If the shader passed is not a program, nothing happens. @@ -168,7 +183,16 @@ public: // on a gl Context and the driver to compile the glsl shader. // Hoppefully in a few years the shader compilation will be completely abstracted in a separate shader compiler library // independant of the graphics api in use underneath (looking at you opengl & vulkan). - static bool makeProgram(Shader& shader, const Shader::BindingSet& bindings = Shader::BindingSet()); + static bool makeProgram(Shader& shader, const Shader::BindingSet& bindings = Shader::BindingSet(), CompilationHandler handler = nullptr); + + // Check the compilation state + bool compilationHasFailed() const { return _compilationHasFailed; } + const CompilationLogs& getCompilationLogs() const { return _compilationLogs; } + + // Set COmpilation logs can only be called by the Backend layers + void setCompilationHasFailed(bool compilationHasFailed) { _compilationHasFailed = compilationHasFailed; } + void setCompilationLogs(const CompilationLogs& logs) const; + const GPUObjectPointer gpuObject {}; @@ -198,6 +222,9 @@ protected: // The type of the shader, the master key Type _type; + // Compilation logs (one for each versions generated) + mutable CompilationLogs _compilationLogs; + // Whether or not the shader compilation failed bool _compilationHasFailed { false }; }; diff --git a/libraries/render/src/render/ShapePipeline.cpp b/libraries/render/src/render/ShapePipeline.cpp index fb58c21dde..597a6a0fab 100644 --- a/libraries/render/src/render/ShapePipeline.cpp +++ b/libraries/render/src/render/ShapePipeline.cpp @@ -72,34 +72,36 @@ void ShapePlumber::addPipeline(const Filter& filter, const gpu::ShaderPointer& p BatchSetter batchSetter, ItemSetter itemSetter) { ShapeKey key{ filter._flags }; - gpu::Shader::BindingSet slotBindings; - slotBindings.insert(gpu::Shader::Binding(std::string("lightingModelBuffer"), Slot::BUFFER::LIGHTING_MODEL)); - slotBindings.insert(gpu::Shader::Binding(std::string("skinClusterBuffer"), Slot::BUFFER::SKINNING)); - slotBindings.insert(gpu::Shader::Binding(std::string("materialBuffer"), Slot::BUFFER::MATERIAL)); - slotBindings.insert(gpu::Shader::Binding(std::string("texMapArrayBuffer"), Slot::BUFFER::TEXMAPARRAY)); - slotBindings.insert(gpu::Shader::Binding(std::string("albedoMap"), Slot::MAP::ALBEDO)); - slotBindings.insert(gpu::Shader::Binding(std::string("roughnessMap"), Slot::MAP::ROUGHNESS)); - slotBindings.insert(gpu::Shader::Binding(std::string("normalMap"), Slot::MAP::NORMAL)); - slotBindings.insert(gpu::Shader::Binding(std::string("metallicMap"), Slot::MAP::METALLIC)); - slotBindings.insert(gpu::Shader::Binding(std::string("emissiveMap"), Slot::MAP::EMISSIVE_LIGHTMAP)); - slotBindings.insert(gpu::Shader::Binding(std::string("occlusionMap"), Slot::MAP::OCCLUSION)); - slotBindings.insert(gpu::Shader::Binding(std::string("scatteringMap"), Slot::MAP::SCATTERING)); - slotBindings.insert(gpu::Shader::Binding(std::string("keyLightBuffer"), Slot::BUFFER::KEY_LIGHT)); - slotBindings.insert(gpu::Shader::Binding(std::string("lightBuffer"), Slot::BUFFER::LIGHT)); - slotBindings.insert(gpu::Shader::Binding(std::string("lightAmbientBuffer"), Slot::BUFFER::LIGHT_AMBIENT_BUFFER)); - slotBindings.insert(gpu::Shader::Binding(std::string("skyboxMap"), Slot::MAP::LIGHT_AMBIENT)); - slotBindings.insert(gpu::Shader::Binding(std::string("fadeMaskMap"), Slot::MAP::FADE_MASK)); - slotBindings.insert(gpu::Shader::Binding(std::string("fadeParametersBuffer"), Slot::BUFFER::FADE_PARAMETERS)); - slotBindings.insert(gpu::Shader::Binding(std::string("hazeBuffer"), Slot::BUFFER::HAZE_MODEL)); + if (program->getInputs().empty()) { + gpu::Shader::BindingSet slotBindings; + slotBindings.insert(gpu::Shader::Binding(std::string("lightingModelBuffer"), Slot::BUFFER::LIGHTING_MODEL)); + slotBindings.insert(gpu::Shader::Binding(std::string("skinClusterBuffer"), Slot::BUFFER::SKINNING)); + slotBindings.insert(gpu::Shader::Binding(std::string("materialBuffer"), Slot::BUFFER::MATERIAL)); + slotBindings.insert(gpu::Shader::Binding(std::string("texMapArrayBuffer"), Slot::BUFFER::TEXMAPARRAY)); + slotBindings.insert(gpu::Shader::Binding(std::string("albedoMap"), Slot::MAP::ALBEDO)); + slotBindings.insert(gpu::Shader::Binding(std::string("roughnessMap"), Slot::MAP::ROUGHNESS)); + slotBindings.insert(gpu::Shader::Binding(std::string("normalMap"), Slot::MAP::NORMAL)); + slotBindings.insert(gpu::Shader::Binding(std::string("metallicMap"), Slot::MAP::METALLIC)); + slotBindings.insert(gpu::Shader::Binding(std::string("emissiveMap"), Slot::MAP::EMISSIVE_LIGHTMAP)); + slotBindings.insert(gpu::Shader::Binding(std::string("occlusionMap"), Slot::MAP::OCCLUSION)); + slotBindings.insert(gpu::Shader::Binding(std::string("scatteringMap"), Slot::MAP::SCATTERING)); + slotBindings.insert(gpu::Shader::Binding(std::string("keyLightBuffer"), Slot::BUFFER::KEY_LIGHT)); + slotBindings.insert(gpu::Shader::Binding(std::string("lightBuffer"), Slot::BUFFER::LIGHT)); + slotBindings.insert(gpu::Shader::Binding(std::string("lightAmbientBuffer"), Slot::BUFFER::LIGHT_AMBIENT_BUFFER)); + slotBindings.insert(gpu::Shader::Binding(std::string("skyboxMap"), Slot::MAP::LIGHT_AMBIENT)); + slotBindings.insert(gpu::Shader::Binding(std::string("fadeMaskMap"), Slot::MAP::FADE_MASK)); + slotBindings.insert(gpu::Shader::Binding(std::string("fadeParametersBuffer"), Slot::BUFFER::FADE_PARAMETERS)); + slotBindings.insert(gpu::Shader::Binding(std::string("hazeBuffer"), Slot::BUFFER::HAZE_MODEL)); - if (key.isTranslucent()) { - slotBindings.insert(gpu::Shader::Binding(std::string("clusterGridBuffer"), Slot::BUFFER::LIGHT_CLUSTER_GRID_CLUSTER_GRID_SLOT)); - slotBindings.insert(gpu::Shader::Binding(std::string("clusterContentBuffer"), Slot::BUFFER::LIGHT_CLUSTER_GRID_CLUSTER_CONTENT_SLOT)); - slotBindings.insert(gpu::Shader::Binding(std::string("frustumGridBuffer"), Slot::BUFFER::LIGHT_CLUSTER_GRID_FRUSTUM_GRID_SLOT)); + if (key.isTranslucent()) { + slotBindings.insert(gpu::Shader::Binding(std::string("clusterGridBuffer"), Slot::BUFFER::LIGHT_CLUSTER_GRID_CLUSTER_GRID_SLOT)); + slotBindings.insert(gpu::Shader::Binding(std::string("clusterContentBuffer"), Slot::BUFFER::LIGHT_CLUSTER_GRID_CLUSTER_CONTENT_SLOT)); + slotBindings.insert(gpu::Shader::Binding(std::string("frustumGridBuffer"), Slot::BUFFER::LIGHT_CLUSTER_GRID_FRUSTUM_GRID_SLOT)); + } + + gpu::Shader::makeProgram(*program, slotBindings); } - gpu::Shader::makeProgram(*program, slotBindings); - auto locations = std::make_shared(); locations->albedoTextureUnit = program->getTextures().findLocation("albedoMap"); From d5e52834ef7873270e28d0b86785d34a0679a642 Mon Sep 17 00:00:00 2001 From: samcake Date: Tue, 30 Jan 2018 13:07:20 -0800 Subject: [PATCH 02/13] cherry picking Tony's fix for shader compilation time not taking soo long and adding better feedback from shader compilation --- libraries/gl/src/gl/GLShaders.h | 2 +- libraries/gpu-gl/src/gpu/gl/GLBackend.cpp | 4 ++-- libraries/gpu-gles/src/gpu/gl/GLBackend.cpp | 4 ++-- libraries/gpu/src/gpu/Shader.h | 3 --- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/libraries/gl/src/gl/GLShaders.h b/libraries/gl/src/gl/GLShaders.h index c5262b6b61..fc070d7659 100644 --- a/libraries/gl/src/gl/GLShaders.h +++ b/libraries/gl/src/gl/GLShaders.h @@ -22,7 +22,7 @@ namespace gl { bool compileShader(GLenum shaderDomain, const std::string& shaderSource, const std::string& defines, GLuint &shaderObject, std::string& message); #endif - GLuint compileProgram(const std::vector& glshaders, std::string& message, , std::vector& binary); + GLuint compileProgram(const std::vector& glshaders, std::string& message, std::vector& binary); } diff --git a/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp b/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp index eb6de5df13..2a052c5210 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp +++ b/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp @@ -68,8 +68,8 @@ GLBackend& getBackend() { return *INSTANCE; } -bool GLBackend::makeProgram(Shader& shader, const Shader::BindingSet& slotBindings) { - return GLShader::makeProgram(getBackend(), shader, slotBindings); +bool GLBackend::makeProgram(Shader& shader, const Shader::BindingSet& slotBindings, Shader::CompilationHandler handler) { + return GLShader::makeProgram(getBackend(), shader, slotBindings, handler); } GLBackend::CommandCall GLBackend::_commandCalls[Batch::NUM_COMMANDS] = diff --git a/libraries/gpu-gles/src/gpu/gl/GLBackend.cpp b/libraries/gpu-gles/src/gpu/gl/GLBackend.cpp index 6fd5df6f81..8a118b7b71 100644 --- a/libraries/gpu-gles/src/gpu/gl/GLBackend.cpp +++ b/libraries/gpu-gles/src/gpu/gl/GLBackend.cpp @@ -61,8 +61,8 @@ GLBackend& getBackend() { return *INSTANCE; } -bool GLBackend::makeProgram(Shader& shader, const Shader::BindingSet& slotBindings) { - return GLShader::makeProgram(getBackend(), shader, slotBindings); +bool GLBackend::makeProgram(Shader& shader, const Shader::BindingSet& slotBindings, Shader::CompilationHandler handler) { + return GLShader::makeProgram(getBackend(), shader, slotBindings, handler); } diff --git a/libraries/gpu/src/gpu/Shader.h b/libraries/gpu/src/gpu/Shader.h index 33449fe656..d78077a396 100755 --- a/libraries/gpu/src/gpu/Shader.h +++ b/libraries/gpu/src/gpu/Shader.h @@ -138,9 +138,6 @@ public: bool isProgram() const { return getType() > NUM_DOMAINS; } bool isDomain() const { return getType() < NUM_DOMAINS; } - void setCompilationHasFailed(bool compilationHasFailed) { _compilationHasFailed = compilationHasFailed; } - bool compilationHasFailed() const { return _compilationHasFailed; } - const Source& getSource() const { return _source; } const Shaders& getShaders() const { return _shaders; } From f078ff611a4b19ac779d6e3926b09db4ec3ad4fc Mon Sep 17 00:00:00 2001 From: samcake Date: Tue, 30 Jan 2018 15:10:52 -0800 Subject: [PATCH 03/13] Refining the declaraion signatures and adding the binary capture --- .../src/display-plugins/hmd/HmdDisplayPlugin.cpp | 3 ++- libraries/gpu-gl/src/gpu/gl/GLBackend.h | 4 ++-- libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp | 8 +++++--- libraries/gpu-gl/src/gpu/gl/GLShader.cpp | 2 +- libraries/gpu-gl/src/gpu/gl/GLShader.h | 2 +- libraries/gpu-gles/src/gpu/gl/GLBackend.h | 2 +- libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp | 4 ++-- libraries/gpu/src/gpu/Context.h | 2 +- libraries/gpu/src/gpu/null/NullBackend.h | 2 +- plugins/openvr/src/OpenVrDisplayPlugin.cpp | 3 ++- tests/shaders/src/main.cpp | 3 ++- 11 files changed, 20 insertions(+), 15 deletions(-) diff --git a/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.cpp index 90bb83a663..b64836627c 100644 --- a/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.cpp +++ b/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.cpp @@ -398,7 +398,8 @@ void HmdDisplayPlugin::HUDRenderer::updatePipeline() { auto vs = gpu::Shader::createVertex(std::string(hmd_ui_vert)); auto ps = gpu::Shader::createPixel(std::string(hmd_ui_frag)); auto program = gpu::Shader::createProgram(vs, ps); - gpu::gl::GLBackend::makeProgram(*program, gpu::Shader::BindingSet()); + // gpu::gl::GLBackend::makeProgram(*program, gpu::Shader::BindingSet()); + gpu::Shader::makeProgram(*program, gpu::Shader::BindingSet()); uniformsLocation = program->getUniformBuffers().findLocation("hudBuffer"); gpu::StatePointer state = gpu::StatePointer(new gpu::State()); diff --git a/libraries/gpu-gl/src/gpu/gl/GLBackend.h b/libraries/gpu-gl/src/gpu/gl/GLBackend.h index 004bfe1c85..f0b74803e4 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLBackend.h +++ b/libraries/gpu-gl/src/gpu/gl/GLBackend.h @@ -64,7 +64,7 @@ protected: explicit GLBackend(bool syncCache); GLBackend(); public: - static bool makeProgram(Shader& shader, const Shader::BindingSet& slotBindings = Shader::BindingSet(), Shader::CompilationHandler handler = nullptr); + static bool makeProgram(Shader& shader, const Shader::BindingSet& slotBindings, Shader::CompilationHandler handler); virtual ~GLBackend(); @@ -423,7 +423,7 @@ protected: } _pipeline; // Backend dependant compilation of the shader - virtual GLShader* compileBackendProgram(const Shader& program); + virtual GLShader* compileBackendProgram(const Shader& program, Shader::CompilationHandler handler); virtual GLShader* compileBackendShader(const Shader& shader, Shader::CompilationHandler handler); virtual std::string getBackendShaderHeader() const; virtual void makeProgramBindings(ShaderObject& shaderObject); diff --git a/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp b/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp index 84170dbffd..40b9c94610 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp +++ b/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp @@ -101,7 +101,7 @@ GLShader* GLBackend::compileBackendShader(const Shader& shader, Shader::Compilat return object; } -GLShader* GLBackend::compileBackendProgram(const Shader& program) { +GLShader* GLBackend::compileBackendProgram(const Shader& program, Shader::CompilationHandler handler) { if (!program.isProgram()) { return nullptr; } @@ -116,11 +116,13 @@ GLShader* GLBackend::compileBackendProgram(const Shader& program) { // Let's go through every shaders and make sure they are ready to go std::vector< GLuint > shaderGLObjects; for (auto subShader : program.getShaders()) { - auto object = GLShader::sync((*this), *subShader); + auto object = GLShader::sync((*this), *subShader, handler); if (object) { shaderGLObjects.push_back(object->_shaderObjects[version].glshader); } else { qCWarning(gpugllogging) << "GLBackend::compileBackendProgram - One of the shaders of the program is not compiled?"; + compilationLogs[version].compiled = false; + compilationLogs[version].message = std::string("Failed to compile, one of the shaders of the program is not compiled ?"); program.setCompilationLogs(compilationLogs); return nullptr; } @@ -132,7 +134,7 @@ GLShader* GLBackend::compileBackendProgram(const Shader& program) { program.setCompilationLogs(compilationLogs); return nullptr; } - + compilationLogs[version].compiled = true; programObject.glprogram = glprogram; makeProgramBindings(programObject); diff --git a/libraries/gpu-gl/src/gpu/gl/GLShader.cpp b/libraries/gpu-gl/src/gpu/gl/GLShader.cpp index a626376e27..42d4fe3845 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLShader.cpp +++ b/libraries/gpu-gl/src/gpu/gl/GLShader.cpp @@ -39,7 +39,7 @@ GLShader* GLShader::sync(GLBackend& backend, const Shader& shader, Shader::Compi } // need to have a gpu object? if (shader.isProgram()) { - GLShader* tempObject = backend.compileBackendProgram(shader); + GLShader* tempObject = backend.compileBackendProgram(shader, handler); if (tempObject) { object = tempObject; Backend::setGPUObject(shader, object); diff --git a/libraries/gpu-gl/src/gpu/gl/GLShader.h b/libraries/gpu-gl/src/gpu/gl/GLShader.h index 42d63f8dfb..8625b3e64a 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLShader.h +++ b/libraries/gpu-gl/src/gpu/gl/GLShader.h @@ -22,7 +22,7 @@ struct ShaderObject { class GLShader : public GPUObject { public: static GLShader* sync(GLBackend& backend, const Shader& shader, Shader::CompilationHandler handler = nullptr); - static bool makeProgram(GLBackend& backend, Shader& shader, const Shader::BindingSet& slotBindings, Shader::CompilationHandler handler = nullptr); + static bool makeProgram(GLBackend& backend, Shader& shader, const Shader::BindingSet& slotBindings, Shader::CompilationHandler handler); enum Version { Mono = 0, diff --git a/libraries/gpu-gles/src/gpu/gl/GLBackend.h b/libraries/gpu-gles/src/gpu/gl/GLBackend.h index bb8e15bad3..0db46985f7 100644 --- a/libraries/gpu-gles/src/gpu/gl/GLBackend.h +++ b/libraries/gpu-gles/src/gpu/gl/GLBackend.h @@ -420,7 +420,7 @@ protected: } _pipeline; // Backend dependant compilation of the shader - virtual GLShader* compileBackendProgram(const Shader& program); + virtual GLShader* compileBackendProgram(const Shader& program, Shader::CompilationHandler handler); virtual GLShader* compileBackendShader(const Shader& shader, Shader::CompilationHandler handler); virtual std::string getBackendShaderHeader() const; virtual void makeProgramBindings(ShaderObject& shaderObject); diff --git a/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp b/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp index 26dbb12cf7..1d7636c159 100644 --- a/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp +++ b/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp @@ -140,7 +140,7 @@ GLShader* GLBackend::compileBackendShader(const Shader& shader, Shader::Compilat return object; } -GLShader* GLBackend::compileBackendProgram(const Shader& program) { +GLShader* GLBackend::compileBackendProgram(const Shader& program, Shader::CompilationHandler handler) { if (!program.isProgram()) { return nullptr; } @@ -155,7 +155,7 @@ GLShader* GLBackend::compileBackendProgram(const Shader& program) { // Let's go through every shaders and make sure they are ready to go std::vector< GLuint > shaderGLObjects; for (auto subShader : program.getShaders()) { - auto object = GLShader::sync((*this), *subShader); + auto object = GLShader::sync((*this), *subShader, handler); if (object) { shaderGLObjects.push_back(object->_shaderObjects[version].glshader); } else { diff --git a/libraries/gpu/src/gpu/Context.h b/libraries/gpu/src/gpu/Context.h index 7d04463609..e22cc57570 100644 --- a/libraries/gpu/src/gpu/Context.h +++ b/libraries/gpu/src/gpu/Context.h @@ -262,7 +262,7 @@ protected: // makeProgramShader(...) make a program shader ready to be used in a Batch. // It compiles the sub shaders, link them and defines the Slots and their bindings. // If the shader passed is not a program, nothing happens. - static bool makeProgram(Shader& shader, const Shader::BindingSet& bindings, Shader::CompilationHandler handler = nullptr); + static bool makeProgram(Shader& shader, const Shader::BindingSet& bindings, Shader::CompilationHandler handler); static CreateBackend _createBackendCallback; static MakeProgram _makeProgramCallback; diff --git a/libraries/gpu/src/gpu/null/NullBackend.h b/libraries/gpu/src/gpu/null/NullBackend.h index c9d249aec7..abaa24812f 100644 --- a/libraries/gpu/src/gpu/null/NullBackend.h +++ b/libraries/gpu/src/gpu/null/NullBackend.h @@ -28,7 +28,7 @@ class Backend : public gpu::Backend { friend class gpu::Context; static void init() {} static gpu::Backend* createBackend() { return new Backend(); } - static bool makeProgram(Shader& shader, const Shader::BindingSet& slotBindings) { return true; } + static bool makeProgram(Shader& shader, const Shader::BindingSet& slotBindings, Shader::CompilationHandler handler) { return true; } protected: explicit Backend(bool syncCache) : Parent() { } diff --git a/plugins/openvr/src/OpenVrDisplayPlugin.cpp b/plugins/openvr/src/OpenVrDisplayPlugin.cpp index 4403eaeff7..4b5f0e6517 100644 --- a/plugins/openvr/src/OpenVrDisplayPlugin.cpp +++ b/plugins/openvr/src/OpenVrDisplayPlugin.cpp @@ -200,9 +200,10 @@ public: std::string fsSource = HMD_REPROJECTION_FRAG; GLuint vertexShader { 0 }, fragmentShader { 0 }; std::string error; + std::vector binary; ::gl::compileShader(GL_VERTEX_SHADER, vsSource, "", vertexShader, error); ::gl::compileShader(GL_FRAGMENT_SHADER, fsSource, "", fragmentShader, error); - _program = ::gl::compileProgram({ { vertexShader, fragmentShader } }, error); + _program = ::gl::compileProgram({ { vertexShader, fragmentShader } }, error, binary); glDeleteShader(vertexShader); glDeleteShader(fragmentShader); qDebug() << "Rebuild proigram"; diff --git a/tests/shaders/src/main.cpp b/tests/shaders/src/main.cpp index cd307ba362..3f48e37a76 100644 --- a/tests/shaders/src/main.cpp +++ b/tests/shaders/src/main.cpp @@ -137,12 +137,13 @@ const std::string PIXEL_SHADER_DEFINES{ R"GLSL( void testShaderBuild(const char* vs_src, const char * fs_src) { std::string error; + std::vector binary; GLuint vs, fs; if (!gl::compileShader(GL_VERTEX_SHADER, vs_src, VERTEX_SHADER_DEFINES, vs, error) || !gl::compileShader(GL_FRAGMENT_SHADER, fs_src, PIXEL_SHADER_DEFINES, fs, error)) { throw std::runtime_error("Failed to compile shader"); } - auto pr = gl::compileProgram({ vs, fs }, error); + auto pr = gl::compileProgram({ vs, fs }, error, binary); if (!pr) { throw std::runtime_error("Failed to link shader"); } From 9ea6c78ac2526faea3035cb52b18a6e9c1993b86 Mon Sep 17 00:00:00 2001 From: Sam Gateau Date: Tue, 30 Jan 2018 15:31:34 -0800 Subject: [PATCH 04/13] fix stupid copy paste --- interface/src/Application.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index f1d0378792..3390e71b07 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2283,7 +2283,7 @@ void Application::initializeGL() { #ifdef Q_OS_OSX // FIXME: on mac os the shaders take up to 1 minute to compile, so we pause the deadlock watchdog thread. - +DeadlockWatchdogThread::pause(); + DeadlockWatchdogThread::pause(); #endif // Set up the render engine From 63371ff59c129184970435255ab2cc003bf85331 Mon Sep 17 00:00:00 2001 From: samcake Date: Tue, 30 Jan 2018 17:17:22 -0800 Subject: [PATCH 05/13] Fix the android build --- .../gpu-gles/src/gpu/gl/GLBackendShader.cpp | 35 ------------------- 1 file changed, 35 deletions(-) diff --git a/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp b/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp index 1d7636c159..350e95b46a 100644 --- a/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp +++ b/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp @@ -56,41 +56,6 @@ static const std::array VERSION_DEFINES { { stereoVersion } }; -GLShader* GLBackend::compileBackendShader(const Shader& shader, Shader::CompilationHandler handler) { - // Any GLSLprogram ? normally yes... - const std::string& shaderSource = shader.getSource().getCode(); - GLenum shaderDomain = SHADER_DOMAINS[shader.getType()]; - GLShader::ShaderObjects shaderObjects; - Shader::CompilationLogs compilationLogs(GLShader::NumVersions); - - for (int version = 0; version < GLShader::NumVersions; version++) { - auto& shaderObject = shaderObjects[version]; - - std::string shaderDefines = getBackendShaderHeader() + "\n" + DOMAIN_DEFINES[shader.getType()] + "\n" + VERSION_DEFINES[version] - + "\n#extension GL_EXT_texture_buffer : enable" - + "\nprecision lowp float; // check precision 2" - + "\nprecision lowp samplerBuffer;" - + "\nprecision lowp sampler2DShadow;"; - std::string error; - -#ifdef SEPARATE_PROGRAM - bool result = ::gl::compileShader(shaderDomain, shaderSource, shaderDefines, shaderObject.glshader, shaderObject.glprogram, error); -#else - bool result = ::gl::compileShader(shaderDomain, shaderSource, shaderDefines, shaderObject.glshader, error); -#endif - if (!result) { - qCWarning(gpugllogging) << "GLBackend::compileBackendProgram - Shader didn't compile:\n" << error.c_str(); - return nullptr; - } - } - - // So far so good, the shader is created successfully - GLShader* object = new GLShader(this->shared_from_this()); - object->_shaderObjects = shaderObjects; - - return object; -} - GLShader* GLBackend::compileBackendShader(const Shader& shader, Shader::CompilationHandler handler) { // Any GLSLprogram ? normally yes... const std::string& shaderSource = shader.getSource().getCode(); From f044cf76d6a35b65733fc20c54810042d6f6c8a5 Mon Sep 17 00:00:00 2001 From: samcake Date: Tue, 30 Jan 2018 18:31:30 -0800 Subject: [PATCH 06/13] add the count of compilations of shaders to avoid recompiling them if ot needed --- .../src/display-plugins/hmd/HmdDisplayPlugin.cpp | 1 - libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp | 2 ++ libraries/gpu/src/gpu/Shader.cpp | 4 ++++ libraries/gpu/src/gpu/Shader.h | 4 ++++ libraries/render/src/render/ShapePipeline.cpp | 2 +- 5 files changed, 11 insertions(+), 2 deletions(-) diff --git a/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.cpp b/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.cpp index b64836627c..d35d5c5317 100644 --- a/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.cpp +++ b/libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.cpp @@ -398,7 +398,6 @@ void HmdDisplayPlugin::HUDRenderer::updatePipeline() { auto vs = gpu::Shader::createVertex(std::string(hmd_ui_vert)); auto ps = gpu::Shader::createPixel(std::string(hmd_ui_frag)); auto program = gpu::Shader::createProgram(vs, ps); - // gpu::gl::GLBackend::makeProgram(*program, gpu::Shader::BindingSet()); gpu::Shader::makeProgram(*program, gpu::Shader::BindingSet()); uniformsLocation = program->getUniformBuffers().findLocation("hudBuffer"); diff --git a/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp b/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp index 40b9c94610..e9d23b543f 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp +++ b/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp @@ -62,6 +62,7 @@ GLShader* GLBackend::compileBackendShader(const Shader& shader, Shader::Compilat GLenum shaderDomain = SHADER_DOMAINS[shader.getType()]; GLShader::ShaderObjects shaderObjects; Shader::CompilationLogs compilationLogs(GLShader::NumVersions); + shader.incrementCompilationAttempt(); for (int version = 0; version < GLShader::NumVersions; version++) { auto& shaderObject = shaderObjects[version]; @@ -108,6 +109,7 @@ GLShader* GLBackend::compileBackendProgram(const Shader& program, Shader::Compil GLShader::ShaderObjects programObjects; + program.incrementCompilationAttempt(); Shader::CompilationLogs compilationLogs(GLShader::NumVersions); for (int version = 0; version < GLShader::NumVersions; version++) { diff --git a/libraries/gpu/src/gpu/Shader.cpp b/libraries/gpu/src/gpu/Shader.cpp index 35c9ad8ae2..b6b90e98fa 100755 --- a/libraries/gpu/src/gpu/Shader.cpp +++ b/libraries/gpu/src/gpu/Shader.cpp @@ -95,3 +95,7 @@ void Shader::setCompilationLogs(const CompilationLogs& logs) const { _compilationLogs.emplace_back(CompilationLog(log)); } } + +void Shader::incrementCompilationAttempt() const { + _numCompilationAttempts++; +} \ No newline at end of file diff --git a/libraries/gpu/src/gpu/Shader.h b/libraries/gpu/src/gpu/Shader.h index d78077a396..a489146e36 100755 --- a/libraries/gpu/src/gpu/Shader.h +++ b/libraries/gpu/src/gpu/Shader.h @@ -185,10 +185,12 @@ public: // Check the compilation state bool compilationHasFailed() const { return _compilationHasFailed; } const CompilationLogs& getCompilationLogs() const { return _compilationLogs; } + uint32_t getNumCompilationAttempts() const { return _numCompilationAttempts; } // Set COmpilation logs can only be called by the Backend layers void setCompilationHasFailed(bool compilationHasFailed) { _compilationHasFailed = compilationHasFailed; } void setCompilationLogs(const CompilationLogs& logs) const; + void incrementCompilationAttempt() const; const GPUObjectPointer gpuObject {}; @@ -219,6 +221,8 @@ protected: // The type of the shader, the master key Type _type; + // Number of attempts to compile the shader + mutable uint32_t _numCompilationAttempts{ 0 }; // Compilation logs (one for each versions generated) mutable CompilationLogs _compilationLogs; diff --git a/libraries/render/src/render/ShapePipeline.cpp b/libraries/render/src/render/ShapePipeline.cpp index 597a6a0fab..a6d73897ae 100644 --- a/libraries/render/src/render/ShapePipeline.cpp +++ b/libraries/render/src/render/ShapePipeline.cpp @@ -72,7 +72,7 @@ void ShapePlumber::addPipeline(const Filter& filter, const gpu::ShaderPointer& p BatchSetter batchSetter, ItemSetter itemSetter) { ShapeKey key{ filter._flags }; - if (program->getInputs().empty()) { + if (program->getNumCompilationAttempts() < 1) { gpu::Shader::BindingSet slotBindings; slotBindings.insert(gpu::Shader::Binding(std::string("lightingModelBuffer"), Slot::BUFFER::LIGHTING_MODEL)); slotBindings.insert(gpu::Shader::Binding(std::string("skinClusterBuffer"), Slot::BUFFER::SKINNING)); From 89f4fe1c04a132fbd73093a657a4f42d60a6dd4c Mon Sep 17 00:00:00 2001 From: samcake Date: Wed, 31 Jan 2018 17:28:45 -0800 Subject: [PATCH 07/13] Adding the map of existing shaders and programs and ways to identify them to avoid recompiling them --- libraries/gpu/src/gpu/Shader.cpp | 114 +++++++++++++++++++++++-------- libraries/gpu/src/gpu/Shader.h | 50 +++++++++++++- 2 files changed, 132 insertions(+), 32 deletions(-) diff --git a/libraries/gpu/src/gpu/Shader.cpp b/libraries/gpu/src/gpu/Shader.cpp index b6b90e98fa..649e7d9e66 100755 --- a/libraries/gpu/src/gpu/Shader.cpp +++ b/libraries/gpu/src/gpu/Shader.cpp @@ -17,59 +17,112 @@ using namespace gpu; -Shader::Shader(Type type, const Source& source): +std::atomic Shader::_nextShaderID( 1 ); +Shader::DomainShaderMaps Shader::_domainShaderMaps; +Shader::ProgramMap Shader::_programMap; + + +Shader::Shader(Type type, const Source& source) : _source(source), - _type(type) + _type(type), + _ID(_nextShaderID++) { } -Shader::Shader(Type type, const Pointer& vertex, const Pointer& pixel): - _type(type) +Shader::Shader(Type type, const Pointer& vertex, const Pointer& geometry, const Pointer& pixel): + _type(type), + _ID(_nextShaderID++) { - _shaders.resize(2); - _shaders[VERTEX] = vertex; - _shaders[PIXEL] = pixel; -} - -Shader::Shader(Type type, const Pointer& vertex, const Pointer& geometry, const Pointer& pixel) : -_type(type) { - _shaders.resize(3); - _shaders[VERTEX] = vertex; - _shaders[GEOMETRY] = geometry; - _shaders[PIXEL] = pixel; + if (geometry) { + _shaders.resize(3); + _shaders[VERTEX] = vertex; + _shaders[GEOMETRY] = geometry; + _shaders[PIXEL] = pixel; + } else { + _shaders.resize(2); + _shaders[VERTEX] = vertex; + _shaders[PIXEL] = pixel; + } } Shader::~Shader() { } +Shader::Pointer Shader::createOrReuseDomainShader(Type type, const Source& source) { + auto found = _domainShaderMaps[type].find(source); + if (found != _domainShaderMaps[type].end()) { + auto sharedShader = (*found).second.lock(); + if (sharedShader) { + return sharedShader; + } + } + auto shader = Pointer(new Shader(type, source)); + _domainShaderMaps[type].emplace(source, std::weak_ptr(shader)); + return shader; +} + Shader::Pointer Shader::createVertex(const Source& source) { - return Pointer(new Shader(VERTEX, source)); + return createOrReuseDomainShader(VERTEX, source); } Shader::Pointer Shader::createPixel(const Source& source) { - return Pointer(new Shader(PIXEL, source)); + return createOrReuseDomainShader(PIXEL, source); } Shader::Pointer Shader::createGeometry(const Source& source) { - return Pointer(new Shader(GEOMETRY, source)); + return createOrReuseDomainShader(GEOMETRY, source); } -Shader::Pointer Shader::createProgram(const Pointer& vertexShader, const Pointer& pixelShader) { - if (vertexShader && vertexShader->getType() == VERTEX && - pixelShader && pixelShader->getType() == PIXEL) { - return Pointer(new Shader(PROGRAM, vertexShader, pixelShader)); +ShaderPointer Shader::createOrReuseProgramShader(Type type, const Pointer& vertexShader, const Pointer& geometryShader, const Pointer& pixelShader) { + ProgramMapKey key(0); + + if (vertexShader && vertexShader->getType() == VERTEX) { + key.x = vertexShader->getID(); + } else { + // Shader is not valid, exit + return Pointer(); } - return Pointer(); + + if (pixelShader && pixelShader->getType() == PIXEL) { + key.y = pixelShader->getID(); + } else { + // Shader is not valid, exit + return Pointer(); + } + + if (geometryShader) { + if (geometryShader->getType() == GEOMETRY) { + key.z = geometryShader->getID(); + } else { + // Shader is not valid, exit + return Pointer(); + } + } + + // program key is defined, now try to reuse + auto found = _programMap.find(key); + if (found != _programMap.end()) { + auto sharedShader = (*found).second.lock(); + if (sharedShader) { + return sharedShader; + } + } + + // Program is a new one, let's create it + auto program = Pointer(new Shader(type, vertexShader, geometryShader, pixelShader)); + _programMap.emplace(key, std::weak_ptr(program)); + return program; +} + + +Shader::Pointer Shader::createProgram(const Pointer& vertexShader, const Pointer& pixelShader) { + return createOrReuseProgramShader(PROGRAM, vertexShader, nullptr, pixelShader); } Shader::Pointer Shader::createProgram(const Pointer& vertexShader, const Pointer& geometryShader, const Pointer& pixelShader) { - if (vertexShader && vertexShader->getType() == VERTEX && - geometryShader && geometryShader->getType() == GEOMETRY && - pixelShader && pixelShader->getType() == PIXEL) { - return Pointer(new Shader(PROGRAM, vertexShader, geometryShader, pixelShader)); - } - return Pointer(); + return createOrReuseProgramShader(PROGRAM, vertexShader, geometryShader, pixelShader); + } void Shader::defineSlots(const SlotSet& uniforms, const SlotSet& uniformBuffers, const SlotSet& resourceBuffers, const SlotSet& textures, const SlotSet& samplers, const SlotSet& inputs, const SlotSet& outputs) { @@ -98,4 +151,5 @@ void Shader::setCompilationLogs(const CompilationLogs& logs) const { void Shader::incrementCompilationAttempt() const { _numCompilationAttempts++; -} \ No newline at end of file +} + diff --git a/libraries/gpu/src/gpu/Shader.h b/libraries/gpu/src/gpu/Shader.h index a489146e36..1e251e6f67 100755 --- a/libraries/gpu/src/gpu/Shader.h +++ b/libraries/gpu/src/gpu/Shader.h @@ -15,6 +15,7 @@ #include #include #include +#include #include @@ -22,6 +23,8 @@ namespace gpu { class Shader { public: + // unique identifier of a shader + using ID = uint32_t; typedef std::shared_ptr< Shader > Pointer; typedef std::vector< Pointer > Shaders; @@ -39,6 +42,11 @@ public: virtual const std::string& getCode() const { return _code; } + class Less { + public: + bool operator() (const Source& x, const Source& y) const { if (x._lang == y._lang) { return x._code < y._code; } else { return (x._lang < y._lang); } } + }; + protected: std::string _code; Language _lang = GLSL; @@ -134,6 +142,8 @@ public: ~Shader(); + ID getID() const { return _ID; } + Type getType() const { return _type; } bool isProgram() const { return getType() > NUM_DOMAINS; } bool isDomain() const { return getType() < NUM_DOMAINS; } @@ -194,15 +204,15 @@ public: const GPUObjectPointer gpuObject {}; - + protected: Shader(Type type, const Source& source); - Shader(Type type, const Pointer& vertex, const Pointer& pixel); Shader(Type type, const Pointer& vertex, const Pointer& geometry, const Pointer& pixel); Shader(const Shader& shader); // deep copy of the sysmem shader Shader& operator=(const Shader& shader); // deep copy of the sysmem texture + // Source contains the actual source code or nothing if the shader is a program Source _source; @@ -221,6 +231,9 @@ protected: // The type of the shader, the master key Type _type; + // The unique identifier of a shader in the GPU lib + uint32_t _ID{ 0 }; + // Number of attempts to compile the shader mutable uint32_t _numCompilationAttempts{ 0 }; // Compilation logs (one for each versions generated) @@ -228,6 +241,39 @@ protected: // Whether or not the shader compilation failed bool _compilationHasFailed { false }; + + + // Global maps of the shaders + // Unique shader ID + static std::atomic _nextShaderID; + + using ShaderMap = std::map, Source::Less>; + using DomainShaderMaps = std::array; + static DomainShaderMaps _domainShaderMaps; + + static ShaderPointer createOrReuseDomainShader(Type type, const Source& source); + + using ProgramMapKey = glm::uvec3; // THe IDs of the shaders in a progrma make its key + class ProgramKeyLess { + public: + bool operator() (const ProgramMapKey& l, const ProgramMapKey& r) const { + if (l.x == r.x) { + if (l.y == r.y) { + return (l.z < l.z); + } + else { + return (l.y < l.y); + } + } + else { + return (l.x < r.x); + } + } + }; + using ProgramMap = std::map, ProgramKeyLess>; + static ProgramMap _programMap; + + static ShaderPointer createOrReuseProgramShader(Type type, const Pointer& vertexShader, const Pointer& geometryShader, const Pointer& pixelShader); }; typedef Shader::Pointer ShaderPointer; From edb416d24eacc2046d46f1f13c16d54d408cc343 Mon Sep 17 00:00:00 2001 From: Sam Gateau Date: Wed, 31 Jan 2018 21:32:22 -0800 Subject: [PATCH 08/13] Fixing the program map sorting, good to go --- libraries/gpu/src/gpu/Shader.cpp | 1 - libraries/gpu/src/gpu/Shader.h | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/libraries/gpu/src/gpu/Shader.cpp b/libraries/gpu/src/gpu/Shader.cpp index 649e7d9e66..e428c758b1 100755 --- a/libraries/gpu/src/gpu/Shader.cpp +++ b/libraries/gpu/src/gpu/Shader.cpp @@ -122,7 +122,6 @@ Shader::Pointer Shader::createProgram(const Pointer& vertexShader, const Pointer Shader::Pointer Shader::createProgram(const Pointer& vertexShader, const Pointer& geometryShader, const Pointer& pixelShader) { return createOrReuseProgramShader(PROGRAM, vertexShader, geometryShader, pixelShader); - } void Shader::defineSlots(const SlotSet& uniforms, const SlotSet& uniformBuffers, const SlotSet& resourceBuffers, const SlotSet& textures, const SlotSet& samplers, const SlotSet& inputs, const SlotSet& outputs) { diff --git a/libraries/gpu/src/gpu/Shader.h b/libraries/gpu/src/gpu/Shader.h index 1e251e6f67..b7ca000d43 100755 --- a/libraries/gpu/src/gpu/Shader.h +++ b/libraries/gpu/src/gpu/Shader.h @@ -259,10 +259,10 @@ protected: bool operator() (const ProgramMapKey& l, const ProgramMapKey& r) const { if (l.x == r.x) { if (l.y == r.y) { - return (l.z < l.z); + return (l.z < r.z); } else { - return (l.y < l.y); + return (l.y < r.y); } } else { From 60238fa644b4ae603a3011f2abbdf8818ed8f37c Mon Sep 17 00:00:00 2001 From: Sam Gateau Date: Wed, 31 Jan 2018 23:15:19 -0800 Subject: [PATCH 09/13] Fixing the android build --- libraries/gpu-gles/src/gpu/gl/GLShader.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/gpu-gles/src/gpu/gl/GLShader.cpp b/libraries/gpu-gles/src/gpu/gl/GLShader.cpp index a626376e27..42d4fe3845 100644 --- a/libraries/gpu-gles/src/gpu/gl/GLShader.cpp +++ b/libraries/gpu-gles/src/gpu/gl/GLShader.cpp @@ -39,7 +39,7 @@ GLShader* GLShader::sync(GLBackend& backend, const Shader& shader, Shader::Compi } // need to have a gpu object? if (shader.isProgram()) { - GLShader* tempObject = backend.compileBackendProgram(shader); + GLShader* tempObject = backend.compileBackendProgram(shader, handler); if (tempObject) { object = tempObject; Backend::setGPUObject(shader, object); From a650c5bbc62c8fc5d61f87b843b83fc373131691 Mon Sep 17 00:00:00 2001 From: Sam Gateau Date: Wed, 31 Jan 2018 23:31:31 -0800 Subject: [PATCH 10/13] Move a define to resume watchdog a bit further --- interface/src/Application.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 808f6eb890..f7c30869c2 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -2296,10 +2296,6 @@ void Application::initializeGL() { #endif _renderEngine->addJob("RenderMainView", cullFunctor, !DISABLE_DEFERRED); -#ifdef Q_OS_OSX - DeadlockWatchdogThread::resume(); -#endif - _renderEngine->load(); _renderEngine->registerScene(_main3DScene); From 351f619555c736a7ae878c06c4dcb6775c727284 Mon Sep 17 00:00:00 2001 From: Sam Gateau Date: Wed, 31 Jan 2018 23:35:16 -0800 Subject: [PATCH 11/13] Fixing a shader error on android --- libraries/render-utils/src/LightAmbient.slh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/render-utils/src/LightAmbient.slh b/libraries/render-utils/src/LightAmbient.slh index acd30d527d..89d3f4faee 100644 --- a/libraries/render-utils/src/LightAmbient.slh +++ b/libraries/render-utils/src/LightAmbient.slh @@ -46,7 +46,7 @@ vec3 evalAmbientSpecularIrradiance(LightAmbient ambient, SurfaceData surface) { float levels = getLightAmbientMapNumMips(ambient); float m = 12.0 / (1.0+11.0*surface.roughness); float lod = levels - m; - lod = max(lod, 0); + lod = max(lod, 0.0); specularLight = evalSkyboxLight(lightDir, lod).xyz; } <@endif@> From fd501cf3af75357cb1de1a5496e8ae9c58fb79e8 Mon Sep 17 00:00:00 2001 From: samcake Date: Thu, 1 Feb 2018 10:37:54 -0800 Subject: [PATCH 12/13] Addressing review comments --- libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp | 3 +++ libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp | 3 +++ libraries/gpu/src/gpu/Shader.h | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp b/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp index e9d23b543f..42e95ba6c6 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp +++ b/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp @@ -71,6 +71,9 @@ GLShader* GLBackend::compileBackendShader(const Shader& shader, Shader::Compilat if (handler) { bool retest = true; std::string currentSrc = shaderSource; + // When a Handler is specified, we can try multiple times to build the shader and let the handler change the source if the compilation fails. + // The retest bool is set to false as soon as the compilation succeed to wexit the while loop. + // The handler tells us if we should retry or not while returning a modified version of the source. while (retest) { bool result = ::gl::compileShader(shaderDomain, currentSrc, shaderDefines, shaderObject.glshader, compilationLogs[version].message); compilationLogs[version].compiled = result; diff --git a/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp b/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp index 350e95b46a..b799fb0037 100644 --- a/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp +++ b/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp @@ -74,6 +74,9 @@ GLShader* GLBackend::compileBackendShader(const Shader& shader, Shader::Compilat if (handler) { bool retest = true; std::string currentSrc = shaderSource; + // When a Handler is specified, we can try multiple times to build the shader and let the handler change the source if the compilation fails. + // The retest bool is set to false as soon as the compilation succeed to wexit the while loop. + // The handler tells us if we should retry or not while returning a modified version of the source. while (retest) { bool result = ::gl::compileShader(shaderDomain, currentSrc, shaderDefines, shaderObject.glshader, compilationLogs[version].message); compilationLogs[version].compiled = result; diff --git a/libraries/gpu/src/gpu/Shader.h b/libraries/gpu/src/gpu/Shader.h index b7ca000d43..07800fa14a 100755 --- a/libraries/gpu/src/gpu/Shader.h +++ b/libraries/gpu/src/gpu/Shader.h @@ -253,7 +253,7 @@ protected: static ShaderPointer createOrReuseDomainShader(Type type, const Source& source); - using ProgramMapKey = glm::uvec3; // THe IDs of the shaders in a progrma make its key + using ProgramMapKey = glm::uvec3; // The IDs of the shaders in a program make its key class ProgramKeyLess { public: bool operator() (const ProgramMapKey& l, const ProgramMapKey& r) const { From 9867b479fd30f2a3987bf3ef4c7df9703ff58d69 Mon Sep 17 00:00:00 2001 From: samcake Date: Thu, 1 Feb 2018 13:12:28 -0800 Subject: [PATCH 13/13] APplying review feedback --- libraries/gpu-gl/src/gpu/gl/GLBackend.cpp | 2 +- libraries/gpu-gl/src/gpu/gl/GLBackend.h | 6 +++--- libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp | 4 ++-- libraries/gpu-gl/src/gpu/gl/GLShader.cpp | 4 ++-- libraries/gpu-gl/src/gpu/gl/GLShader.h | 4 ++-- libraries/gpu-gles/src/gpu/gl/GLBackend.cpp | 2 +- libraries/gpu-gles/src/gpu/gl/GLBackend.h | 6 +++--- libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp | 4 ++-- libraries/gpu-gles/src/gpu/gl/GLShader.cpp | 4 ++-- libraries/gpu-gles/src/gpu/gl/GLShader.h | 4 ++-- libraries/gpu/src/gpu/Context.cpp | 2 +- libraries/gpu/src/gpu/Context.h | 4 ++-- libraries/gpu/src/gpu/Shader.cpp | 2 +- libraries/gpu/src/gpu/Shader.h | 11 +++++++++-- libraries/gpu/src/gpu/null/NullBackend.h | 2 +- 15 files changed, 34 insertions(+), 27 deletions(-) diff --git a/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp b/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp index 2a052c5210..08bd20be66 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp +++ b/libraries/gpu-gl/src/gpu/gl/GLBackend.cpp @@ -68,7 +68,7 @@ GLBackend& getBackend() { return *INSTANCE; } -bool GLBackend::makeProgram(Shader& shader, const Shader::BindingSet& slotBindings, Shader::CompilationHandler handler) { +bool GLBackend::makeProgram(Shader& shader, const Shader::BindingSet& slotBindings, const Shader::CompilationHandler& handler) { return GLShader::makeProgram(getBackend(), shader, slotBindings, handler); } diff --git a/libraries/gpu-gl/src/gpu/gl/GLBackend.h b/libraries/gpu-gl/src/gpu/gl/GLBackend.h index f0b74803e4..18916ac18c 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLBackend.h +++ b/libraries/gpu-gl/src/gpu/gl/GLBackend.h @@ -64,7 +64,7 @@ protected: explicit GLBackend(bool syncCache); GLBackend(); public: - static bool makeProgram(Shader& shader, const Shader::BindingSet& slotBindings, Shader::CompilationHandler handler); + static bool makeProgram(Shader& shader, const Shader::BindingSet& slotBindings, const Shader::CompilationHandler& handler); virtual ~GLBackend(); @@ -423,8 +423,8 @@ protected: } _pipeline; // Backend dependant compilation of the shader - virtual GLShader* compileBackendProgram(const Shader& program, Shader::CompilationHandler handler); - virtual GLShader* compileBackendShader(const Shader& shader, Shader::CompilationHandler handler); + virtual GLShader* compileBackendProgram(const Shader& program, const Shader::CompilationHandler& handler); + virtual GLShader* compileBackendShader(const Shader& shader, const Shader::CompilationHandler& handler); virtual std::string getBackendShaderHeader() const; virtual void makeProgramBindings(ShaderObject& shaderObject); class ElementResource { diff --git a/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp b/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp index 42e95ba6c6..93c9b0d2ff 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp +++ b/libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp @@ -56,7 +56,7 @@ static const std::array VERSION_DEFINES { { stereoVersion } }; -GLShader* GLBackend::compileBackendShader(const Shader& shader, Shader::CompilationHandler handler) { +GLShader* GLBackend::compileBackendShader(const Shader& shader, const Shader::CompilationHandler& handler) { // Any GLSLprogram ? normally yes... const std::string& shaderSource = shader.getSource().getCode(); GLenum shaderDomain = SHADER_DOMAINS[shader.getType()]; @@ -105,7 +105,7 @@ GLShader* GLBackend::compileBackendShader(const Shader& shader, Shader::Compilat return object; } -GLShader* GLBackend::compileBackendProgram(const Shader& program, Shader::CompilationHandler handler) { +GLShader* GLBackend::compileBackendProgram(const Shader& program, const Shader::CompilationHandler& handler) { if (!program.isProgram()) { return nullptr; } diff --git a/libraries/gpu-gl/src/gpu/gl/GLShader.cpp b/libraries/gpu-gl/src/gpu/gl/GLShader.cpp index 42d4fe3845..010a7c479c 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLShader.cpp +++ b/libraries/gpu-gl/src/gpu/gl/GLShader.cpp @@ -30,7 +30,7 @@ GLShader::~GLShader() { } } -GLShader* GLShader::sync(GLBackend& backend, const Shader& shader, Shader::CompilationHandler handler) { +GLShader* GLShader::sync(GLBackend& backend, const Shader& shader, const Shader::CompilationHandler& handler) { GLShader* object = Backend::getGPUObject(shader); // If GPU object already created then good @@ -56,7 +56,7 @@ GLShader* GLShader::sync(GLBackend& backend, const Shader& shader, Shader::Compi return object; } -bool GLShader::makeProgram(GLBackend& backend, Shader& shader, const Shader::BindingSet& slotBindings, Shader::CompilationHandler handler) { +bool GLShader::makeProgram(GLBackend& backend, Shader& shader, const Shader::BindingSet& slotBindings, const Shader::CompilationHandler& handler) { // First make sure the Shader has been compiled GLShader* object = sync(backend, shader, handler); diff --git a/libraries/gpu-gl/src/gpu/gl/GLShader.h b/libraries/gpu-gl/src/gpu/gl/GLShader.h index 8625b3e64a..3259982e93 100644 --- a/libraries/gpu-gl/src/gpu/gl/GLShader.h +++ b/libraries/gpu-gl/src/gpu/gl/GLShader.h @@ -21,8 +21,8 @@ struct ShaderObject { class GLShader : public GPUObject { public: - static GLShader* sync(GLBackend& backend, const Shader& shader, Shader::CompilationHandler handler = nullptr); - static bool makeProgram(GLBackend& backend, Shader& shader, const Shader::BindingSet& slotBindings, Shader::CompilationHandler handler); + static GLShader* sync(GLBackend& backend, const Shader& shader, const Shader::CompilationHandler& handler = nullptr); + static bool makeProgram(GLBackend& backend, Shader& shader, const Shader::BindingSet& slotBindings, const Shader::CompilationHandler& handler); enum Version { Mono = 0, diff --git a/libraries/gpu-gles/src/gpu/gl/GLBackend.cpp b/libraries/gpu-gles/src/gpu/gl/GLBackend.cpp index 8a118b7b71..fc1bc39929 100644 --- a/libraries/gpu-gles/src/gpu/gl/GLBackend.cpp +++ b/libraries/gpu-gles/src/gpu/gl/GLBackend.cpp @@ -61,7 +61,7 @@ GLBackend& getBackend() { return *INSTANCE; } -bool GLBackend::makeProgram(Shader& shader, const Shader::BindingSet& slotBindings, Shader::CompilationHandler handler) { +bool GLBackend::makeProgram(Shader& shader, const Shader::BindingSet& slotBindings, const Shader::CompilationHandler& handler) { return GLShader::makeProgram(getBackend(), shader, slotBindings, handler); } diff --git a/libraries/gpu-gles/src/gpu/gl/GLBackend.h b/libraries/gpu-gles/src/gpu/gl/GLBackend.h index 0db46985f7..3681fc0492 100644 --- a/libraries/gpu-gles/src/gpu/gl/GLBackend.h +++ b/libraries/gpu-gles/src/gpu/gl/GLBackend.h @@ -61,7 +61,7 @@ protected: explicit GLBackend(bool syncCache); GLBackend(); public: - static bool makeProgram(Shader& shader, const Shader::BindingSet& slotBindings = Shader::BindingSet(), Shader::CompilationHandler handler = nullptr); + static bool makeProgram(Shader& shader, const Shader::BindingSet& slotBindings = Shader::BindingSet(), const Shader::CompilationHandler& handler = nullptr); virtual ~GLBackend(); @@ -420,8 +420,8 @@ protected: } _pipeline; // Backend dependant compilation of the shader - virtual GLShader* compileBackendProgram(const Shader& program, Shader::CompilationHandler handler); - virtual GLShader* compileBackendShader(const Shader& shader, Shader::CompilationHandler handler); + virtual GLShader* compileBackendProgram(const Shader& program, const Shader::CompilationHandler& handler); + virtual GLShader* compileBackendShader(const Shader& shader, const Shader::CompilationHandler& handler); virtual std::string getBackendShaderHeader() const; virtual void makeProgramBindings(ShaderObject& shaderObject); class ElementResource { diff --git a/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp b/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp index b799fb0037..677bba97ca 100644 --- a/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp +++ b/libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp @@ -56,7 +56,7 @@ static const std::array VERSION_DEFINES { { stereoVersion } }; -GLShader* GLBackend::compileBackendShader(const Shader& shader, Shader::CompilationHandler handler) { +GLShader* GLBackend::compileBackendShader(const Shader& shader, const Shader::CompilationHandler& handler) { // Any GLSLprogram ? normally yes... const std::string& shaderSource = shader.getSource().getCode(); GLenum shaderDomain = SHADER_DOMAINS[shader.getType()]; @@ -108,7 +108,7 @@ GLShader* GLBackend::compileBackendShader(const Shader& shader, Shader::Compilat return object; } -GLShader* GLBackend::compileBackendProgram(const Shader& program, Shader::CompilationHandler handler) { +GLShader* GLBackend::compileBackendProgram(const Shader& program, const Shader::CompilationHandler& handler) { if (!program.isProgram()) { return nullptr; } diff --git a/libraries/gpu-gles/src/gpu/gl/GLShader.cpp b/libraries/gpu-gles/src/gpu/gl/GLShader.cpp index 42d4fe3845..010a7c479c 100644 --- a/libraries/gpu-gles/src/gpu/gl/GLShader.cpp +++ b/libraries/gpu-gles/src/gpu/gl/GLShader.cpp @@ -30,7 +30,7 @@ GLShader::~GLShader() { } } -GLShader* GLShader::sync(GLBackend& backend, const Shader& shader, Shader::CompilationHandler handler) { +GLShader* GLShader::sync(GLBackend& backend, const Shader& shader, const Shader::CompilationHandler& handler) { GLShader* object = Backend::getGPUObject(shader); // If GPU object already created then good @@ -56,7 +56,7 @@ GLShader* GLShader::sync(GLBackend& backend, const Shader& shader, Shader::Compi return object; } -bool GLShader::makeProgram(GLBackend& backend, Shader& shader, const Shader::BindingSet& slotBindings, Shader::CompilationHandler handler) { +bool GLShader::makeProgram(GLBackend& backend, Shader& shader, const Shader::BindingSet& slotBindings, const Shader::CompilationHandler& handler) { // First make sure the Shader has been compiled GLShader* object = sync(backend, shader, handler); diff --git a/libraries/gpu-gles/src/gpu/gl/GLShader.h b/libraries/gpu-gles/src/gpu/gl/GLShader.h index 42d63f8dfb..f2a144a81c 100644 --- a/libraries/gpu-gles/src/gpu/gl/GLShader.h +++ b/libraries/gpu-gles/src/gpu/gl/GLShader.h @@ -21,8 +21,8 @@ struct ShaderObject { class GLShader : public GPUObject { public: - static GLShader* sync(GLBackend& backend, const Shader& shader, Shader::CompilationHandler handler = nullptr); - static bool makeProgram(GLBackend& backend, Shader& shader, const Shader::BindingSet& slotBindings, Shader::CompilationHandler handler = nullptr); + static GLShader* sync(GLBackend& backend, const Shader& shader, const Shader::CompilationHandler& handler = nullptr); + static bool makeProgram(GLBackend& backend, Shader& shader, const Shader::BindingSet& slotBindings, const Shader::CompilationHandler& handler = nullptr); enum Version { Mono = 0, diff --git a/libraries/gpu/src/gpu/Context.cpp b/libraries/gpu/src/gpu/Context.cpp index 2399d3ddc3..d7d86c3ef7 100644 --- a/libraries/gpu/src/gpu/Context.cpp +++ b/libraries/gpu/src/gpu/Context.cpp @@ -127,7 +127,7 @@ void Context::executeFrame(const FramePointer& frame) const { _frameStats.evalDelta(beginStats, endStats); } -bool Context::makeProgram(Shader& shader, const Shader::BindingSet& bindings, Shader::CompilationHandler handler) { +bool Context::makeProgram(Shader& shader, const Shader::BindingSet& bindings, const Shader::CompilationHandler& handler) { // If we're running in another DLL context, we need to fetch the program callback out of the application // FIXME find a way to do this without reliance on Qt app properties if (!_makeProgramCallback) { diff --git a/libraries/gpu/src/gpu/Context.h b/libraries/gpu/src/gpu/Context.h index e22cc57570..195565f438 100644 --- a/libraries/gpu/src/gpu/Context.h +++ b/libraries/gpu/src/gpu/Context.h @@ -143,7 +143,7 @@ class Context { public: using Size = Resource::Size; typedef BackendPointer (*CreateBackend)(); - typedef bool (*MakeProgram)(Shader& shader, const Shader::BindingSet& bindings, Shader::CompilationHandler handler); + typedef bool (*MakeProgram)(Shader& shader, const Shader::BindingSet& bindings, const Shader::CompilationHandler& handler); // This one call must happen before any context is created or used (Shader::MakeProgram) in order to setup the Backend and any singleton data needed @@ -262,7 +262,7 @@ protected: // makeProgramShader(...) make a program shader ready to be used in a Batch. // It compiles the sub shaders, link them and defines the Slots and their bindings. // If the shader passed is not a program, nothing happens. - static bool makeProgram(Shader& shader, const Shader::BindingSet& bindings, Shader::CompilationHandler handler); + static bool makeProgram(Shader& shader, const Shader::BindingSet& bindings, const Shader::CompilationHandler& handler); static CreateBackend _createBackendCallback; static MakeProgram _makeProgramCallback; diff --git a/libraries/gpu/src/gpu/Shader.cpp b/libraries/gpu/src/gpu/Shader.cpp index e428c758b1..aa7898569b 100755 --- a/libraries/gpu/src/gpu/Shader.cpp +++ b/libraries/gpu/src/gpu/Shader.cpp @@ -134,7 +134,7 @@ void Shader::defineSlots(const SlotSet& uniforms, const SlotSet& uniformBuffers, _outputs = outputs; } -bool Shader::makeProgram(Shader& shader, const Shader::BindingSet& bindings, CompilationHandler handler) { +bool Shader::makeProgram(Shader& shader, const Shader::BindingSet& bindings, const CompilationHandler& handler) { if (shader.isProgram()) { return Context::makeProgram(shader, bindings, handler); } diff --git a/libraries/gpu/src/gpu/Shader.h b/libraries/gpu/src/gpu/Shader.h index 07800fa14a..4504337789 100755 --- a/libraries/gpu/src/gpu/Shader.h +++ b/libraries/gpu/src/gpu/Shader.h @@ -175,7 +175,14 @@ public: const SlotSet& inputs, const SlotSet& outputs); - typedef bool(*CompilationHandler)(const Shader& shader, const std::string& src, CompilationLog& log, std::string& newSrc); + // Compilation Handler can be passed while compiling a shader (in the makeProgram call) to be able to give the hand to + // the caller thread if the comilation fails and to prvide a different version of the source for it + // @param0 the Shader object that just failed to compile + // @param1 the original source code as submited to the compiler + // @param2 the compilation log containing the error message + // @param3 a new string ready to be filled with the new version of the source that could be proposed from the handler functor + // @return boolean true if the backend should keep trying to compile the shader with the new source returned or false to stop and fail that shader compilation + using CompilationHandler = std::function; // makeProgram(...) make a program shader ready to be used in a Batch. // It compiles the sub shaders, link them and defines the Slots and their bindings. @@ -190,7 +197,7 @@ public: // on a gl Context and the driver to compile the glsl shader. // Hoppefully in a few years the shader compilation will be completely abstracted in a separate shader compiler library // independant of the graphics api in use underneath (looking at you opengl & vulkan). - static bool makeProgram(Shader& shader, const Shader::BindingSet& bindings = Shader::BindingSet(), CompilationHandler handler = nullptr); + static bool makeProgram(Shader& shader, const Shader::BindingSet& bindings = Shader::BindingSet(), const CompilationHandler& handler = nullptr); // Check the compilation state bool compilationHasFailed() const { return _compilationHasFailed; } diff --git a/libraries/gpu/src/gpu/null/NullBackend.h b/libraries/gpu/src/gpu/null/NullBackend.h index abaa24812f..57b8fbafbc 100644 --- a/libraries/gpu/src/gpu/null/NullBackend.h +++ b/libraries/gpu/src/gpu/null/NullBackend.h @@ -28,7 +28,7 @@ class Backend : public gpu::Backend { friend class gpu::Context; static void init() {} static gpu::Backend* createBackend() { return new Backend(); } - static bool makeProgram(Shader& shader, const Shader::BindingSet& slotBindings, Shader::CompilationHandler handler) { return true; } + static bool makeProgram(Shader& shader, const Shader::BindingSet& slotBindings, const Shader::CompilationHandler& handler) { return true; } protected: explicit Backend(bool syncCache) : Parent() { }