HomeWildfire Games

Fix a crash on some system when Alt-tabbing during game setup.
AuditedrP22314

Description

Fix a crash on some system when Alt-tabbing during game setup.

Move checks for rendering a frame in Render() to avoid missing calls to this functions, which can crash on certain systems.

Move the sound manager's idle task out of Render().
Move the buffer swapping in Render() since we do not need to swap buffers unless we are rendering.

Patch By: Angen

Reviewed By: wraitii

Differential Revision: https://code.wildfiregames.com/D1495

Event Timeline

Stan raised a concern with this commit.Jun 16 2019, 11:54 PM
Stan added a subscriber: Stan.

This breaks sounds for me. I believe that maybe g_SoundManager->IdleTask(); gets called too often ?

This commit now has outstanding concerns.Jun 16 2019, 11:54 PM
In rP22314#34004, @Stan wrote:

This breaks sounds for me. I believe that maybe g_SoundManager->IdleTask(); gets called too often ?

Does it break sound always? Or only if you minimised?
Can you try:

  • Putting SwapBuffers()s in the main loop after Render();
  • Putting the same if for the sound manager as for Render();

It has to be one of these two.

Angen added a subscriber: Angen.Jun 17 2019, 9:05 AM
In rP22314#34004, @Stan wrote:

This breaks sounds for me. I believe that maybe g_SoundManager->IdleTask(); gets called too often ?

Does it break sound always? Or only if you minimised?
Can you try:

  • Putting SwapBuffers()s in the main loop after Render();
  • Putting the same if for the sound manager as for Render();

It has to be one of these two.

I think it is moved sound manager. just move sound manager inside renderer after if check as it was before should solve the problem. I do not think we want duplication for the if condition, one is enough :)

In rP22314#34004, @Stan wrote:

This breaks sounds for me. I believe that maybe g_SoundManager->IdleTask(); gets called too often ?

Does it break sound always? Or only if you minimised?
Can you try:

  • Putting SwapBuffers()s in the main loop after Render();
  • Putting the same if for the sound manager as for Render();

It has to be one of these two.

I think it is moved sound manager. just move sound manager inside renderer after if check as it was before should solve the problem. I do not think we want duplication for the if condition, one is enough :)

I would disagree, this function is called "Render" and putting sound management there is unexpected. If the lack of If condition is what breaks sound, then we should rather:

  • Move the sound call in a GameSetup.cpp function called "SoundTask()" or something.
  • Wrap the if condition in a standalone function in GameSetup.cpp
  • Use the same if condition in both functions
Stan added inline comments.Jun 17 2019, 10:42 AM
/ps/trunk/source/main.cpp
411

Replacing that bit of code by

if (g_SoundManager && g_app_minimized || (!g_app_has_focus && g_VideoMode.IsInFullscreen()))
    g_SoundManager->IdleTask();

doesn't work

Stan added inline comments.Jun 17 2019, 11:05 AM
/ps/trunk/source/main.cpp
411
	if (g_SoundManager && !g_app_minimized && (g_app_has_focus || !g_VideoMode.IsInFullscreen()))
		g_SoundManager->IdleTask();
	if (g_SoundManager && !(g_app_minimized || (!g_app_has_focus && g_VideoMode.IsInFullscreen())))
		g_SoundManager->IdleTask();

don't work either

elexis added a subscriber: elexis.Jun 25 2019, 3:26 AM

@Stan what does "break sound for me" mean, no sound at all (on windows?)? Is this reproduced by others (on windows)?

@Angen: What were the original steps to reproduce other than alt+tab just after the loading screen?
Perhaps one can reproduce it with some code modification (triggering the unfocused window code after the loading screen programmatically)
You say "it is kind of rare but not impossible", which sounds like it could not be reproduced reliably, which means the absence of the bug could also not be confirmed, is that right?

Stan added a comment.Jun 25 2019, 8:52 AM

Sorry, should have been more specific.

After this commit (after running dichotomia on all other commits) when I run the simulation in Atlas (Doesn't seem to happen in game) and make a 4v4 archer fight, they will stop making sounds after a short time, maybe 3-4 arrows, then the game units go completely silent.

I believe @Angen reproduced the bug as well.

/ps/trunk/source/ps/GameSetup/GameSetup.cpp
207

No GL code there, right ?

Angen added a comment.Jun 25 2019, 3:21 PM

Bug n 1.
Run game with postprocessing on in fullscreen, alt tab and go back (solved by D1212)
Bug n 2.
Postprocessing on in fullscreen, alt tab while loading match, return back after match started ( original solution https://code.wildfiregames.com/D1495?id=7570) commited version (this)

Stan added a comment.Jun 29 2019, 5:16 PM

Was anybody else able to reproduce ?

Angen added a comment.Jun 30 2019, 5:48 PM

@wraitii @Stan
g_Soundmanager->IdleTask has to be inside Render()
Events changing g_app_minimised, g_app_has_focus, isinfullscreen can ( and looks like they do mainly in atlas ) come after g_SoundIdleTask and before Render so Idle task is called but Render not

Angen raised a concern with this commit.Jun 30 2019, 6:28 PM

Do g_SoundManager->IdleTask iff Render is done
Atlas has own gameLoop
Atlas does own BufferSwapping (dynamicly sometimes at 2fps) so move SwapBuffers back to main

See GameLoop.cpp

Angen added inline comments.Jul 21 2019, 1:32 PM
/ps/trunk/source/ps/GameSetup/GameSetup.cpp
207

I would leave it so that way we are sure we have no graphic errors when entering this function

Angen added a comment.Jul 28 2019, 9:43 AM

@Stan can you please check if it works for you?

Stan accepted this commit.Aug 1 2019, 3:25 PM

Re-Compiled everything with the fix, works now. Has there been an atlas Autobuild since then ?

All concerns with this commit have now been addressed.Aug 1 2019, 3:25 PM