Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Swapping ImDrawLists not rendering anything #4763

Open
Cascades opened this issue Nov 26, 2021 · 4 comments
Open

Swapping ImDrawLists not rendering anything #4763

Cascades opened this issue Nov 26, 2021 · 4 comments

Comments

@Cascades
Copy link

Cascades commented Nov 26, 2021

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 #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
Copy link
Owner

ocornut commented Nov 30, 2021

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:

  • not calling window->DrawList->_ResetForNewFrame() when that flag is set
  • returning false when that flag is set.
    Probably more things but that's the way to get started with this idea.

@Cascades
Copy link
Author

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
Copy link
Owner

ocornut commented Nov 30, 2021

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 added a commit that referenced this issue Jul 12, 2023
…DrawData itself. Faclitate user-manipulation of the array (#6406, #4879, #1878) + deep swap. (#6597, #6475, #6167, #5776, #5109, #4763, #3515, #1860)

+ Metrics: avoid misleadingly iterating all layers of DrawDataBuilder as everything is flattened into Layers[0] at this point.

# Conflicts:
#	imgui.cpp
#	imgui_internal.h
ocornut added a commit that referenced this issue Aug 6, 2023
ocornut added a commit that referenced this issue May 7, 2024
…#7556, #5116 , #4076, #2749, #2268)

currently: ImGui::SetNextWindowRefreshPolicy(ImGuiWindowRefreshFlags_TryToAvoidRefresh);
- This is NOT meant to replace frame-wide/app-wide idle mode.
- This is another tool: the idea that a given window could avoid refresh and reuse last frame contents.
- I think it needs to be backed by a careful and smart design overall (refresh policy, load balancing, making it easy and obvious to user).
- It's not there yet, this is currently a toy for experimenting.
My other issues with this:
- It appears to be very simple, but skipping most of Begin() logic will inevitably lead to tricky/confusing bugs. Let's see how it goes.
- I don't like very much that this opens a door to varying inconsistencies
- I don't like very much that it can lead us to situation where the lazy refresh gets disabled in bulk due to some reason (e.g. resizing a dock space) and we get sucked in the temptation to update for idle rather than update for dynamism.
@ocornut
Copy link
Owner

ocornut commented May 7, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants