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

Reset ImGui::GetIO().BackendFlags on shutdown #6334

Closed

Conversation

GereonV
Copy link
Contributor

@GereonV GereonV commented Apr 14, 2023

Relevant code in ImGui_ImplOpenGL3_Init():

#ifdef IMGUI_IMPL_OPENGL_MAY_HAVE_VTX_OFFSET
if (bd->GlVersion >= 320)
    io.BackendFlags |= ImGuiBackendFlags_RendererHasVtxOffset;  // We can honor the ImDrawCmd::VtxOffset field, allowing for large meshes.
#endif

(Note: can always be reset, regardless of whether it was actually set on initialization)


Relevant code in ImGui_ImplGlfw_Init():

io.BackendFlags |= ImGuiBackendFlags_HasMouseCursors;         // We can honor GetMouseCursor() values (optional)
io.BackendFlags |= ImGuiBackendFlags_HasSetMousePos;          // We can honor io.WantSetMousePos requests (optional, rarely used)

@ocornut
Copy link
Owner

ocornut commented Apr 14, 2023

Hello,
I assume your use case is you are switching backends while a same imgui context is being reused? Which backends are you using? That code should be applied to every backends.

@GereonV
Copy link
Contributor Author

GereonV commented Apr 15, 2023

Hi,
although I didn't check you're probably right and most backends need a change like this. I realized this issue when I read through backends/imgui_impl_glfw.cpp on my quest to understand how the library works as a whole. I thought of the time I used the same ImGui context for GLFW and SDL. I realized this could be a problem when switching backends in general. I later read backends/imgui_impl_opengl3.cpp and found the same thing. I only checked these, because they're the backends I currently use. I can check the other backends for io.BackendFlags (shouldn't be hard) and add the reset there as well. This is my very first PR (except for the other one I opened 20 mins prior to this one) so I don't know whether it's normal to ask: can I add these changes?
PS: excuse me for not being concise 😅

@ocornut
Copy link
Owner

ocornut commented Apr 15, 2023

Hello,

I think the changes are fine and correct but:

  • they would need to be applied to all backends.
  • multiple clear can be done together, e.g.
io.BackendFlags &= ~ImGuiBackendFlags_HasMouseCursors;
io.BackendFlags &= ~ImGuiBackendFlags_HasSetMousePos;

Can be:

io.BackendFlags &= ~(ImGuiBackendFlags_HasMouseCursors | ImGuiBackendFlags_HasSetMousePos);
  • And ultimately this needs to be completed with a second commit on the docking branch.

As this is relatively easy I don't mind doing it on my side, but if you are interested in pushing this to make a neat PR be my guest :)

Metal and OSX require `ImGui_Impl*_GetBackendData() != nullptr` during
shutdown anyway so this is not a breaking change. Actually, before this
PR there were potential `nullptr` derefs in the respective
`ImGui_Impl*_DestroyBackendData()`s. This PR both prevents this and also
resets `io.BackendRendererUserData`, `io.BackendRendererName` and
`io.BackendFlags`.
@GereonV GereonV force-pushed the glfw_and_opengl3_backendflags_fix branch from a2374a9 to 7d40dd4 Compare April 15, 2023 10:58
@GereonV
Copy link
Contributor Author

GereonV commented Apr 15, 2023

@ocornut I'm done. The branch name obviously isn't accurate anymore, because all backends that set io.BackendFlags while initializing have been adjusted and not just GLFW and OpenGL3.

I commited the changes for Metal and OSX separately because I wanted to comment on it in the commit message: it also IM_ASSERTs that the backend has been initialized and resets io.Backend*UserData and io.Backend*Name to nullptr in addition to clearing the bits of io.BackendFlags.

ocornut pushed a commit that referenced this pull request Apr 17, 2023
…ear BackendPlatformName. (#6334, #6335)

Amended with fix for missing clear for ImGuiBackendFlags_HasGamepad.
@ocornut
Copy link
Owner

ocornut commented Apr 17, 2023

Merged as 055e715
I have folded #6335 into the same, added the missing clear for ImGuiBackendFlags_HasGamepad in the 5 platform backends (if you grep for BackendFlags in backends/ folder you would find every use) and minor aesthetic tweaks.
Thank you!

@GereonV
Copy link
Contributor Author

GereonV commented Apr 19, 2023

I am happy to have contributed ^^

@GereonV GereonV deleted the glfw_and_opengl3_backendflags_fix branch April 19, 2023 02:10
ocornut added a commit that referenced this pull request Apr 19, 2023
@ocornut
Copy link
Owner

ocornut commented Apr 19, 2023

Also pushed 1f2b84a with equivalent flag clear for docking branch.

ocornut pushed a commit that referenced this pull request May 4, 2023
)

Co-authored-by: Alexander Rath <alex@ist.besonders.cool>
kjblanchard pushed a commit to kjblanchard/imgui that referenced this pull request May 5, 2023
…ear BackendPlatformName. (ocornut#6334, ocornut#6335)

Amended with fix for missing clear for ImGuiBackendFlags_HasGamepad.
kjblanchard pushed a commit to kjblanchard/imgui that referenced this pull request May 5, 2023
, ocornut#6334)

Co-authored-by: Alexander Rath <alex@ist.besonders.cool>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants