pthom / imgui_bundle

Dear ImGui Bundle: an extensive set of Ready-to-use widgets and libraries, based on ImGui. Start your first app in 5 lines of code, or less. Whether you prefer Python or C++, this pack has your back!
https://pthom.github.io/imgui_bundle/
MIT License
592 stars 63 forks source link

IDStackTool and TEST_ENGINE #166

Closed dcnieho closed 5 months ago

dcnieho commented 5 months ago

There is a perhaps unintended consequence of compiling with IMGUI_ENABLE_TEST_ENGINE, see here: https://github.com/ocornut/imgui/blob/8a3dfda8d082caa4eefa75d8a53ca5074b2c651d/imgui.cpp#L15387

That means that I cannot display the id stack window from my python application without first initializing the test engine. For a while doing this seemed to work:

self._test_engine_context = imgui.test_engine.create_context()  # need to keep a ref somewhere
imgui.test_engine.start(self._test_engine_context, imgui.get_current_context())

But this has started freezing on me on the imgui.test_engine.start call. Is this not the way i am supposed to use this? More generally, I wanted to report this, as i assume it was not the plan to for people to initialize the test_engine just to be able to show the id stack tool, correct?

pthom commented 5 months ago

There is a perhaps unintended consequence of compiling with IMGUI_ENABLE_TEST_ENGINE, see here: https://github.com/ocornut/imgui/blob/8a3dfda8d082caa4eefa75d8a53ca5074b2c651d/imgui.cpp#L15387

Yes

But this has started freezing on me on the imgui.test_engine.start call. Is this not the way i am supposed to use this? Could you try

runner_params.use_imgui_test_engine = True

And tell me if it helps. This should initialize and keep the reference for you. The test engine uses threads, so that using it from python is tricky.

More generally, I wanted to report this, as i assume it was not the plan to for people to initialize the test_engine just to be able to show the id stack tool, correct?

No it was not. However, this requires a modification inside ImGui.

dcnieho commented 5 months ago

ah, I forgot to explain why i did things this way. I assumed (but have no idea) that setting runner_params.use_imgui_test_engine = True would lead to additional overhead, and the application is heavy enough as it is. I normally do not need the test engine, just when a debug window is opened that may include this id stack window. So i create and attach it myself only when needed. So i have some questions:

  1. does having the test engine initialized cause overhead/slowdown?
  2. can i use your functionality to initialize the test engine at a later time, once its needed (does setting runner_params.use_imgui_test_engine = True at a later time once running for a while already have an effect)?
pthom commented 5 months ago

Hello Diederick,

I fixed this with this commit

And I also proposed an upstream patch to ImGui

dcnieho commented 5 months ago

Thanks for the fix! Regarding upstreaming things, do you think some flag like IMGUI_USE_STD_FUNCTION or so would be amenable to ocornut? IIRC, he has a policy to not use the standard library in the interface, but having it opt in and thereby making the life of people generating bindings (and perhaps other use cases) easier may be worth it.

pthom commented 5 months ago

do you think some flag like IMGUI_USE_STD_FUNCTION or so would be amenable to ocornut? IIRC, he has a policy to not use the standard library in the interface, but having it opt in and thereby making the life of people generating bindings (and perhaps other use cases) easier may be worth it.

Hum, we could ask him. However, it might be difficult. Here is some context

imgui test engine uses a similar construct

imgui_te_config.h

    // [Optional, default 0] Using std::function and <functional> for function pointers such as ImGuiTest::TestFunc and ImGuiTest::GuiFunc
    #ifndef IMGUI_TEST_ENGINE_ENABLE_STD_FUNCTION
    #define IMGUI_TEST_ENGINE_ENABLE_STD_FUNCTION 0
    #endif

imgui_te_utils.h

    #if IMGUI_TEST_ENGINE_ENABLE_STD_FUNCTION
    #include <functional>
    #define ImFuncPtr(FUNC_TYPE)        std::function<FUNC_TYPE>
    #else
    #define ImFuncPtr(FUNC_TYPE)        FUNC_TYPE*
    #endif

so, we could imagine doing something similar within ImGui:

imconfig.h

    //---- Callbacks: Enable usage of std::function and <functional> for callbacks (default false)
    // #define IMGUI_ENABLE_STD_FUNCTION

imgui.h

    // Top of file
    #ifdef IMGUI_ENABLE_STD_FUNCTION
        #include <functional>
        #define ImFuncPtr(FUNC_TYPE)        std::function<FUNC_TYPE>
    #else
        #define ImFuncPtr(FUNC_TYPE)        FUNC_TYPE*
    #endif

    ...
    ...

    // Callback and functions types
    using ImGuiInputTextCallback = ImFuncPtr(int  (ImGuiInputTextCallbackData* data));  // Callback function for ImGui::InputText()
    using ImGuiSizeCallback = ImFuncPtr(void(ImGuiSizeCallbackData*));                  // Callback function for ImGui::SetNextWindowSizeConstraints()

    ...
    ...

    #ifndef ImDrawCallback
    // typedef void (*ImDrawCallback)(const ImDrawList* parent_list, const ImDrawCmd* cmd);
    using ImDrawCallback = ImFuncPtr(void(const ImDrawList* parent_list, const ImDrawCmd* cmd));
    #endif

    ...

However, we would quickly reach a problem with the following code inside imgui.h:

inside imgui.h:

    // Special Draw callback value to request renderer backend to reset the graphics/render state.
    // The renderer backend needs to handle this special value, otherwise it will crash trying to call a function at this address.
    // This is useful, for example, if you submitted callbacks which you know have altered the render state and you want it to be restored.
    // Render state is not reset by default because they are many perfectly useful way of altering render state (e.g. changing shader/blending settings before an Image call).
    #define ImDrawCallback_ResetRenderState     (ImDrawCallback)(-8)  

So, ImDrawCallback_ResetRenderState is a special value (an int!), not a function pointer...

usage inside imgui_impl_open_gl3.cpp:

void    ImGui_ImplOpenGL3_RenderDrawData(ImDrawData* draw_data)
{
    ...
    // User callback, registered via ImDrawList::AddCallback()
    // (ImDrawCallback_ResetRenderState is a special callback value used by the user to request the renderer to reset render state.)
    if (pcmd->UserCallback == ImDrawCallback_ResetRenderState)
        ImGui_ImplOpenGL3_SetupRenderState(draw_data, fb_width, fb_height, vertex_array_object);
    else
        pcmd->UserCallback(cmd_list, pcmd);
}

I will not ask him at this moment, but we can keep this as a reference for the future.

dcnieho commented 5 months ago

Oh i see! (Without investigating further) that looks rather hacky and indeed is an issue. So this isn't as simple as the define solution you drafted there, as the functionality of this special value would have to be achieved in a different way somehow...