APplying review feedback

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);
 }
 

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 {

libraries/gpu-gl/src/gpu/gl/GLBackendShader.cpp

@@ -56,7 +56,7 @@ static const std::array<std::string, GLShader::NumVersions> 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;
     }

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<GLShader>(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);

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,

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);
 }
 

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 {

libraries/gpu-gles/src/gpu/gl/GLBackendShader.cpp

@@ -56,7 +56,7 @@ static const std::array<std::string, GLShader::NumVersions> 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;
     }

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<GLShader>(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);

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,

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) {

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;

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);
     }

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<bool (const Shader&, const std::string&, CompilationLog&, std::string&)>; 
 
     // 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; }

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() { }