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

[sokol_imgui] Manage ImDrawCallback_ResetRenderState #1000

Closed
rcases opened this issue Feb 24, 2024 · 3 comments
Closed

[sokol_imgui] Manage ImDrawCallback_ResetRenderState #1000

rcases opened this issue Feb 24, 2024 · 3 comments

Comments

@rcases
Copy link

rcases commented Feb 24, 2024

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

floooh commented Feb 25, 2024

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

floooh commented Feb 25, 2024

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: cimgui/cimgui#261

@floooh floooh closed this as completed in 443c0cb Feb 25, 2024
@floooh
Copy link
Owner

floooh commented Feb 25, 2024

...443c0cb 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.

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

No branches or pull requests

2 participants