From 9b97ba14629e33f06798af049d0cb1c91d7aaf43 Mon Sep 17 00:00:00 2001 From: Hans Goudey Date: Fri, 1 Nov 2024 20:00:31 +0100 Subject: [PATCH] Cleanup: GPU: Avoid raw pointers for shader API strings Avoid measuring the length of strings repeatedly by passing their length along with their data with `StringRefNull`. Null termination seems to be necessary still for passing the shader sources to OpenGL. Though I doubt this is a bottleneck, it's still nice to avoid overhead from string operations and this helps move in that direction. Pull Request: https://projects.blender.org/blender/blender/pulls/127702 --- .../intern/draw_cache_impl_subdivision.cc | 28 ++- source/blender/gpu/CMakeLists.txt | 1 + source/blender/gpu/GPU_shader.hh | 50 ++--- source/blender/gpu/intern/gpu_shader.cc | 182 +++++++++--------- .../gpu/intern/gpu_shader_create_info.hh | 2 +- .../gpu/intern/gpu_shader_dependency.cc | 15 +- .../intern/gpu_shader_dependency_private.hh | 7 +- .../gpu/intern/gpu_shader_interface.hh | 20 +- source/blender/gpu/intern/gpu_shader_log.cc | 4 +- .../blender/gpu/intern/gpu_shader_private.hh | 12 +- source/blender/gpu/metal/mtl_immediate.mm | 4 +- source/blender/gpu/metal/mtl_shader.hh | 8 +- source/blender/gpu/metal/mtl_shader.mm | 42 ++-- .../blender/gpu/metal/mtl_shader_generator.mm | 8 +- source/blender/gpu/opengl/gl_debug.cc | 24 ++- source/blender/gpu/opengl/gl_shader.cc | 67 ++++--- source/blender/gpu/opengl/gl_shader.hh | 22 +-- source/blender/gpu/vulkan/vk_debug.hh | 4 +- source/blender/gpu/vulkan/vk_shader.cc | 30 ++- source/blender/gpu/vulkan/vk_shader.hh | 10 +- 20 files changed, 273 insertions(+), 267 deletions(-) diff --git a/source/blender/draw/intern/draw_cache_impl_subdivision.cc b/source/blender/draw/intern/draw_cache_impl_subdivision.cc index 3e82fd1f373..8a66232141d 100644 --- a/source/blender/draw/intern/draw_cache_impl_subdivision.cc +++ b/source/blender/draw/intern/draw_cache_impl_subdivision.cc @@ -97,7 +97,7 @@ static GPUShader *g_subdiv_shaders[NUM_SHADERS]; static GPUShader *g_subdiv_custom_data_shaders[SHADER_CUSTOM_DATA_INTERP_MAX_DIMENSIONS][GPU_COMP_MAX]; -static const char *get_shader_code(int shader_type) +static StringRefNull get_shader_code(int shader_type) { switch (shader_type) { case SHADER_BUFFER_LINES: @@ -147,7 +147,7 @@ static const char *get_shader_code(int shader_type) return nullptr; } -static const char *get_shader_name(int shader_type) +static StringRefNull get_shader_name(int shader_type) { switch (shader_type) { case SHADER_BUFFER_LINES: { @@ -215,9 +215,9 @@ static const char *get_shader_name(int shader_type) static GPUShader *get_patch_evaluation_shader(int shader_type) { if (g_subdiv_shaders[shader_type] == nullptr) { - const char *compute_code = get_shader_code(shader_type); + const StringRefNull compute_code = get_shader_code(shader_type); - const char *defines = nullptr; + std::optional defines; if (shader_type == SHADER_PATCH_EVALUATION) { defines = "#define OSD_PATCH_BASIS_GLSL\n" @@ -253,12 +253,11 @@ static GPUShader *get_patch_evaluation_shader(int shader_type) } /* Merge OpenSubdiv library code with our own library code. */ - const char *patch_basis_source = openSubdiv_getGLSLPatchBasisSource(); - const char *subdiv_lib_code = datatoc_common_subdiv_lib_glsl; - char *library_code = BLI_string_joinN(patch_basis_source, subdiv_lib_code); + const StringRefNull patch_basis_source = openSubdiv_getGLSLPatchBasisSource(); + const StringRefNull subdiv_lib_code = datatoc_common_subdiv_lib_glsl; + std::string library_code = patch_basis_source + subdiv_lib_code; g_subdiv_shaders[shader_type] = GPU_shader_create_compute( compute_code, library_code, defines, get_shader_name(shader_type)); - MEM_freeN(library_code); } return g_subdiv_shaders[shader_type]; @@ -282,8 +281,8 @@ static GPUShader *get_subdiv_shader(int shader_type) SHADER_COMP_CUSTOM_DATA_INTERP_4D)); if (g_subdiv_shaders[shader_type] == nullptr) { - const char *compute_code = get_shader_code(shader_type); - const char *defines = nullptr; + const StringRefNull compute_code = get_shader_code(shader_type); + std::optional defines; if (ELEM(shader_type, SHADER_BUFFER_LINES, @@ -326,7 +325,8 @@ static GPUShader *get_subdiv_custom_data_shader(int comp_type, int dimensions) GPUShader *&shader = g_subdiv_custom_data_shaders[dimensions - 1][comp_type]; if (shader == nullptr) { - const char *compute_code = get_shader_code(SHADER_COMP_CUSTOM_DATA_INTERP_1D + dimensions - 1); + const StringRefNull compute_code = get_shader_code(SHADER_COMP_CUSTOM_DATA_INTERP_1D + + dimensions - 1); int shader_type = SHADER_COMP_CUSTOM_DATA_INTERP_1D + dimensions - 1; @@ -347,10 +347,8 @@ static GPUShader *get_subdiv_custom_data_shader(int comp_type, int dimensions) break; } - shader = GPU_shader_create_compute(compute_code, - datatoc_common_subdiv_lib_glsl, - defines.c_str(), - get_shader_name(shader_type)); + shader = GPU_shader_create_compute( + compute_code, datatoc_common_subdiv_lib_glsl, defines, get_shader_name(shader_type)); } return shader; } diff --git a/source/blender/gpu/CMakeLists.txt b/source/blender/gpu/CMakeLists.txt index 82bd839d3b4..c0462ba2e94 100644 --- a/source/blender/gpu/CMakeLists.txt +++ b/source/blender/gpu/CMakeLists.txt @@ -391,6 +391,7 @@ set(LIB PRIVATE bf::intern::atomic PRIVATE bf::intern::clog PRIVATE bf::intern::guardedalloc + PRIVATE bf::extern::fmtlib ) # Select Backend source based on availability diff --git a/source/blender/gpu/GPU_shader.hh b/source/blender/gpu/GPU_shader.hh index d0c41cc58b7..cd5d65fe988 100644 --- a/source/blender/gpu/GPU_shader.hh +++ b/source/blender/gpu/GPU_shader.hh @@ -10,8 +10,12 @@ #pragma once +#include + #include "BLI_span.hh" +#include "BLI_string_ref.hh" #include "BLI_vector.hh" + #include "GPU_common_types.hh" #include "GPU_shader_builtin.hh" @@ -271,32 +275,32 @@ enum eGPUShaderTFBType { GPU_SHADER_TFB_TRIANGLES = 3, }; -GPUShader *GPU_shader_create(const char *vertcode, - const char *fragcode, - const char *geomcode, - const char *libcode, - const char *defines, - const char *shname); -GPUShader *GPU_shader_create_compute(const char *computecode, - const char *libcode, - const char *defines, - const char *shname); -GPUShader *GPU_shader_create_from_python(const char *vertcode, - const char *fragcode, - const char *geomcode, - const char *libcode, - const char *defines, - const char *name); -GPUShader *GPU_shader_create_ex(const char *vertcode, - const char *fragcode, - const char *geomcode, - const char *computecode, - const char *libcode, - const char *defines, +GPUShader *GPU_shader_create(std::optional vertcode, + std::optional fragcode, + std::optional geomcode, + std::optional libcode, + std::optional defines, + blender::StringRefNull shname); +GPUShader *GPU_shader_create_compute(std::optional computecode, + std::optional libcode, + std::optional defines, + blender::StringRefNull shname); +GPUShader *GPU_shader_create_from_python(std::optional vertcode, + std::optional fragcode, + std::optional geomcode, + std::optional libcode, + std::optional defines, + std::optional name); +GPUShader *GPU_shader_create_ex(std::optional vertcode, + std::optional fragcode, + std::optional geomcode, + std::optional computecode, + std::optional libcode, + std::optional defines, eGPUShaderTFBType tf_type, const char **tf_names, int tf_count, - const char *shname); + blender::StringRefNull shname); /** * Returns true if transform feedback was successfully enabled. diff --git a/source/blender/gpu/intern/gpu_shader.cc b/source/blender/gpu/intern/gpu_shader.cc index 84e7bf135d6..79f85fc003b 100644 --- a/source/blender/gpu/intern/gpu_shader.cc +++ b/source/blender/gpu/intern/gpu_shader.cc @@ -64,7 +64,7 @@ Shader::~Shader() delete interface; } -static void standard_defines(Vector &sources) +static void standard_defines(Vector &sources) { BLI_assert(sources.is_empty()); /* Version and specialization constants needs to be first. @@ -118,26 +118,26 @@ static void standard_defines(Vector &sources) } } -GPUShader *GPU_shader_create_ex(const char *vertcode, - const char *fragcode, - const char *geomcode, - const char *computecode, - const char *libcode, - const char *defines, +GPUShader *GPU_shader_create_ex(const std::optional vertcode, + const std::optional fragcode, + const std::optional geomcode, + const std::optional computecode, + const std::optional libcode, + const std::optional defines, const eGPUShaderTFBType tf_type, const char **tf_names, const int tf_count, - const char *shname) + const StringRefNull shname) { /* At least a vertex shader and a fragment shader are required, or only a compute shader. */ - BLI_assert(((fragcode != nullptr) && (vertcode != nullptr) && (computecode == nullptr)) || - ((fragcode == nullptr) && (vertcode == nullptr) && (geomcode == nullptr) && - (computecode != nullptr))); + BLI_assert((fragcode.has_value() && vertcode.has_value() && !computecode.has_value()) || + (!fragcode.has_value() && !vertcode.has_value() && !geomcode.has_value() && + computecode.has_value())); - Shader *shader = GPUBackend::get()->shader_alloc(shname); + Shader *shader = GPUBackend::get()->shader_alloc(shname.c_str()); if (vertcode) { - Vector sources; + Vector sources; standard_defines(sources); sources.append("#define GPU_VERTEX_SHADER\n"); sources.append("#define IN_OUT out\n"); @@ -145,15 +145,15 @@ GPUShader *GPU_shader_create_ex(const char *vertcode, sources.append("#define USE_GEOMETRY_SHADER\n"); } if (defines) { - sources.append(defines); + sources.append(*defines); } - sources.append(vertcode); + sources.append(*vertcode); shader->vertex_shader_from_glsl(sources); } if (fragcode) { - Vector sources; + Vector sources; standard_defines(sources); sources.append("#define GPU_FRAGMENT_SHADER\n"); sources.append("#define IN_OUT in\n"); @@ -161,39 +161,39 @@ GPUShader *GPU_shader_create_ex(const char *vertcode, sources.append("#define USE_GEOMETRY_SHADER\n"); } if (defines) { - sources.append(defines); + sources.append(*defines); } if (libcode) { - sources.append(libcode); + sources.append(*libcode); } - sources.append(fragcode); + sources.append(*fragcode); shader->fragment_shader_from_glsl(sources); } if (geomcode) { - Vector sources; + Vector sources; standard_defines(sources); sources.append("#define GPU_GEOMETRY_SHADER\n"); if (defines) { - sources.append(defines); + sources.append(*defines); } - sources.append(geomcode); + sources.append(*geomcode); shader->geometry_shader_from_glsl(sources); } if (computecode) { - Vector sources; + Vector sources; standard_defines(sources); sources.append("#define GPU_COMPUTE_SHADER\n"); if (defines) { - sources.append(defines); + sources.append(*defines); } if (libcode) { - sources.append(libcode); + sources.append(*libcode); } - sources.append(computecode); + sources.append(*computecode); shader->compute_shader_from_glsl(sources); } @@ -222,17 +222,17 @@ void GPU_shader_free(GPUShader *shader) /** \name Creation utils * \{ */ -GPUShader *GPU_shader_create(const char *vertcode, - const char *fragcode, - const char *geomcode, - const char *libcode, - const char *defines, - const char *shname) +GPUShader *GPU_shader_create(const std::optional vertcode, + const std::optional fragcode, + const std::optional geomcode, + const std::optional libcode, + const std::optional defines, + const StringRefNull shname) { return GPU_shader_create_ex(vertcode, fragcode, geomcode, - nullptr, + std::nullopt, libcode, defines, GPU_SHADER_TFB_NONE, @@ -241,14 +241,14 @@ GPUShader *GPU_shader_create(const char *vertcode, shname); } -GPUShader *GPU_shader_create_compute(const char *computecode, - const char *libcode, - const char *defines, - const char *shname) +GPUShader *GPU_shader_create_compute(const std::optional computecode, + const std::optional libcode, + const std::optional defines, + const StringRefNull shname) { - return GPU_shader_create_ex(nullptr, - nullptr, - nullptr, + return GPU_shader_create_ex(std::nullopt, + std::nullopt, + std::nullopt, computecode, libcode, defines, @@ -288,7 +288,7 @@ GPUShader *GPU_shader_create_from_info_name(const char *info_name) const GPUShaderCreateInfo *_info = gpu_shader_create_info_get(info_name); const ShaderCreateInfo &info = *reinterpret_cast(_info); if (!info.do_static_compilation_) { - std::cerr << "Warning: Trying to compile \"" << info.name_.c_str() + std::cerr << "Warning: Trying to compile \"" << info.name_ << "\" which was not marked for static compilation.\n"; } return GPU_shader_create_from_info(_info); @@ -333,20 +333,21 @@ GPUShader *GPU_shader_create_from_info_python(const GPUShaderCreateInfo *_info) return result; } -GPUShader *GPU_shader_create_from_python(const char *vertcode, - const char *fragcode, - const char *geomcode, - const char *libcode, - const char *defines, - const char *name) +GPUShader *GPU_shader_create_from_python(std::optional vertcode, + std::optional fragcode, + std::optional geomcode, + std::optional libcode, + const std::optional defines, + const std::optional name) { - char *libcodecat = nullptr; + std::string libcodecat; - if (libcode == nullptr) { + if (!libcode) { libcode = datatoc_gpu_shader_colorspace_lib_glsl; } else { - libcode = libcodecat = BLI_strdupcat(libcode, datatoc_gpu_shader_colorspace_lib_glsl); + libcodecat = *libcode + datatoc_gpu_shader_colorspace_lib_glsl; + libcode = libcodecat; } std::string vertex_source_processed; @@ -354,30 +355,30 @@ GPUShader *GPU_shader_create_from_python(const char *vertcode, std::string geometry_source_processed; std::string library_source_processed; - if (vertcode != nullptr) { - vertex_source_processed = preprocess_source(vertcode); - vertcode = vertex_source_processed.c_str(); + if (vertcode.has_value()) { + vertex_source_processed = preprocess_source(*vertcode); + vertcode = vertex_source_processed; } - if (fragcode != nullptr) { - fragment_source_processed = preprocess_source(fragcode); - fragcode = fragment_source_processed.c_str(); + if (fragcode.has_value()) { + fragment_source_processed = preprocess_source(*fragcode); + fragcode = fragment_source_processed; } - if (geomcode != nullptr) { - geometry_source_processed = preprocess_source(geomcode); - geomcode = geometry_source_processed.c_str(); + if (geomcode.has_value()) { + geometry_source_processed = preprocess_source(*geomcode); + geomcode = geometry_source_processed; } - if (libcode != nullptr) { - library_source_processed = preprocess_source(libcode); - libcode = library_source_processed.c_str(); + if (libcode.has_value()) { + library_source_processed = preprocess_source(*libcode); + libcode = library_source_processed; } /* Use pyGPUShader as default name for shader. */ - const char *shname = name != nullptr ? name : "pyGPUShader"; + blender::StringRefNull shname = name.value_or("pyGPUShader"); GPUShader *sh = GPU_shader_create_ex(vertcode, fragcode, geomcode, - nullptr, + std::nullopt, libcode, defines, GPU_SHADER_TFB_NONE, @@ -385,7 +386,6 @@ GPUShader *GPU_shader_create_from_python(const char *vertcode, 0, shname); - MEM_SAFE_FREE(libcodecat); return sh; } @@ -485,7 +485,7 @@ GPUShader *GPU_shader_get_bound() const char *GPU_shader_get_name(GPUShader *shader) { - return unwrap(shader)->name_get(); + return unwrap(shader)->name_get().c_str(); } /** \} */ @@ -872,7 +872,7 @@ Shader *ShaderCompiler::compile(const shader::ShaderCreateInfo &info, bool is_ba const std::string error = info.check_error(); if (!error.empty()) { - std::cerr << error.c_str() << "\n"; + std::cerr << error << "\n"; BLI_assert(false); } @@ -887,34 +887,34 @@ Shader *ShaderCompiler::compile(const shader::ShaderCreateInfo &info, bool is_ba defines += "#define USE_GPU_SHADER_CREATE_INFO\n"; } - Vector typedefs; + Vector typedefs; if (!info.typedef_sources_.is_empty() || !info.typedef_source_generated.empty()) { typedefs.append(gpu_shader_dependency_get_source("GPU_shader_shared_utils.hh").c_str()); } if (!info.typedef_source_generated.empty()) { - typedefs.append(info.typedef_source_generated.c_str()); + typedefs.append(info.typedef_source_generated); } for (auto filename : info.typedef_sources_) { - typedefs.append(gpu_shader_dependency_get_source(filename).c_str()); + typedefs.append(gpu_shader_dependency_get_source(filename)); } if (!info.vertex_source_.is_empty()) { auto code = gpu_shader_dependency_get_resolved_source(info.vertex_source_); std::string interface = shader->vertex_interface_declare(info); - Vector sources; + Vector sources; standard_defines(sources); sources.append("#define GPU_VERTEX_SHADER\n"); if (!info.geometry_source_.is_empty()) { sources.append("#define USE_GEOMETRY_SHADER\n"); } - sources.append(defines.c_str()); + sources.append(defines); sources.extend(typedefs); - sources.append(resources.c_str()); - sources.append(interface.c_str()); + sources.append(resources); + sources.append(interface); sources.extend(code); sources.extend(info.dependencies_generated); - sources.append(info.vertex_source_generated.c_str()); + sources.append(info.vertex_source_generated); shader->vertex_shader_from_glsl(sources); } @@ -923,19 +923,19 @@ Shader *ShaderCompiler::compile(const shader::ShaderCreateInfo &info, bool is_ba auto code = gpu_shader_dependency_get_resolved_source(info.fragment_source_); std::string interface = shader->fragment_interface_declare(info); - Vector sources; + Vector sources; standard_defines(sources); sources.append("#define GPU_FRAGMENT_SHADER\n"); if (!info.geometry_source_.is_empty()) { sources.append("#define USE_GEOMETRY_SHADER\n"); } - sources.append(defines.c_str()); + sources.append(defines); sources.extend(typedefs); - sources.append(resources.c_str()); - sources.append(interface.c_str()); + sources.append(resources); + sources.append(interface); sources.extend(code); sources.extend(info.dependencies_generated); - sources.append(info.fragment_source_generated.c_str()); + sources.append(info.fragment_source_generated); shader->fragment_shader_from_glsl(sources); } @@ -945,15 +945,15 @@ Shader *ShaderCompiler::compile(const shader::ShaderCreateInfo &info, bool is_ba std::string layout = shader->geometry_layout_declare(info); std::string interface = shader->geometry_interface_declare(info); - Vector sources; + Vector sources; standard_defines(sources); sources.append("#define GPU_GEOMETRY_SHADER\n"); - sources.append(defines.c_str()); + sources.append(defines); sources.extend(typedefs); - sources.append(resources.c_str()); - sources.append(layout.c_str()); - sources.append(interface.c_str()); - sources.append(info.geometry_source_generated.c_str()); + sources.append(resources); + sources.append(layout); + sources.append(interface); + sources.append(info.geometry_source_generated); sources.extend(code); shader->geometry_shader_from_glsl(sources); @@ -963,16 +963,16 @@ Shader *ShaderCompiler::compile(const shader::ShaderCreateInfo &info, bool is_ba auto code = gpu_shader_dependency_get_resolved_source(info.compute_source_); std::string layout = shader->compute_layout_declare(info); - Vector sources; + Vector sources; standard_defines(sources); sources.append("#define GPU_COMPUTE_SHADER\n"); - sources.append(defines.c_str()); + sources.append(defines); sources.extend(typedefs); - sources.append(resources.c_str()); - sources.append(layout.c_str()); + sources.append(resources); + sources.append(layout); sources.extend(code); sources.extend(info.dependencies_generated); - sources.append(info.compute_source_generated.c_str()); + sources.append(info.compute_source_generated); shader->compute_shader_from_glsl(sources); } diff --git a/source/blender/gpu/intern/gpu_shader_create_info.hh b/source/blender/gpu/intern/gpu_shader_create_info.hh index 68a9bef4a31..7bb5e79f54d 100644 --- a/source/blender/gpu/intern/gpu_shader_create_info.hh +++ b/source/blender/gpu/intern/gpu_shader_create_info.hh @@ -621,7 +621,7 @@ struct ShaderCreateInfo { std::string geometry_source_generated = ""; std::string typedef_source_generated = ""; /** Manually set generated dependencies. */ - Vector dependencies_generated; + Vector dependencies_generated; #define TEST_EQUAL(a, b, _member) \ if (!((a)._member == (b)._member)) { \ diff --git a/source/blender/gpu/intern/gpu_shader_dependency.cc b/source/blender/gpu/intern/gpu_shader_dependency.cc index 9abadaf9772..af7c900bd9f 100644 --- a/source/blender/gpu/intern/gpu_shader_dependency.cc +++ b/source/blender/gpu/intern/gpu_shader_dependency.cc @@ -399,12 +399,12 @@ struct GPUSource { } /* Returns the final string with all includes done. */ - void build(Vector &result) const + void build(Vector &result) const { for (auto *dep : dependencies) { - result.append(dep->source.c_str()); + result.append(dep->source); } - result.append(source.c_str()); + result.append(source); } shader::BuiltinBits builtins_get() const @@ -532,10 +532,10 @@ BuiltinBits gpu_shader_dependency_get_builtins(const StringRefNull shader_source return source->builtins_get(); } -Vector gpu_shader_dependency_get_resolved_source( +Vector gpu_shader_dependency_get_resolved_source( const StringRefNull shader_source_name) { - Vector result; + Vector result; GPUSource *src = g_sources->lookup_default(shader_source_name, nullptr); if (src == nullptr) { std::cerr << "Error source not found : " << shader_source_name << std::endl; @@ -553,11 +553,10 @@ StringRefNull gpu_shader_dependency_get_source(const StringRefNull shader_source return src->source; } -StringRefNull gpu_shader_dependency_get_filename_from_source_string( - const StringRefNull source_string) +StringRefNull gpu_shader_dependency_get_filename_from_source_string(const StringRef source_string) { for (auto &source : g_sources->values()) { - if (source->source.c_str() == source_string.c_str()) { + if (source->source == source_string) { return source->filename; } } diff --git a/source/blender/gpu/intern/gpu_shader_dependency_private.hh b/source/blender/gpu/intern/gpu_shader_dependency_private.hh index 7f7fa668e87..2811f414cc8 100644 --- a/source/blender/gpu/intern/gpu_shader_dependency_private.hh +++ b/source/blender/gpu/intern/gpu_shader_dependency_private.hh @@ -46,15 +46,14 @@ struct PrintfFormat { const PrintfFormat &gpu_shader_dependency_get_printf_format(uint32_t format_hash); -Vector gpu_shader_dependency_get_resolved_source(const StringRefNull source_name); -StringRefNull gpu_shader_dependency_get_source(const StringRefNull source_name); +Vector gpu_shader_dependency_get_resolved_source(StringRefNull source_name); +StringRefNull gpu_shader_dependency_get_source(StringRefNull source_name); /** * \brief Find the name of the file from which the given string was generated. * \return filename or empty string. * \note source_string needs to be identical to the one given by gpu_shader_dependency_get_source() */ -StringRefNull gpu_shader_dependency_get_filename_from_source_string( - const StringRefNull source_string); +StringRefNull gpu_shader_dependency_get_filename_from_source_string(StringRef source_string); } // namespace blender::gpu::shader diff --git a/source/blender/gpu/intern/gpu_shader_interface.hh b/source/blender/gpu/intern/gpu_shader_interface.hh index 4ea3a70f600..4631734fc54 100644 --- a/source/blender/gpu/intern/gpu_shader_interface.hh +++ b/source/blender/gpu/intern/gpu_shader_interface.hh @@ -82,7 +82,7 @@ class ShaderInterface { void debug_print() const; - inline const ShaderInput *attr_get(const char *name) const + inline const ShaderInput *attr_get(const StringRefNull name) const { return input_lookup(inputs_, attr_len_, name); } @@ -91,7 +91,7 @@ class ShaderInterface { return input_lookup(inputs_, attr_len_, binding); } - inline const ShaderInput *ubo_get(const char *name) const + inline const ShaderInput *ubo_get(const StringRefNull name) const { return input_lookup(inputs_ + attr_len_, ubo_len_, name); } @@ -100,7 +100,7 @@ class ShaderInterface { return input_lookup(inputs_ + attr_len_, ubo_len_, binding); } - inline const ShaderInput *uniform_get(const char *name) const + inline const ShaderInput *uniform_get(const StringRefNull name) const { return input_lookup(inputs_ + attr_len_ + ubo_len_, uniform_len_, name); } @@ -110,7 +110,7 @@ class ShaderInterface { return input_lookup(inputs_ + attr_len_ + ubo_len_, uniform_len_, binding); } - inline const ShaderInput *ssbo_get(const char *name) const + inline const ShaderInput *ssbo_get(const StringRefNull name) const { return input_lookup(inputs_ + attr_len_ + ubo_len_ + uniform_len_, ssbo_len_, name); } @@ -119,7 +119,7 @@ class ShaderInterface { return input_lookup(inputs_ + attr_len_ + ubo_len_ + uniform_len_, ssbo_len_, binding); } - inline const ShaderInput *constant_get(const char *name) const + inline const ShaderInput *constant_get(const StringRefNull name) const { return input_lookup( inputs_ + attr_len_ + ubo_len_ + uniform_len_ + ssbo_len_, constant_len_, name); @@ -162,7 +162,7 @@ class ShaderInterface { private: inline const ShaderInput *input_lookup(const ShaderInput *const inputs, uint inputs_len, - const char *name) const; + StringRefNull name) const; inline const ShaderInput *input_lookup(const ShaderInput *const inputs, uint inputs_len, @@ -275,16 +275,16 @@ inline void ShaderInterface::copy_input_name(ShaderInput *input, inline const ShaderInput *ShaderInterface::input_lookup(const ShaderInput *const inputs, const uint inputs_len, - const char *name) const + const StringRefNull name) const { - const uint name_hash = BLI_hash_string(name); + const uint name_hash = BLI_hash_string(name.c_str()); /* Simple linear search for now. */ for (int i = inputs_len - 1; i >= 0; i--) { if (inputs[i].name_hash == name_hash) { if ((i > 0) && UNLIKELY(inputs[i - 1].name_hash == name_hash)) { /* Hash collision resolve. */ for (; i >= 0 && inputs[i].name_hash == name_hash; i--) { - if (STREQ(name, name_buffer_ + inputs[i].name_offset)) { + if (name == (name_buffer_ + inputs[i].name_offset)) { return inputs + i; /* not found */ } } @@ -294,7 +294,7 @@ inline const ShaderInput *ShaderInterface::input_lookup(const ShaderInput *const /* This is a bit dangerous since we could have a hash collision. * where the asked uniform that does not exist has the same hash * as a real uniform. */ - BLI_assert(STREQ(name, name_buffer_ + inputs[i].name_offset)); + BLI_assert(name == (name_buffer_ + inputs[i].name_offset)); return inputs + i; } } diff --git a/source/blender/gpu/intern/gpu_shader_log.cc b/source/blender/gpu/intern/gpu_shader_log.cc index 5cfbf494b8a..f50daddac9f 100644 --- a/source/blender/gpu/intern/gpu_shader_log.cc +++ b/source/blender/gpu/intern/gpu_shader_log.cc @@ -37,7 +37,7 @@ namespace blender::gpu { */ #define DEBUG_DEPENDENCIES 0 -void Shader::print_log(Span sources, +void Shader::print_log(Span sources, const char *log, const char *stage, const bool error, @@ -63,7 +63,7 @@ void Shader::print_log(Span sources, #endif Vector sources_end_line; - for (StringRefNull src : sources) { + for (StringRef src : sources) { int64_t cursor = 0, line_count = 0; while ((cursor = src.find('\n', cursor) + 1)) { line_count++; diff --git a/source/blender/gpu/intern/gpu_shader_private.hh b/source/blender/gpu/intern/gpu_shader_private.hh index 778a2cadae1..b3bb3bb018e 100644 --- a/source/blender/gpu/intern/gpu_shader_private.hh +++ b/source/blender/gpu/intern/gpu_shader_private.hh @@ -82,10 +82,10 @@ class Shader { * `GPU_shader_batch`. Backends that use the `ShaderCompilerGeneric` can ignore it. */ virtual void init(const shader::ShaderCreateInfo &info, bool is_batch_compilation) = 0; - virtual void vertex_shader_from_glsl(MutableSpan sources) = 0; - virtual void geometry_shader_from_glsl(MutableSpan sources) = 0; - virtual void fragment_shader_from_glsl(MutableSpan sources) = 0; - virtual void compute_shader_from_glsl(MutableSpan sources) = 0; + virtual void vertex_shader_from_glsl(MutableSpan sources) = 0; + virtual void geometry_shader_from_glsl(MutableSpan sources) = 0; + virtual void fragment_shader_from_glsl(MutableSpan sources) = 0; + virtual void compute_shader_from_glsl(MutableSpan sources) = 0; virtual bool finalize(const shader::ShaderCreateInfo *info = nullptr) = 0; /* Pre-warms PSOs using parent shader's cached PSO descriptors. Limit specifies maximum PSOs to * warm. If -1, compiles all PSO permutations in parent shader. @@ -122,7 +122,7 @@ class Shader { virtual bool get_uses_ssbo_vertex_fetch() const = 0; virtual int get_ssbo_vertex_fetch_output_num_verts() const = 0; - inline const char *name_get() const + inline StringRefNull name_get() const { return name; } @@ -142,7 +142,7 @@ class Shader { static void set_framebuffer_srgb_target(int use_srgb_to_linear); protected: - void print_log(Span sources, + void print_log(Span sources, const char *log, const char *stage, bool error, diff --git a/source/blender/gpu/metal/mtl_immediate.mm b/source/blender/gpu/metal/mtl_immediate.mm index 70ccb855e28..a926fa89690 100644 --- a/source/blender/gpu/metal/mtl_immediate.mm +++ b/source/blender/gpu/metal/mtl_immediate.mm @@ -71,12 +71,12 @@ void MTLImmediate::end() active_mtl_shader->get_interface() == nullptr) { - const char *ptr = (active_mtl_shader) ? active_mtl_shader->name_get() : nullptr; + const StringRefNull ptr = (active_mtl_shader) ? active_mtl_shader->name_get() : ""; MTL_LOG_WARNING( "MTLImmediate::end -- cannot perform draw as active shader is NULL or invalid (likely " "unimplemented) (shader %p '%s')", active_mtl_shader, - ptr); + ptr.c_str()); return; } diff --git a/source/blender/gpu/metal/mtl_shader.hh b/source/blender/gpu/metal/mtl_shader.hh index 72e66ce77b2..37c6e2e75c8 100644 --- a/source/blender/gpu/metal/mtl_shader.hh +++ b/source/blender/gpu/metal/mtl_shader.hh @@ -287,10 +287,10 @@ class MTLShader : public Shader { void init(const shader::ShaderCreateInfo & /*info*/, bool is_batch_compilation) override; /* Assign GLSL source. */ - void vertex_shader_from_glsl(MutableSpan sources) override; - void geometry_shader_from_glsl(MutableSpan sources) override; - void fragment_shader_from_glsl(MutableSpan sources) override; - void compute_shader_from_glsl(MutableSpan sources) override; + void vertex_shader_from_glsl(MutableSpan sources) override; + void geometry_shader_from_glsl(MutableSpan sources) override; + void fragment_shader_from_glsl(MutableSpan sources) override; + void compute_shader_from_glsl(MutableSpan sources) override; /* Compile and build - Return true if successful. */ bool finalize(const shader::ShaderCreateInfo *info = nullptr) override; diff --git a/source/blender/gpu/metal/mtl_shader.mm b/source/blender/gpu/metal/mtl_shader.mm index 4cca98b939d..6af06004876 100644 --- a/source/blender/gpu/metal/mtl_shader.mm +++ b/source/blender/gpu/metal/mtl_shader.mm @@ -198,7 +198,7 @@ void MTLShader::init(const shader::ShaderCreateInfo & /*info*/, bool is_batch_co /** \name Shader stage creation. * \{ */ -void MTLShader::vertex_shader_from_glsl(MutableSpan sources) +void MTLShader::vertex_shader_from_glsl(MutableSpan sources) { /* Flag source as not being compiled from native MSL. */ BLI_assert(shd_builder_ != nullptr); @@ -215,12 +215,12 @@ void MTLShader::vertex_shader_from_glsl(MutableSpan sources) shd_builder_->glsl_vertex_source_ = ss.str(); } -void MTLShader::geometry_shader_from_glsl(MutableSpan /*sources*/) +void MTLShader::geometry_shader_from_glsl(MutableSpan /*sources*/) { MTL_LOG_ERROR("MTLShader::geometry_shader_from_glsl - Geometry shaders unsupported!"); } -void MTLShader::fragment_shader_from_glsl(MutableSpan sources) +void MTLShader::fragment_shader_from_glsl(MutableSpan sources) { /* Flag source as not being compiled from native MSL. */ BLI_assert(shd_builder_ != nullptr); @@ -238,7 +238,7 @@ void MTLShader::fragment_shader_from_glsl(MutableSpan sources) shd_builder_->glsl_fragment_source_ = ss.str(); } -void MTLShader::compute_shader_from_glsl(MutableSpan sources) +void MTLShader::compute_shader_from_glsl(MutableSpan sources) { /* Flag source as not being compiled from native MSL. */ BLI_assert(shd_builder_ != nullptr); @@ -259,7 +259,7 @@ bool MTLShader::finalize(const shader::ShaderCreateInfo *info) { /* Check if Shader has already been finalized. */ if (this->is_valid()) { - MTL_LOG_ERROR("Shader (%p) '%s' has already been finalized!", this, this->name_get()); + MTL_LOG_ERROR("Shader (%p) '%s' has already been finalized!", this, this->name_get().c_str()); } /* Compute shaders. */ @@ -280,7 +280,7 @@ bool MTLShader::finalize(const shader::ShaderCreateInfo *info) BLI_assert_msg(false, "Shader translation from GLSL to MSL has failed. \n"); /* Create empty interface to allow shader to be silently used. */ - MTLShaderInterface *mtl_interface = new MTLShaderInterface(this->name_get()); + MTLShaderInterface *mtl_interface = new MTLShaderInterface(this->name_get().c_str()); this->set_interface(mtl_interface); /* Release temporary compilation resources. */ @@ -388,15 +388,11 @@ bool MTLShader::finalize(const shader::ShaderCreateInfo *info) NSNotFound) { const char *errors_c_str = [[error localizedDescription] UTF8String]; - const char *sources_c_str = (is_compute) ? shd_builder_->glsl_compute_source_.c_str() : - shd_builder_->glsl_fragment_source_.c_str(); + const StringRefNull source = (is_compute) ? shd_builder_->glsl_compute_source_ : + shd_builder_->glsl_fragment_source_; MTLLogParser parser; - print_log(Span(&sources_c_str, 1), - errors_c_str, - to_string(src_stage), - true, - &parser); + print_log({source}, errors_c_str, to_string(src_stage), true, &parser); /* Release temporary compilation resources. */ delete shd_builder_; @@ -517,7 +513,7 @@ void MTLShader::bind() MTL_LOG_WARNING( "MTLShader::bind - Shader '%s' has no valid implementation in Metal, draw calls will be " "skipped.", - this->name_get()); + this->name_get().c_str()); } ctx->pipeline_state.active_shader = this; } @@ -536,7 +532,8 @@ void MTLShader::uniform_float(int location, int comp_len, int array_size, const } MTLShaderInterface *mtl_interface = get_interface(); if (location < 0 || location >= mtl_interface->get_total_uniforms()) { - MTL_LOG_WARNING("Uniform location %d is not valid in Shader %s", location, this->name_get()); + MTL_LOG_WARNING( + "Uniform location %d is not valid in Shader %s", location, this->name_get().c_str()); return; } @@ -651,7 +648,8 @@ void MTLShader::uniform_int(int location, int comp_len, int array_size, const in } if (location < 0 || location >= mtl_interface->get_total_uniforms()) { - MTL_LOG_WARNING("Uniform is not valid at location %d - Shader %s", location, this->name_get()); + MTL_LOG_WARNING( + "Uniform is not valid at location %d - Shader %s", location, this->name_get().c_str()); return; } @@ -1227,11 +1225,10 @@ MTLRenderPipelineStateInstance *MTLShader::bake_pipeline_state( NSNotFound); const char *errors_c_str = [[error localizedDescription] UTF8String]; - const char *sources_c_str = shd_builder_->glsl_fragment_source_.c_str(); + const StringRefNull source = shd_builder_->glsl_fragment_source_.c_str(); MTLLogParser parser; - print_log( - Span(&sources_c_str, 1), errors_c_str, "VertShader", has_error, &parser); + print_log({source}, errors_c_str, "VertShader", has_error, &parser); /* Only exit out if genuine error and not warning */ if (has_error) { @@ -1250,11 +1247,10 @@ MTLRenderPipelineStateInstance *MTLShader::bake_pipeline_state( NSNotFound); const char *errors_c_str = [[error localizedDescription] UTF8String]; - const char *sources_c_str = shd_builder_->glsl_fragment_source_.c_str(); + const StringRefNull source = shd_builder_->glsl_fragment_source_; MTLLogParser parser; - print_log( - Span(&sources_c_str, 1), errors_c_str, "FragShader", has_error, &parser); + print_log({source}, errors_c_str, "FragShader", has_error, &parser); /* Only exit out if genuine error and not warning */ if (has_error) { @@ -1770,7 +1766,7 @@ void MTLShader::ssbo_vertex_fetch_bind_attributes_end( MTL_LOG_WARNING( "Unassigned Shader attribute: %s, Attr Name: %s -- Binding NULL BUFFER to " "slot %d", - this->name_get(), + this->name_get().c_str(), mtl_interface->get_name_at_offset(mtl_shader_attribute->name_offset), null_attr_buffer_slot); } diff --git a/source/blender/gpu/metal/mtl_shader_generator.mm b/source/blender/gpu/metal/mtl_shader_generator.mm index c6b0eacdc00..6680d55c3e9 100644 --- a/source/blender/gpu/metal/mtl_shader_generator.mm +++ b/source/blender/gpu/metal/mtl_shader_generator.mm @@ -215,7 +215,7 @@ static bool extract_ssbo_pragma_info(const MTLShader *shader, } else { MTL_LOG_ERROR("Unsupported output primitive type for SSBO VERTEX FETCH MODE. Shader: %s", - shader->name_get()); + shader->name_get().c_str()); return false; } @@ -426,7 +426,7 @@ bool MTLShader::generate_msl_from_glsl(const shader::ShaderCreateInfo *info) if (!uses_create_info) { MTL_LOG_WARNING("Unable to compile shader %p '%s' as no create-info was provided!", this, - this->name_get()); + this->name_get().c_str()); valid_ = false; return false; } @@ -1635,7 +1635,7 @@ bool MSLGeneratorInterface::use_argument_buffer_for_samplers() const "Compiled Shader '%s' is falling back to bindless via argument buffers due to having a " "texture sampler of Index: %u Which exceeds the limit of 15+1. However shader only uses " "%d textures. Consider optimising bind points with .auto_resource_location(true).", - parent_shader_.name_get(), + parent_shader_.name_get().c_str(), max_tex_bind_index, (int)texture_samplers.size()); } @@ -3119,7 +3119,7 @@ void MSLGeneratorInterface::resolve_input_attribute_locations() /* Error if could not assign attribute. */ MTL_LOG_ERROR("Could not assign attribute location to attribute %s for shader %s", attr.name.c_str(), - this->parent_shader_.name_get()); + this->parent_shader_.name_get().c_str()); } } } diff --git a/source/blender/gpu/opengl/gl_debug.cc b/source/blender/gpu/opengl/gl_debug.cc index 9ed696ac311..919c5707340 100644 --- a/source/blender/gpu/opengl/gl_debug.cc +++ b/source/blender/gpu/opengl/gl_debug.cc @@ -245,9 +245,10 @@ void check_gl_resources(const char *info) if ((ubo_needed & 1) != 0) { const ShaderInput *ubo_input = interface->ubo_get(i); const char *ubo_name = interface->input_name_get(ubo_input); - const char *sh_name = ctx->shader->name_get(); + const StringRefNull sh_name = ctx->shader->name_get(); char msg[256]; - SNPRINTF(msg, "Missing UBO bind at slot %d : %s > %s : %s", i, sh_name, ubo_name, info); + SNPRINTF( + msg, "Missing UBO bind at slot %d : %s > %s : %s", i, sh_name.c_str(), ubo_name, info); debug_callback(0, GL_DEBUG_TYPE_ERROR, 0, GL_DEBUG_SEVERITY_HIGH, 0, msg, nullptr); } } @@ -256,9 +257,10 @@ void check_gl_resources(const char *info) if ((ssbo_needed & 1) != 0) { const ShaderInput *ssbo_input = interface->ssbo_get(i); const char *ssbo_name = interface->input_name_get(ssbo_input); - const char *sh_name = ctx->shader->name_get(); + const StringRefNull sh_name = ctx->shader->name_get(); char msg[256]; - SNPRINTF(msg, "Missing SSBO bind at slot %d : %s > %s : %s", i, sh_name, ssbo_name, info); + SNPRINTF( + msg, "Missing SSBO bind at slot %d : %s > %s : %s", i, sh_name.c_str(), ssbo_name, info); debug_callback(0, GL_DEBUG_TYPE_ERROR, 0, GL_DEBUG_SEVERITY_HIGH, 0, msg, nullptr); } } @@ -268,9 +270,14 @@ void check_gl_resources(const char *info) /* FIXME: texture_get might return an image input instead. */ const ShaderInput *tex_input = interface->texture_get(i); const char *tex_name = interface->input_name_get(tex_input); - const char *sh_name = ctx->shader->name_get(); + const StringRefNull sh_name = ctx->shader->name_get(); char msg[256]; - SNPRINTF(msg, "Missing Texture bind at slot %d : %s > %s : %s", i, sh_name, tex_name, info); + SNPRINTF(msg, + "Missing Texture bind at slot %d : %s > %s : %s", + i, + sh_name.c_str(), + tex_name, + info); debug_callback(0, GL_DEBUG_TYPE_ERROR, 0, GL_DEBUG_SEVERITY_HIGH, 0, msg, nullptr); } } @@ -280,9 +287,10 @@ void check_gl_resources(const char *info) /* FIXME: texture_get might return a texture input instead. */ const ShaderInput *tex_input = interface->texture_get(i); const char *tex_name = interface->input_name_get(tex_input); - const char *sh_name = ctx->shader->name_get(); + const StringRefNull sh_name = ctx->shader->name_get(); char msg[256]; - SNPRINTF(msg, "Missing Image bind at slot %d : %s > %s : %s", i, sh_name, tex_name, info); + SNPRINTF( + msg, "Missing Image bind at slot %d : %s > %s : %s", i, sh_name.c_str(), tex_name, info); debug_callback(0, GL_DEBUG_TYPE_ERROR, 0, GL_DEBUG_SEVERITY_HIGH, 0, msg, nullptr); } } diff --git a/source/blender/gpu/opengl/gl_shader.cc b/source/blender/gpu/opengl/gl_shader.cc index 7a29d3bf684..baf2104003a 100644 --- a/source/blender/gpu/opengl/gl_shader.cc +++ b/source/blender/gpu/opengl/gl_shader.cc @@ -1007,12 +1007,12 @@ bool GLShader::do_geometry_shader_injection(const shader::ShaderCreateInfo *info /** \name Shader stage creation * \{ */ -static const char *glsl_patch_default_get() +static StringRefNull glsl_patch_default_get() { /** Used for shader patching. Init once. */ static std::string patch; if (!patch.empty()) { - return patch.c_str(); + return patch; } std::stringstream ss; @@ -1063,15 +1063,15 @@ static const char *glsl_patch_default_get() ss << datatoc_glsl_shader_defines_glsl; patch = ss.str(); - return patch.c_str(); + return patch; } -static const char *glsl_patch_compute_get() +static StringRefNull glsl_patch_compute_get() { /** Used for shader patching. Init once. */ static std::string patch; if (!patch.empty()) { - return patch.c_str(); + return patch; } std::stringstream ss; @@ -1085,10 +1085,10 @@ static const char *glsl_patch_compute_get() ss << datatoc_glsl_shader_defines_glsl; patch = ss.str(); - return patch.c_str(); + return patch; } -const char *GLShader::glsl_patch_get(GLenum gl_stage) +StringRefNull GLShader::glsl_patch_get(GLenum gl_stage) { if (gl_stage == GL_COMPUTE_SHADER) { return glsl_patch_compute_get(); @@ -1097,12 +1097,12 @@ const char *GLShader::glsl_patch_get(GLenum gl_stage) } GLuint GLShader::create_shader_stage(GLenum gl_stage, - MutableSpan sources, + MutableSpan sources, GLSources &gl_sources) { /* Patch the shader sources to include specialization constants. */ std::string constants_source; - Vector recreated_sources; + Vector recreated_sources; const bool has_specialization_constants = !constants.types.is_empty(); if (has_specialization_constants) { constants_source = constants_declare(); @@ -1114,7 +1114,7 @@ GLuint GLShader::create_shader_stage(GLenum gl_stage, /* Patch the shader code using the first source slot. */ sources[SOURCES_INDEX_VERSION] = glsl_patch_get(gl_stage); - sources[SOURCES_INDEX_SPECIALIZATION_CONSTANTS] = constants_source.c_str(); + sources[SOURCES_INDEX_SPECIALIZATION_CONSTANTS] = constants_source; if (async_compilation_) { gl_sources[SOURCES_INDEX_VERSION].source = std::string(sources[SOURCES_INDEX_VERSION]); @@ -1141,7 +1141,7 @@ GLuint GLShader::create_shader_stage(GLenum gl_stage, } debug_source += "\n\n----------" + source_type + "----------\n\n"; - for (const char *source : sources) { + for (StringRefNull source : sources) { debug_source.append(source); } } @@ -1157,7 +1157,11 @@ GLuint GLShader::create_shader_stage(GLenum gl_stage, return 0; } - glShaderSource(shader, sources.size(), sources.data(), nullptr); + Array c_str_sources(sources.size()); + for (const int i : sources.index_range()) { + c_str_sources[i] = sources[i].c_str(); + } + glShaderSource(shader, c_str_sources.size(), c_str_sources.data(), nullptr); glCompileShader(shader); GLint status; @@ -1194,7 +1198,7 @@ GLuint GLShader::create_shader_stage(GLenum gl_stage, } void GLShader::update_program_and_sources(GLSources &stage_sources, - MutableSpan sources) + MutableSpan sources) { const bool store_sources = !constants.types.is_empty() || async_compilation_; if (store_sources && stage_sources.is_empty()) { @@ -1204,28 +1208,28 @@ void GLShader::update_program_and_sources(GLSources &stage_sources, init_program(); } -void GLShader::vertex_shader_from_glsl(MutableSpan sources) +void GLShader::vertex_shader_from_glsl(MutableSpan sources) { update_program_and_sources(vertex_sources_, sources); program_active_->vert_shader = this->create_shader_stage( GL_VERTEX_SHADER, sources, vertex_sources_); } -void GLShader::geometry_shader_from_glsl(MutableSpan sources) +void GLShader::geometry_shader_from_glsl(MutableSpan sources) { update_program_and_sources(geometry_sources_, sources); program_active_->geom_shader = this->create_shader_stage( GL_GEOMETRY_SHADER, sources, geometry_sources_); } -void GLShader::fragment_shader_from_glsl(MutableSpan sources) +void GLShader::fragment_shader_from_glsl(MutableSpan sources) { update_program_and_sources(fragment_sources_, sources); program_active_->frag_shader = this->create_shader_stage( GL_FRAGMENT_SHADER, sources, fragment_sources_); } -void GLShader::compute_shader_from_glsl(MutableSpan sources) +void GLShader::compute_shader_from_glsl(MutableSpan sources) { update_program_and_sources(compute_sources_, sources); program_active_->compute_shader = this->create_shader_stage( @@ -1240,10 +1244,10 @@ bool GLShader::finalize(const shader::ShaderCreateInfo *info) if (info && do_geometry_shader_injection(info)) { std::string source = workaround_geometry_shader_source_create(*info); - Vector sources; + Vector sources; sources.append("version"); sources.append("/* Specialization Constants. */\n"); - sources.append(source.c_str()); + sources.append(source); geometry_shader_from_glsl(sources); } @@ -1418,7 +1422,7 @@ int GLShader::program_handle_get() const /* -------------------------------------------------------------------- */ /** \name Sources * \{ */ -GLSource::GLSource(const char *other) +GLSource::GLSource(StringRefNull other) { if (!gpu_shader_dependency_get_filename_from_source_string(other).is_empty()) { source = ""; @@ -1426,19 +1430,19 @@ GLSource::GLSource(const char *other) } else { source = other; - source_ref = nullptr; + source_ref = std::nullopt; } } -GLSources &GLSources::operator=(Span other) +GLSources &GLSources::operator=(Span other) { clear(); reserve(other.size()); - for (const char *other_source : other) { + for (StringRefNull other_source : other) { /* Don't store empty string as compilers can optimize these away and result in pointing to a * string that isn't c-str compliant anymore. */ - if (other_source[0] == '\0') { + if (other_source.is_empty()) { continue; } append(GLSource(other_source)); @@ -1447,17 +1451,17 @@ GLSources &GLSources::operator=(Span other) return *this; } -Vector GLSources::sources_get() const +Vector GLSources::sources_get() const { - Vector result; + Vector result; result.reserve(size()); for (const GLSource &source : *this) { if (source.source_ref) { - result.append(source.source_ref); + result.append(*source.source_ref); } else { - result.append(source.source.c_str()); + result.append(source.source); } } return result; @@ -1468,7 +1472,7 @@ std::string GLSources::to_string() const std::string result; for (const GLSource &source : *this) { if (source.source_ref) { - result.append(source.source_ref); + result.append(*source.source_ref); } else { result.append(source.source); @@ -1540,9 +1544,8 @@ bool GLShader::check_link_status() if (!status) { char log[5000]; glGetProgramInfoLog(program_id, sizeof(log), nullptr, log); - Span sources = {debug_source.c_str()}; GLLogParser parser; - print_log(sources, log, "Linking", true, &parser); + print_log({debug_source}, log, "Linking", true, &parser); } return bool(status); @@ -1578,7 +1581,7 @@ GLuint GLShader::program_get() program_active_ = &program_cache_.lookup_or_add_default(constants.values); if (!program_active_->program_id) { - MutableSpan no_sources; + MutableSpan no_sources; if (!vertex_sources_.is_empty()) { program_active_->vert_shader = create_shader_stage( GL_VERTEX_SHADER, no_sources, vertex_sources_); diff --git a/source/blender/gpu/opengl/gl_shader.hh b/source/blender/gpu/opengl/gl_shader.hh index e121515d64c..6dfac705791 100644 --- a/source/blender/gpu/opengl/gl_shader.hh +++ b/source/blender/gpu/opengl/gl_shader.hh @@ -35,15 +35,15 @@ namespace blender::gpu { */ struct GLSource { std::string source; - const char *source_ref; + std::optional source_ref; GLSource() = default; - GLSource(const char *other_source); + GLSource(StringRefNull other_source); }; class GLSources : public Vector { public: - GLSources &operator=(Span other); - Vector sources_get() const; + GLSources &operator=(Span other); + Vector sources_get() const; std::string to_string() const; }; @@ -131,7 +131,7 @@ class GLShader : public Shader { */ void init_program(); - void update_program_and_sources(GLSources &stage_sources, MutableSpan sources); + void update_program_and_sources(GLSources &stage_sources, MutableSpan sources); /** * Link the active program. @@ -159,10 +159,10 @@ class GLShader : public Shader { void init(const shader::ShaderCreateInfo &info, bool is_batch_compilation) override; /** Return true on success. */ - void vertex_shader_from_glsl(MutableSpan sources) override; - void geometry_shader_from_glsl(MutableSpan sources) override; - void fragment_shader_from_glsl(MutableSpan sources) override; - void compute_shader_from_glsl(MutableSpan sources) override; + void vertex_shader_from_glsl(MutableSpan sources) override; + void geometry_shader_from_glsl(MutableSpan sources) override; + void fragment_shader_from_glsl(MutableSpan sources) override; + void compute_shader_from_glsl(MutableSpan sources) override; bool finalize(const shader::ShaderCreateInfo *info = nullptr) override; bool post_finalize(const shader::ShaderCreateInfo *info = nullptr); void warm_cache(int /*limit*/) override{}; @@ -214,11 +214,11 @@ class GLShader : public Shader { GLSourcesBaked get_sources(); private: - const char *glsl_patch_get(GLenum gl_stage); + StringRefNull glsl_patch_get(GLenum gl_stage); /** Create, compile and attach the shader stage to the shader program. */ GLuint create_shader_stage(GLenum gl_stage, - MutableSpan sources, + MutableSpan sources, GLSources &gl_sources); /** diff --git a/source/blender/gpu/vulkan/vk_debug.hh b/source/blender/gpu/vulkan/vk_debug.hh index 5dd19815335..c0dd2cff020 100644 --- a/source/blender/gpu/vulkan/vk_debug.hh +++ b/source/blender/gpu/vulkan/vk_debug.hh @@ -34,7 +34,7 @@ class VKDebuggingTools { }; void object_label(VkObjectType vk_object_type, uint64_t object_handle, const char *name); -template void object_label(T vk_object_type, const char *name) +template void object_label(T vk_object_type, const StringRefNull name) { if (!(G.debug & G_DEBUG_GPU)) { return; @@ -43,7 +43,7 @@ template void object_label(T vk_object_type, const char *name) char label[label_size]; memset(label, 0, label_size); static int stats = 0; - SNPRINTF(label, "%s_%d", name, stats++); + SNPRINTF(label, "%s_%d", name.c_str(), stats++); object_label(to_vk_object_type(vk_object_type), (uint64_t)vk_object_type, (const char *)label); }; diff --git a/source/blender/gpu/vulkan/vk_shader.cc b/source/blender/gpu/vulkan/vk_shader.cc index a861ae681fa..cf0dba2bb8c 100644 --- a/source/blender/gpu/vulkan/vk_shader.cc +++ b/source/blender/gpu/vulkan/vk_shader.cc @@ -24,6 +24,8 @@ #include "BKE_global.hh" +#include + using namespace blender::gpu::shader; namespace blender::gpu { @@ -379,14 +381,14 @@ static void print_resource(std::ostream &os, case ShaderCreateInfo::Resource::BindType::UNIFORM_BUFFER: array_offset = res.uniformbuf.name.find_first_of("["); name_no_array = (array_offset == -1) ? res.uniformbuf.name : - StringRef(res.uniformbuf.name.c_str(), array_offset); + StringRef(res.uniformbuf.name.data(), array_offset); os << "uniform _" << name_no_array << " { " << res.uniformbuf.type_name << " " << res.uniformbuf.name << "; };\n"; break; case ShaderCreateInfo::Resource::BindType::STORAGE_BUFFER: array_offset = res.storagebuf.name.find_first_of("["); name_no_array = (array_offset == -1) ? res.storagebuf.name : - StringRef(res.storagebuf.name.c_str(), array_offset); + StringRef(res.storagebuf.name.data(), array_offset); print_qualifier(os, res.storagebuf.qualifiers); os << "buffer _"; os << name_no_array << " { " << res.storagebuf.type_name << " " << res.storagebuf.name @@ -481,12 +483,9 @@ static std::string main_function_wrapper(std::string &pre_main, std::string &pos return ss.str(); } -static std::string combine_sources(Span sources) +static std::string combine_sources(Span sources) { - char *sources_combined = BLI_string_join_arrayN((const char **)sources.data(), sources.size()); - std::string result(sources_combined); - MEM_freeN(sources_combined); - return result; + return fmt::to_string(fmt::join(sources, "")); } VKShader::VKShader(const char *name) : Shader(name) @@ -517,7 +516,7 @@ VKShader::~VKShader() vk_descriptor_set_layout_ = VK_NULL_HANDLE; } -void VKShader::build_shader_module(MutableSpan sources, +void VKShader::build_shader_module(MutableSpan sources, shaderc_shader_kind stage, VKShaderModule &r_shader_module) { @@ -537,22 +536,22 @@ void VKShader::build_shader_module(MutableSpan sources, } } -void VKShader::vertex_shader_from_glsl(MutableSpan sources) +void VKShader::vertex_shader_from_glsl(MutableSpan sources) { build_shader_module(sources, shaderc_vertex_shader, vertex_module); } -void VKShader::geometry_shader_from_glsl(MutableSpan sources) +void VKShader::geometry_shader_from_glsl(MutableSpan sources) { build_shader_module(sources, shaderc_geometry_shader, geometry_module); } -void VKShader::fragment_shader_from_glsl(MutableSpan sources) +void VKShader::fragment_shader_from_glsl(MutableSpan sources) { build_shader_module(sources, shaderc_fragment_shader, fragment_module); } -void VKShader::compute_shader_from_glsl(MutableSpan sources) +void VKShader::compute_shader_from_glsl(MutableSpan sources) { build_shader_module(sources, shaderc_compute_shader, compute_module); } @@ -573,9 +572,9 @@ bool VKShader::finalize(const shader::ShaderCreateInfo *info) if (do_geometry_shader_injection(info)) { std::string source = workaround_geometry_shader_source_create(*info); - Vector sources; + Vector sources; sources.append("version"); - sources.append(source.c_str()); + sources.append(source); geometry_shader_from_glsl(sources); } @@ -626,8 +625,7 @@ bool VKShader::finalize_shader_module(VKShaderModule &shader_module, const char if (bool(shader_module.compilation_result.GetNumWarnings() + shader_module.compilation_result.GetNumErrors())) { - const char *sources = shader_module.combined_sources.c_str(); - print_log(Span(&sources, 1), + print_log({shader_module.combined_sources}, shader_module.compilation_result.GetErrorMessage().c_str(), stage_name, bool(shader_module.compilation_result.GetNumErrors()), diff --git a/source/blender/gpu/vulkan/vk_shader.hh b/source/blender/gpu/vulkan/vk_shader.hh index 0eee8335fb1..6545fdd8091 100644 --- a/source/blender/gpu/vulkan/vk_shader.hh +++ b/source/blender/gpu/vulkan/vk_shader.hh @@ -59,10 +59,10 @@ class VKShader : public Shader { void init(const shader::ShaderCreateInfo &info, bool is_batch_compilation) override; - void vertex_shader_from_glsl(MutableSpan sources) override; - void geometry_shader_from_glsl(MutableSpan sources) override; - void fragment_shader_from_glsl(MutableSpan sources) override; - void compute_shader_from_glsl(MutableSpan sources) override; + void vertex_shader_from_glsl(MutableSpan sources) override; + void geometry_shader_from_glsl(MutableSpan sources) override; + void fragment_shader_from_glsl(MutableSpan sources) override; + void compute_shader_from_glsl(MutableSpan sources) override; bool finalize(const shader::ShaderCreateInfo *info = nullptr) override; bool finalize_post(); @@ -131,7 +131,7 @@ class VKShader : public Shader { } private: - void build_shader_module(MutableSpan sources, + void build_shader_module(MutableSpan sources, shaderc_shader_kind stage, VKShaderModule &r_shader_module); bool finalize_shader_module(VKShaderModule &shader_module, const char *stage_name);