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().BackendPlatformName in Android and GLUT shutdown #6335

Closed
wants to merge 2 commits into from

Conversation

GereonV
Copy link
Contributor

@GereonV GereonV commented Apr 15, 2023

Android and GLUT backends set io.BackendPlatformName during initialization. These backends' shutdowns have previously been no-ops and hence failed to reset io.BackendPlatformName. Because they don't use io.BackendPlatformUserData only ImGui::GetCurrentContext() can be checked. Other backends check for ImGui::GetCurrentContext() != nullptr which also fails when ImGui::GetCurrentContext() == nullptr and emit the same error message regardless of whether there was no context or no backend data. For this reason it's perfectly consistent to emit the same error message. Consideration about compatibility should be taken: programs may break when they relied on these backends' shutdown not failing. However arguably those could be considered misuses and additionally they'd need to have read the implementation of ImGui_Impl*_Shutdown to reason about the safety of calling it out-of-place or multiple times. Having done that they probably wouldn't have called the shutdown function at all (because it didn't do anything). Furthermore, if someone's program breaks due to this change all it takes to exactly restore the old behavior is to not call the shutdown function.

Android and GLUT backends set `io.BackendPlatformName` during
initialization. These backends' shutdowns have previously been no-ops
and hence failed to reset `io.BackendPlatformName`. Because they don't
use `io.BackendPlatformUserData` only `ImGui::GetCurrentContext()` can
be checked. Other backends check for `ImGui::GetCurrentContext() !=
nullptr` which also fails when `ImGui::GetCurrentContext() == nullptr`
and emit the same error message regardless of whether there was no
context or no backend data. For this reason it's perfectly consistent
to emit the same error message. Consideration about compatibility should
be taken: programs may break when they relied on these backends'
shutdown not failing. However arguably those could be considered misuses
and additionally they'd need to have read the implementation of
`ImGui_Impl*_Shutdown` to reason about the safety of calling it
out-of-place or multiple times. Having done that they probably wouldn't
have called the shutdown function at all (because it didn't do
anything). Furthermore, if someone's program breaks due to this change
all it takes to exactly restore the old behavior is to not call the
shutdown function.
@GereonV GereonV changed the title Reset ImGui::GetIO().BackendPlatformName Reset ImGui::GetIO().BackendPlatformName in Android and GLUT shutdown Apr 15, 2023
@GereonV
Copy link
Contributor Author

GereonV commented Apr 15, 2023

maybe add assertion io.BackendPlatformName != nullptr to prevent multiple shutdowns or change error message to something along the lines of "no context" / remove ", or already shutdown?"

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 part of #6334, thank you!

@ocornut ocornut closed this Apr 17, 2023
@GereonV GereonV deleted the android_glut_reset branch April 19, 2023 02:09
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

Successfully merging this pull request may close these issues.

2 participants