From 0d8ecb9508b1936bf2b378763d926b3b95f61dfa Mon Sep 17 00:00:00 2001 From: Jake <> Date: Fri, 8 Jul 2022 02:52:17 -0700 Subject: [PATCH 1/7] Add compile and link status enums --- include/gl_layer/private/types.h | 16 ++++++++++++++-- src/shader.cpp | 12 ++++++------ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/include/gl_layer/private/types.h b/include/gl_layer/private/types.h index 67b9956..30f2090 100644 --- a/include/gl_layer/private/types.h +++ b/include/gl_layer/private/types.h @@ -16,6 +16,18 @@ enum GLShaderInfoParam { GL_SHADER_SOURCE_LENGTH = 0x8B88 }; +enum class CompileStatus { + UNCHECKED = -1, + FAILED = 0, + OK = 1, +}; + +enum class LinkStatus { + UNCHECKED = -1, + FAILED = 0, + OK = 1, +}; + const char* enum_str(GLenum v); struct Version { @@ -28,7 +40,7 @@ struct Shader { unsigned int handle {}; // If this is -1, this means the compile status was never checked by the host application. // This is an error that should be reported. - int compile_status = -1; + CompileStatus compile_status = CompileStatus::UNCHECKED; }; // Represents a shader program returned by glCreateProgram @@ -36,7 +48,7 @@ struct Program { unsigned int handle {}; std::vector shaders {}; // If this is -1, this means the status was never checked by the host application. - int link_status = -1; + LinkStatus link_status = LinkStatus::UNCHECKED; }; } diff --git a/src/shader.cpp b/src/shader.cpp index 7bd6770..e4c43b0 100644 --- a/src/shader.cpp +++ b/src/shader.cpp @@ -27,7 +27,7 @@ void Context::glGetShaderiv(unsigned int handle, GLenum param, int* params) { return; } - it->second.compile_status = *params; + it->second.compile_status = static_cast(*params); } } @@ -39,9 +39,9 @@ void Context::glAttachShader(unsigned int program, unsigned int shader) { return; } - if (it->second.compile_status == -1) { + if (it->second.compile_status == CompileStatus::UNCHECKED) { output_fmt("glAttachShader(program = %u, shader = %u): Always check shader compilation status before trying to use the object.", program, shader); - } else if (it->second.compile_status == false) { + } else if (it->second.compile_status == CompileStatus::FAILED) { output_fmt("glAttachShader(program = %u, shader = %u): Attached shader has a compilation error.", program, shader); } @@ -67,7 +67,7 @@ void Context::glGetProgramiv(unsigned int handle, GLenum param, int* params) { return; } - it->second.link_status = *params; + it->second.link_status = static_cast(*params); } } @@ -78,12 +78,12 @@ void Context::glUseProgram(unsigned int handle) { return; } - if (it->second.link_status == -1) { + if (it->second.link_status == LinkStatus::UNCHECKED) { output_fmt("glUseProgram(program = %u): Always check program link status before trying to use the object.", handle); return; } - if (it->second.link_status == false) { + if (it->second.link_status == LinkStatus::FAILED) { output_fmt("glUseProgram(program = %u): Program has a linker error.", handle); return; } From 0e5cb051275875aeff20638cc5525dc9467f3fb3 Mon Sep 17 00:00:00 2001 From: Jake <> Date: Fri, 8 Jul 2022 04:08:49 -0700 Subject: [PATCH 2/7] Added the includes to cmake so I can actually see the headers in VS --- CMakeLists.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9053b9d..dd6e9f5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,6 +8,9 @@ option(GL_VALIDATION_LAYER_BUILD_TESTS "Build tests for the OpenGL Validation La add_library(gl_validation_layer src/context.cpp src/shader.cpp + include/gl_layer/context.h + include/gl_layer/private/context.h + include/gl_layer/private/types.h ) target_include_directories(gl_validation_layer PUBLIC include/) From 89c425bebea75d4fa427fc0887be800854e45630 Mon Sep 17 00:00:00 2001 From: Jake <> Date: Fri, 8 Jul 2022 06:21:52 -0700 Subject: [PATCH 3/7] Added ContextGLFunctions struct (tentative name), program uniform tracking, and glLinkProgram validation. Also updated the test #yolo --- include/gl_layer/context.h | 11 +++- include/gl_layer/private/context.h | 4 +- include/gl_layer/private/types.h | 23 +++++++- src/context.cpp | 10 ++-- src/shader.cpp | 43 ++++++++++++++- tests/main.cpp | 86 ++++++++++++++++++++---------- 6 files changed, 141 insertions(+), 36 deletions(-) diff --git a/include/gl_layer/context.h b/include/gl_layer/context.h index 949974e..f6722e5 100644 --- a/include/gl_layer/context.h +++ b/include/gl_layer/context.h @@ -5,13 +5,22 @@ extern "C" { #endif +typedef struct ContextGLFunctions +{ + // TODO: replace the types in the parameters with proper platform-detected ones + // TODO: evaluate whether * should be replaced with something like APIENTRYP in glad + void (*GetActiveUniform)(unsigned, unsigned, int, int*, int*, unsigned*, char*); + int (*GetUniformLocation)(unsigned, const char*); + void (*GetProgramiv)(unsigned, unsigned, int*); +}ContextGLFunctions; + /** * @brief Initialize the OpenGL Validation Layer * @param gl_version_major OpenGL context major version. * @param gl_version_minor OpenGL context minor version. * @return 0 on success, any other value on error. */ -int gl_layer_init(unsigned int gl_version_major, unsigned int gl_version_minor); +int gl_layer_init(unsigned int gl_version_major, unsigned int gl_version_minor, const ContextGLFunctions* gl_functions); /** * @brief Terminate the OpenGL Validation Layer. diff --git a/include/gl_layer/private/context.h b/include/gl_layer/private/context.h index b214856..3866551 100644 --- a/include/gl_layer/private/context.h +++ b/include/gl_layer/private/context.h @@ -11,7 +11,7 @@ namespace gl_layer { class Context { public: - explicit Context(Version version); + explicit Context(Version version, const ContextGLFunctions* gl_functions); void set_output_callback(GLLayerOutputFun callback, void* user_data); @@ -20,6 +20,7 @@ class Context { void glAttachShader(unsigned int program, unsigned int shader); void glGetProgramiv(unsigned int handle, GLenum param, int* params); + void glLinkProgram(GLuint program); void glUseProgram(unsigned int handle); private: @@ -27,6 +28,7 @@ class Context { GLLayerOutputFun output_fun = nullptr; void* output_user_data = nullptr; + ContextGLFunctions gl; std::unordered_map shaders{}; std::unordered_map programs{}; diff --git a/include/gl_layer/private/types.h b/include/gl_layer/private/types.h index 30f2090..41e7009 100644 --- a/include/gl_layer/private/types.h +++ b/include/gl_layer/private/types.h @@ -2,10 +2,16 @@ #define GL_VALIDATION_LAYER_TYPES_H_ #include +#include +#include namespace gl_layer { -using GLenum = unsigned int; +using GLenum = std::uint32_t; +using GLuint = std::uint32_t; +using GLint = std::int32_t; +using GLsizei = std::int32_t; +using GLchar = char; enum GLShaderInfoParam { GL_SHADER_TYPE = 0x8B4F, @@ -13,7 +19,9 @@ enum GLShaderInfoParam { GL_COMPILE_STATUS = 0x8B81, GL_LINK_STATUS = 0x8B82, GL_INFO_LOG_LENGTH = 0x8B84, - GL_SHADER_SOURCE_LENGTH = 0x8B88 + GL_SHADER_SOURCE_LENGTH = 0x8B88, + GL_ACTIVE_UNIFORM_MAX_LENGTH = 0x8B87, + GL_ACTIVE_UNIFORMS = 0x8B86 }; enum class CompileStatus { @@ -35,6 +43,16 @@ struct Version { unsigned int minor = 0; }; +struct UniformInfo { + GLint array_size; + GLenum type; + // std::string name // the name is probably not useful for us + + bool operator==(const UniformInfo& other) const { + return array_size == other.array_size && type == other.type; + } +}; + // Represents a shader returned by glCreateShader struct Shader { unsigned int handle {}; @@ -47,6 +65,7 @@ struct Shader { struct Program { unsigned int handle {}; std::vector shaders {}; + std::unordered_map uniforms {}; // If this is -1, this means the status was never checked by the host application. LinkStatus link_status = LinkStatus::UNCHECKED; }; diff --git a/src/context.cpp b/src/context.cpp index 8db7747..3e61587 100644 --- a/src/context.cpp +++ b/src/context.cpp @@ -26,7 +26,8 @@ static void default_output_func(const char* text, void* = nullptr) { printf("%s\n", text); } -Context::Context(Version version) : gl_version(version) { +Context::Context(Version version, const ContextGLFunctions* gl_functions) + : gl_version(version), gl(*gl_functions) { output_fun = &default_output_func; } @@ -46,8 +47,8 @@ static bool is_func(const char* name, std::string_view func) { return func == name; } -int gl_layer_init(unsigned int gl_version_major, unsigned int gl_version_minor) { - gl_layer::g_context = new gl_layer::Context(gl_layer::Version{ gl_version_major, gl_version_minor }); +int gl_layer_init(unsigned int gl_version_major, unsigned int gl_version_minor, const ContextGLFunctions* gl_functions) { + gl_layer::g_context = new gl_layer::Context(gl_layer::Version{ gl_version_major, gl_version_minor }, gl_functions); if (gl_layer::g_context) return 0; return -1; @@ -86,6 +87,9 @@ void gl_layer_callback(const char* name, void* func_ptr, int num_args, ...) { } else if (is_func(name, "glUseProgram")) { unsigned int program = va_arg(args, unsigned int); gl_layer::g_context->glUseProgram(program); + } else if (is_func(name, "glLinkProgram")) { + unsigned int program = va_arg(args, unsigned int); + gl_layer::g_context->glLinkProgram(program); } va_end(args); diff --git a/src/shader.cpp b/src/shader.cpp index e4c43b0..045e830 100644 --- a/src/shader.cpp +++ b/src/shader.cpp @@ -1,6 +1,6 @@ #include #include - +#include namespace gl_layer { @@ -14,7 +14,7 @@ void Context::glCompileShader(unsigned int handle) { return; } - shaders.insert({ handle, Shader { handle } }); + shaders.emplace(handle, Shader{ handle }); } void Context::glGetShaderiv(unsigned int handle, GLenum param, int* params) { @@ -71,6 +71,45 @@ void Context::glGetProgramiv(unsigned int handle, GLenum param, int* params) { } } +void Context::glLinkProgram(GLuint program) +{ + auto program_it = programs.find(program); + if (program_it == programs.end()) { + output_fmt("glLinkProgram(program = %u): Invalid program handle.", program); + return; + } + + auto& programr = program_it->second; + + GLint uniform_count{}; + gl.GetProgramiv(program, GL_ACTIVE_UNIFORMS, &uniform_count); + if (uniform_count > 0) + { + GLint max_name_len{}; + gl.GetProgramiv(program, GL_ACTIVE_UNIFORM_MAX_LENGTH, &max_name_len); + + auto uniform_name = std::make_unique(max_name_len); + + for (int i = 0; i < uniform_count; i++) + { + GLsizei uniform_name_length{}; + UniformInfo uniform_info{}; + gl.GetActiveUniform( + program, + i, + max_name_len, + &uniform_name_length, + &uniform_info.array_size, + &uniform_info.type, + uniform_name.get()); + + auto loc = gl.GetUniformLocation(program, uniform_name.get()); + + programr.uniforms.emplace(loc, uniform_info); + } + } +} + void Context::glUseProgram(unsigned int handle) { auto it = programs.find(handle); if (it == programs.end()) { diff --git a/tests/main.cpp b/tests/main.cpp index 5d35792..c0c96ad 100644 --- a/tests/main.cpp +++ b/tests/main.cpp @@ -1,4 +1,6 @@ #include +#include +#include #include #include @@ -18,32 +20,22 @@ void gl_error_callback([[maybe_unused]] GLenum source, std::cout << "OpenGL Error (default validation): " << type << " message: " << message << std::endl; } -unsigned int create_shader(const char* vtx_path, const char* frag_path) { - using namespace std::literals::string_literals; - - std::fstream file(vtx_path); - - if (!file.good()) { - return 0; - } - - std::stringstream buf; - buf << file.rdbuf(); - - std::string vtx_source(buf.str()); +std::optional load_file(std::string_view path) +{ + std::fstream file(path.data()); - file.close(); - buf = std::stringstream{}; // reset buffer - file.open(frag_path); + if (!file.good()) { + return std::nullopt; + } - if (!file.good()) { - return 0; - } + std::stringstream buf; + buf << file.rdbuf(); - buf << file.rdbuf(); + return std::string(buf.str()); +} - std::string frag_source(buf.str()); - buf = std::stringstream{}; +unsigned int create_shader(std::string_view vtx_source, std::string_view frag_source) { + using namespace std::literals::string_literals; unsigned int vtx_shader = 0, frag_shader = 0; vtx_shader = glCreateShader(GL_VERTEX_SHADER); @@ -51,8 +43,8 @@ unsigned int create_shader(const char* vtx_path, const char* frag_path) { // This is wrapped inside a lambda to limit the scope of vtx_carr and // frag_carr [&vtx_source, &frag_source, &vtx_shader, &frag_shader]() { - const char* vtx_carr = vtx_source.c_str(); - const char* frag_carr = frag_source.c_str(); + const char* vtx_carr = vtx_source.data(); + const char* frag_carr = frag_source.data(); glShaderSource(vtx_shader, 1, &vtx_carr, nullptr); glShaderSource(frag_shader, 1, &frag_carr, nullptr); }(); @@ -102,6 +94,17 @@ unsigned int create_shader(const char* vtx_path, const char* frag_path) { return shaderProgram; } +unsigned create_shader_from_files(std::string_view vtx_path, std::string_view frag_path) +{ + auto vtx_source = load_file(vtx_path); + if (!vtx_source) return 0; + + auto frag_source = load_file(frag_path); + if (!frag_source) return 0; + + return create_shader(*vtx_source, *frag_source); +} + int main() { // Initialize GLFW. @@ -111,14 +114,18 @@ int main() { glfwWindowHint(GLFW_OPENGL_PROFILE, GLFW_OPENGL_CORE_PROFILE); glfwWindowHint(GLFW_OPENGL_DEBUG_CONTEXT, GL_TRUE); glfwWindowHint(GLFW_SRGB_CAPABLE, GL_TRUE); - + GLFWwindow* window = glfwCreateWindow(800, 600, "OpenGL Validation Layer - tests", nullptr, nullptr); glfwMakeContextCurrent(window); // Load GLAD. gladLoadGLLoader((GLADloadproc)glfwGetProcAddress); // Initialize our library. - int error = gl_layer_init(3, 3); + ContextGLFunctions glFuncs; + glFuncs.GetActiveUniform = glad_glGetActiveUniform; + glFuncs.GetUniformLocation = glad_glGetUniformLocation; + glFuncs.GetProgramiv = glad_glGetProgramiv; + int error = gl_layer_init(3, 3, &glFuncs); if (error) { std::cerr << "Could not initialize OpenGL Validation Layer\n"; return -1; @@ -128,7 +135,31 @@ int main() { // glDebugMessageCallback(&gl_error_callback, nullptr); glad_set_post_callback(&gl_layer_callback); - unsigned int program = create_shader("shaders/vert.glsl", "shaders/frag.glsl"); + const char* vtx_source2 = R"( +#version 330 core + +layout(location = 0) in vec3 iPos; + +void main() { + gl_Position = vec4(iPos, 1); +} +)"; + + const char* frag_source2 = R"( +#version 330 core + +uniform vec3 u_color; +uniform float u_alpha; + +out vec4 o_color; + +void main() { + o_color = vec4(u_color, u_alpha); +} +)"; + + unsigned int program = create_shader_from_files("shaders/vert.glsl", "shaders/frag.glsl"); + unsigned int program2 = create_shader(vtx_source2, frag_source2); // Main application loop while(!glfwWindowShouldClose(window)) { @@ -137,6 +168,7 @@ int main() { glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT); glUseProgram(program); + glUseProgram(program2); glfwSwapBuffers(window); } From b1bf8d01c6a7647be0d14c7bef38c78981244e96 Mon Sep 17 00:00:00 2001 From: Jake <> Date: Sun, 10 Jul 2022 03:18:24 -0700 Subject: [PATCH 4/7] Moar program validation --- include/gl_layer/private/context.h | 5 +++ include/gl_layer/private/types.h | 2 + src/context.cpp | 10 ++++- src/shader.cpp | 61 ++++++++++++++++++++++++++---- 4 files changed, 69 insertions(+), 9 deletions(-) diff --git a/include/gl_layer/private/context.h b/include/gl_layer/private/context.h index 3866551..0ea722a 100644 --- a/include/gl_layer/private/context.h +++ b/include/gl_layer/private/context.h @@ -22,6 +22,10 @@ class Context { void glGetProgramiv(unsigned int handle, GLenum param, int* params); void glLinkProgram(GLuint program); void glUseProgram(unsigned int handle); + void glDeleteProgram(GLuint program); + + void validate_program_bound(std::string_view func_name); + bool validate_program_status(GLuint program); private: Version gl_version; @@ -29,6 +33,7 @@ class Context { GLLayerOutputFun output_fun = nullptr; void* output_user_data = nullptr; ContextGLFunctions gl; + GLuint bound_program = 0; std::unordered_map shaders{}; std::unordered_map programs{}; diff --git a/include/gl_layer/private/types.h b/include/gl_layer/private/types.h index 41e7009..86091be 100644 --- a/include/gl_layer/private/types.h +++ b/include/gl_layer/private/types.h @@ -12,6 +12,8 @@ using GLuint = std::uint32_t; using GLint = std::int32_t; using GLsizei = std::int32_t; using GLchar = char; +using GLfloat = float; +using GLboolean = bool; enum GLShaderInfoParam { GL_SHADER_TYPE = 0x8B4F, diff --git a/src/context.cpp b/src/context.cpp index 3e61587..f1a234b 100644 --- a/src/context.cpp +++ b/src/context.cpp @@ -43,10 +43,14 @@ Context* g_context = nullptr; } // namespace gl_layer -static bool is_func(const char* name, std::string_view func) { +static bool is_func(std::string_view name, std::string_view func) { return func == name; } +static bool func_has(std::string_view name, std::string_view substr) { + return name.find(substr) != std::string_view::npos; +} + int gl_layer_init(unsigned int gl_version_major, unsigned int gl_version_minor, const ContextGLFunctions* gl_functions) { gl_layer::g_context = new gl_layer::Context(gl_layer::Version{ gl_version_major, gl_version_minor }, gl_functions); if (gl_layer::g_context) return 0; @@ -64,6 +68,10 @@ void gl_layer_callback(const char* name, void* func_ptr, int num_args, ...) { return; } + using namespace gl_layer; // GL types + + std::string_view name = name_c; + va_list args; va_start(args, num_args); diff --git a/src/shader.cpp b/src/shader.cpp index 045e830..29061f0 100644 --- a/src/shader.cpp +++ b/src/shader.cpp @@ -79,8 +79,9 @@ void Context::glLinkProgram(GLuint program) return; } - auto& programr = program_it->second; + auto& programInfo = program_it->second; + // Store all active uniforms in the program (location, array size, and type) GLint uniform_count{}; gl.GetProgramiv(program, GL_ACTIVE_UNIFORMS, &uniform_count); if (uniform_count > 0) @@ -105,26 +106,70 @@ void Context::glLinkProgram(GLuint program) auto loc = gl.GetUniformLocation(program, uniform_name.get()); - programr.uniforms.emplace(loc, uniform_info); + programInfo.uniforms.emplace(loc, uniform_info); } } } void Context::glUseProgram(unsigned int handle) { - auto it = programs.find(handle); + if (handle == 0) { + bound_program = 0; + return; + } + + if (!validate_program_status(handle)) { + return; + } + + // TODO: add optional performance warning for rebinding the same program + //if (handle == bound_program) { + // output_fmt("glUseProgram(program = %u): Program is already bound.", handle); + //} + + bound_program = handle; +} + +void Context::glDeleteProgram(GLuint program) +{ + auto it = programs.find(program); if (it == programs.end()) { - output_fmt("glUseProgram(program = %u): Invalid program handle.", handle); + output_fmt("glUseProgram(program = %u): Invalid program handle.", program); return; } - if (it->second.link_status == LinkStatus::UNCHECKED) { - output_fmt("glUseProgram(program = %u): Always check program link status before trying to use the object.", handle); + // note: this does not change which program is bound, even if it becomes invalid here + + programs.erase(it); +} + +void Context::validate_program_bound(std::string_view func_name) +{ + if (bound_program == 0) { + output_fmt("%s: No program bound.", func_name.data()); return; } +} + +bool Context::validate_program_status(GLuint program) +{ + auto it = programs.find(program); + if (it == programs.end()) { + output_fmt("glUseProgram(program = %u): Invalid program handle.", program); + return false; + } + + if (it->second.link_status == LinkStatus::UNCHECKED) { + output_fmt("glUseProgram(program = %u): Always check program link status before trying to use the object.", program); + return false; + } if (it->second.link_status == LinkStatus::FAILED) { - output_fmt("glUseProgram(program = %u): Program has a linker error.", handle); - return; + output_fmt("glUseProgram(program = %u): Program has a linker error.", program); + return false; + } + + return true; +} } } From d171901a9573ecf4e4b64d25ac07ca4f4c04d74c Mon Sep 17 00:00:00 2001 From: Jake <> Date: Sun, 10 Jul 2022 03:31:45 -0700 Subject: [PATCH 5/7] Added build folder to gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 990936c..9e92679 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ .idea/ cmake-build-debug/ +/build From 3d8c4af3f90230d3817c0760c46d77c258e60790 Mon Sep 17 00:00:00 2001 From: Jake <> Date: Sun, 10 Jul 2022 04:25:05 -0700 Subject: [PATCH 6/7] Fixed some silly errors and switched to GL types where needed (for consistency). --- include/gl_layer/private/context.h | 14 +++++----- src/context.cpp | 25 +++++++++--------- src/shader.cpp | 42 +++++++++++++----------------- 3 files changed, 38 insertions(+), 43 deletions(-) diff --git a/include/gl_layer/private/context.h b/include/gl_layer/private/context.h index 0ea722a..68f336e 100644 --- a/include/gl_layer/private/context.h +++ b/include/gl_layer/private/context.h @@ -15,13 +15,13 @@ class Context { void set_output_callback(GLLayerOutputFun callback, void* user_data); - void glCompileShader(unsigned int handle); - void glGetShaderiv(unsigned int handle, GLenum param, int* params); - void glAttachShader(unsigned int program, unsigned int shader); + void glCompileShader(GLuint program); + void glGetShaderiv(GLuint program, GLenum param, GLint* params); + void glAttachShader(GLuint program, GLuint shader); - void glGetProgramiv(unsigned int handle, GLenum param, int* params); + void glGetProgramiv(GLuint program, GLenum param, GLint* params); void glLinkProgram(GLuint program); - void glUseProgram(unsigned int handle); + void glUseProgram(GLuint program); void glDeleteProgram(GLuint program); void validate_program_bound(std::string_view func_name); @@ -35,8 +35,8 @@ class Context { ContextGLFunctions gl; GLuint bound_program = 0; - std::unordered_map shaders{}; - std::unordered_map programs{}; + std::unordered_map shaders{}; + std::unordered_map programs{}; template void output_fmt(const char* fmt, Args&& ... args) { diff --git a/src/context.cpp b/src/context.cpp index f1a234b..568048f 100644 --- a/src/context.cpp +++ b/src/context.cpp @@ -62,7 +62,7 @@ void gl_layer_terminate() { delete gl_layer::g_context; } -void gl_layer_callback(const char* name, void* func_ptr, int num_args, ...) { +void gl_layer_callback(const char* name_c, void* func_ptr, int num_args, ...) { if (!gl_layer::g_context) { // Report error: context not initialized. return; @@ -76,27 +76,28 @@ void gl_layer_callback(const char* name, void* func_ptr, int num_args, ...) { va_start(args, num_args); if (is_func(name, "glCompileShader")) { - gl_layer::g_context->glCompileShader(va_arg(args, unsigned int)); + auto shader = va_arg(args, GLuint); + gl_layer::g_context->glCompileShader(shader); } else if (is_func(name, "glGetShaderiv")) { - unsigned int shader = va_arg(args, unsigned int); - gl_layer::GLenum param = va_arg(args, gl_layer::GLenum); - int* params = va_arg(args, int*); + auto shader = va_arg(args, GLuint); + gl_layer::GLenum param = va_arg(args, GLenum); + auto* params = va_arg(args, GLint*); gl_layer::g_context->glGetShaderiv(shader, param, params); } else if (is_func(name, "glAttachShader")) { // Note that the parameters must be pulled outside the function call, since there is no guarantee that they will evaluate in order! - unsigned int program = va_arg(args, unsigned int); - unsigned int shader = va_arg(args, unsigned int); + auto program = va_arg(args, GLuint); + auto shader = va_arg(args, GLuint); gl_layer::g_context->glAttachShader(program, shader); } else if (is_func(name, "glGetProgramiv")) { - unsigned int program = va_arg(args, unsigned int); - gl_layer::GLenum param = va_arg(args, gl_layer::GLenum); - int* params = va_arg(args, int*); + auto program = va_arg(args, GLuint); + gl_layer::GLenum param = va_arg(args, GLenum); + auto* params = va_arg(args, GLint*); gl_layer::g_context->glGetProgramiv(program, param, params); } else if (is_func(name, "glUseProgram")) { - unsigned int program = va_arg(args, unsigned int); + auto program = va_arg(args, GLuint); gl_layer::g_context->glUseProgram(program); } else if (is_func(name, "glLinkProgram")) { - unsigned int program = va_arg(args, unsigned int); + auto program = va_arg(args, GLuint); gl_layer::g_context->glLinkProgram(program); } diff --git a/src/shader.cpp b/src/shader.cpp index 29061f0..1ac2d83 100644 --- a/src/shader.cpp +++ b/src/shader.cpp @@ -4,26 +4,26 @@ namespace gl_layer { -void Context::glCompileShader(unsigned int handle) { +void Context::glCompileShader(GLuint program) { // We will use glCompileShader to add shaders to our internal structure, since // we cannot access the return value from glCreateShader() - auto it = shaders.find(handle); + auto it = shaders.find(program); if (it != shaders.end()) { - output_fmt("glCompileShader(shader = %u): Shader is already compiled.", handle); + output_fmt("glCompileShader(shader = %u): Shader is already compiled.", program); return; } - shaders.emplace(handle, Shader{ handle }); + shaders.emplace(program, Shader{ program }); } -void Context::glGetShaderiv(unsigned int handle, GLenum param, int* params) { +void Context::glGetShaderiv(GLuint program, GLenum param, GLint* params) { assert(params && "params may not be nullptr"); if (param == GL_COMPILE_STATUS) { - auto it = shaders.find(handle); + auto it = shaders.find(program); if (it == shaders.end()) { - output_fmt("glGetShaderiv(handle = %u, param = %s, params = %p): Invalid shader handle.", handle, enum_str(param), static_cast(params)); + output_fmt("glGetShaderiv(handle = %u, param = %s, params = %p): Invalid shader handle.", program, enum_str(param), static_cast(params)); return; } @@ -31,7 +31,7 @@ void Context::glGetShaderiv(unsigned int handle, GLenum param, int* params) { } } -void Context::glAttachShader(unsigned int program, unsigned int shader) { +void Context::glAttachShader(GLuint program, GLuint shader) { // Make sure compile status was checked and successful when attaching a shader. auto it = shaders.find(shader); if (it == shaders.end()) { @@ -57,13 +57,13 @@ void Context::glAttachShader(unsigned int program, unsigned int shader) { } } -void Context::glGetProgramiv(unsigned int handle, GLenum param, int* params) { +void Context::glGetProgramiv(GLuint program, GLenum param, GLint* params) { assert(params && "params may not be nullptr"); if (param == GL_LINK_STATUS) { - auto it = programs.find(handle); + auto it = programs.find(program); if (it == programs.end()) { - output_fmt("glGetProgramiv(handle = %u, param = %s, params = %p): Invalid program handle.", handle, enum_str(param), static_cast(params)); + output_fmt("glGetProgramiv(handle = %u, param = %s, params = %p): Invalid program handle.", program, enum_str(param), static_cast(params)); return; } @@ -111,13 +111,13 @@ void Context::glLinkProgram(GLuint program) } } -void Context::glUseProgram(unsigned int handle) { - if (handle == 0) { +void Context::glUseProgram(GLuint program) { + if (program == 0) { bound_program = 0; return; } - if (!validate_program_status(handle)) { + if (!validate_program_status(program)) { return; } @@ -126,11 +126,10 @@ void Context::glUseProgram(unsigned int handle) { // output_fmt("glUseProgram(program = %u): Program is already bound.", handle); //} - bound_program = handle; + bound_program = program; } -void Context::glDeleteProgram(GLuint program) -{ +void Context::glDeleteProgram(GLuint program) { auto it = programs.find(program); if (it == programs.end()) { output_fmt("glUseProgram(program = %u): Invalid program handle.", program); @@ -142,16 +141,14 @@ void Context::glDeleteProgram(GLuint program) programs.erase(it); } -void Context::validate_program_bound(std::string_view func_name) -{ +void Context::validate_program_bound(std::string_view func_name) { if (bound_program == 0) { output_fmt("%s: No program bound.", func_name.data()); return; } } -bool Context::validate_program_status(GLuint program) -{ +bool Context::validate_program_status(GLuint program) { auto it = programs.find(program); if (it == programs.end()) { output_fmt("glUseProgram(program = %u): Invalid program handle.", program); @@ -170,7 +167,4 @@ bool Context::validate_program_status(GLuint program) return true; } - } -} - } \ No newline at end of file From fd3a3b19d8ba84733cc5010c1969ce30eccabd0d Mon Sep 17 00:00:00 2001 From: Jake <> Date: Sun, 10 Jul 2022 04:37:28 -0700 Subject: [PATCH 7/7] Renamed some variables for clarity and consistency --- include/gl_layer/private/context.h | 2 +- src/shader.cpp | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/include/gl_layer/private/context.h b/include/gl_layer/private/context.h index 68f336e..87020e6 100644 --- a/include/gl_layer/private/context.h +++ b/include/gl_layer/private/context.h @@ -33,7 +33,7 @@ class Context { GLLayerOutputFun output_fun = nullptr; void* output_user_data = nullptr; ContextGLFunctions gl; - GLuint bound_program = 0; + GLuint current_program_handle = 0; std::unordered_map shaders{}; std::unordered_map programs{}; diff --git a/src/shader.cpp b/src/shader.cpp index 1ac2d83..1ff2389 100644 --- a/src/shader.cpp +++ b/src/shader.cpp @@ -79,7 +79,7 @@ void Context::glLinkProgram(GLuint program) return; } - auto& programInfo = program_it->second; + auto& program_info = program_it->second; // Store all active uniforms in the program (location, array size, and type) GLint uniform_count{}; @@ -106,14 +106,14 @@ void Context::glLinkProgram(GLuint program) auto loc = gl.GetUniformLocation(program, uniform_name.get()); - programInfo.uniforms.emplace(loc, uniform_info); + program_info.uniforms.emplace(loc, uniform_info); } } } void Context::glUseProgram(GLuint program) { if (program == 0) { - bound_program = 0; + current_program_handle = 0; return; } @@ -122,11 +122,11 @@ void Context::glUseProgram(GLuint program) { } // TODO: add optional performance warning for rebinding the same program - //if (handle == bound_program) { + //if (handle == current_program_handle) { // output_fmt("glUseProgram(program = %u): Program is already bound.", handle); //} - bound_program = program; + current_program_handle = program; } void Context::glDeleteProgram(GLuint program) { @@ -142,7 +142,7 @@ void Context::glDeleteProgram(GLuint program) { } void Context::validate_program_bound(std::string_view func_name) { - if (bound_program == 0) { + if (current_program_handle == 0) { output_fmt("%s: No program bound.", func_name.data()); return; }