GPU: Refactored +cleanup compilation log parsing.

Old implementation has a single parser of many different
formats. With the introduction of Vulkan this would lead
to another parser in the same function. This patch
separates the log parsing using a visitor pattern so the
log parsing can be configured per GPU backend or even
per driver.

With Vulkan we manage the compiler our self so the parsing
will become more straight forward. The OpenGL part depends
on many factors (OS, Driver) and perhaps even GPU.

6 files changed, 197 insertions, 92 deletions

diff --git a/source/blender/gpu/CMakeLists.txt b/source/blender/gpu/CMakeLists.txt
index 87e3b5cd733..abb7330d292 100644
--- a/source/blender/gpu/CMakeLists.txt
+++ b/source/blender/gpu/CMakeLists.txt
@@ -103,6 +103,7 @@ set(SRC
   opengl/gl_index_buffer.cc
   opengl/gl_query.cc
   opengl/gl_shader.cc
+  opengl/gl_shader_log.cc
   opengl/gl_shader_interface.cc
   opengl/gl_state.cc
   opengl/gl_texture.cc
diff --git a/source/blender/gpu/intern/gpu_shader_log.cc b/source/blender/gpu/intern/gpu_shader_log.cc
index cde7f2763d1..5859afb7fbf 100644
--- a/source/blender/gpu/intern/gpu_shader_log.cc
+++ b/source/blender/gpu/intern/gpu_shader_log.cc
@@ -13,7 +13,7 @@
  * along with this program; if not, write to the Free Software Foundation,
  * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
  *
- * The Original Code is Copyright (C) 2005 Blender Foundation.
+ * The Original Code is Copyright (C) 2021 Blender Foundation.
  * All rights reserved.
  */
 
@@ -41,7 +41,11 @@ namespace blender::gpu {
 /** \name Debug functions
  * \{ */
 
-void Shader::print_log(Span<const char *> sources, char *log, const char *stage, const bool error)
+void Shader::print_log(Span<const char *> sources,
+                       char *log,
+                       const char *stage,
+                       const bool error,
+                       GPULogParser *parser)
 {
   const char line_prefix[] = "      | ";
   char err_col[] = "\033[31;1m";
@@ -58,8 +62,9 @@ void Shader::print_log(Span<const char *> sources, char *log, const char *stage,
   BLI_dynstr_appendf(dynstr, "\n");
 
   char *log_line = log, *line_end;
-  char *error_line_number_end;
-  int error_line, error_char, last_error_line = -2, last_error_char = -1;
+
+  LogCursor previous_location;
+
   bool found_line_id = false;
   while ((line_end = strchr(log_line, '\n'))) {
     /* Skip empty lines. */
@@ -67,82 +72,33 @@ void Shader::print_log(Span<const char *> sources, char *log, const char *stage,
       log_line++;
       continue;
     }
-    /* 0 = error, 1 = warning. */
-    int type = -1;
-    /* Skip ERROR: or WARNING:. */
-    const char *prefix[] = {"ERROR", "WARNING"};
-    for (int i = 0; i < ARRAY_SIZE(prefix); i++) {
-      if (STREQLEN(log_line, prefix[i], strlen(prefix[i]))) {
-        log_line += strlen(prefix[i]);
-        type = i;
-        break;
-      }
-    }
-    /* Skip whitespaces and separators. */
-    while (ELEM(log_line[0], ':', '(', ' ')) {
-      log_line++;
-    }
-    /* Parse error line & char numbers. */
-    error_line = error_char = -1;
-    if (log_line[0] >= '0' && log_line[0] <= '9') {
-      error_line = (int)strtol(log_line, &error_line_number_end, 10);
-      /* Try to fetch the error character (not always available). */
-      if (ELEM(error_line_number_end[0], '(', ':') && error_line_number_end[1] != ' ') {
-        error_char = (int)strtol(error_line_number_end + 1, &log_line, 10);
-      }
-      else {
-        log_line = error_line_number_end;
-      }
-      /* There can be a 3rd number (case of mesa driver). */
-      if (ELEM(log_line[0], '(', ':') && log_line[1] >= '0' && log_line[1] <= '9') {
-        error_line = error_char;
-        error_char = (int)strtol(log_line + 1, &error_line_number_end, 10);
-        log_line = error_line_number_end;
-      }
-    }
-    /* Skip whitespaces and separators. */
-    while (ELEM(log_line[0], ':', ')', ' ')) {
-      log_line++;
-    }
-    if (error_line == -1) {
+
+    GPULogItem log_item;
+    log_line = parser->parse_line(log_line, log_item);
+
+    if (log_item.cursor.row == -1) {
       found_line_id = false;
     }
+
     const char *src_line = sources_combined;
-    if ((error_line != -1) && (error_char != -1)) {
-      if (GPU_type_matches(GPU_DEVICE_ATI, GPU_OS_UNIX, GPU_DRIVER_OFFICIAL)) {
-        /* source:line */
-        int error_source = error_line;
-        if (error_source < sources.size()) {
-          src_line = sources[error_source];
-          error_line = error_char;
-          error_char = -1;
-        }
-      }
-      else if (GPU_type_matches(GPU_DEVICE_NVIDIA, GPU_OS_ANY, GPU_DRIVER_OFFICIAL) ||
-               GPU_type_matches(GPU_DEVICE_INTEL, GPU_OS_MAC, GPU_DRIVER_OFFICIAL)) {
-        /* 0:line */
-        error_line = error_char;
-        error_char = -1;
-      }
-      else {
-        /* line:char */
-      }
-    }
+
     /* Separate from previous block. */
-    if (last_error_line != error_line) {
+    if (previous_location.source != log_item.cursor.source ||
+        previous_location.row != log_item.cursor.row) {
       BLI_dynstr_appendf(dynstr, "%s%s%s\n", info_col, line_prefix, reset_col);
     }
-    else if (error_char != last_error_char) {
+    else if (log_item.cursor.column != previous_location.column) {
       BLI_dynstr_appendf(dynstr, "%s\n", line_prefix);
     }
     /* Print line from the source file that is producing the error. */
-    if ((error_line != -1) && (error_line != last_error_line || error_char != last_error_char)) {
+    if ((log_item.cursor.row != -1) && (log_item.cursor.row != previous_location.row ||
+                                        log_item.cursor.column != previous_location.column)) {
       const char *src_line_end;
       found_line_id = false;
       /* error_line is 1 based in this case. */
       int src_line_index = 1;
       while ((src_line_end = strchr(src_line, '\n'))) {
-        if (src_line_index == error_line) {
+        if (src_line_index == log_item.cursor.row) {
           found_line_id = true;
           break;
         }
@@ -157,7 +113,7 @@ void Shader::print_log(Span<const char *> sources, char *log, const char *stage,
       }
       /* Print error source. */
       if (found_line_id) {
-        if (error_line != last_error_line) {
+        if (log_item.cursor.row != previous_location.row) {
           BLI_dynstr_appendf(dynstr, "%5d | ", src_line_index);
         }
         else {
@@ -166,8 +122,8 @@ void Shader::print_log(Span<const char *> sources, char *log, const char *stage,
         BLI_dynstr_nappend(dynstr, src_line, (src_line_end + 1) - src_line);
         /* Print char offset. */
         BLI_dynstr_appendf(dynstr, line_prefix);
-        if (error_char != -1) {
-          for (int i = 0; i < error_char; i++) {
+        if (log_item.cursor.column != -1) {
+          for (int i = 0; i < log_item.cursor.column; i++) {
             BLI_dynstr_appendf(dynstr, " ");
           }
           BLI_dynstr_appendf(dynstr, "^");
@@ -176,23 +132,11 @@ void Shader::print_log(Span<const char *> sources, char *log, const char *stage,
       }
     }
     BLI_dynstr_appendf(dynstr, line_prefix);
-    /* Skip to message. Avoid redundant info. */
-    const char *keywords[] = {"error", "warning"};
-    for (int i = 0; i < ARRAY_SIZE(prefix); i++) {
-      if (STREQLEN(log_line, keywords[i], strlen(keywords[i]))) {
-        log_line += strlen(keywords[i]);
-        type = i;
-        break;
-      }
-    }
-    /* Skip and separators. */
-    while (ELEM(log_line[0], ':', ')')) {
-      log_line++;
-    }
-    if (type == 0) {
+
+    if (log_item.severity == Severity::Error) {
       BLI_dynstr_appendf(dynstr, "%s%s%s: ", err_col, "Error", info_col);
     }
-    else if (type == 1) {
+    else if (log_item.severity == Severity::Error) {
       BLI_dynstr_appendf(dynstr, "%s%s%s: ", warn_col, "Warning", info_col);
     }
     /* Print the error itself. */
@@ -201,8 +145,7 @@ void Shader::print_log(Span<const char *> sources, char *log, const char *stage,
     BLI_dynstr_append(dynstr, reset_col);
     /* Continue to next line. */
     log_line = line_end + 1;
-    last_error_line = error_line;
-    last_error_char = error_char;
+    previous_location = log_item.cursor;
   }
   MEM_freeN(sources_combined);
 
@@ -218,6 +161,30 @@ void Shader::print_log(Span<const char *> sources, char *log, const char *stage,
   BLI_dynstr_free(dynstr);
 }
 
+char *GPULogParser::skip_severity(char *log_line,
+                                  GPULogItem &log_item,
+                                  const char *error_msg,
+                                  const char *warning_msg) const
+{
+  if (STREQLEN(log_line, error_msg, strlen(error_msg))) {
+    log_line += strlen(error_msg);
+    log_item.severity = Severity::Error;
+  }
+  else if (STREQLEN(log_line, warning_msg, strlen(warning_msg))) {
+    log_line += strlen(warning_msg);
+    log_item.severity = Severity::Warning;
+  }
+  return log_line;
+}
+
+char *GPULogParser::skip_separators(char *log_line, char sep1, char sep2, char sep3) const
+{
+  while (ELEM(log_line[0], sep1, sep2, sep3)) {
+    log_line++;
+  }
+  return log_line;
+}
+
 /** \} */
 
 }  // namespace blender::gpu
diff --git a/source/blender/gpu/intern/gpu_shader_private.hh b/source/blender/gpu/intern/gpu_shader_private.hh
index 281f01dbc22..ebdfc3478f8 100644
--- a/source/blender/gpu/intern/gpu_shader_private.hh
+++ b/source/blender/gpu/intern/gpu_shader_private.hh
@@ -29,6 +29,8 @@
 namespace blender {
 namespace gpu {
 
+class GPULogParser;
+
 /**
  * Implementation of shader compilation and uniforms handling.
  * Base class which is then specialized for each implementation (GL, VK, ...).
@@ -74,7 +76,11 @@ class Shader {
   };
 
  protected:
-  void print_log(Span<const char *> sources, char *log, const char *stage, const bool error);
+  void print_log(Span<const char *> sources,
+                 char *log,
+                 const char *stage,
+                 const bool error,
+                 GPULogParser *parser);
 };
 
 /* Syntactic sugar. */
@@ -91,6 +97,37 @@ static inline const Shader *unwrap(const GPUShader *vert)
   return reinterpret_cast<const Shader *>(vert);
 }
 
+enum class Severity {
+  Unknown,
+  Warning,
+  Error,
+};
+
+struct LogCursor {
+  int source = -1;
+  int row = -1;
+  int column = -1;
+};
+
+struct GPULogItem {
+  LogCursor cursor;
+  Severity severity = Severity::Unknown;
+};
+
+class GPULogParser {
+ public:
+  virtual char *parse_line(char *log_line, GPULogItem &log_item) = 0;
+
+ protected:
+  char *skip_severity(char *log_line,
+                      GPULogItem &log_item,
+                      const char *error_msg,
+                      const char *warning_msg) const;
+  char *skip_separators(char *log_line, char sep1, char sep2, char sep3) const;
+
+  MEM_CXX_CLASS_ALLOC_FUNCS("GPULogParser");
+};
+
 }  // namespace gpu
 }  // namespace blender
 
diff --git a/source/blender/gpu/opengl/gl_shader.cc b/source/blender/gpu/opengl/gl_shader.cc
index e77347d99eb..66a1bd5ceb7 100644
--- a/source/blender/gpu/opengl/gl_shader.cc
+++ b/source/blender/gpu/opengl/gl_shader.cc
@@ -158,18 +158,19 @@ GLuint GLShader::create_shader_stage(GLenum gl_stage, MutableSpan<const char *>
     char log[5000] = "";
     glGetShaderInfoLog(shader, sizeof(log), nullptr, log);
     if (log[0] != '\0') {
+      GLLogParser parser;
       switch (gl_stage) {
         case GL_VERTEX_SHADER:
-          this->print_log(sources, log, "VertShader", !status);
+          this->print_log(sources, log, "VertShader", !status, &parser);
           break;
         case GL_GEOMETRY_SHADER:
-          this->print_log(sources, log, "GeomShader", !status);
+          this->print_log(sources, log, "GeomShader", !status, &parser);
           break;
         case GL_FRAGMENT_SHADER:
-          this->print_log(sources, log, "FragShader", !status);
+          this->print_log(sources, log, "FragShader", !status, &parser);
           break;
         case GL_COMPUTE_SHADER:
-          this->print_log(sources, log, "ComputeShader", !status);
+          this->print_log(sources, log, "ComputeShader", !status, &parser);
           break;
       }
     }
@@ -220,7 +221,8 @@ bool GLShader::finalize()
     char log[5000];
     glGetProgramInfoLog(shader_program_, sizeof(log), nullptr, log);
     Span<const char *> sources;
-    this->print_log(sources, log, "Linking", true);
+    GLLogParser parser;
+    this->print_log(sources, log, "Linking", true, &parser);
     return false;
   }
 
diff --git a/source/blender/gpu/opengl/gl_shader.hh b/source/blender/gpu/opengl/gl_shader.hh
index 48aaaf2283d..770bc29747e 100644
--- a/source/blender/gpu/opengl/gl_shader.hh
+++ b/source/blender/gpu/opengl/gl_shader.hh
@@ -84,5 +84,16 @@ class GLShader : public Shader {
   MEM_CXX_CLASS_ALLOC_FUNCS("GLShader");
 };
 
+class GLLogParser : public GPULogParser {
+ public:
+  char *parse_line(char *log_line, GPULogItem &log_item) override;
+
+ protected:
+  char *skip_severity_prefix(char *log_line, GPULogItem &log_item);
+  char *skip_severity_keyword(char *log_line, GPULogItem &log_item);
+
+  MEM_CXX_CLASS_ALLOC_FUNCS("GLLogParser");
+};
+
 }  // namespace gpu
 }  // namespace blender
diff --git a/source/blender/gpu/opengl/gl_shader_log.cc b/source/blender/gpu/opengl/gl_shader_log.cc
new file mode 100644
index 00000000000..393f852b463
--- /dev/null
+++ b/source/blender/gpu/opengl/gl_shader_log.cc
@@ -0,0 +1,87 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ * The Original Code is Copyright (C) 2021 Blender Foundation.
+ * All rights reserved.
+ */
+
+/** \file
+ * \ingroup gpu
+ */
+
+#include "gl_shader.hh"
+
+#include "GPU_platform.h"
+
+namespace blender::gpu {
+
+char *GLLogParser::parse_line(char *log_line, GPULogItem &log_item)
+{
+  /* Skip ERROR: or WARNING:. */
+  log_line = skip_severity_prefix(log_line, log_item);
+  log_line = skip_separators(log_line, ':', '(', ' ');
+
+  /* Parse error line & char numbers. */
+  if (log_line[0] >= '0' && log_line[0] <= '9') {
+    char *error_line_number_end;
+    log_item.cursor.row = (int)strtol(log_line, &error_line_number_end, 10);
+    /* Try to fetch the error character (not always available). */
+    if (ELEM(error_line_number_end[0], '(', ':') && error_line_number_end[1] != ' ') {
+      log_item.cursor.column = (int)strtol(error_line_number_end + 1, &log_line, 10);
+    }
+    else {
+      log_line = error_line_number_end;
+    }
+    /* There can be a 3rd number (case of mesa driver). */
+    if (ELEM(log_line[0], '(', ':') && log_line[1] >= '0' && log_line[1] <= '9') {
+      log_item.cursor.source = log_item.cursor.row;
+      log_item.cursor.row = log_item.cursor.column;
+      log_item.cursor.column = (int)strtol(log_line + 1, &error_line_number_end, 10);
+      log_line = error_line_number_end;
+    }
+  }
+
+  if ((log_item.cursor.row != -1) && (log_item.cursor.column != -1)) {
+    if (GPU_type_matches(GPU_DEVICE_NVIDIA, GPU_OS_ANY, GPU_DRIVER_OFFICIAL) ||
+        GPU_type_matches(GPU_DEVICE_INTEL, GPU_OS_MAC, GPU_DRIVER_OFFICIAL)) {
+      /* 0:line */
+      log_item.cursor.row = log_item.cursor.column;
+      log_item.cursor.column = -1;
+    }
+    else {
+      /* line:char */
+    }
+  }
+
+  log_line = skip_separators(log_line, ':', ')', ' ');
+
+  /* Skip to message. Avoid redundant info. */
+  log_line = skip_severity_keyword(log_line, log_item);
+  log_line = skip_separators(log_line, ':', ')', ' ');
+
+  return log_line;
+}
+
+char *GLLogParser::skip_severity_prefix(char *log_line, GPULogItem &log_item)
+{
+  return skip_severity(log_line, log_item, "ERROR", "WARNING");
+}
+
+char *GLLogParser::skip_severity_keyword(char *log_line, GPULogItem &log_item)
+{
+  return skip_severity(log_line, log_item, "error", "warning");
+}
+
+}  // namespace blender::gpu