From 5ac152a5f3885cb1ef66156f98c6a7ed687e933d Mon Sep 17 00:00:00 2001 From: jdgleaver Date: Mon, 12 Aug 2019 12:51:11 +0100 Subject: [PATCH] (glslang_util) Remove C++ 'isms' (where possible) --- gfx/drivers_shader/glslang_util.cpp | 301 +++++++++++++------- gfx/drivers_shader/glslang_util.h | 6 +- gfx/drivers_shader/slang_preprocess.cpp | 23 +- libretro-common/include/lists/string_list.h | 13 + libretro-common/include/string/stdstring.h | 22 ++ libretro-common/lists/string_list.c | 53 ++++ libretro-common/string/stdstring.c | 81 ++++++ 7 files changed, 388 insertions(+), 111 deletions(-) diff --git a/gfx/drivers_shader/glslang_util.cpp b/gfx/drivers_shader/glslang_util.cpp index 381cfb03c7..9dd86aade9 100644 --- a/gfx/drivers_shader/glslang_util.cpp +++ b/gfx/drivers_shader/glslang_util.cpp @@ -23,7 +23,6 @@ #include #include #include -#include #include #ifdef HAVE_CONFIG_H @@ -38,105 +37,134 @@ using namespace std; -bool glslang_read_shader_file(const char *path, vector *output, bool root_file) +static void get_include_file( + const char *line, char *include_file, size_t len) +{ + char *start = NULL; + char *end = NULL; + + start = (char*)strchr(line, '\"'); + + if (!start) + return; + + start++; + end = (char*)strchr(start, '\"'); + + if (!end) + return; + + *end = '\0'; + strlcpy(include_file, start, len); +} + +bool glslang_read_shader_file(const char *path, struct string_list *output, bool root_file) { - vector lines; - char include_path[PATH_MAX_LENGTH]; char tmp[PATH_MAX_LENGTH]; - char *ptr = NULL; - char *buf = nullptr; - int64_t len = 0; - const char *basename = path_basename(path); + union string_list_elem_attr attr; + size_t i; + const char *basename = NULL; + uint8_t *buf = NULL; + int64_t buf_len = 0; + struct string_list *lines = NULL; - include_path[0] = tmp[0] = '\0'; + tmp[0] = '\0'; + attr.i = 0; - if (!filestream_read_file(path, (void**)&buf, &len)) + /* Sanity check */ + if (string_is_empty(path) || !output) + return false; + + basename = path_basename(path); + + if (string_is_empty(basename)) + return false; + + /* Read file contents */ + if (!filestream_read_file(path, (void**)&buf, &buf_len)) { RARCH_ERR("Failed to open shader file: \"%s\".\n", path); return false; } - /* Remove Windows \r chars if we encounter them. - * filestream_read_file() allocates one extra for 0 terminator. */ - auto itr = remove_if(buf, buf + len + 1, [](char c) { - return c == '\r'; - }); - - if (itr < buf + len) - *itr = '\0'; - - /* Cannot use string_split since it removes blank lines (strtok). */ - ptr = buf; - - while (ptr && *ptr) + if (buf_len > 0) { - char *next_ptr = NULL; + /* Remove Windows '\r' chars if we encounter them */ + string_remove_all_chars((char*)buf, '\r'); - lines.push_back(ptr); - - next_ptr = strchr(ptr, '\n'); - - if (next_ptr) - { - ptr = next_ptr + 1; - *next_ptr = '\0'; - } - else - ptr = nullptr; + /* Split into lines + * (Blank lines must be included) */ + lines = string_separate((char*)buf, "\n"); } - if (lines.empty()) + /* Buffer is no longer required - clean up */ + if (buf) + free(buf); + + /* Sanity check */ + if (!lines) + return false; + + if (lines->size < 1) goto error; + /* If this is the 'parent' shader file, ensure that first + * line is a 'VERSION' string */ if (root_file) { - if (strstr(lines[0], "#version ") != lines[0]) + const char *line = lines->elems[0].data; + + if (strncmp("#version ", line, STRLEN_CONST("#version "))) { RARCH_ERR("First line of the shader must contain a valid #version string.\n"); goto error; } - output->push_back(lines[0]); + if (!string_list_append(output, line, attr)) + goto error; + /* Allows us to use #line to make dealing with shader errors easier. * This is supported by glslang, but since we always use glslang statically, * this is fine. */ - output->push_back("#extension GL_GOOGLE_cpp_style_line_directive : require"); + + if (!string_list_append(output, "#extension GL_GOOGLE_cpp_style_line_directive : require", attr)) + goto error; } /* At least VIM treats the first line as line #1, * so offset everything by one. */ snprintf(tmp, sizeof(tmp), "#line %u \"%s\"", root_file ? 2 : 1, basename); - output->push_back(tmp); + if (!string_list_append(output, tmp, attr)) + goto error; - for (size_t i = root_file ? 1 : 0; i < lines.size(); i++) + /* Loop through lines of file */ + for (i = root_file ? 1 : 0; i < lines->size; i++) { unsigned push_line = 0; - const char *line = lines[i]; - if (strstr(line, "#include ") == line) + const char *line = lines->elems[i].data; + + /* Check for 'include' statements */ + if (!strncmp("#include ", line, STRLEN_CONST("#include "))) { - char *closing = NULL; - char *c = (char*)strchr(line, '"'); + char include_file[PATH_MAX_LENGTH]; + char include_path[PATH_MAX_LENGTH]; - if (!c) + include_file[0] = '\0'; + include_path[0] = '\0'; + + /* Build include file path */ + get_include_file(line, include_file, sizeof(include_file)); + + if (string_is_empty(include_file)) { RARCH_ERR("Invalid include statement \"%s\".\n", line); goto error; } - c++; - - closing = (char*)strchr(c, '"'); - - if (!closing) - { - RARCH_ERR("Invalid include statement \"%s\".\n", line); - goto error; - } - - *closing = '\0'; - - fill_pathname_resolve_relative(include_path, path, c, sizeof(include_path)); + fill_pathname_resolve_relative( + include_path, path, include_file, sizeof(include_path)); + /* Parse include file */ if (!glslang_read_shader_file(include_path, output, false)) goto error; @@ -144,7 +172,8 @@ bool glslang_read_shader_file(const char *path, vector *output, bool roo * to pull it back to current file. */ push_line = 1; } - else if (strstr(line, "#endif") || strstr(line, "#pragma")) + else if (!strncmp("#endif", line, STRLEN_CONST("#endif")) || + !strncmp("#pragma", line, STRLEN_CONST("#pragma"))) { /* #line seems to be ignored if preprocessor tests fail, * so we should reapply #line after each #endif. @@ -152,52 +181,80 @@ bool glslang_read_shader_file(const char *path, vector *output, bool roo * for the line after this one. */ push_line = 2; - output->push_back(line); + if (!string_list_append(output, line, attr)) + goto error; } else - output->push_back(line); + if (!string_list_append(output, line, attr)) + goto error; if (push_line != 0) { snprintf(tmp, sizeof(tmp), "#line %u \"%s\"", unsigned(i + push_line), basename); - output->push_back(tmp); + if (!string_list_append(output, tmp, attr)) + goto error; } } - free(buf); + string_list_free(lines); + return true; error: - free(buf); + + if (lines) + string_list_free(lines); + return false; } -static string build_stage_source(const vector &lines, const char *stage) +static string build_stage_source(const struct string_list *lines, const char *stage) { + /* Note: since we have to return a std::string anyway, + * there is nothing to be gained from trying to replace + * this ostringstream with a C-based alternative + * (would require a rewrite of deps/glslang/glslang.cpp) */ ostringstream str; bool active = true; + size_t i; + + if (!lines) + return ""; + + if (lines->size < 1) + return ""; /* Version header. */ - str << lines.front(); + str << lines->elems[0].data;; str << '\n'; - for (auto itr = begin(lines) + 1; itr != end(lines); ++itr) + for (i = 1; i < lines->size; i++) { - if (itr->find("#pragma stage ") == 0) + const char *line = lines->elems[i].data; + + /* Identify 'stage' (fragment/vertex) */ + if (!strncmp("#pragma stage ", line, STRLEN_CONST("#pragma stage "))) { - if (stage) + if (!string_is_empty(stage)) { - auto expected = string("#pragma stage ") + stage; - active = itr->find(expected) != string::npos; + char expected[128]; + + expected[0] = '\0'; + + strlcpy(expected, "#pragma stage ", sizeof(expected)); + strlcat(expected, stage, sizeof(expected)); + + active = strcmp(expected, line) == 0; } } - else if (itr->find("#pragma name ") == 0 || - itr->find("#pragma format ") == 0) + else if (!strncmp("#pragma name ", line, STRLEN_CONST("#pragma name ")) || + !strncmp("#pragma format ", line, STRLEN_CONST("#pragma format "))) { /* Ignore */ } else if (active) - str << *itr; + str << line; + str << '\n'; } @@ -288,20 +345,26 @@ static glslang_format glslang_find_format(const char *fmt) return SLANG_FORMAT_UNKNOWN; } -bool glslang_parse_meta(const vector &lines, glslang_meta *meta) +bool glslang_parse_meta(const struct string_list *lines, glslang_meta *meta) { char id[64]; char desc[64]; + size_t i; - id[0] = desc[0] = '\0'; + id[0] = '\0'; + desc[0] = '\0'; - *meta = glslang_meta{}; + if (!lines) + return false; - for (auto &line : lines) + *meta = glslang_meta{}; + + for (i = 0; i < lines->size; i++) { - const char *line_c = line.c_str(); + const char *line = lines->elems[i].data; - if (line.find("#pragma name ") == 0) + /* Check for shader identifier */ + if (!strncmp("#pragma name ", line, STRLEN_CONST("#pragma name "))) { const char *str = NULL; @@ -311,16 +374,18 @@ bool glslang_parse_meta(const vector &lines, glslang_meta *meta) return false; } - str = line_c + STRLEN_CONST("#pragma name "); - + str = line + STRLEN_CONST("#pragma name "); while (*str == ' ') str++; + meta->name = str; } - else if (line.find("#pragma parameter ") == 0) + /* Check for shader parameters */ + else if (!strncmp("#pragma parameter ", line, STRLEN_CONST("#pragma parameter "))) { float initial, minimum, maximum, step; - int ret = sscanf(line_c, "#pragma parameter %63s \"%63[^\"]\" %f %f %f %f", + int ret = sscanf( + line, "#pragma parameter %63s \"%63[^\"]\" %f %f %f %f", id, desc, &initial, &minimum, &maximum, &step); if (ret == 5) @@ -331,19 +396,33 @@ bool glslang_parse_meta(const vector &lines, glslang_meta *meta) if (ret == 6) { - auto itr = find_if(begin(meta->parameters), end(meta->parameters), [&](const glslang_parameter ¶m) { - return param.id == id; - }); + bool parameter_found = false; + size_t parameter_index = 0; + size_t j; + + for (j = 0; j < meta->parameters.size(); j++) + { + /* Note: LHS is a std:string, RHS is a C string. + * (the glslang_meta stuff has to be C++) */ + if (meta->parameters[j].id == id) + { + parameter_found = true; + parameter_index = j; + break; + } + } /* Allow duplicate #pragma parameter, but only * if they are exactly the same. */ - if (itr != end(meta->parameters)) + if (parameter_found) { - if ( itr->desc != desc || - itr->initial != initial || - itr->minimum != minimum || - itr->maximum != maximum || - itr->step != step + const glslang_parameter *parameter = &meta->parameters[parameter_index]; + + if ( parameter->desc != desc || + parameter->initial != initial || + parameter->minimum != minimum || + parameter->maximum != maximum || + parameter->step != step ) { RARCH_ERR("[slang]: Duplicate parameters found for \"%s\", but arguments do not match.\n", id); @@ -355,11 +434,12 @@ bool glslang_parse_meta(const vector &lines, glslang_meta *meta) } else { - RARCH_ERR("[slang]: Invalid #pragma parameter line: \"%s\".\n", line_c); + RARCH_ERR("[slang]: Invalid #pragma parameter line: \"%s\".\n", line); return false; } } - else if (line.find("#pragma format ") == 0) + /* Check for framebuffer format */ + else if (!strncmp("#pragma format ", line, STRLEN_CONST("#pragma format "))) { const char *str = NULL; @@ -369,12 +449,12 @@ bool glslang_parse_meta(const vector &lines, glslang_meta *meta) return false; } - str = line_c + STRLEN_CONST("#pragma format "); - + str = line + STRLEN_CONST("#pragma format "); while (*str == ' ') str++; meta->rt_format = glslang_find_format(str); + if (meta->rt_format == SLANG_FORMAT_UNKNOWN) { RARCH_ERR("[slang]: Failed to find format \"%s\".\n", str); @@ -382,37 +462,50 @@ bool glslang_parse_meta(const vector &lines, glslang_meta *meta) } } } + return true; } #if defined(HAVE_GLSLANG) bool glslang_compile_shader(const char *shader_path, glslang_output *output) { - vector lines; + struct string_list *lines = string_list_new(); + + if (!lines) + return false; RARCH_LOG("[slang]: Compiling shader \"%s\".\n", shader_path); - if (!glslang_read_shader_file(shader_path, &lines, true)) - return false; + if (!glslang_read_shader_file(shader_path, lines, true)) + goto error; if (!glslang_parse_meta(lines, &output->meta)) - return false; + goto error; if ( !glslang::compile_spirv(build_stage_source(lines, "vertex"), glslang::StageVertex, &output->vertex)) { RARCH_ERR("Failed to compile vertex shader stage.\n"); - return false; + goto error; } if ( !glslang::compile_spirv(build_stage_source(lines, "fragment"), glslang::StageFragment, &output->fragment)) { RARCH_ERR("Failed to compile fragment shader stage.\n"); - return false; + goto error; } + string_list_free(lines); + return true; + +error: + + if (lines) + string_list_free(lines); + + return false; } #else bool glslang_compile_shader(const char *shader_path, glslang_output *output) diff --git a/gfx/drivers_shader/glslang_util.h b/gfx/drivers_shader/glslang_util.h index 24f86f190e..8c5c0ae5be 100644 --- a/gfx/drivers_shader/glslang_util.h +++ b/gfx/drivers_shader/glslang_util.h @@ -19,6 +19,8 @@ #include #include +#include + typedef enum glslang_format { SLANG_FORMAT_UNKNOWN = 0, @@ -106,8 +108,8 @@ struct glslang_output bool glslang_compile_shader(const char *shader_path, glslang_output *output); /* Helpers for internal use. */ -bool glslang_read_shader_file(const char *path, std::vector *output, bool root_file); -bool glslang_parse_meta(const std::vector &lines, glslang_meta *meta); +bool glslang_read_shader_file(const char *path, struct string_list *output, bool root_file); +bool glslang_parse_meta(const struct string_list *lines, glslang_meta *meta); #endif void *config_file_new_wrapper(const char *path); diff --git a/gfx/drivers_shader/slang_preprocess.cpp b/gfx/drivers_shader/slang_preprocess.cpp index a814359e06..f92fd52857 100644 --- a/gfx/drivers_shader/slang_preprocess.cpp +++ b/gfx/drivers_shader/slang_preprocess.cpp @@ -20,6 +20,7 @@ #include #include +#include #include "../../verbosity.h" @@ -93,11 +94,23 @@ bool slang_preprocess_parse_parameters(const char *shader_path, struct video_shader *shader) { glslang_meta meta; - vector lines; + bool ret = false; + struct string_list *lines = string_list_new(); - if (!glslang_read_shader_file(shader_path, &lines, true)) - return false; + if (!lines) + goto end; + + if (!glslang_read_shader_file(shader_path, lines, true)) + goto end; if (!glslang_parse_meta(lines, &meta)) - return false; - return slang_preprocess_parse_parameters(meta, shader); + goto end; + + ret = slang_preprocess_parse_parameters(meta, shader); + +end: + + if (lines) + string_list_free(lines); + + return ret; } diff --git a/libretro-common/include/lists/string_list.h b/libretro-common/include/lists/string_list.h index b156c6e484..de26282d76 100644 --- a/libretro-common/include/lists/string_list.h +++ b/libretro-common/include/lists/string_list.h @@ -88,6 +88,19 @@ bool string_list_find_elem_prefix(const struct string_list *list, */ struct string_list *string_split(const char *str, const char *delim); +/** + * string_separate: + * @str : string to turn into a string list + * @delim : delimiter character to use for separating the string. + * + * Creates a new string list based on string @str, delimited by @delim. + * Includes empty strings - i.e. two adjacent delimiters will resolve + * to a string list element of "". + * + * Returns: new string list if successful, otherwise NULL. + */ +struct string_list *string_separate(char *str, const char *delim); + /** * string_list_new: * diff --git a/libretro-common/include/string/stdstring.h b/libretro-common/include/string/stdstring.h index 74c9bdc15c..176045f637 100644 --- a/libretro-common/include/string/stdstring.h +++ b/libretro-common/include/string/stdstring.h @@ -129,6 +129,28 @@ char *string_trim_whitespace(char *const s); char *word_wrap(char *buffer, const char *string, int line_width, bool unicode, unsigned max_lines); +/* Splits string into tokens seperated by 'delim' + * > Returned token string must be free()'d + * > Returns NULL if token is not found + * > After each call, 'str' is set to the position after the + * last found token + * > Tokens *include* empty strings + * Usage example: + * char *str = "1,2,3,4,5,6,7,,,10,"; + * char **str_ptr = &str; + * char *token = NULL; + * while((token = string_tokenize(str_ptr, ","))) + * { + * printf("%s\n", token); + * free(token); + * token = NULL; + * } + */ +char* string_tokenize(char **str, const char *delim); + +/* Removes every instance of character 'c' from 'str' */ +void string_remove_all_chars(char *str, char c); + RETRO_END_DECLS #endif diff --git a/libretro-common/lists/string_list.c b/libretro-common/lists/string_list.c index 93f7c212fb..26d1d58a9b 100644 --- a/libretro-common/lists/string_list.c +++ b/libretro-common/lists/string_list.c @@ -259,6 +259,59 @@ error: return NULL; } +/** + * string_separate: + * @str : string to turn into a string list + * @delim : delimiter character to use for separating the string. + * + * Creates a new string list based on string @str, delimited by @delim. + * Includes empty strings - i.e. two adjacent delimiters will resolve + * to a string list element of "". + * + * Returns: new string list if successful, otherwise NULL. + */ +struct string_list *string_separate(char *str, const char *delim) +{ + char *token = NULL; + char **str_ptr = NULL; + struct string_list *list = NULL; + + /* Sanity check */ + if (!str || string_is_empty(delim)) + goto error; + + str_ptr = &str; + list = string_list_new(); + + if (!list) + goto error; + + token = string_tokenize(str_ptr, delim); + while (token) + { + union string_list_elem_attr attr; + + attr.i = 0; + + if (!string_list_append(list, token, attr)) + goto error; + + free(token); + token = NULL; + + token = string_tokenize(str_ptr, delim); + } + + return list; + +error: + if (token) + free(token); + if (list) + string_list_free(list); + return NULL; +} + /** * string_list_find_elem: * @list : pointer to string list diff --git a/libretro-common/string/stdstring.c b/libretro-common/string/stdstring.c index 4405a78c1e..914e6d31ec 100644 --- a/libretro-common/string/stdstring.c +++ b/libretro-common/string/stdstring.c @@ -237,3 +237,84 @@ char *word_wrap(char* buffer, const char *string, int line_width, bool unicode, return buffer; } + +/* Splits string into tokens seperated by 'delim' + * > Returned token string must be free()'d + * > Returns NULL if token is not found + * > After each call, 'str' is set to the position after the + * last found token + * > Tokens *include* empty strings + * Usage example: + * char *str = "1,2,3,4,5,6,7,,,10,"; + * char **str_ptr = &str; + * char *token = NULL; + * while((token = string_tokenize(str_ptr, ","))) + * { + * printf("%s\n", token); + * free(token); + * token = NULL; + * } + */ +char* string_tokenize(char **str, const char *delim) +{ + /* Taken from https://codereview.stackexchange.com/questions/216956/strtok-function-thread-safe-supports-empty-tokens-doesnt-change-string# */ + char *str_ptr = NULL; + char *delim_ptr = NULL; + char *token = NULL; + size_t token_len = 0; + + /* Sanity checks */ + if (!str || string_is_empty(delim)) + return NULL; + + str_ptr = *str; + + /* Note: we don't check string_is_empty() here, + * empty strings are valid */ + if (!str_ptr) + return NULL; + + /* Search for delimiter */ + delim_ptr = strstr(str_ptr, delim); + + if (delim_ptr) + token_len = delim_ptr - str_ptr; + else + token_len = strlen(str_ptr); + + /* Allocate token string */ + token = (char *)malloc((token_len + 1) * sizeof(char)); + + if (!token) + return NULL; + + /* Copy token */ + strlcpy(token, str_ptr, (token_len + 1) * sizeof(char)); + token[token_len] = '\0'; + + /* Update input string pointer */ + *str = delim_ptr ? delim_ptr + strlen(delim) : NULL; + + return token; +} + +/* Removes every instance of character 'c' from 'str' */ +void string_remove_all_chars(char *str, char c) +{ + char *read_ptr = NULL; + char *write_ptr = NULL; + + if (string_is_empty(str)) + return; + + read_ptr = str; + write_ptr = str; + + while (*read_ptr != '\0') + { + *write_ptr = *read_ptr++; + write_ptr += (*write_ptr != c) ? 1 : 0; + } + + *write_ptr = '\0'; +}