Skip to content

fix issues with imgui backend/renderer cleanup#7278

Merged
notimaginative merged 1 commit into
scp-fs2open:masterfrom
notimaginative:imgui_shutdown_fix
Mar 12, 2026
Merged

fix issues with imgui backend/renderer cleanup#7278
notimaginative merged 1 commit into
scp-fs2open:masterfrom
notimaginative:imgui_shutdown_fix

Conversation

@notimaginative

Copy link
Copy Markdown
Contributor

If the graphics system fails to fully init for some reason then it's possible for the imgui backend and/or renderer to not be initialized. Those subsystems must be initialized before being shut down so we need to check for that to avoid hitting assertions inside of imgui.

Also add a safety check for making sure that an imgui context exists before attempting to initialize the backend/renderer. A simple assertion should work for this as it would take a coder messing something up in order to trip it.

If the graphics system fails to fully init for some reason then it's possible
for the imgui backend and/or renderer to not be initialized. Those subsystems
*must* be initialized before being shut down so we need to check for that to
avoid hitting assertions inside of imgui.

Also add a safety check for making sure that an imgui context exists before
attempting to initialize the backend/renderer. A simple assertion should work
for this as it would take a coder messing something up in order to trip it.
@notimaginative notimaginative added fix A fix for bugs, not-a-bugs, and/or regressions. graphics A feature or issue related to graphics (2d and 3d) labels Mar 10, 2026
@notimaginative

Copy link
Copy Markdown
Contributor Author

This issue was discovered testing the vulkan pr, but it's unrelated to the vulkan code, and could just as easily mess up when using OpenGL in the right circumstances.

@Goober5000 Goober5000 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is initializing it and then immediately shutting it down better than simply skipping shutting it down if it's not initialized?

@notimaginative

Copy link
Copy Markdown
Contributor Author

Is initializing it and then immediately shutting it down better than simply skipping shutting it down if it's not initialized?

Same problem: it can't be initialized twice so you'd have to check that it's not initialized already before initializing.

Ideally we'd only be using imgui when required, which as far as I know is only for the lab and extra settings screens. So it would be set up when you enter the state in question and torn down when you leave that state. Much better than always allocating resources to imgui when it's not needed a vast majority of execution time. It's on my todo list to tackle that as part of an overhaul of that settings ui, but it's not a small job and pretty low priority.

@wookieejedi

Copy link
Copy Markdown
Member

Should this also be tagged for the point release?

@notimaginative

Copy link
Copy Markdown
Contributor Author

Should this also be tagged for the point release?

I don't believe that anyone has encountered it before and it should be fairly rare with just OpenGL. So I don't think it's needed for the point release, but it wouldn't hurt if it happened to be included.

@wookieejedi wookieejedi added this to the Release 25.0.1 milestone Mar 11, 2026
@notimaginative notimaginative merged commit d6c1b36 into scp-fs2open:master Mar 12, 2026
20 checks passed
@wookieejedi wookieejedi added the Point Release Candidate An already merged bugfix that may be merged into a previous stable version label Apr 10, 2026
@JohnAFernandez JohnAFernandez added added to point release and removed Point Release Candidate An already merged bugfix that may be merged into a previous stable version labels Apr 11, 2026
JohnAFernandez pushed a commit that referenced this pull request Apr 11, 2026
If the graphics system fails to fully init for some reason then it's possible
for the imgui backend and/or renderer to not be initialized. Those subsystems
*must* be initialized before being shut down so we need to check for that to
avoid hitting assertions inside of imgui.

Also add a safety check for making sure that an imgui context exists before
attempting to initialize the backend/renderer. A simple assertion should work
for this as it would take a coder messing something up in order to trip it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

added to point release fix A fix for bugs, not-a-bugs, and/or regressions. graphics A feature or issue related to graphics (2d and 3d)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants