Shadow Volumes Blow Up Meshes (Strange Bug)

[Elevations]

Tested with Irrlicht 1.9 and DX9 driver

I have this strange bug where if i attach a standard point light to follow my character, then jump in the air beyond the lights 3D World position, the shadow casted volumes blow up the mesh to a HUGE scale and corrupt all other shadows in the scene.

After many many hours of trying to narrow down the bug that causes Shadows / Shadow Volumes to BLOW UP meshes I believe i have finally found the BUG! and how i fixed it was very simple:

in CShadowVolumeSceneNode.cpp Line 374 i changed getLengthSQ() to getLength() and that fixed the problem now my shadow casted meshes are no longer expanding beyond the lights position into oblivion they appear to stop scaling once i leave the point light position and no longer causing strange bugs.

Code: Select all

	core::vector3df lpos(dl.Position);
	if (dl.CastShadows &&
		fabs((lpos - parentpos).getLength()) <= (dl.Radius*dl.Radius*4.0f))
	{
		matInv.transformVect(lpos);
		createShadowVolume(lpos, false);
	}

[CuteAlien]

I assume you mean CShadowVolumeSceneNode.cpp?

I have some troubles with this patch. First is that the original code looks on first view somewhat correct to me (comparing a square to square). Thought I have to dig deeper later what the *4.0f is about. On first view it looks like some magic limit after which to stop producing shadows. So 4.0 might be too large, but likely it still shouldn't switch to comparing a linear value to a square (maybe some variable instead of a constant, or comparing a fixed value to length instead of using squares or it might depend more on the angles created than on length).

The other trouble I have is that you say this is Line 381. But it's not that line in either Irrlicht 1.8 nor in svn trunk (nor in a few other revisions I just checked). So I'm confused about which Irrlicht version you are working with. And this kinda matters as 1.8 shadows had a lot more bugs than current svn trunk version. And if you have a version which has different lines than trunk it's likely you have a version where some of those bugs are still open. So I'd recommend to update to latest svn trunk first.

[Elevations]

My apologies yes it is CShadowVolumeSceneNode.cpp in the updateShadowVolumes() function ill post the full function below just for reference:

I downloaded version 1.9 from this link here: https://sourceforge.net/p/irrlicht/code ... ree/trunk/

The line number is actually 374 (i accidentally added <iostream> at the top of mine woops) ill update the main post aswell. And yeah i dont know what kind of black magic this is but i did some extra testing and for what ever reason the getLength() just fixes the problem entirely for me.

https://sourceforge.net/p/irrlicht/code ... neNode.cpp

Code: Select all

void CShadowVolumeSceneNode::updateShadowVolumes()
{
	if ( Freeze == ESF_FREEZE )
		return;
	else if ( Freeze == ESF_FREEZE_AFTER_UPDATE )
		Freeze = ESF_FREEZE;

	const u32 oldIndexCount = IndexCount;
	const u32 oldVertexCount = VertexCount;

	VertexCount = 0;
	IndexCount = 0;
	ShadowVolumesUsed = 0;

	const IMesh* const mesh = ShadowMesh;
	if (!mesh)
		return;

	// create as much shadow volumes as there are lights but
	// do not ignore the max light settings.
	const u32 lightCount = SceneManager->getVideoDriver()->getDynamicLightCount();
	if (!lightCount)
		return;

	// calculate total amount of vertices and indices

	u32 i;
	u32 totalVertices = 0;
	u32 totalIndices = 0;
	const u32 bufcnt = mesh->getMeshBufferCount();

	for (i=0; i<bufcnt; ++i)
	{
		const IMeshBuffer* buf = mesh->getMeshBuffer(i);
		if (	buf->getIndexType() == video::EIT_16BIT 
			&& buf->getPrimitiveType() == scene::EPT_TRIANGLES )
		{
			totalIndices += buf->getIndexCount();
			totalVertices += buf->getVertexCount();
		}
		else
		{
			os::Printer::log("ShadowVolumeSceneNode only supports meshbuffers with 16 bit indices and triangles", ELL_WARNING);
			return;
		}
	}
	
	if ( totalIndices != (u32)(u16)totalIndices)
	{
		// We could switch to 32-bit indices, not much work and just bit of extra memory (< 192k) per shadow volume.
		// If anyone ever complains and really needs that just switch it. But huge shadows are usually a bad idea as they will be slow.
		os::Printer::log("ShadowVolumeSceneNode does not yet support shadowvolumes which need more than 16 bit indices", ELL_WARNING);
		return;
	}

	// allocate memory if necessary

	Vertices.set_used(totalVertices);
	Indices.set_used(totalIndices);
	FaceData.set_used(totalIndices / 3);

	// copy mesh 
	// (could speed this up for static meshes by adding some user flag to prevents copying)
	for (i=0; i<bufcnt; ++i)
	{
		const IMeshBuffer* buf = mesh->getMeshBuffer(i);

		const u16* idxp = buf->getIndices();
		const u16* idxpend = idxp + buf->getIndexCount();
		for (; idxp!=idxpend; ++idxp)
			Indices[IndexCount++] = *idxp + VertexCount;

		const u32 vtxcnt = buf->getVertexCount();
		for (u32 j=0; j<vtxcnt; ++j)
			Vertices[VertexCount++] = buf->getPosition(j);
	}

	// recalculate adjacency if necessary
	if (oldVertexCount != VertexCount || oldIndexCount != IndexCount || AdjacencyDirtyFlag)
		calculateAdjacency();

	core::matrix4 matInv(Parent->getAbsoluteTransformation());
	matInv.makeInverse();
	core::matrix4 matTransp(Parent->getAbsoluteTransformation(), core::matrix4::EM4CONST_TRANSPOSED);
	const core::vector3df parentpos = Parent->getAbsolutePosition();

	for (i=0; i<lightCount; ++i)
	{
		const video::SLight& dl = SceneManager->getVideoDriver()->getDynamicLight(i);

		if ( dl.Type == video::ELT_DIRECTIONAL )
		{
			core::vector3df ldir(dl.Direction);
			matTransp.transformVect(ldir);
			createShadowVolume(ldir, true);
		}
		else
		{
			core::vector3df lpos(dl.Position);
			if (dl.CastShadows &&
				fabs((lpos - parentpos).getLength()) <= (dl.Radius*dl.Radius*4.0f))
			{
				matInv.transformVect(lpos);
				createShadowVolume(lpos, false);
			}
		}
	}
}

[CuteAlien]

Ah OK, 374 is current version. And basically it just sets another limit - which is also kinda fixed (as dl.Radius is usually only set once). If I read the code correct (without coding any tests right now) then this line prevents creating a shadow once the center of the parent node is further away than 2 times the light radius. While you stop producing a shadow now a lot later (4 times the squared light radius).

So the problem seems to be that something messes up for you when one shadow is no longer produced. Which is strange - but I can't really tell what's going on without test-case.

Note - this kind of shadow has some problems. I think it goes wrong for example when the light is in between the camera and the node (then it blows up). Not sure I remember the exact case where it fails correct.

[Elevations]

Ah i will admit this is certainly a "rare test case" and most people arent going to be jumping out of point lights with shadows casting enabled on the point light in the same manner that i was doing so.

For example:
The problem never happened when i just had unmoving static point lights. The problem started happening when i attached the point light to follow my character, while the point light moves, and then if i move outside that moving point light, Then i would immediatly get this problem where meshes blow up and all shadows (and the original mesh model) wont even be visible anymore and over-scaled.

So again the test case is not exactly easy to reproduce, But what i will say is, If the GetLength() does indeed fix a rare bug like this and has no down side then it might be worth adding it to the latest trunk as a small patch or at least having a solution for others that might experience this bug.

[CuteAlien]

Yeah, that line might need some change (I also don't like that the distance is independent of the object size - this sounds wrong to me on first view). But this is not the right patch. For example it can increase the cost in other cases (where drawing shadows is no longer needed). And it will make the limit at which disabling shadows happens more confusing (2*radius makes at least a bit more sense than 4 times the squared radius). So for example replacing the 2* with a variable would likely allow you to work around this as well. And then it's adaptable to other situations (for example it can then be set depending on the object size).

Note: the getLengthSQ is just tiny speed optimization which game developers use sometimes (especially back in the days when this stuff was coded). Instead of comparing length to X they avoid the (somewhat expensive) square-root involved in calculating the vector length, by instead comparing the squared length to squared X (with some cost to code readability).

But my main problem with the patch so far is that it doesn't explain at all why keeping some shadows around for longer has an effect on others blowing up. This sounds kinda random, so it's probably just hiding your real bug for a moment instead of fixing it.

Is there any chance you have a screenshot (or video) where it fails? Maybe that gives me more ideas.
edit: Also please tell me which driver you are currently working with (like opengl, d3d, software or burnings), as they don't always behave the same for shadows.

edit: Looking at the code some more I've got an idea what could go wrong. It seems to allocate FaceData earlier (not just reserve memory, but already assumes it will be used). But if createShadowVolume isn't called that's just a bunch of random data. Will need to code me some quick test - but this looks very suspect. And it would explain the behavior - basically you avoid that error case slightly more often (it's probably worst when it happens at the start).
edit2: Bit confusing code, but not the bug (FaceData isn't used before it's initialized).

[Elevations]

So i've made some videos to show you the bug (Please don't laugh at my early demo noob testing lol)
Edit - I'm using D3D9/DirectX9 Driver but i did also encounter it in OpenGL.

Here is the video of the BUG:
https://youtu.be/16edA-_Bacw

And here is video without the bug (I recorded longer to show it never happens):
https://youtu.be/87sgqAaZu1I

I also just want to let you know i tested many other things to fix this problem including inside the createShadowVolume(lpos, false); Function which did not fix the bug at all, Then i also went deeper and tried testing faces inside the createEdgesAndCaps() function which also did not fix the bug. and then after finally hours and hours of testing literally the only solution i found to remove this bug was changing getLengthSQ() to getLength() and then it went away.

I have a funny feeling that even though getLengthSQ() has a speed optimization which is nice BUT in order for calculations to be correct it needs getLength() that is my theory. But i can accept it if thats not correct. (Sometimes optimizations related to speed can cause strange bugs like this) its same thing if you try debug code after applying O2 optimizations you will get like really mangled stack traces.

[CuteAlien]

Thanks for the videos, it started helping me to understand the situation a lot already.

Do you have 2 light sources? One around the center of the map and one which is at the dwarf position but with another height? Is it floor height + around 2*dwarf height maybe? Would help me to reproduce this if you can tell me the exact placement for the light. And please tell me also which light values you are using (radius and any other light values you might have set).

The getLenght()/getLengthSQ() are the (a bit too simple) test if a node is still reached by light. If that's not the case then having a shadow is pointless so the shadow is disabled. The distance (2*light radius) seems a bit arbitrary to me - but it shouldn't matter which distance that happens! All this line does is enable/disable the shadow and your change keeps the shadow a bit longer enabled (assuming your light radius is larger than 1).

So if there is a bug where this causes things to mess up - it's not this line itself. Another distance just means you may avoid the bug for now in your case, but it's not fixing it. It's likely related to enabling/disabling shadows and it's still there. Next guy uses a larger model or tiny bit different light placement and it's back. So what I want to do is to find the real bug.

It might also be related to you having the light inside the model or going inside/outside the model for a moment (and maybe avoiding that case by changing the distance check). As that's likely not a situation well supported by this kind of light. Could be this can't even be fixed (or at least not easily), but then I'll make avoiding it easier (like you do). But to know about a good workaround I still need to know first what's really causing it.

[Elevations]

No problem, And yes i do have 2 light sources, 1 is the point light attached to the character and the other is the campfire, which here is code for that:

Character Pointlight:

Code: Select all

     // Add a pointlight
     scene::ILightSceneNode* pointlight = smgr->addLightSceneNode(
         nullptr,                                 // Parent node (none)
         core::vector3df(0, 0, 0),             // Position of the light
         video::SColorf(1.0f, 1.0f, 1.0f),       // White light color
         21.7f                                  // Radius
     );

     if (pointlight) {
         // Set the light type to pointlight
         pointlight->getLightData().Type = irr::video::ELT_POINT;

         pointlight->enableCastShadow(true); // Only available in some drivers
      }

and here is the campfire point light:

Code: Select all

scene::ILightSceneNode* camplight = smgr->addLightSceneNode(
    nullptr,                                 // Parent node (none)
    core::vector3df(-10, 20, -20),             // Position of the light
    video::SColorf(1.0f, 1.0f, 1.0f),       // White light color
    10.7f                                  // Radius
);

if (camplight) {
    // Set the light type to spotlight
    camplight->getLightData().Type = irr::video::ELT_POINT;

    // Set light color to red (RGB values for red)
    camplight->getLightData().DiffuseColor = video::SColorf(1.0f, 0.2f, 0.0f); // Full red, with green to make orange

    camplight->enableCastShadow(false); // Only available in some drivers


}

Also just letting you know this problem still happened even if i only had the character spotlight. And mainly happens because of the moving character pointlight not the static campfire one.

[CuteAlien]

You say it's attached to the parent. But addLightSceneNode doesn't set parent yet, so I assume you do later. Parent is the dwarf? And it's simply attached without any offset? So the light is in the middle of the dwarf (so in reality you would lighten up his heart...or his shoes in this case probably... have to check where it has it's orgin)?

[Elevations]

Oh its actually not "attached" in that manner it just simply follows the dwarfs position:

Code: Select all

spotlight->setPosition(core::vector3df(dwarfNode->getPosition().X, 20, dwarfNode->getPosition().Z));

[CuteAlien]

Ah OK, so it's fixed height. I already wondered because it looked like that to me in the video (light area not changing when jumping). Floor then probably at height 0 I guess. I'll try if I can reproduce it.

edit: Sorry, I need some more infos: In your addShadowVolumeSceneNode to which values do you set zfail and infinity? Or did you keep defaults? And do you change the camera far value in some way? If you did please tell to which value you set that one.

[Elevations]

here is how i structrued my addshadowvolumes but just letting you know this also happened without setting infinity and leaving them default:

Code: Select all

campfireNode->addShadowVolumeSceneNode(campfireMesh, 0, true);
dwarfNode->addShadowVolumeSceneNode(dwarfMesh, 0, true, 150.0);
skeleNode->addShadowVolumeSceneNode(skeleMesh, 1, true, 0.01);

[CuteAlien]

Tried to reproduce your values but getting very different results from the start. Thought kinda what I actually expected with lights around shoe height: https://www.youtube.com/watch?v=dGBk5XYTVrs
It's some quickly put together code, but in theory light values should be as you described: https://github.com/mzeilfelder/irr-play ... _scene.cpp (if you copy it over some example you could run it).

So ... missing something. Looking at your video again it feels like the light should come from a higher position probably. Do you maybe scale the dwarf down?

[Elevations]

oh yes my dwarf is scaled down because i reimported it into blender with mixamo animations and that increased its scale so i had to make it smaller:

Code: Select all

 dwarfNode->setScale(core::vector3df(0.1f, 0.1f, 0.1f));