We encountered a rare crash in egRenderer.cpp Renderer::drawModels (during the call to updateShadowMaps)
Reproduction: 100% with specific scene.
Callstack:
(Lines in callstack are bit off, due to some debugging information, but the actual line is 1721, egRenderer.cpp, svn rev 237)
Code:
#0 0x00072271 in Renderer::drawModels at egRenderer.cpp:1721
#1 0x0006c1ce in Renderer::drawRenderables at egRenderer.cpp:1524
#2 0x0006e060 in Renderer::updateShadowMap at egRenderer.cpp:1018
#3 0x0007309f in Renderer::drawLightGeometry at egRenderer.cpp:1362
#4 0x00073bf0 in Renderer::render at egRenderer.cpp:2018
#5 0x0003f278 in h3dRender at egMain.cpp:111
#6 0x000059cd in Application::mainLoop at app.cpp:250
#7 0x00006eeb in main at main.cpp:227
Actual Line 1721 (svn rev 237)
Code:
if( curShader->uni_worldMat >= 0 )
with curShader == 0 -> bad access -> crash
Analysis:
The problem stems from the fact the call
Code:
if( !Modules::renderer().setMaterial( meshNode->getMaterialRes(), shaderContext ) )
has a side-effect, e.g. setting Renderer's _curShader to 0 (which is ok).
But the drawModels loop always assumes that, if it has the same curMatRes Ptr (as it didn't change by a Mesh),
that there is no need to re-set the material.
Now consider this order of meshes (with materials).
Mesh A - Material A -> setMaterial will result in _curShader = ShaderA, curMatRes = Material A
Mesh B - Material B -> setMaterial will result in _curShader = 0, curMatRes = Material A (stays the same)
Mesh C - Material A -> setMaterial will not be called, because curMatRes == MeshC->Material (=MaterialA)
hence Modules::Renderer().getCurShader() (line 1703) will return 0 and curShader will be 0 -> crash.
I am not sure which is the cleanest and best solution (also in terms of performance).
If there are no further side effects of
Modules::renderer().setMaterialReca call to
Modules::renderer().setShaderComb(prevShader) would suffice, in case the call to
Code:
if( !Modules::renderer().setMaterial( meshNode->getMaterialRes(), shaderContext )
in line 1681 failed.
But if
setMaterialRec has more side-effects (GL State, Texture wise, ...), we may need to set curMatRes=0 after a failed call in line 1681,
so the material gets re-set for the next Mesh. This is what we did actually and worked out.
BTW: This is the first real crash in Horde during normal operation which we encountered so far! So it's really amazing that Horde is so stable (besides this rare crash) and it has very few bugs so far. Great job, Guys!