AllenDang / cimgui-go

Auto generated Go wrapper for Dear ImGui via cimgui
MIT License
298 stars 43 forks source link

Calling backend.DeleteTexture() crashes program #279

Closed tenten8401 closed 2 months ago

tenten8401 commented 2 months ago

I'm making an app that shows a webcam preview in the GUI using GoCV (OpenCV). I need to delete the texture after I'm done drawing it to conserve GPU memory as every frame will be getting kept in VRAM. Calling backend.DeleteTexture() is crashing the entire program from within C function igDeleteTexture().

I can reproduce it with the following code example:

previewRgba, _ := imgui.LoadImage("test.jpeg")
previewTexture := backend.CreateTextureRgba(previewRgba, previewRgba.Bounds().Dx(), previewRgba.Bounds().Dy())

// imgui.Image(previewTexture, imgui.NewVec2(300, 300))

backend.DeleteTexture(previewTexture)

image image

Is there anything obvious that would be causing this? I am calling runtime.LockOSThread() and the crash is happening in goroutine 1 so I'm pretty sure glDeleteTextures() is getting called from the main thread as it should.

gucio321 commented 2 months ago

In pretty sure that giu deletes textures so it might be a good starting point

tenten8401 commented 2 months ago

In pretty sure that giu deletes textures so it might be a good starting point

Any chance you'd be able to locate any portion of code that does? I'm able to find a bunch in giu that create textures but none that delete / unload them :(

gucio321 commented 2 months ago

You're right, and I can confirm it crashes... I think we have texture unloading implemented incorrectly

gucio321 commented 2 months ago

I'm wondering what this glBindTexture is expected to do here... @AllenDang ?

void igDeleteTexture(ImTextureID id) {
  glBindTexture(GL_TEXTURE_2D, 0);
  glDeleteTextures(1, (GLuint *)id);
}

ref: https://registry.khronos.org/OpenGL-Refpages/es2.0/xhtml/glBindTexture.xml

AllenDang commented 2 months ago

@gucio321 Seems it is unnecessary to bind texture to 0 before delete.

tenten8401 commented 2 months ago

Tried commenting out the glBindTexture() and it still crashes. I'm suspecting it may be a type conversion error between the ImTextureID and GLuint* but I'm not sure. I tried enabling OpenGL debug logging by putting glEnable(GL_DEBUG_OUTPUT) inside of igDeleteTexture() to get a better error on why it's crashing but I'm not entirely sure what I'm doing and got stuck :( image

gucio321 commented 2 months ago

I'm pretty sure this is an issue with conversion there. In CreateTexture we have:

    GLuint texId;
    // ...
    return ImTextureID((intptr_t(texId)));

and in DeleteTexture:

  glDeleteTextures(1, (GLuint *)id);

so in my opinion we should do (GLuint*)&id

gucio321 commented 2 months ago

@tenten8401 now it works for me

tenten8401 commented 2 months ago

@gucio321 Program no longer crashes for me after the changes but calling DeleteTexture() seems to break all new texture creation attempts for me :(

Am I correct in assuming this should render something on the screen instead've being black? removing backend.DeleteTexture() makes it work properly again, just with the same VRAM issue :(

previewTexture := imgui.NewTextureFromRgba(imgui.ImageToRgba(previewImg))
imgui.Image(previewTexture.ID, imgui.NewVec2(300, 300))
backend.DeleteTexture(previewTexture.ID)

image image

gucio321 commented 2 months ago

could you check if it works when you remove this BindTexture call?

tenten8401 commented 2 months ago

It does not unfortunately :(

gucio321 commented 2 months ago

I suppose I know whats wrong.

Lets look at this

  glGetIntegerv(GL_TEXTURE_BINDING_2D, &last_texture);
  glGenTextures(1, &texId);
  glBindTexture(GL_TEXTURE_2D, texId);
  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR
  glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR

  // Restore state
  glBindTexture(GL_TEXTURE_2D, last_texture);

^this is from Create Texture so this reffers to last texture, but we have no last texture because we bind 0 to it (or if we removed binding call in delete texture call there is something strange there. Null?)

we should deffinitly proceed this in DeleteTexture somehow

gucio321 commented 2 months ago

I come with my research: according to OpenGL documentation, bind is used to set current texture, and deleteTexture automatically bindst to 0 if deleted texture is current.

I've looked at your code and I think I know whats going on. In my opinion creating/deleting texture every frame is really bad idea. Also, opengl probably enqueues texture loading so that it takes a few miliseconds to get loaded.

I suggest loading texture out of main loop, only once.

tenten8401 commented 2 months ago

@gucio321 I agree that making it every frame isn't great, but it's also a live webcam feed and I don't think there's a clean way to get that into a format imgui can render on the screen without creating a new texture.

I also did consider the few milliseconds to load thing and added a 1 second delay between every line of code there and I saw no change in behavior, so I don't believe it has anything to do with timing.

gucio321 commented 2 months ago

My suppose is that rendering does not happen when calling imgui.Image but at the end of the loop(when calling Render). Try to cache your texture until the next frame and then delete it

tenten8401 commented 2 months ago

@gucio321 Good call, re-wrote my code to treat the rendering as async, persisting the texture between frames and only deleting it when a new image is ready and I now have a stable camera feed with no memory leaks :D

image