floooh / sokol

minimal cross-platform standalone C headers
https://floooh.github.io/sokol-html5
zlib License
6.54k stars 469 forks source link

[sokol_imgui] Manage ImDrawCallback_ResetRenderState #1000

Closed rcases closed 4 months ago

rcases commented 4 months ago

I am trying to use [https://github.com/andyborrell/imgui_tex_inspect]() using sokol as backend. internally it makes a call of the type: ImGui::GetWindowDrawList()->AddCallback(ImDrawCallback_ResetRenderState, nullptr); which is defined in "imgui.h" as:

// 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)

This causes an access violation in sokol_imgui.

The backends delivered with imgui use this type of construction:

        for (int cmd_i = 0; cmd_i < cmd_list->CmdBuffer.Size; cmd_i++)
        {
            const ImDrawCmd* pcmd = &cmd_list->CmdBuffer[cmd_i];
            if (pcmd->UserCallback != nullptr)
            {
                // 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);
            }
            else
            {
...
floooh commented 4 months ago

Hmm, even after reading the description I'm not quite sure what's the purpose of the special ImDrawCallback_ResetRenderState UserCallback is, but I can skip this in sokol_imgui.h, one sec.

PS: or maybe it's better to call sg_reset_state_cache(), but this would only make sense when there are direct calls to the 3D APIs outside of sokol_gfx.h, but OTH it also shouldn't do any harm.

floooh commented 4 months ago

On minor issue: that define isn't exposed in the cimgui.h header, so with a straightforward fix sokol_imgui.h doesn't build in C mode. I'll put a workaround in by using the hardwired constant (ImDrawCallback)(-8) constant instead.

I wrote a ticket for cimgui, but not sure if this is actually going to be fixed: https://github.com/cimgui/cimgui/issues/261

floooh commented 4 months ago

...https://github.com/floooh/sokol/commit/443c0cbea1bc0c79116cd9d6a883ec22a4d7b351 should fix the crash.

However: I'm not sure if that makes your use case work, because in general I reset some render state after every user callback, which seems to clash with Dear ImGui's documentation for ImDrawCallback_ResetRenderState.

The other option would be to change the code like this:

if (pcmd->UserCallback != ImDrawCallback_ResetRenderState) {
    pcmd->UserCallback(cl, pcmd);
} else {
    sg_reset_state_cache();
    sg_apply_viewport(0, 0, fb_width, fb_height, true);
    sg_apply_pipeline(_simgui.def_pip);
    sg_apply_uniforms(SG_SHADERSTAGE_VS, 0, SG_RANGE_REF(vs_params));
    sg_apply_bindings(&bind);
}

...which I guess is more inline with Dear ImGui's intentions, but would require to change the imgui-usercallback-sapp.c sample to issue an additional ImDrawCallback_ResetRenderState after each regular user callback (no big deal of course).

If the fix doesn't work for you, please open this issue again, or if you feel like it provide a PR.