Skip to content

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

Closed
GereonV wants to merge 5 commits intoocornut:masterfrom
GereonV:glfw_and_opengl3_backendflags_fix
Closed

Reset ImGui::GetIO().BackendFlags on shutdown#6334
GereonV wants to merge 5 commits intoocornut:masterfrom
GereonV:glfw_and_opengl3_backendflags_fix

Conversation

@GereonV
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 :)

GereonV added 4 commits April 15, 2023 12:55
@GereonV
Reset ImGui::GetIO().BackendFlags on shutdown
9f320e9
@GereonV
Adjust style to ocornut's suggestion
e2c4f18
@GereonV
Apply changes to other backends
078aa92
@GereonV
Shutdown safely and correctly on Apple
7d40dd4 
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
Copy Markdown
Contributor Author

GereonV commented Apr 15, 2023 •
edited
Loading

@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
@GereonV @ocornut
Backends: clear bits set in io.BackendFlags on backend Shutdown(). Cl…
055e715 
…ear BackendPlatformName. (#6334, #6335)

Amended with fix for missing clear for ImGuiBackendFlags_HasGamepad.
@ocornut
Copy link
Copy Markdown
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!

@ocornut ocornut closed this Apr 17, 2023
@ocornut ocornut mentioned this pull request Apr 17, 2023
Closed
@GereonV
Copy link
Copy Markdown
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
Backends: clear viewport specific bits in io.BackendFlags on backend …
1f2b84a 
…Shutdown(). (#6334)

Amend 055e715
@ocornut
Copy link
Copy Markdown
Owner

ocornut commented Apr 19, 2023

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

@iRath96 iRath96 mentioned this pull request Apr 29, 2023
Merged
ocornut pushed a commit that referenced this pull request May 4, 2023
@iRath96
Backends: avoid null dereference in metal and osx shutdown (#6385, #6334
70cca1e 
)

Co-authored-by: Alexander Rath <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

backends

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@GereonV @ocornut