ArjunNair / egui_sdl2_gl

Egui backend for SDL2 + OpenGL
MIT License
52 stars 35 forks source link

Vertex array leaked - user vertex array's data gets overwritten #6

Closed FrankvdStam closed 3 years ago

FrankvdStam commented 3 years ago

Take all of this with a grain of salt; I'm far from an opengl expert, theres are just my findings when trying to use this code with glfw instead of sdl2 (which is a tiny tiny change, it would be incredibly easy to set this up so that both can be used).

        let program = link_program(vert_shader, frag_shader);
        let mut index_buffer = 0;
        let mut pos_buffer = 0;
        let mut tc_buffer = 0;
        let mut color_buffer = 0;
        gl::GenVertexArrays(1, &mut index_buffer);
        gl::BindVertexArray(index_buffer);
        //This overwrites the ID of the generated vertex array with data for a new buffer, effectively leaking video memory.
        gl::GenBuffers(1, &mut index_buffer);
        gl::GenBuffers(1, &mut pos_buffer);
        gl::GenBuffers(1, &mut tc_buffer);
        gl::GenBuffers(1, &mut color_buffer);

You generate a vertex array into index_buffer, then later forget about the vertex array by overwriting it with another buffer. Later on I see index_buffer used to store indices (ELEMENT_ARRAY_BUFFER). But then later on it gets deleted in cleanup with gl::DeleteVertexArrays(1, &self.index_buffer); - which should be gl::DeleteBuffers if it is used to store indices.

I think newer versions of opengl require you to bind a vertex array before you're allowed to do anything; my driver already pre-generates one when I make a new context. What ends up happening in my code is this:

Set up vertex array 1. bind vertex array 1. Buffer triangle vertices/indices yadayada to it. (the vertex and index buffer are internally linked to the vertex_array) Setup a shader.

egui code starts set up vertex array 2 bind vertex array 2 overwrite vertex array with regular buffer - 2 is lost -> we get index buffer 12 back for all I care, this depends on GL state that we don't know about setup other buffers and data?

main loop: do inputs yadayada

draw my own triangle: bind vertex array 1 bind shader for vertex array 1 (for some reason, it seems you DO have to rebind shaders, they seem detached from vertex arrays) draw vertex array 1

egui draws: DONT BIND VERTEX ARRAY buffer data -> this data overwrites the triangle I drew before because opengl considers it bound draw vertex array 1 because it is still bound by opengl

swap buffers: I can see my own triangle and the egui stuff being drawn. On to the 2nd frame

draw my own triangle: bind vertex array 1 -> this has been overwritten by egui! does it draw egui? does it draw a triangle? does it draw garabage? I honestly don't know... draw vertex array 1??

egui draws: DONT BIND VERTEX ARRAY buffer data -> data was already overwritten, nothing really happens draw vertex array 1 because it is still bound by opengl

swap buffers: I can no longer see my own triangle, I can only see the egui stuff being rendered.

The fix is very simple, just setup proper VA managemen. Let me add my changes to a PR.

ArjunNair commented 3 years ago

That makes perfect sense to me. Thanks for pointing this out and fixing it. I'll merge the PR.

FrankvdStam commented 3 years ago

Awesome!