ocornut / imgui

Dear ImGui: Bloat-free Graphical User interface for C++ with minimal dependencies
MIT License
57.8k stars 9.91k forks source link

Drawlists callbacks questions #7665

Open astotrzynascie opened 4 weeks ago

astotrzynascie commented 4 weeks ago

Version/Branch of Dear ImGui:

Version 1.90.3, Branch: master

Back-ends:

imgui_impl_sdl2.cpp + imgui_impl_opengl3.cpp

Compiler, OS:

Windows 10 + Clang-cl 18.1.6

Full config/build information:

No response

Details:

Hello.

I'm using drawlists callbacks the usual(?) way:

if (ImGui::Begin("test", NULL, flags)                   // flags can make a difference
    {
    ImDrawList *draw_list { ImGui::GetWindowDrawList() };       // or GetBackgroundDrawList()
    draw_list->PushClipRect(position_min, position_max);        // or PushClipRectFullScreen();
    draw_list->AddDrawCmd();
    draw_list->PopClipRect();

    TCallbackData *callback_data = new TCallbackData(...);

    draw_list->AddCallback(callback_function, callback_data);
    draw_list->AddCallback(ImDrawCallback_ResetRenderState, nullptr);
    }
ImGui::End();

In callback_function I juggle textures, bind to units, use glsl programs, the whole zoo is involved. That's why I add ImDrawCallback_ResetRenderState.

At first I didn't add AddDrawCmd(); to redraw the rectangle that I will be processing in callback, but I noticed that I have to do it in more and more cases.

First question: is there any clue about when I should or shouldn't refresh / force redraw this part of image?

Second question is about different draw lists in this context, maybe this will help me understand better the ImGui flow.

If I have a single callback in the frame (or all callbacks are in the same draw lists) skipping redraw (AddDrawCmd) results in messed up rectangle(s) that was processed in the callback(s). And now the interesting observation which I mention because it was quite hard to debug/trace it: if callbacks are from different draw lists (GetWindowDrawList/GetBackgroundDrawList) and one is lacking the redraw, then weird and terrible things are happening. In particular, the fonts looks like someone throw a grenade into textures store (font atlas gets textures from random locations?). The whole ImGui goes loco. Additionally, changing window properties, like making window invisible (no background, titlebar, borders) causes the mess looks slightly different.

Is there a short explanation for this?

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

No response

GamingMinds-DanielC commented 4 weeks ago

AddDrawCmd() does not issue any kind of redraw, you should see it more as a pipeline flush. It prevents submitted primitives from before and after the call being merged into the same command. State changes like pushing or popping a clip rect will flush internally, there should be no need to explicitly call this function there. Adding a callback will also call it internally. If explicitly calling or not calling AddDrawCmd() makes a tangible difference, that is an indicator that something is wrong with your callbacks regardless of the call, but the problem may manifest in different ways.

You can process geometry in callbacks, but you shouldn't process geometry that you submitted to ImGui. In that case, you should use the callback to just set up render state, then submit the geometry to ImGui, then add another callback to reset or restore the render state to something that is expected. If you forget to do that, changed render state will affect everything that comes after, even other draw lists that are executed afterwards. So if you are not careful, you can mess up all rendering down the line, and you don't have direct control over the order in which draw lists are executed. But they are executed sequentially, so basically you just need to make sure to fix the render state before the end of the draw list. This is not a bug of the library, just an advanced feature that puts the responsibility of proper use on the user.

If you need to identify your callbacks, you can put identifying information into the user data pointer you are prividing to your callback. If you want to analyze draw calls and render state to get a better understanding and see where it goes wrong, RenderDoc is a great tool to do so.

astotrzynascie commented 4 weeks ago

Thank you for the response.

AddDrawCmd() does not issue any kind of redraw (...)

Ok, got it. For a test I commented out the code and in one project everything looks broken and in second only if I change window draw list to background. Looks like I need to rework the callbacks. I'm not sure what you mean by 'submitting geometry to ImGui', at the moment callbacks look more or less like this:

void callback_example(const ImDrawList *_parent_list, const ImDrawCmd *_cmd)
    {
    // one VAO for the project
    glBindVertexArray(gl::VAO);

    // glActiveTexture & glBindTexture, the texture I use to render ImGui on
    // (bind it's framebuffer before ImGui::Render())
    rebind_texture(ETexture::imgui);  

    TCallbackData *callback_data = static_cast<TCallbackData*>(_cmd->UserCallbackData);

    ImVec2 &position_min = callback_data->position_min;
    ImVec2 &position_max = callback_data->position_max;

    some_shader.use();

    bind_framebuffer(ETexture::imgui);

    set_some_shader_stuff();

    // gl::vertices_set/reset is used set the vertices to draw only position_min - position_max
    // at the end of these are (only one VBO at the project):
    // glBindBuffer(GL_ARRAY_BUFFER, VBO);
    // glBufferData(GL_ARRAY_BUFFER, sizeof(vertices), vertices, GL_STATIC_DRAW);

    gl::vertices_set(position_min.x, position_min.y, position_max.x, position_max.y);
    glDrawElements(GL_TRIANGLES, 6, GL_UNSIGNED_INT, 0);
    generate_mipmap(ETexture::imgui);
    gl::vertices_reset();

    delete callback_data;
    }

I suspect I shouldn't use something (glDrawElements?) or render to a completely different texture and after the callback use draw_list->AddImage or something. In the later case, is it ok (also performance-wise) to create/delete textures for each window's callback every frame or just make one and use it for all callbacks and just manipulate corners? Regarding AddImage, if the texture is premultiplied, is it ok to use glBlendFunc before/after?

Oh, and in some callbacks I use more shaders, giving some ETexture::imgui as an additional texture (for blurring the background for example). In these cases I render to a helper texture of course and then render it back on ETexture::imgui.

Adding ImDrawCallback_ResetRenderState after each callback.

Any hints appreciated.

GamingMinds-DanielC commented 4 weeks ago

I'm not sure what you mean by 'submitting geometry to ImGui', at the moment callbacks look more or less like this:

What I meant was using a callback to set up shaders and other state without drawing anything, then calling other drawing functions like f.e. AddRectFilled() before finally cleaning up with another callback. That way you can submit geometry to ImGui that is rendered with the shaders you supplied. But that doesn't apply to the code you just posted.

So now you need to find out what state is wrong, maybe ImDrawCallback_ResetRenderState doesn't handle every eventuality. To do that I recommend RenderDoc, that tool is made for rendering analysis like this. Draw something after your callback/reset combo and replace these callbacks with a single AddDrawCmd() to make sure the following draw doesn't get merged into the previous one. Make captures with your callback and with the inhibited merge, then compare states between those versions. There you will see what state is different than it should be.

astotrzynascie commented 3 weeks ago

I prepared the data for examination:

First callback:

    ImDrawList *draw_list { ImGui::GetWindowDrawList() };
    TCallback1Data *callback_1_data = new TCallback1Data(...);
    draw_list->AddCallback(callback_1_function, callback_1_data);
    draw_list->AddCallback(ImDrawCallback_ResetRenderState, nullptr);

Second callback

    ImDrawList *draw_list { ImGui::GetBackgroundDrawList() };

    draw_list->AddDrawCmd(); // commented out in 'broken' version

    TCallback2Data *callback_2_data = new TCallback2Data(...);
    draw_list->AddCallback(callback_2_function, callback_2_data);
    draw_list->AddCallback(ImDrawCallback_ResetRenderState, nullptr);

RenderDoc showed me: RenderDoc_difference I checked, only difference is in these two positions (glScissors full screen and glBindTexture), the rest is continued in next position. Number 204 didn't tell me much, was empty in texture inspector and had dimensions 2048x4096, like none of my textures. So 204 must be an ImGui texture. To be sure, I debugged draw_list->AddDrawCmd();, it led me to

    void ImDrawList::AddDrawCmd()
    {
        ImDrawCmd draw_cmd;
        draw_cmd.ClipRect = _CmdHeader.ClipRect;    // Same as calling ImDrawCmd_HeaderCopy()
        draw_cmd.TextureId = _CmdHeader.TextureId;
        draw_cmd.VtxOffset = _CmdHeader.VtxOffset;
        draw_cmd.IdxOffset = IdxBuffer.Size;

        IM_ASSERT(draw_cmd.ClipRect.x <= draw_cmd.ClipRect.z && draw_cmd.ClipRect.y <= draw_cmd.ClipRect.w);
        CmdBuffer.push_back(draw_cmd);
    }

...and TextureId was number 9. So I checked the whole project for glGenTextures and found it in imgui_impl_opengl3.cpp: GL_CALL(glGenTextures(1, &bd->FontTexture)); when debugged, it showed also 9. Anyways, there aren't many textures as I cut down the project to the working minimum with two callbacks.

So, it looks like font texture wasn't bound anywhere(?). I'm always rebinding textures with

        glActiveTexture(GL_TEXTURE0 + _texture.unit);
        glBindTexture(GL_TEXTURE_2D, _texture.tex);

but when I looked up for glActiveTexture in ImGui I found only:

void ImGui_ImplOpenGL3_RenderDrawData(ImDrawData* draw_data)
{
(...)
// Backup GL state
GLenum last_active_texture; glGetIntegerv(GL_ACTIVE_TEXTURE, (GLint*)&last_active_texture);
glActiveTexture(GL_TEXTURE0);
(...)
    // Restore modified GL state
(...)
#ifdef IMGUI_IMPL_OPENGL_MAY_HAVE_BIND_SAMPLER
    if (bd->GlVersion >= 330 || bd->GlProfileIsES3)
        glBindSampler(0, last_sampler);
#endif
    glActiveTexture(last_active_texture);

At this point, I'm a little confused, should I assume that ImGui is using GL_TEXTURE0 ('+0') unit only? I'm not using it at all, my textures start from GL_TEXTURE0 + 1.

So, somehow just glBindTexture is enough, without setting glActiveTexture first ? Looks risky, what if someone just set and used glActiveTexture for his texture and this bind will overwrite it? Someone should set glActiveTexture on unused unit just for ImGui after any operations? Edit: that was a little hasty

And why this binding (if it was bound to GL_TEXTURE0 + 0) was no more?

As an extra exercise I set ImGui::GetWindowDrawList() in both callbacks (so it's also fine, everything looks ok, especially fonts). In this case RenderDoc reported only one Colour Pass completely skipping glDrawElements, glGenerateTextureMipmap and glDrawElementsBaseVertex.

It's my second OpenGL project, first one with ImGui and I really appreciate the help.

The only additional info in Errors and Warnings in RenderDoc were API Info Miscellaneous (...) Buffer detailed info: Buffer object (...) will use VIDEO memory as the source for buffer object operations., so I assume it's fine since it has Severity set to info.

astotrzynascie commented 3 weeks ago

I just did an extra check and in 'full' project I removed all the

    draw_list->PushClipRect(..);
    draw_list->AddDrawCmd();
    draw_list->PopClipRect();

lines and checked that callbacks use the same drawlist (ImGui::GetWindowDrawList()). Results are very similar, same broken font. So, in 'reduced' project I need to switch draw list in one callback to break fonts, and in 'full' project all callbacks are using the same lists and the font is broken anyway (unless I add draw_list->AddDrawCmd();).

astotrzynascie commented 3 weeks ago

If, instead of draw_list->AddDrawCmd(); I put glBindTexture(GL_TEXTURE_2D, (GLuint)(intptr_t)ImGui::GetIO().Fonts->TexID); then fonts look correct. Still, the question is, what breaks it?

Edit: there is just one glActiveTexture (with GL_TEXTURE0, at the beginning of frame/1st color pass) before the 'cutting point', no suspicious texture binding, or something

Edit2: After analyzing all 3 saved cases, it may look like sometimes ImGui is adding glScissors + glBindTexture(GL_TEXTURE_2D, Texture_with_font) before glDrawElementsBaseVertex(number_other_than_font_id) and sometimes not. Sometimes it's just the separated group with glDrawElementsBaseVertex(font_id_number). My experience is still too low to say anything more.

astotrzynascie commented 3 weeks ago

Investigation, part 2.

The case can be narrowed to one callback. Callback should be added to background drawlist. This list is in ImGui_ImplOpenGL3_RenderDrawData drawed first, it may be the difference. The foreground and window list callbacks don't spoil rendering fonts.

If we take a ImGui-OpenGL project containing only one window with one callback, then we get situation in which in the ImGui_ImplOpenGL3_RenderDrawData loop all but one draw list have CmdBuffer.Size == 1. If this list with callback is background or foreground, then it's buffer has 3 commands: callback, reset, and 'finishing command' scissors, bind fonts texture, draw. If this list is the window's one, then we got 4 commands, with the same finishing command at the very beginning (because of PushClipRect(window->InnerClipRect.Min, window->InnerClipRect.Max, true); line in window begin(...) ). Anyways, as all the lists command buffers contain at least one command, before window/foreground lists there are scissord, bind fonts texture, draw commands. In case of background list there isn't any.

Callback could contain only the binding of the texture that later on the ImGui will be rendered on (binding it shouldn't change anything, right?).

    glBindVertexArray(one_and_only_VAO);
    glActiveTexture(GL_TEXTURE0 + n); // n > 0
    glBindTexture(GL_TEXTURE_2D, texture_which_framebuffer_will_be_bound_before_ImGui_ImplOpenGL3_RenderDrawData);

and reset command should be submitted after the callback

    draw_list->AddCallback(callback_function, doesnt_matter);
    draw_list->AddCallback(ImDrawCallback_ResetRenderState, nullptr);

From the RenderDoc point of view it looks like if we add draw_list->AddDrawCmd(); before the callback then additional pre-binding of fonts texture is submited. And this is what matters (as I checked exchanging draw command with glBindTexture(GL_TEXTURE_2D, (GLuint)(intptr_t)ImGui::GetIO().Fonts->TexID);), but I still don't know why, as it seems like binding it just before glDrawElementsBaseVertex / glDrawElements should be enough.