From 1b942ef90c9b4b7fe98af50357aa76b74754f7ff Mon Sep 17 00:00:00 2001 From: Jeroen Bakker Date: Mon, 28 Jun 2021 12:20:59 +0200 Subject: 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. --- source/blender/gpu/CMakeLists.txt | 1 + source/blender/gpu/intern/gpu_shader_log.cc | 139 +++++++++--------------- source/blender/gpu/intern/gpu_shader_private.hh | 39 ++++++- source/blender/gpu/opengl/gl_shader.cc | 12 +- source/blender/gpu/opengl/gl_shader.hh | 11 ++ source/blender/gpu/opengl/gl_shader_log.cc | 87 +++++++++++++++ 6 files changed, 197 insertions(+), 92 deletions(-) create mode 100644 source/blender/gpu/opengl/gl_shader_log.cc (limited to 'source') 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 sources, char *log, const char *stage, const bool error) +void Shader::print_log(Span 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 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 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 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 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 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 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 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 sources, char *log, const char *stage, const bool error); + void print_log(Span 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(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 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 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 -- cgit v1.2.3