Refactored GLSL patching for future extension.

Every patch would be its own class this would keep the data and logic contained.
When a source is unchained during patching it would reuse the original source.

3 files changed, 156 insertions, 75 deletions

diff --git a/source/blender/gpu/opengl/gl_shader.cc b/source/blender/gpu/opengl/gl_shader.cc
index 78544035416..9ad9ffebd00 100644
--- a/source/blender/gpu/opengl/gl_shader.cc
+++ b/source/blender/gpu/opengl/gl_shader.cc
@@ -149,7 +149,7 @@ GLuint GLShader::create_shader_stage(GLenum gl_stage, MutableSpan<const char *>
   /* Patch the shader code using the first source slot. */
   sources[0] = glsl_patch_get(gl_stage);
   converter_.patch(sources);
-  if (converter_.has_error()) {
+  if (converter_.has_errors()) {
     compilation_failed_ = true;
     return 0;
   }
diff --git a/source/blender/gpu/opengl/gl_shader_converter.cc b/source/blender/gpu/opengl/gl_shader_converter.cc
index c2f06b74e31..d055006f2e9 100644
--- a/source/blender/gpu/opengl/gl_shader_converter.cc
+++ b/source/blender/gpu/opengl/gl_shader_converter.cc
@@ -25,86 +25,169 @@
 
 #include "gl_shader_converter.hh"
 
-namespace blender::gpu {
+#include <optional>
 
-void GLShaderConverter::patch(MutableSpan<const char *> sources)
-{
-  for (int i = 0; i < sources.size(); i++) {
-    std::string patched_source = patch(sources[i]);
-    patched_sources_.append(patched_source);
-    sources[i] = patched_sources_.last().c_str();
-  }
-}
+#include "CLG_log.h"
 
-bool GLShaderConverter::has_error() const
-{
-  return status != GLShaderConverterState::Ok;
-}
+static CLG_LogRef LOG = {"gpu.gl.shader.converter"};
 
-void GLShaderConverter::free()
-{
-  patched_sources_.clear();
-}
+namespace blender::gpu {
 
-std::string GLShaderConverter::patch(StringRef src)
+static bool is_error_state(GLShaderConverterState state)
 {
-  std::string result = patch_push_constants(src);
-  return result;
+  return !ELEM(state, GLShaderConverterState::OkChanged, GLShaderConverterState::OkUnchanged);
 }
 
-StringRef GLShaderConverter::skip_whitespace(StringRef ref)
-{
-  static constexpr StringRef WHITESPACES = " \t\n\v\f\r";
-
-  size_t skip = ref.find_first_not_of(WHITESPACES);
-  if (skip == blender::StringRef::not_found) {
-    return ref;
+struct GLSLPatchResult {
+  std::optional<std::string> patched_glsl;
+  GLShaderConverterState state = GLShaderConverterState::OkUnchanged;
+
+  void merge(const GLSLPatchResult &other, const StringRef unchanged_result)
+  {
+    switch (other.state) {
+      case GLShaderConverterState::OkUnchanged:
+        patched_glsl = unchanged_result;
+        break;
+      case GLShaderConverterState::OkChanged:
+        patched_glsl = other.patched_glsl;
+        if (state == GLShaderConverterState::OkUnchanged) {
+          state = GLShaderConverterState::OkChanged;
+        }
+        break;
+      case GLShaderConverterState::MismatchedPushConstantNames:
+        state = other.state;
+        break;
+    }
   }
-  return ref.drop_prefix(skip);
-}
+};
 
-StringRef GLShaderConverter::extract_name(StringRef src)
-{
-  static constexpr StringRef VALID_CHARS =
+class GLSLPatch {
+ private:
+  static constexpr StringRef WHITESPACES = " \t\n\v\f\r";
+  static constexpr StringRef VALID_NAME_CHARS =
       "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ01232456789_";
-  StringRef result = src;
 
-  size_t skip = result.find_first_not_of(VALID_CHARS);
-  BLI_assert(skip != StringRef::not_found);
-  return result.substr(0, skip);
-}
+ public:
+  virtual GLSLPatchResult patch(PatchContext &context, StringRef source) = 0;
 
-std::string GLShaderConverter::patch_push_constants(StringRef src)
-{
+ protected:
+  static StringRef skip_whitespace(StringRef ref)
+  {
+    size_t skip = ref.find_first_not_of(WHITESPACES);
+    if (skip == blender::StringRef::not_found) {
+      return ref;
+    }
+    return ref.drop_prefix(skip);
+  }
+
+  static StringRef extract_name(StringRef src)
+  {
+    StringRef result = src;
+    size_t skip = result.find_first_not_of(VALID_NAME_CHARS);
+    BLI_assert(skip != StringRef::not_found);
+    return result.substr(0, skip);
+  }
+};
+
+class PatchPushConstants : public GLSLPatch {
   static constexpr StringRef LAYOUT_PUSH_CONSTANTS = "layout(push_constant)";
   static constexpr StringRef LAYOUT_STD140 = "layout(std140)";
 
-  size_t pos = src.find(LAYOUT_PUSH_CONSTANTS);
-  if (pos == StringRef::not_found) {
-    return src;
+ public:
+  GLSLPatchResult patch(PatchContext &context, StringRef source) override
+  {
+    GLSLPatchResult result;
+
+    size_t pos = source.find(LAYOUT_PUSH_CONSTANTS);
+    if (pos == StringRef::not_found) {
+      result.state = GLShaderConverterState::OkUnchanged;
+      return result;
+    }
+
+    std::stringstream patched_glsl;
+    patched_glsl << source.substr(0, pos);
+    patched_glsl << LAYOUT_STD140;
+    patched_glsl << source.substr(pos + LAYOUT_PUSH_CONSTANTS.size());
+
+    StringRef name = source.substr(pos + LAYOUT_PUSH_CONSTANTS.size());
+    name = skip_whitespace(name);
+    name = name.drop_known_prefix("uniform");
+    name = skip_whitespace(name);
+    name = extract_name(name);
+
+    if (context.push_constants.name.empty()) {
+      context.push_constants.name = name;
+    }
+    else if (context.push_constants.name != name) {
+      CLOG_ERROR(&LOG,
+                 "Detected different push_constants binding names ('%s' and '%s'). push_constants "
+                 "binding names must be identical across all stages.",
+                 context.push_constants.name.c_str(),
+                 std::string(name).c_str());
+      result.state = GLShaderConverterState::MismatchedPushConstantNames;
+      return result;
+    }
+
+    std::string patched_glsl_str = patched_glsl.str();
+    result.state = GLShaderConverterState::OkChanged;
+    GLSLPatchResult recursive_result = patch(context, patched_glsl_str);
+    result.merge(recursive_result, patched_glsl_str);
+    return result;
+  };
+};
+
+class GLSLPatcher : public GLSLPatch {
+ private:
+  static void patch(PatchContext &context,
+                    GLSLPatch &patch,
+                    StringRef source,
+                    GLSLPatchResult &r_result)
+  {
+    /* Do not patch when result is in error so the error state won't be rewritten. */
+    if (is_error_state(r_result.state)) {
+      return;
+    }
+
+    GLSLPatchResult patch_result = patch.patch(context, source);
+    r_result.merge(patch_result, source);
   }
-  std::stringstream result;
-  result << src.substr(0, pos);
-  result << LAYOUT_STD140;
-  result << src.substr(pos + LAYOUT_PUSH_CONSTANTS.size());
-
-  StringRef name = src.substr(pos + LAYOUT_PUSH_CONSTANTS.size());
-  name = skip_whitespace(name);
-  name = name.drop_known_prefix("uniform");
-  name = skip_whitespace(name);
-  name = extract_name(name);
-
-  if (push_constants.name.empty()) {
-    push_constants.name = name;
+
+ public:
+  GLSLPatchResult patch(PatchContext &context, StringRef source) override
+  {
+    GLSLPatchResult result;
+    PatchPushConstants push_constants;
+    patch(context, push_constants, source, result);
+    return result;
   }
-  else {
-    /* Push constant name must be the same across all stages. */
-    if (push_constants.name != name) {
-      status = GLShaderConverterState::NOT_MATCHING_PUSH_CONSTANT_NAME;
+};
+
+void GLShaderConverter::patch(MutableSpan<const char *> sources)
+{
+  for (int i = 0; i < sources.size(); i++) {
+    GLSLPatcher patcher;
+    const char *source = sources[i];
+    GLSLPatchResult patch_result = patcher.patch(context_, source);
+    if (is_error_state(patch_result.state)) {
+      state = patch_result.state;
+      return;
+    }
+    if (patch_result.state == GLShaderConverterState::OkChanged) {
+      BLI_assert(patch_result.patched_glsl);
+      patched_sources_.append(*patch_result.patched_glsl);
+      sources[i] = patched_sources_.last().c_str();
     }
   }
+}
 
-  return patch_push_constants(result.str());
+bool GLShaderConverter::has_errors() const
+{
+  return is_error_state(state);
+}
+
+void GLShaderConverter::free()
+{
+  patched_sources_.clear();
 }
 
 }  // namespace blender::gpu
diff --git a/source/blender/gpu/opengl/gl_shader_converter.hh b/source/blender/gpu/opengl/gl_shader_converter.hh
index 0ba58cccf4c..84826fee38a 100644
--- a/source/blender/gpu/opengl/gl_shader_converter.hh
+++ b/source/blender/gpu/opengl/gl_shader_converter.hh
@@ -29,32 +29,30 @@
 namespace blender::gpu {
 
 enum class GLShaderConverterState {
-  Ok,
-  NOT_MATCHING_PUSH_CONSTANT_NAME,
+  OkUnchanged,
+  OkChanged,
+  MismatchedPushConstantNames,
 };
-class GLShaderConverter {
- public:
+
+struct PatchContext {
   struct {
     std::string name;
   } push_constants;
-  GLShaderConverterState status = GLShaderConverterState::Ok;
+};
+
+class GLShaderConverter {
+ public:
+  GLShaderConverterState state = GLShaderConverterState::OkUnchanged;
 
  private:
   Vector<std::string> patched_sources_;
+  PatchContext context_;
 
  public:
   void patch(MutableSpan<const char *> sources);
-  bool has_error() const;
+  bool has_errors() const;
   void free();
 
- private:
-  std::string patch(StringRef src);
-  bool is_valid_name_char(const char c) const;
-  StringRef skip_whitespace(StringRef src);
-
-  StringRef extract_name(StringRef src);
-  std::string patch_push_constants(StringRef src);
-
   MEM_CXX_CLASS_ALLOC_FUNCS("GLShaderConverter");
 };