Problem retrieving shaders stored inside std::map-CodePudding

So I've been learning OpenGL for sometimes now and today I come up with an idea to put multiple shaders in one std::map, like this

std::map<std::string, Shader>     Shaders;

the key holds the name of the shader and the value is the Shader class I created. Then I worte both add and get shader function

void AddShader(std::string filepath, std::string name)
{
    Shader shader(filepath);
    Shaders[name] = shader;
    if (glIsProgram(Shaders[name].m_RendererID) != GL_TRUE)
    {
        std::cout << "invalid program" << std::endl;
        __debugbreak();
    }
}

Shader GetShader(std::string name)
{
    //added for debug purposes.
    if (glIsProgram(Shaders[name].m_RendererID) != GL_TRUE)
    {
        std::cout << "invalid program" << std::endl;
        __debugbreak();
    }
    return Shaders[name];
}

Now here comes the problem. Whenever I tried to call GetShader function I would recieve invalid program error and bind shader wouldn't work either as it would return GL_INVALID_VALUE

here is shader class I abstracted:

Shader.h

#pragma once
#include <string>
#include "glm/glm.hpp"
#include <glad/glad.h>
struct ShaderProgramSources
{
    std::string VertexSource;
    std::string FragmentSource;
};

class Shader
{
private:
    
    std::string m_FilePath;
    int GetUniformLocation(const std::string& name);
public:
    unsigned int m_RendererID;
    Shader(const std::string& filepath);
    Shader(void) {}
    ~Shader();

    void Bind() const;
    void Unbind() const;

    ShaderProgramSources ParseShader(const std::string& filepath);
    unsigned int CompileShader(unsigned int type, const std::string& source);
    unsigned int CreateShader(const std::string& vertexShader, const std::string& fragmentShader);

Shader.cpp

#include "Shader.h"
#include <iostream>
#include <string> 

#include <fstream>
#include <string>
#include <sstream>

Shader::Shader(const std::string& filepath)
    : m_FilePath(filepath), m_RendererID(0)
{
    ShaderProgramSources source = ParseShader(filepath);
    m_RendererID = CreateShader(source.VertexSource, source.FragmentSource);

}

Shader::~Shader()
{
    glDeleteProgram(m_RendererID);
}

void Shader::Bind() const
{
    glUseProgram(m_RendererID);
}

void Shader::Unbind() const
{
    glUseProgram(0);
}

unsigned int Shader::CreateShader(const std::string& vertexShader, const std::string& fragmentShader)
{
    unsigned int program = glCreateProgram();
    unsigned int vs = CompileShader(GL_VERTEX_SHADER, vertexShader);
    unsigned int fs = CompileShader(GL_FRAGMENT_SHADER, fragmentShader);

    glAttachShader(program, vs);
    glAttachShader(program, fs);
    glLinkProgram(program);
    glValidateProgram(program);

    glDeleteShader(vs);
    glDeleteShader(fs);

    return program;
}

unsigned int Shader::CompileShader(unsigned int type, const std::string& source)
{
    unsigned int id = glCreateShader(type);
    const char* src = source.c_str();
    glShaderSource(id, 1, &src, nullptr);
    glCompileShader(id);
    int result;
    glGetShaderiv(id, GL_COMPILE_STATUS, &result);
    if (result == GL_FALSE)
    {
        int length;
        glGetShaderiv(id, GL_INFO_LOG_LENGTH, &length);
        char* message = (char*)alloca(length * sizeof(char));
        glGetShaderInfoLog(id, length, &length, message);
        std::cout << "Failed to compile " << std::endl;
        if (type == GL_VERTEX_SHADER)
            std::cout << "vertex " << std::endl;
        else
            std::cout << "fragment " << std::endl;
        std::cout << message << std::endl;
        glDeleteShader(id);
        return 0;
    }

    return id;
}

ShaderProgramSources Shader::ParseShader(const std::string& filepath)
{
    std::ifstream stream(filepath);
    enum class ShaderType
    {
        NONE = -1, VERTEX = 0, FRAGMENT = 1
    };

    std::string line;
    std::stringstream ss[2];
    ShaderType type = ShaderType::NONE;
    while (getline(stream, line))
    {
        if (line.find("#shader") != std::string::npos)
        {
            if (line.find("vertex") != std::string::npos)
            {
                type = ShaderType::VERTEX;
            }
            if (line.find("fragment") != std::string::npos)
            {
                type = ShaderType::FRAGMENT;
            }
        }
        else
        {
            ss[(int)type] << line << '\n';
        }

    }
    ShaderProgramSources sources;
    sources.VertexSource = ss[0].str();
    sources.FragmentSource = ss[1].str();

    return sources;

}

both vertex and fragment shader are put in one single file identified by "#shader vertex" and "#shader fragment" at the beginning of each shader. Like this:

#shader vertex
#version 330 core
...

#shader fragment
#version 330 core
...

Main file


int main(int argc, char* argv[])
{
    glfwInit();
    glfwWindowHint(GLFW_CONTEXT_VERSION_MAJOR, 4);
    glfwWindowHint(GLFW_CONTEXT_VERSION_MINOR, 4);
    glfwWindowHint(GLFW_OPENGL_PROFILE, GLFW_OPENGL_CORE_PROFILE);
    glfwWindowHint(GLFW_OPENGL_DEBUG_CONTEXT, GL_TRUE);

    GLFWwindow* window = glfwCreateWindow(SCREEN_WIDTH, SCREEN_HEIGHT, "TITLE", nullptr, nullptr);
    glfwMakeContextCurrent(window);

    if (!gladLoadGLLoader((GLADloadproc)glfwGetProcAddress))
    {
        std::cout << "Failed to initialize GLAD" << std::endl;
        return -1;
    }

    
    glViewport(0, 0, SCREEN_WIDTH, SCREEN_HEIGHT);
    
    glEnable(GL_BLEND);
    glBlendFunc(GL_SRC_ALPHA, GL_ONE_MINUS_SRC_ALPHA);
    glDebugMessageCallback([](GLenum source, GLenum type, GLuint id, GLenum serverity,
        GLsizei length, const GLchar* message, const void* userParam)
        {
            std::cout << source << message << std::endl;
            __debugbreak();
        }, NULL);
    
    AddShader("Shader/test1.shader", "shader1");
    AddShader("Shader/test2.shader", "shader2");
    
    Shader shader = GetShader("shader1");
    Shader shader2 = GetShader("shader2");
    shader.Bind();
    float deltaTime = 0.0f;
    float lastFrame = 0.0f;
    while (!glfwWindowShouldClose(window))
    {
        float currentFrame = glfwGetTime();
        deltaTime = currentFrame - lastFrame;
        lastFrame = currentFrame;
        glfwPollEvents();

        

        glClearColor(0.0f, 0.0f, 0.0f, 1.0f);
        glClear(GL_COLOR_BUFFER_BIT);

        

        glfwSwapBuffers(window);

    }

    glfwTerminate();
    return 0;

}

Can somebody please tell me where the problems are? Thanks

CodePudding user response：

Look at your AddShader code:

    Shader shader(filepath);

So you have a Shader object local to this function.

    Shaders[name] = shader;

Now this copies the Shader object into the container, using the implicit copy constructor. You have now two Shader objects, both referencing the same GL object ID, one in the map, and one in your local variable.

When the function is left, the local variable gets out of scope, and the destructor is called, which destroys the GL object. What's left is the copy in the map, which still references the now-destroyed GL object ID.

Naively wrapping GL objects in C objects isn't going to work. You will need a more elaborate scheme for proper OpenGL object management.

CodePudding user response：

if (glIsProgram(Shaders[name].m_RendererID) != GL_TRUE)

Shaders is a std::map. Its operator[] default-constructs the object if the key does not exist in the map.

Your Shader class's default constructor fails to initialize m_RendererID. As such, this results in undefined behavior, whenever the name that gets passed in does not exist.

You can use your debugger to set a breakpoint here, when that happens, then work your way up the call stack and determine the reason for the logical error. You will discover that m_RendererID gets set to the return value of glCreateProgram, and based on its description, a comparison against GL_TRUE appears to be meaningless.