Added detection for switching alt + tab while in fullscreen mode while in game.
Plus I had got error on minimize, because need_render did not catch g_app_minimized change, so I moved it to the condition too.
Details
- Reviewers
vladislavbelov - Commits
- rP21476: Fixes the Alt + Tab crash in the fullscreen mode. Refs #4181.
- Trac Tickets
- #4181
A bit hard, if you had not this kind of problem.
At least look if something is not broken with this patch :)
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 5364 Build 9101: Vulcan Build Jenkins Build 9100: arc lint + arc unit
Event Timeline
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
source/main.cpp | ||
---|---|---|
374 | (x && y) || !y is the same as x || !y |
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
source/main.cpp | ||
---|---|---|
307 | Comments usually start with caps. I'm not sure that comment is really necessary though. |
!g_app_minimised:
In some way, I cant say why exactly, when I minimised my window, g_app_minimised was seted to true, but need_render already used old value of g_app_minimised. I guess that maybe window event runs in different thread as game itself, so when I minimised the window probably need_render was computed before the g_app_minimised changed in the memory, then window was minimised so there was not area where to render and it tries to do it. There is also chance that it is something wrong with specific graphic cards family.
To the alt + tab
(g_app_has_focus || !g_VideoMode.isInFullscreen()):
Maybe it is only windows thing, but when I debugged alt+tab, minimised window event was not used, so after alt+tab window is not minimised and therefore kept rendering, but since game was in fullscreen and not displayed, graphic card maybe could not find rendering area correctly, so it crashed.
The point of this solution is that if you are in fullscreen mode and game is not focused, you are not playing game but doing something else, so you do not see it, no point to render.
Got someone to test it on a Intel HD530 and he confirmed, that this patch fixes the problem.
So you can reproduce the crash? Could you get a stack trace of the crash?
anything helpfull from stack:
pyrogenesis.exe!CGame::Update(const double deltaRealTime, bool doInterpolate) Line 422 C++
and this from output:
First-chance exception at 0x528D8120 (ig75icd32.dll) in pyrogenesis.exe: 0xC0000005: Access violation reading location 0x00000018.
CallStack of alt+Tab error:
pyrogenesis_dbg.exe!CRenderer::RenderSubmissions(const CBoundingBoxAligned & waterScissor) Line 1596 C++ pyrogenesis_dbg.exe!CRenderer::RenderScene(Scene & scene) Line 1898 C++ pyrogenesis_dbg.exe!CGameView::Render() Line 490 C++ pyrogenesis_dbg.exe!Render() Line 219 C++ pyrogenesis_dbg.exe!Frame() Line 378 C++ pyrogenesis_dbg.exe!RunGameOrAtlas(int argc, const char * * argv) Line 590 C++ pyrogenesis_dbg.exe!SDL_main(int argc, char * * argv) Line 632 C++
L1596 is this part of CRenderer::RenderSubmissions
if (m_Options.m_Silhouettes) { RenderSilhouettes(context); }
Can we find out which line therein is the one crashing?
Either by adding a debug_printf("hello world\n") before the statements, or by using the debugger differently.
Also can you test if it doesnt crash if silouettes are disabled?
It crashes after this function
void CPostprocManager::ReleaseRenderOutput()
called as
if (m_Options.m_Postproc) { m->postprocManager.ApplyPostproc(); m->postprocManager.ReleaseRenderOutput(); }
in
void CRenderer::RenderSubmissions(const CBoundingBoxAligned& waterScissor)
It has nothing to do with silouettes.
Just now I enabled everything except post processing and no crash.
Ten I disabled everything and enabled postprocesing, it crashed.
Just tested with someone called Jongleur on #0ad and the patch indeed fixes the bug. http://irclogs.wildfiregames.com/2018-02/2018-02-11-QuakeNet-%230ad.log
Since only Intel HD 4000 users experience it, it must be a driver bug.
I agree with Vladislav that we should try to get a stacktrace to see which call exactly crashes.
Perhaps we can catch it differently.
Also maybe there is a newer driver and we can just tell people to install that?
If not, then I guess we can commit this if there is a comment added that this is needed for this specific graphics card family on windows,
so that we can maybe remove it eventually.
source/main.cpp | ||
---|---|---|
372 | The new condition says that if fullscreen mode is enabled, that it shouldn't render if the window isn't focused. Also, there will be a merge conflict with D1212. It would be great if someone can test that this still doesn't crash after we have committed that. |
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/51/display/redirect
@echotangoecho proposed to detect the graphics card family in hwdetect.js and set a flag enabling the workaround.
The detection looks easy, just a string match. But I doubt that we want to add a JS interface function for this workaround.
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/52/display/redirect
Call stack from build game as debug is here
More when it crashes is here:
and insurance about postprocessing
I mean on which line inside RenderSubmissions, could you modify the code like:
debug_printf("breakpoint"); if (m_Options.m_Silhouettes) { debug_printf("breakpoint"); RenderSilhouettes(context); } debug_printf("breakpoint");
And run it? We need to know a line.
As I said earlier. It is postprocessing.
And to it be more specific, it breaks after, not inside. Mystery.
For info:
it is this code which is case of break
pglBlitFramebufferEXT(0, 0, m_Width, m_Height, 0, 0, m_Width, m_Height, GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT | GL_STENCIL_BUFFER_BIT, GL_NEAREST);
source/main.cpp | ||
---|---|---|
372 | I think, the comment isn't right, because the !g_app_minimized part is valid, so it isn't the fix. I suggest to use comment like: // We don't have to render an inactive fullscreen frame, because it can lead to errors for some graphic card families. Or something like that, at least it needs to check a gramma. | |
source/ps/VideoMode.cpp | ||
403 | This function doesn't change the object state, so it should be const. |
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/164/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/202/display/redirect