Behaviour of glTexImage2D call regarding memory access

From the reference page:

void glTexImage2D(GLenum target, …, const void * data);
If target is GL_TEXTURE_2D, GL_TEXTURE_RECTANGLE or one of the GL_TEXTURE_CUBE_MAP targets, data is read from data (…)

So it seems like it doesn’t matter how data looks like after the glTexImage2D call.
Now I have code that looks something like this:

    std::vector<unsigned char> image;
    unsigned int width, height;
    load_png(image, width, height, "file/path/to/image.png");
    glBindTexture(GL_TEXTURE_2D, a);
    ...
    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, width, height, 0, GL_RGBA, GL_UNSIGNED_BYTE, image.data());
    ...
    load_png(image, width, height, "./other_image.png");
    glBindTexture(GL_TEXTURE_2D, b);
    ...
    glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, width, height, 0, GL_RGBA, GL_UNSIGNED_BYTE, image.data());
    ...

The problem is that the first image (a) looks nothing like the image I would expect, more like the second image (b) with lots of noise. So I assume that glTexImage2D returns before it is fully done reading image.data().
The program works fine if I work with two different std::vectors for each image.
Is this really how OpenGL should work and if yes, is there a way to wait until it is guaranteed that the image has been fully read?

It’s more likely something going on in your code. Behind those ... bits. As stated in the SO question " When does OpenGL get finished with pointers in functions?", all OpenGL functions with the exception of a function ending with the word “Pointer” will consume all pointers to client memory by the time they have returned.

Okay, here is a more complete version of my code:

template<typename T>
class GlObject
{
public:
    GlObject() :
        m_copy_counter(new size_t(1)),
        m_id(T::create())
    {}

    ~GlObject()
    {
        destroy();
    }

    // copy constructor
    GlObject(const GlObject<T>& other) :
        m_copy_counter(other.m_copy_counter),
        m_id(other.m_id)
    {
        *m_copy_counter += 1;
    }

    // copy operator=
    GlObject<T>& operator=(const GlObject<T>& other)
    {
        destroy();
        m_id = other.m_id;
        m_copy_counter = other.m_copy_counter;
        *m_copy_counter += 1;
        return *this;
    }

    // move constructor
    GlObject(GlObject<T>&& other) = delete;

    // move operator=
    GlObject<T>& operator=(GlObject<T>&& other) = delete;

    [[nodiscard]] GLuint id() const
    {
        return m_id;
    }

private:

    void destroy()
    {
        assert(m_copy_counter != nullptr);
        assert(*m_copy_counter > 0);
        *m_copy_counter -= 1;
        if (*m_copy_counter == 0)
        {
            T::destroy(m_id);
            m_id = 0;
            delete m_copy_counter;
        }
    }

    size_t* m_copy_counter = nullptr;

    GLuint m_id = 0;
};

class Texture2D
{
    friend class Framebuffer;

    friend class Program;

public:
    Texture2D(
        const GLint internal_format,
        const GLsizei width,
        const GLsizei height,
        const GLenum format,
        const GLenum type
    )
    {
        assert(m_object.id() != 0);
        glBindTexture(GL_TEXTURE_2D, m_object.id());
        glTexImage2D(GL_TEXTURE_2D, 0, internal_format, width, height, 0, format, type, nullptr);
    }

    Texture2D(const std::vector<unsigned char>& image, const int width, const int height)
    {
        assert(m_object.id() != 0);

        glBindTexture(GL_TEXTURE_2D, m_object.id());
        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
        glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, width, height, 0, GL_RGBA, GL_UNSIGNED_BYTE, image.data());
        //enable mip mapping
        glGenerateMipmap(GL_TEXTURE_2D);
        glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR_MIPMAP_LINEAR);
        glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_LOD_BIAS, -0.3f);
    }

    Texture2D() = default;

    void setParameter(const GLenum pname, const GLint param)
    {
        glBindTexture(GL_TEXTURE_2D, m_object.id());
        glTexParameteri(GL_TEXTURE_2D, pname, param);
    }

private:
    GlObject<GlTextureTraits> m_object;
};

...

for (const auto& o : obj)
{
    ...

    std::vector<unsigned char> image;
    unsigned int width, height;

    lodepng::decode(image, width, height, o.material.diffuse_map);
    gl::Texture2D texture = gl::Texture2D(image, (int)width, (int)height);

    lodepng::decode(image, width, height, o.material.normal_map);
    gl::Texture2D normal_map = gl::Texture2D(image, (int)width, (int)height);

    ...
}

To be honest i don’t see a reason why that shouldn’t work but this does:

std::vector<unsigned char> image_texture;
std::vector<unsigned char> image_normal;
unsigned int width, height;

lodepng::decode(image_texture, width, height, o.material.diffuse_map);
gl::Texture2D texture = gl::Texture2D(image_texture, (int)width, (int)height);

lodepng::decode(image_normal, width, height, o.material.normal_map);
gl::Texture2D normal_map = gl::Texture2D(image_normal, (int)width, (int)height);

other than that glTexImage2D returns early. But I may miss something obvious so I won’t rule that out.

lodepng lacks reasonable documentation, so I had to read the source code to find out what’s going on. Basically, lodepng::decode will append the data to the given vector. Because writing directly to the vector could mean avoiding an extra copy operation, and why would a C++ user care about performance? :roll_eyes:

So you should clear the vector between the two calls. Or better yet, stop using terribly written libraries :wink:


That having been said, there are issues with the code you presented. GlObject is not good. It is almost never a good idea to make a C++ object type copyable but not moveable. Move is an optimized copy, so even if you don’t have any optimizations you could perform on it, you could just do exactly what copy does. Users of C++ will assume that copyability implies moveability, and your code breaks that assumption.

Also, reference-counting for objects like this is a dubious prospect. Not everyone needs the reference count, so having one always in the object isn’t helping them. It’d be better to make the base GlObject act like a unique_ptr, and then have users who need a reference count just make a std::shared_ptr to one. It’s more flexible that way, plus shared_ptr has atomic reference count increments and decrements for basic thread safety.

1 Like

Oh lol. Of course, I was missing something obvious. Also thanks for your input for the rest of my code.