ocornut / imgui

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

Swapping ImDrawLists not rendering anything #4763

Open Cascades opened 2 years ago

Cascades commented 2 years ago

Version/Branch of Dear ImGui:

Version: 1.64 Branch: master

Back-end/Renderer/Compiler/OS

Back-ends: Custom OpenGL implementation of this (looks identical) Compiler: Ninja 1.10.2 Operating System: Windows (also building for Linux, but I'm on Windows usually)

My Issue/Question:

Hi, I currently have a project which, while using a custom backend and various custom widgets, is otherwise fairly normal in terms of Dear ImGui flow (in my opinion). The only potential abnormality is the number of windows being used. Dear ImGui is starting to bottleneck a bit, and I'm investigating caching methods. I've had a look through #3515, #2391, #1878 and this external issue. I thought I'd give an approach similar to #3515 a go, but am getting nothing rendered.

I have attached a minimal code example of what I'm currently trying. When I run it, I get everything rendered properly for the first 10 frames (I disable the caching attempt for that amount of time), and then once it switches over to caching, I get nothing. Looking at RenderDoc, it looks like most of the render pass just doesn't happen, should I also be updating something else when I cache?

Edit 1: I have updated things a little, and now I get an abort() in ImGui somewhere. I'll update once I know where (in ImGui::Render() almost certainly.

Edit 2: Hmmm, this is what I'm getting

Assertion failed: draw_list->VtxBuffer.Size == 0 || draw_list->_VtxWritePtr == draw_list->VtxBuffer.Data + draw_list->VtxBuffer.Size, file Imgui.cpp, AddDrawListToDrawData 
Assertion failed: (g.FrameCount == 0 || g.FrameCountEnded == g.FrameCount) && "Forgot to call Render() or EndFrame() at the end of the previous frame?", file imgui.cpp, line 2862

Edit 3: Looks like draw_list->VtxBuffer.Size == 0 || draw_list->_VtxWritePtr == draw_list->VtxBuffer.Data + draw_list->VtxBuffer.Size is failing with: draw_list->VtxBuffer.Size == 1798 ((draw_list->VtxBuffer.Data + draw_list->VtxBuffer.Size) - draw_list->_VtxWritePtr) / sizeof(ImDrawVert) == 1797

Edit 4: And with end() not back(), I think it's doing what I expect it to now! I'd appreciate any comments on improvements or thoughts on performance (perhaps reducing the number of allocated objects), but otherwise I'll close this Monday afternoon.

Screenshots/Video

Don't think this is applicable. My output before frame 10 are my windows, the output after 10 frames is nothing.

Standalone, minimal, complete and verifiable example: (see https://github.com/ocornut/imgui/issues/2261)

I am sorry, this is neither complete nor verifiable, but is just a very rough pseudocode idea of what I'm trying to achieve.

struct MyWindowClass
{
    /* some kind of state */
    std::string id;
    ImGuiWindowFlags flags;
    bool is_background_red = false;
    /* some more state */

    ImDrawList* cached_draw_list;

    bool potentially_should_change()
    {
        /* logic to decide if there's a chance changes have been made
           Perhaps just if the mouse is currently over the window */
    }

    void draw()
    {
         /* ImDrawList drawing stuff */
    }
}

void MyImGuiRenderFunction(ImDrawData* draw_data)
{
    /* Looks basically the same as 
       https://github.com/ocornut/imgui/blob/65f4be1a10932545a29faa54b8619eef7c0fe678/imgui.cpp#L291
       but in OpenGL land */
}

int main()
{
    ImGui::NewFrame();
    std::vector<MyWindowClass> my_windows;

    /* setup code */

    assert(my_windows.size() >= 1000); // let's say this passes

    while(true) // main loop
    {
        /* main loop stuff */

        static initial_loop_count = 0; // used to bypass the first few frames

        for(auto& window : my_windows)
        {
            if(ImGui::Begin(window.id.c_str(), nullptr, window.flags))
            {
                if (!window.potentially_should_change() && (initial_loop_count >= 10)) // use cached version
                {
                    // from ImDrawList::CloneOutput(), imgui_draw.cpp:441
                    auto curr_draw_list = ImGui::GetWindowDrawList();
                    curr_draw_list->CmdBuffer = m_drawList->CmdBuffer;
                    curr_draw_list->IdxBuffer = m_drawList->IdxBuffer;
                    curr_draw_list->VtxBuffer = m_drawList->VtxBuffer;
                    curr_draw_list->Flags = m_drawList->Flags;
                    curr_draw_list->_VtxCurrentIdx = curr_draw_list->VtxBuffer.Size;
                    curr_draw_list->_VtxWritePtr = &curr_draw_list->VtxBuffer.end();
                    curr_draw_list->_IdxWritePtr = &curr_draw_list->IdxBuffer.end();
                }
                else  // run Dear ImGui commands as normal
                {
                    window.draw()
                    // cache actual geo data at the end
                    if (m_drawList)
                    {
                        IM_DELETE(m_drawList);
                    }
                    m_drawList = ImGui::GetWindowDrawList()->CloneOutput();
                }
            }
            ImGui::End()

        }

        ImGui::Render()
        MyImGuiRenderFunction(ImGui::GetDrawData())

        if(initial_loop_count < 10)
        {
            ++initial_loop_count;
        }
    }
}
ocornut commented 2 years ago

I think a much easier approach for that idea would be to leave last ImDrawList of a specific window untouched and keep rendering that at the end of the frame. It would only work if the user is strictly respecting not submitting anything directly to the ImDrawList instance if Begin() returns false.

I think you should investigate that idea, perhaps by adding a custom window flags to do that. It MIGHT be as simple as:

Cascades commented 2 years ago

Thanks for the reply Omar! I realise you're extremely busy (always), so appreciate your time.

Having ImDrawLists persist over frames like this would be ideal! I think I'm correct in saying that ImDrawList::_ResetForNewFrame() is the new version of ImDrawList::Clear() in v1.64 (my company's current version).

I can see some of the equivalent calls you're referring to, e.g. in Begin(), but are you suggesting I can supress these without modifying the ImGui source? I can of course guard calls in ImGui source with if statements or similar, but can I do so while staying on the consumer side of the ImGui interface?

ocornut commented 2 years ago

but can I do so while staying on the consumer side of the ImGui interface?

You cannot currently, but I think this is the way forward and we can aim to support a feature like that. First we need to validat the proof of concept then we can introduce policies (e.g. interlaced refresh rates, with different setting for focused or hovered window than for other windows).

If you make local mods my suggestion to enforce a standard such as making all changes under a specific defines:

#ifdef !IMGUI_MYCOMPANY
   if (flags & ImGuiWindowsFlags_NoRefresh)
      window->DrawList->_ResetForNewFrame() 
#else
      window->DrawList->_ResetForNewFrame() 
#endif

This way it is easier to rebase.

PS: You should really update ihmo, 1.64 is getting very old and the most often you update the less issues you'll get into.

ocornut commented 6 months ago

I pushed an experiment: d449544 You can toy with it as ImGui::SetNextWindowRefreshPolicy(ImGuiWindowRefreshFlags_TryToAvoidRefresh); (require imgui_internal.h) Note that this behave at the Window level, not on a per-DrawList basis. So it doesn't exactly full-fill the title of this topic. See comments/caveats in the commit description. I think it'll be a good tool for reduce costs of "normally heavy" UI traversal, but may be an inadequate tool to reduce cost of "abnormally heavy" computation, as for that later you'll want very precise control and guaranteed avoidance that e.g. something is not done two frames in a row.