gl_shader_util: Move shader utility code to a separate file

src/video_core/renderer_opengl/gl_resource_manager.cpp

@@ -83,18 +83,6 @@ void OGLSampler::Release() {
     handle = 0;
 }
 
-void OGLShader::Create(std::string_view source, GLenum type) {
-    if (handle != 0) {
-        return;
-    }
-    if (source.empty()) {
-        return;
-    }
-
-    MICROPROFILE_SCOPE(OpenGL_ResourceCreation);
-    handle = GLShader::LoadShader(source, type);
-}
-
 void OGLShader::Release() {
     if (handle == 0)
         return;
@@ -104,21 +92,6 @@ void OGLShader::Release() {
     handle = 0;
 }
 
-void OGLProgram::CreateFromSource(const char* vert_shader, const char* geo_shader,
-                                  const char* frag_shader, bool separable_program,
-                                  bool hint_retrievable) {
-    OGLShader vert, geo, frag;
-    if (vert_shader)
-        vert.Create(vert_shader, GL_VERTEX_SHADER);
-    if (geo_shader)
-        geo.Create(geo_shader, GL_GEOMETRY_SHADER);
-    if (frag_shader)
-        frag.Create(frag_shader, GL_FRAGMENT_SHADER);
-
-    MICROPROFILE_SCOPE(OpenGL_ResourceCreation);
-    Create(separable_program, hint_retrievable, vert.handle, geo.handle, frag.handle);
-}
-
 void OGLProgram::Release() {
     if (handle == 0)
         return;

src/video_core/renderer_opengl/gl_resource_manager.h

@@ -8,7 +8,6 @@
 #include <utility>
 #include <glad/glad.h>
 #include "common/common_types.h"
-#include "video_core/renderer_opengl/gl_shader_util.h"
 
 namespace OpenGL {
 
@@ -128,8 +127,6 @@ public:
         return *this;
     }
 
-    void Create(std::string_view source, GLenum type);
-
     void Release();
 
     GLuint handle = 0;
@@ -151,17 +148,6 @@ public:
         return *this;
     }
 
-    template <typename... T>
-    void Create(bool separable_program, bool hint_retrievable, T... shaders) {
-        if (handle != 0)
-            return;
-        handle = GLShader::LoadProgram(separable_program, hint_retrievable, shaders...);
-    }
-
-    /// Creates a new internal OpenGL resource and stores the handle
-    void CreateFromSource(const char* vert_shader, const char* geo_shader, const char* frag_shader,
-                          bool separable_program = false, bool hint_retrievable = false);
-
     /// Deletes the internal OpenGL resource
     void Release();
 

src/video_core/renderer_opengl/gl_shader_cache.cpp

@@ -31,6 +31,7 @@
 #include "video_core/renderer_opengl/gl_rasterizer.h"
 #include "video_core/renderer_opengl/gl_resource_manager.h"
 #include "video_core/renderer_opengl/gl_shader_cache.h"
+#include "video_core/renderer_opengl/gl_shader_util.h"
 #include "video_core/renderer_opengl/gl_state_tracker.h"
 #include "video_core/shader_cache.h"
 #include "video_core/shader_environment.h"
@@ -53,77 +54,6 @@ auto MakeSpan(Container& container) {
     return std::span(container.data(), container.size());
 }
 
-void AddShader(GLenum stage, GLuint program, std::span<const u32> code) {
-    OGLShader shader;
-    shader.handle = glCreateShader(stage);
-
-    glShaderBinary(1, &shader.handle, GL_SHADER_BINARY_FORMAT_SPIR_V_ARB, code.data(),
-                   static_cast<GLsizei>(code.size_bytes()));
-    glSpecializeShader(shader.handle, "main", 0, nullptr, nullptr);
-    glAttachShader(program, shader.handle);
-    if (!Settings::values.renderer_debug) {
-        return;
-    }
-    GLint shader_status{};
-    glGetShaderiv(shader.handle, GL_COMPILE_STATUS, &shader_status);
-    if (shader_status == GL_FALSE) {
-        LOG_ERROR(Render_OpenGL, "Failed to build shader");
-    }
-    GLint log_length{};
-    glGetShaderiv(shader.handle, GL_INFO_LOG_LENGTH, &log_length);
-    if (log_length == 0) {
-        return;
-    }
-    std::string log(log_length, 0);
-    glGetShaderInfoLog(shader.handle, log_length, nullptr, log.data());
-    if (shader_status == GL_FALSE) {
-        LOG_ERROR(Render_OpenGL, "{}", log);
-    } else {
-        LOG_WARNING(Render_OpenGL, "{}", log);
-    }
-}
-
-void LinkProgram(GLuint program) {
-    glLinkProgram(program);
-    if (!Settings::values.renderer_debug) {
-        return;
-    }
-    GLint link_status{};
-    glGetProgramiv(program, GL_LINK_STATUS, &link_status);
-
-    GLint log_length{};
-    glGetProgramiv(program, GL_INFO_LOG_LENGTH, &log_length);
-    if (log_length == 0) {
-        return;
-    }
-    std::string log(log_length, 0);
-    glGetProgramInfoLog(program, log_length, nullptr, log.data());
-    if (link_status == GL_FALSE) {
-        LOG_ERROR(Render_OpenGL, "{}", log);
-    } else {
-        LOG_WARNING(Render_OpenGL, "{}", log);
-    }
-}
-
-OGLAssemblyProgram CompileProgram(std::string_view code, GLenum target) {
-    OGLAssemblyProgram program;
-    glGenProgramsARB(1, &program.handle);
-    glNamedProgramStringEXT(program.handle, target, GL_PROGRAM_FORMAT_ASCII_ARB,
-                            static_cast<GLsizei>(code.size()), code.data());
-    if (Settings::values.renderer_debug) {
-        const auto err = reinterpret_cast<const char*>(glGetString(GL_PROGRAM_ERROR_STRING_NV));
-        if (err && *err) {
-            if (std::strstr(err, "error")) {
-                LOG_CRITICAL(Render_OpenGL, "\n{}", err);
-                LOG_INFO(Render_OpenGL, "\n{}", code);
-            } else {
-                LOG_WARNING(Render_OpenGL, "\n{}", err);
-            }
-        }
-    }
-    return program;
-}
-
 GLenum Stage(size_t stage_index) {
     switch (stage_index) {
     case 0:
@@ -492,9 +422,8 @@ std::unique_ptr<GraphicsPipeline> ShaderCache::CreateGraphicsPipeline(
     if (!device.UseAssemblyShaders()) {
         source_program.handle = glCreateProgram();
     }
-
+    const size_t first_index = uses_vertex_a && uses_vertex_b ? 1 : 0;
-    for (size_t index = uses_vertex_a && uses_vertex_b ? 1 : 0; index < Maxwell::MaxShaderProgram;
+    for (size_t index = first_index; index < Maxwell::MaxShaderProgram; ++index) {
-         ++index) {
         if (key.unique_hashes[index] == 0) {
             continue;
         }
@@ -510,7 +439,7 @@ std::unique_ptr<GraphicsPipeline> ShaderCache::CreateGraphicsPipeline(
             assembly_programs[stage_index] = CompileProgram(code, AssemblyStage(stage_index));
         } else {
             const std::vector<u32> code{EmitSPIRV(profile, runtime_info, program, binding)};
-            AddShader(Stage(stage_index), source_program.handle, code);
+            AttachShader(Stage(stage_index), source_program.handle, code);
         }
     }
     if (!device.UseAssemblyShaders()) {
@@ -565,7 +494,7 @@ std::unique_ptr<ComputePipeline> ShaderCache::CreateComputePipeline(ShaderPools&
     } else {
         const std::vector<u32> code{EmitSPIRV(profile, program)};
         source_program.handle = glCreateProgram();
-        AddShader(GL_COMPUTE_SHADER, source_program.handle, code);
+        AttachShader(GL_COMPUTE_SHADER, source_program.handle, code);
         LinkProgram(source_program.handle);
     }
     return std::make_unique<ComputePipeline>(device, texture_cache, buffer_cache, gpu_memory,

src/video_core/renderer_opengl/gl_shader_util.cpp

@@ -5,57 +5,100 @@
 #include <string_view>
 #include <vector>
 #include <glad/glad.h>
+
 #include "common/assert.h"
 #include "common/logging/log.h"
+#include "common/settings.h"
 #include "video_core/renderer_opengl/gl_shader_util.h"
 
-namespace OpenGL::GLShader {
+namespace OpenGL {
 
-namespace {
+static void LogShader(GLuint shader) {
-
+    GLint shader_status{};
-std::string_view StageDebugName(GLenum type) {
+    glGetShaderiv(shader, GL_COMPILE_STATUS, &shader_status);
-    switch (type) {
+    if (shader_status == GL_FALSE) {
-    case GL_VERTEX_SHADER:
+        LOG_ERROR(Render_OpenGL, "Failed to build shader");
-        return "vertex";
-    case GL_GEOMETRY_SHADER:
-        return "geometry";
-    case GL_FRAGMENT_SHADER:
-        return "fragment";
-    case GL_COMPUTE_SHADER:
-        return "compute";
     }
-    UNIMPLEMENTED();
+    GLint log_length{};
-    return "unknown";
+    glGetShaderiv(shader, GL_INFO_LOG_LENGTH, &log_length);
+    if (log_length == 0) {
+        return;
     }
-
+    std::string log(log_length, 0);
-} // Anonymous namespace
+    glGetShaderInfoLog(shader, log_length, nullptr, log.data());
-
+    if (shader_status == GL_FALSE) {
-GLuint LoadShader(std::string_view source, GLenum type) {
+        LOG_ERROR(Render_OpenGL, "{}", log);
-    const std::string_view debug_type = StageDebugName(type);
-    const GLuint shader_id = glCreateShader(type);
-
-    const GLchar* source_string = source.data();
-    const GLint source_length = static_cast<GLint>(source.size());
-
-    glShaderSource(shader_id, 1, &source_string, &source_length);
-    LOG_DEBUG(Render_OpenGL, "Compiling {} shader...", debug_type);
-    glCompileShader(shader_id);
-
-    GLint result = GL_FALSE;
-    GLint info_log_length;
-    glGetShaderiv(shader_id, GL_COMPILE_STATUS, &result);
-    glGetShaderiv(shader_id, GL_INFO_LOG_LENGTH, &info_log_length);
-
-    if (info_log_length > 1) {
-        std::string shader_error(info_log_length, ' ');
-        glGetShaderInfoLog(shader_id, info_log_length, nullptr, &shader_error[0]);
-        if (result == GL_TRUE) {
-            LOG_DEBUG(Render_OpenGL, "{}", shader_error);
     } else {
-            LOG_ERROR(Render_OpenGL, "Error compiling {} shader:\n{}", debug_type, shader_error);
+        LOG_WARNING(Render_OpenGL, "{}", log);
     }
 }
-    return shader_id;
-}
 
-} // namespace OpenGL::GLShader
+void AttachShader(GLenum stage, GLuint program, std::string_view code) {
+    OGLShader shader;
+    shader.handle = glCreateShader(stage);
+
+    const GLint length = static_cast<GLint>(code.size());
+    const GLchar* const code_ptr = code.data();
+    glShaderSource(shader.handle, 1, &code_ptr, &length);
+    glCompileShader(shader.handle);
+    glAttachShader(program, shader.handle);
+    if (Settings::values.renderer_debug) {
+        LogShader(shader.handle);
+    }
+}
+
+void AttachShader(GLenum stage, GLuint program, std::span<const u32> code) {
+    OGLShader shader;
+    shader.handle = glCreateShader(stage);
+
+    glShaderBinary(1, &shader.handle, GL_SHADER_BINARY_FORMAT_SPIR_V_ARB, code.data(),
+                   static_cast<GLsizei>(code.size_bytes()));
+    glSpecializeShader(shader.handle, "main", 0, nullptr, nullptr);
+    glAttachShader(program, shader.handle);
+    if (Settings::values.renderer_debug) {
+        LogShader(shader.handle);
+    }
+}
+
+void LinkProgram(GLuint program) {
+    glLinkProgram(program);
+    if (!Settings::values.renderer_debug) {
+        return;
+    }
+    GLint link_status{};
+    glGetProgramiv(program, GL_LINK_STATUS, &link_status);
+
+    GLint log_length{};
+    glGetProgramiv(program, GL_INFO_LOG_LENGTH, &log_length);
+    if (log_length == 0) {
+        return;
+    }
+    std::string log(log_length, 0);
+    glGetProgramInfoLog(program, log_length, nullptr, log.data());
+    if (link_status == GL_FALSE) {
+        LOG_ERROR(Render_OpenGL, "{}", log);
+    } else {
+        LOG_WARNING(Render_OpenGL, "{}", log);
+    }
+}
+
+OGLAssemblyProgram CompileProgram(std::string_view code, GLenum target) {
+    OGLAssemblyProgram program;
+    glGenProgramsARB(1, &program.handle);
+    glNamedProgramStringEXT(program.handle, target, GL_PROGRAM_FORMAT_ASCII_ARB,
+                            static_cast<GLsizei>(code.size()), code.data());
+    if (Settings::values.renderer_debug) {
+        const auto err = reinterpret_cast<const char*>(glGetString(GL_PROGRAM_ERROR_STRING_NV));
+        if (err && *err) {
+            if (std::strstr(err, "error")) {
+                LOG_CRITICAL(Render_OpenGL, "\n{}", err);
+                LOG_INFO(Render_OpenGL, "\n{}", code);
+            } else {
+                LOG_WARNING(Render_OpenGL, "\n{}", err);
+            }
+        }
+    }
+    return program;
+}
+
+} // namespace OpenGL

src/video_core/renderer_opengl/gl_shader_util.h

@@ -4,92 +4,25 @@
 
 #pragma once
 
+#include <span>
 #include <string>
+#include <string_view>
 #include <vector>
+
 #include <glad/glad.h>
+
 #include "common/assert.h"
 #include "common/logging/log.h"
+#include "video_core/renderer_opengl/gl_resource_manager.h"
 
-namespace OpenGL::GLShader {
+namespace OpenGL {
 
-/**
+void AttachShader(GLenum stage, GLuint program, std::string_view code);
- * Utility function to log the source code of a list of shaders.
- * @param shaders The OpenGL shaders whose source we will print.
- */
-template <typename... T>
-void LogShaderSource(T... shaders) {
-    auto shader_list = {shaders...};
 
-    for (const auto& shader : shader_list) {
+void AttachShader(GLenum stage, GLuint program, std::span<const u32> code);
-        if (shader == 0)
-            continue;
 
-        GLint source_length;
+void LinkProgram(GLuint program);
-        glGetShaderiv(shader, GL_SHADER_SOURCE_LENGTH, &source_length);
 
-        std::string source(source_length, ' ');
+OGLAssemblyProgram CompileProgram(std::string_view code, GLenum target);
-        glGetShaderSource(shader, source_length, nullptr, &source[0]);
-        LOG_INFO(Render_OpenGL, "Shader source {}", source);
-    }
-}
 
-/**
+} // namespace OpenGL
- * Utility function to create and compile an OpenGL GLSL shader
- * @param source String of the GLSL shader program
- * @param type Type of the shader (GL_VERTEX_SHADER, GL_GEOMETRY_SHADER or GL_FRAGMENT_SHADER)
- */
-GLuint LoadShader(std::string_view source, GLenum type);
-
-/**
- * Utility function to create and compile an OpenGL GLSL shader program (vertex + fragment shader)
- * @param separable_program whether to create a separable program
- * @param shaders ID of shaders to attach to the program
- * @returns Handle of the newly created OpenGL program object
- */
-template <typename... T>
-GLuint LoadProgram(bool separable_program, bool hint_retrievable, T... shaders) {
-    // Link the program
-    LOG_DEBUG(Render_OpenGL, "Linking program...");
-
-    GLuint program_id = glCreateProgram();
-
-    ((shaders == 0 ? (void)0 : glAttachShader(program_id, shaders)), ...);
-
-    if (separable_program) {
-        glProgramParameteri(program_id, GL_PROGRAM_SEPARABLE, GL_TRUE);
-    }
-    if (hint_retrievable) {
-        glProgramParameteri(program_id, GL_PROGRAM_BINARY_RETRIEVABLE_HINT, GL_TRUE);
-    }
-
-    glLinkProgram(program_id);
-
-    // Check the program
-    GLint result = GL_FALSE;
-    GLint info_log_length;
-    glGetProgramiv(program_id, GL_LINK_STATUS, &result);
-    glGetProgramiv(program_id, GL_INFO_LOG_LENGTH, &info_log_length);
-
-    if (info_log_length > 1) {
-        std::string program_error(info_log_length, ' ');
-        glGetProgramInfoLog(program_id, info_log_length, nullptr, &program_error[0]);
-        if (result == GL_TRUE) {
-            LOG_DEBUG(Render_OpenGL, "{}", program_error);
-        } else {
-            LOG_ERROR(Render_OpenGL, "Error linking shader:\n{}", program_error);
-        }
-    }
-
-    if (result == GL_FALSE) {
-        // There was a problem linking the shader, print the source for debugging purposes.
-        LogShaderSource(shaders...);
-    }
-
-    ASSERT_MSG(result == GL_TRUE, "Shader not linked");
-
-    ((shaders == 0 ? (void)0 : glDetachShader(program_id, shaders)), ...);
-
-    return program_id;
-}
-
-} // namespace OpenGL::GLShader

src/video_core/renderer_opengl/renderer_opengl.cpp

@@ -24,6 +24,7 @@
 #include "video_core/host_shaders/opengl_present_frag.h"
 #include "video_core/host_shaders/opengl_present_vert.h"
 #include "video_core/renderer_opengl/gl_rasterizer.h"
+#include "video_core/renderer_opengl/gl_shader_util.h"
 #include "video_core/renderer_opengl/gl_shader_manager.h"
 #include "video_core/renderer_opengl/renderer_opengl.h"
 #include "video_core/textures/decoders.h"
@@ -230,13 +231,10 @@ void RendererOpenGL::LoadColorToActiveGLTexture(u8 color_r, u8 color_g, u8 color
 
 void RendererOpenGL::InitOpenGLObjects() {
     // Create shader programs
-    OGLShader vertex_shader;
+    present_program.handle = glCreateProgram();
-    vertex_shader.Create(HostShaders::OPENGL_PRESENT_VERT, GL_VERTEX_SHADER);
+    AttachShader(GL_VERTEX_SHADER, present_program.handle, HostShaders::OPENGL_PRESENT_VERT);
-
+    AttachShader(GL_FRAGMENT_SHADER, present_program.handle, HostShaders::OPENGL_PRESENT_FRAG);
-    OGLShader fragment_shader;
+    LinkProgram(present_program.handle);
-    fragment_shader.Create(HostShaders::OPENGL_PRESENT_FRAG, GL_FRAGMENT_SHADER);
-
-    present_program.Create(false, false, vertex_shader.handle, fragment_shader.handle);
 
     // Generate presentation sampler
     present_sampler.Create();

src/video_core/renderer_opengl/util_shaders.cpp

@@ -17,6 +17,7 @@
 #include "video_core/host_shaders/opengl_copy_bgra_comp.h"
 #include "video_core/host_shaders/pitch_unswizzle_comp.h"
 #include "video_core/renderer_opengl/gl_shader_manager.h"
+#include "video_core/renderer_opengl/gl_shader_util.h"
 #include "video_core/renderer_opengl/gl_texture_cache.h"
 #include "video_core/renderer_opengl/util_shaders.h"
 #include "video_core/texture_cache/accelerated_swizzle.h"
@@ -40,13 +41,12 @@ using VideoCommon::Accelerated::MakeBlockLinearSwizzle3DParams;
 using VideoCore::Surface::BytesPerBlock;
 
 namespace {
-
 OGLProgram MakeProgram(std::string_view source) {
-    OGLShader shader;
-    shader.Create(source, GL_COMPUTE_SHADER);
-
     OGLProgram program;
-    program.Create(true, false, shader.handle);
+    OGLShader shader;
+    program.handle = glCreateProgram();
+    AttachShader(GL_COMPUTE_SHADER, program.handle, source);
+    LinkProgram(program.handle);
     return program;
 }
 
@@ -54,7 +54,6 @@ size_t NumPixelsInCopy(const VideoCommon::ImageCopy& copy) {
     return static_cast<size_t>(copy.extent.width * copy.extent.height *
                                copy.src_subresource.num_layers);
 }
-
 } // Anonymous namespace
 
 UtilShaders::UtilShaders(ProgramManager& program_manager_)