Upgrading from 1.11.2 to 1.12.13 Topic is solved

Problems building or running the engine, queries about how to use features etc.
rpgplayerrobin
Gnoll
Posts: 617
Joined: Wed Mar 18, 2009 3:03 am
x 353

Upgrading from 1.11.2 to 1.12.13

Post by rpgplayerrobin »

Ogre Version: 1.12.13
Operating System: Windows 10
Render System: Direct3D11

Hey!

I will place all bugs and stuff regarding upgrading here in one thread if that is ok?

I found a bug regarding generating tangents for Direct3D11.
This has only to do with Direct3D11, because it works correctly in Direct3D9.

Say you have created your own mesh from ManualObject (or just your own mesh loaded from somewhere) and you want to generate new tangents for it (lets say the tangents on the mesh currently are just placeholders):

Code: Select all

unsigned short src, dest;
tmpMeshPtr->suggestTangentVectorBuildParams(VES_TANGENT, src, dest);
tmpMeshPtr->buildTangentVectors(VES_TANGENT, src, dest);

Then in "TangentSpaceCalc::insertTangents" it does this line:

Code: Select all

unsigned char* pDest = static_cast<unsigned char*>(targetBuffer->lock(HardwareBuffer::HBL_DISCARD));

That discards the whole vertex buffer and that makes the mesh behave very incorrectly on Direct3D11 (works for Direct3D9 for some reason).

It seems to work correctly when a mesh has not created tangents yet though (because of needsToBeCreated in the loop at that code), but that is not always.
Either the code should always use something else than discard (HBL_WRITE_ONLY makes the bug go away for me) or it should lock it to HBL_DISCARD only if needsToBeCreated is true.
So I suggest this code to fix it (which seems to work for me):

Code: Select all

unsigned char* pDest = static_cast<unsigned char*>(targetBuffer->lock(needsToBeCreated ? HardwareBuffer::HBL_DISCARD : HardwareBuffer::HBL_WRITE_ONLY));

Woo! That bug only took 8 hours or so to understand and fix so it no longer makes objects emit vertices randomly! :D

rpgplayerrobin
Gnoll
Posts: 617
Joined: Wed Mar 18, 2009 3:03 am
x 353

Re: Upgrading from 1.11.2 to 1.12.13

Post by rpgplayerrobin »

To my understanding, locking TANGENT and using DISCARD should not mess with the other vertex data, like POSITION, NORMAL, etc, right?
But with this bug it does. Maybe the bug is actually in Direct3D11 or something?

It seems this issue is also from 1.11.2. I can never use DISCARD in Direct3D11 on any vertex data without making the whole mesh invalid. Is this right?

Anyway, here is a code to reproduce the issue (just place it in a keypressed event and then press "H" multiple times, sometimes the quads show up but they will mostly be invalid):

Code: Select all

if (evt.keycode == CSDL_Keycode::SDLK_h)
{
	static int uniqueIDCreator = 0;

// Create the manual object
ManualObject* m_manualObject = app->m_SceneManager->createManualObject("UniqueTest" + std::to_string(uniqueIDCreator++));

// Add data to the manual object (a quad with two triangles) and any material that exists
m_manualObject->begin("floor0", RenderOperation::OT_TRIANGLE_LIST);

const Vector3 tmpNormal = Vector3::UNIT_Y;
const Vector3 tmpTangent = Vector3::UNIT_Y;
const float tmpUVScale = 0.25f;

m_manualObject->position(Vector3(0.0f, 0.0f, 0.0f));
m_manualObject->normal(tmpNormal);
m_manualObject->textureCoord(1.0f * tmpUVScale, 7.0f * tmpUVScale);
m_manualObject->tangent(tmpTangent);

m_manualObject->position(Vector3(0.0f, 0.0f, 1.0f));
m_manualObject->normal(tmpNormal);
m_manualObject->textureCoord(1.0f * tmpUVScale, 8.0f * tmpUVScale);
m_manualObject->tangent(tmpTangent);

m_manualObject->position(Vector3(1.0f, 0.0f, 1.0f));
m_manualObject->normal(tmpNormal);
m_manualObject->textureCoord(2.0f * tmpUVScale, 8.0f * tmpUVScale);
m_manualObject->tangent(tmpTangent);

m_manualObject->position(Vector3(1.0f, 0.0f, 0.0f));
m_manualObject->normal(tmpNormal);
m_manualObject->textureCoord(2.0f * tmpUVScale, 7.0f * tmpUVScale);
m_manualObject->tangent(tmpTangent);

m_manualObject->triangle(0, 1, 2);
m_manualObject->triangle(0, 2, 3);

m_manualObject->end();

// Convert the manual object to a mesh
MeshPtr tmpMeshPtr = m_manualObject->convertToMesh("UniqueTest" + std::to_string(uniqueIDCreator++));

// Build tangents for the mesh
unsigned short src, dest;
tmpMeshPtr->suggestTangentVectorBuildParams(VES_TANGENT, src, dest);
tmpMeshPtr->buildTangentVectors(VES_TANGENT, src, dest);

// Create an entity and attach it to a scene node
SceneNode* tmpSceneNode = app->m_SceneManager->getRootSceneNode()->createChildSceneNode("UniqueTest" + std::to_string(uniqueIDCreator++));
Entity* tmpEntity = app->m_SceneManager->createEntity(tmpMeshPtr->getName());
tmpSceneNode->attachObject(tmpEntity);

// Set the position of the scene node with an offset each time this code is called
static Vector3 tmpPosition = Vector3(0.0f, 0.0f, 0.0f);
//if (CGeneric::IsCapslockToggled()) // If capslock is toggled, set the position randomly instead (this shows even greater issues)
//	tmpPosition = Vector3(Math::RangeRandom(-5.0f, 5.0f), Math::RangeRandom(1.0f, 2.0f), Math::RangeRandom(-5.0f, 5.0f));
tmpSceneNode->setPosition(tmpPosition);
tmpPosition.x += 1.1f;
}
paroj
OGRE Team Member
OGRE Team Member
Posts: 1993
Joined: Sun Mar 30, 2014 2:51 pm
x 1073
Contact:

Re: Upgrading from 1.11.2 to 1.12.13

Post by paroj »

rpgplayerrobin wrote: Sun May 01, 2022 1:41 am

To my understanding, locking TANGENT and using DISCARD should not mess with the other vertex data, like POSITION, NORMAL, etc, right?

no, those are unrelated. DISCARD drops all of the buffer contents - even if it holds multiple attributes.

fix: https://github.com/OGRECave/ogre/pull/2449/files

rpgplayerrobin
Gnoll
Posts: 617
Joined: Wed Mar 18, 2009 3:03 am
x 353

Re: Upgrading from 1.11.2 to 1.12.13

Post by rpgplayerrobin »

In Direct3D11 I have a strange bug that causes it to crash on exit when using debug (assert).

When I delete the root, goes into D3D11DeviceResourceManager::(~)D3D11DeviceResourceManager and asserts on the line "assert(mResources.empty());".

There are 5 resources still remaining in that list (which are my resource, not any ogre-internal resources):

Code: Select all

FixedFunctionShader_Diffuse.hlsl (vertex shader)
FixedFunctionShader_Diffuse.hlsl (fragment shader)
ZPrePass (vertex shader)
ZPrePass (fragment shader)
white.tga

But to double check that they are actually not unloaded, I use this code to generate a list of everything still used in the game the line before I delete the root:

Code: Select all

std::string tmpStr;
std::vector<MaterialPtr> tmpMaterials;
std::string tmpTexturesUsed;
std::string tmpShadersUsed;
ResourceManager::ResourceMapIterator tmpResourceIterator = MaterialManager::getSingleton().getResourceIterator();
while (tmpResourceIterator.hasMoreElements())
{
	ResourcePtr tmpResourcePtr = tmpResourceIterator.getNext();
	std::string tmpName = tmpResourcePtr->getName();
	tmpStr += "Resource found: " + tmpName + "\n";

const Ogre::String tmpResourceType = tmpResourcePtr->getCreator()->getResourceType();
if (tmpResourceType == "Material")
{
	MaterialPtr tmpMaterialPtr = Ogre::static_pointer_cast<Material>(tmpResourcePtr);
	if (tmpMaterialPtr)
		tmpMaterials.push_back(tmpMaterialPtr);
}
}
ResourceManager::ResourceMapIterator tmpItr = GpuProgramManager::getSingleton().getResourceIterator();
if (tmpItr.begin() == tmpItr.end())
	tmpItr = HighLevelGpuProgramManager::getSingleton().getResourceIterator();
for (ResourceManager::ResourceMapIterator::const_iterator i = tmpItr.begin(); i != tmpItr.end(); ++i)
{
	const ResourcePtr tmpResourcePtr = i->second;
	std::string tmpName = tmpResourcePtr->getName();
	tmpStr += "GPU program found: " + tmpName + "\n";
}
for (auto& it : Ogre::ResourceGroupManager::getSingleton().getResourceManagers())
{
	tmpStr += "Resource group still existing: " + it.first + "\n";

tmpItr = it.second->getResourceIterator();
for (ResourceManager::ResourceMapIterator::const_iterator i = tmpItr.begin(); i != tmpItr.end(); ++i)
{
	const ResourcePtr tmpResourcePtr = i->second;
	std::string tmpName = tmpResourcePtr->getName();
	tmpStr += "Resource found in group " + it.first + ": " + tmpName + "\n";

	const Ogre::String tmpResourceType = tmpResourcePtr->getCreator()->getResourceType();
	if (tmpResourceType == "Material")
	{
		MaterialPtr tmpMaterialPtr = Ogre::static_pointer_cast<Material>(tmpResourcePtr);
		if (tmpMaterialPtr)
			tmpMaterials.push_back(tmpMaterialPtr);
	}
}
}
for (size_t i = 0; i < tmpMaterials.size(); i++)
{
	MaterialPtr tmpMaterialPtr = tmpMaterials[i];
	for (unsigned short n = 0; n < tmpMaterialPtr->getNumTechniques(); n++)
	{
		Technique* tmpTechnique = tmpMaterialPtr->getTechnique(n);
		for (unsigned short m = 0; m < tmpTechnique->getNumPasses(); m++)
		{
			Pass* tmpPass = tmpTechnique->getPass(m);

		if (tmpPass->hasVertexProgram())
			tmpStr += tmpMaterialPtr->getName() + ", shader used (VS): " + tmpPass->getVertexProgramName() + "\n";

		if(tmpPass->hasFragmentProgram())
			tmpStr += tmpMaterialPtr->getName() + ", shader (PS): " + tmpPass->getFragmentProgramName() + "\n";

		for (unsigned short o = 0; o < tmpPass->getNumTextureUnitStates(); o++)
		{
			TextureUnitState* tmpTextureUnitState = tmpPass->getTextureUnitState(o);
			std::string tmpName = tmpTextureUnitState->getTextureName();
			if(tmpName != "")
				tmpStr += tmpMaterialPtr->getName() + ", texture used: " + tmpName + "\n";
		}
	}
}
}
tmpMaterials.clear();

That gives me this result below, which shows specifically that there is no shader used at all and the texture "white.tga" is not used either:

Code: Select all

Resource found: DefaultSettings
Resource found: BaseWhite
Resource found: BaseWhiteNoLighting
Resource found: Ogre/ShadowTexture5MatSceneManagerUnique1
Resource group still existing: Compositor
Resource group still existing: GpuProgram
Resource group still existing: HighLevelGpuProgram
Resource group still existing: Material
Resource found in group Material: DefaultSettings
Resource found in group Material: BaseWhite
Resource found in group Material: BaseWhiteNoLighting
Resource found in group Material: Ogre/ShadowTexture5MatSceneManagerUnique1
Resource group still existing: Mesh
Resource found in group Mesh: Ogre/Debug/AxesMesh
Resource found in group Mesh: Prefab_Sphere
Resource found in group Mesh: Prefab_Cube
Resource found in group Mesh: Prefab_Plane
Resource group still existing: Skeleton
Resource group still existing: Texture
Resource found in group Texture: Ogre/ShadowTextureNull3
Resource found in group Texture: Ogre/ShadowTexture5
Resource found in group Texture: Warning
Ogre/ShadowTexture5MatSceneManagerUnique1, texture used: Ogre/ShadowTexture5
Ogre/ShadowTexture5MatSceneManagerUnique1, texture used: Ogre/ShadowTexture5

Is there some bug on Direct3D11 that does not remove resources from its internal list correctly?

User avatar
suny
Greenskin
Posts: 137
Joined: Thu Mar 12, 2020 5:53 pm
x 60

Re: Upgrading from 1.11.2 to 1.12.13

Post by suny »

I have the same assert when I exit from DX11.
S.

paroj
OGRE Team Member
OGRE Team Member
Posts: 1993
Joined: Sun Mar 30, 2014 2:51 pm
x 1073
Contact:

Re: Upgrading from 1.11.2 to 1.12.13

Post by paroj »

rpgplayerrobin wrote: Mon May 02, 2022 1:14 pm

Is there some bug on Direct3D11 that does not remove resources from its internal list correctly?

no, it is working as intended. See: https://ogrecave.github.io/ogre/api/lat ... ddcf7b3f19

it means that you removed the resource, but still hold on to a shared_ptr to it somewhere. D3D11 assets here as it likely breaks device lost handling. Maybe you can just ignore that if you are terminating the process anyway.

rpgplayerrobin
Gnoll
Posts: 617
Joined: Wed Mar 18, 2009 3:03 am
x 353

Re: Upgrading from 1.11.2 to 1.12.13

Post by rpgplayerrobin »

paroj wrote: Mon May 02, 2022 3:31 pm
rpgplayerrobin wrote: Mon May 02, 2022 1:14 pm

Is there some bug on Direct3D11 that does not remove resources from its internal list correctly?

no, it is working as intended. See: https://ogrecave.github.io/ogre/api/lat ... ddcf7b3f19

it means that you removed the resource, but still hold on to a shared_ptr to it somewhere. D3D11 assets here as it likely breaks device lost handling. Maybe you can just ignore that if you are terminating the process anyway.

Thank you! That explains a lot and helped me to fix it.

For anyone else that has this issue and want to fix it (@suny maybe), here is how I fixed it.

On the destruction/exit of the application, before all resource groups are destroyed, use this code:

Code: Select all

// Get all resources into a list
std::vector<ResourcePtr> tmpResources;
ResourceManager::ResourceMapIterator tmpResourceIterator = MaterialManager::getSingleton().getResourceIterator();
while (tmpResourceIterator.hasMoreElements())
{
	ResourcePtr tmpResourcePtr = tmpResourceIterator.getNext();
	tmpResources.push_back(tmpResourcePtr);
}
ResourceManager::ResourceMapIterator tmpItr = GpuProgramManager::getSingleton().getResourceIterator();
if (tmpItr.begin() == tmpItr.end())
	tmpItr = HighLevelGpuProgramManager::getSingleton().getResourceIterator();
for (ResourceManager::ResourceMapIterator::const_iterator i = tmpItr.begin(); i != tmpItr.end(); ++i)
{
	const ResourcePtr tmpResourcePtr = i->second;
	tmpResources.push_back(tmpResourcePtr);
}

After the resource groups have been destroyed (but before destroying Root), use this code:

Code: Select all

// Remove all resources with only one use count
for (size_t i = 0; i < tmpResources.size(); i++)
{
	ResourcePtr& tmpResource = tmpResources[i];
	if (tmpResource.use_count() == 1 ||
		tmpResource->getGroup() == "OgreInternal")
	{
		std::vector<ResourcePtr>::iterator del = tmpResources.begin();
		del += i;
		tmpResources.erase(del);
		i--;
	}
}

// The resources left here will only be the ones still in D3D11DeviceResourceManager::(~)D3D11DeviceResourceManager.
// Find them in your user code and fix them.
tmpResources.clear(); // Debug this line here.

Put a breakpoint in the line at the end that contains "// Debug this line here".

Then run the application and exit it and let the breakpoint happen.
Look through the list of resources, they will be the ones causing issues, get their names and fix them before you destroy all resource groups.
In my application, it was the material called "white" (my own resource) that caused the problems, and this code before destroying the resource groups solved it:

Code: Select all

// Fix specific materials before we exit, since they will otherwise not be unloaded correctly on Direct3D11 and cause a crash
std::vector<MaterialPtr> tmpMaterials;
tmpMaterials.push_back(MaterialManager::getSingleton().getByName("white"));
for (size_t i = 0; i < tmpMaterials.size(); i++)
{
	MaterialPtr tmpMaterial = tmpMaterials[i];
	if(tmpMaterial)
		tmpMaterial->removeAllTechniques();
}
tmpMaterials.clear();

I made an #ifdef for this in my code to fix future issues regarding this, full code here:

Code: Select all

//#define FIND_DIRECT3D11_DEBUG_RESOURCE_CRASH
#ifdef FIND_DIRECT3D11_DEBUG_RESOURCE_CRASH
// Get all resources into a list
std::vector<ResourcePtr> tmpResources;
ResourceManager::ResourceMapIterator tmpResourceIterator = MaterialManager::getSingleton().getResourceIterator();
while (tmpResourceIterator.hasMoreElements())
{
	ResourcePtr tmpResourcePtr = tmpResourceIterator.getNext();
	tmpResources.push_back(tmpResourcePtr);
}
ResourceManager::ResourceMapIterator tmpItr = GpuProgramManager::getSingleton().getResourceIterator();
if (tmpItr.begin() == tmpItr.end())
	tmpItr = HighLevelGpuProgramManager::getSingleton().getResourceIterator();
for (ResourceManager::ResourceMapIterator::const_iterator i = tmpItr.begin(); i != tmpItr.end(); ++i)
{
	const ResourcePtr tmpResourcePtr = i->second;
	tmpResources.push_back(tmpResourcePtr);
}
#endif



// Fix specific materials before we exit, since they will otherwise not be unloaded correctly on Direct3D11 and cause a crash
std::vector<MaterialPtr> tmpMaterials;
tmpMaterials.push_back(MaterialManager::getSingleton().getByName("white"));
for (size_t i = 0; i < tmpMaterials.size(); i++)
{
	MaterialPtr tmpMaterial = tmpMaterials[i];
	if(tmpMaterial)
		tmpMaterial->removeAllTechniques();
}
tmpMaterials.clear();

// Destroy all resource managers
delete m_inGameResourceManager;
delete m_alwaysLoadedResourceManager;
delete m_beforeLoadingScreenResourceManager;




#ifdef FIND_DIRECT3D11_DEBUG_RESOURCE_CRASH
// Remove all resources with only one use count
for (size_t i = 0; i < tmpResources.size(); i++)
{
	ResourcePtr& tmpResource = tmpResources[i];
	if (tmpResource.use_count() == 1 ||
		tmpResource->getGroup() == "OgreInternal")
	{
		std::vector<ResourcePtr>::iterator del = tmpResources.begin();
		del += i;
		tmpResources.erase(del);
		i--;
	}
}

// The resources left here will only be the ones still in D3D11DeviceResourceManager::(~)D3D11DeviceResourceManager.
// Find them in your user code and fix them.
tmpResources.clear(); // Debug this line here.
#endif



// Destroy the root
delete m_Root;
User avatar
suny
Greenskin
Posts: 137
Joined: Thu Mar 12, 2020 5:53 pm
x 60

Re: Upgrading from 1.11.2 to 1.12.13

Post by suny »

Thanks!
All my undestroyed resources are materials from RTShader though, so I didn't create them myself :/
S.

rpgplayerrobin
Gnoll
Posts: 617
Joined: Wed Mar 18, 2009 3:03 am
x 353

Re: Upgrading from 1.11.2 to 1.12.13

Post by rpgplayerrobin »

When I resize my window in Direct3D9, my scene becomes completely black.
The GUI from Gorilla and particles still renders though, but the scene does not.
This happens only for Direct3D9 and not for Direct3D11.

This does not happen in Ogre 1.11.2 though, with the exact same SDL code handling the window and the exact same SDL version (SDL2, 2.0.20 built with x64 just like with OgreSDK and my game).

Since SDL and the code regarding the window has not changed, what might have changed in Ogre regarding this?

paroj
OGRE Team Member
OGRE Team Member
Posts: 1993
Joined: Sun Mar 30, 2014 2:51 pm
x 1073
Contact:

Re: Upgrading from 1.11.2 to 1.12.13

Post by paroj »

rpgplayerrobin
Gnoll
Posts: 617
Joined: Wed Mar 18, 2009 3:03 am
x 353

Re: Upgrading from 1.11.2 to 1.12.13

Post by rpgplayerrobin »

paroj wrote: Tue May 03, 2022 7:51 pm

probably this: viewtopic.php?p=550570#p550570

Thank you! No more black screen! :D

loath
Platinum Sponsor
Platinum Sponsor
Posts: 290
Joined: Tue Jan 17, 2012 5:18 am
x 67

Re: Upgrading from 1.11.2 to 1.12.13

Post by loath »

suny wrote: Tue May 03, 2022 6:02 pm

Thanks!
All my undestroyed resources are materials from RTShader though, so I didn't create them myself :/
S.

i also had this assertion when switching from dx9 to dx11. this is caused by not freeing a scene node or entity that you created which has active references to a material, shader, or other reference counted resource.

verify all the nodes and entities you've created are destroyed via scene->destroyEntity () or scene->destroySceneNode (node) etc.

loath
Platinum Sponsor
Platinum Sponsor
Posts: 290
Joined: Tue Jan 17, 2012 5:18 am
x 67

Re: Upgrading from 1.11.2 to 1.12.13

Post by loath »

When I delete the root, goes into D3D11DeviceResourceManager::(~)D3D11DeviceResourceManager and asserts on the line "assert(mResources.empty());".

this assert needs a comment like: "verify you've destroyed all nodes, entities, and reference counted resources before shutdown"

rpgplayerrobin
Gnoll
Posts: 617
Joined: Wed Mar 18, 2009 3:03 am
x 353

Re: Upgrading from 1.11.2 to 1.12.13

Post by rpgplayerrobin »

I am having some problems with spotlights.
It seems when I rotate a spotlight, with this code for example:

Code: Select all

m_light->getParentNode()->rotate(Vector3::UNIT_X, Degree(-30.0f * app->m_DT), Node::TS_LOCAL);

Then it does not update the light list of objects around it.

If I physically set the position of the light back and forth between two frames that it gets updated and objects around it gets correctly lit, or if I call sceneManager->notifyLightsDirty().

But should the direction of the light (only for spotlights) not also be included in this automatically, like the position is (in SceneManager::findLightsAffectingFrustum)?
Or should I just call sceneManager->_notifyLightsDirty() everytime I set the direction of a light to force it to update?

paroj
OGRE Team Member
OGRE Team Member
Posts: 1993
Joined: Sun Mar 30, 2014 2:51 pm
x 1073
Contact:

Re: Upgrading from 1.11.2 to 1.12.13

Post by paroj »

this assert needs a comment like: "verify you've destroyed all nodes, entities, and reference counted resources before shutdown"

good idea https://github.com/OGRECave/ogre/pull/2 ... 2d3ab556ef

It seems when I rotate a spotlight, Then it does not update the light list of objects around it.

is that a new issue, that you did not encounter before?

from the code, the spotlight should be in the lightlist regardless of its orientation:
https://github.com/OGRECave/ogre/blob/e ... .cpp#L2697

this code did not change since 1.6.

rpgplayerrobin
Gnoll
Posts: 617
Joined: Wed Mar 18, 2009 3:03 am
x 353

Re: Upgrading from 1.11.2 to 1.12.13

Post by rpgplayerrobin »

I am not really sure if it existed before.

All I know is that when I rotate a light, it will not get sent to the right objects correctly unless I call that function (_notifyLightsDirty) or physically move the lights between two frames (which SceneManager::findLightsAffectingFrustum seems to handle).

I also found another bug (again, not sure if it existed before or not, but its the same kind of a bug). When I get into a scene and I am using a lot of lights in it (say 20 or so), but that any object can just receive the 3 nearest lights (param_named_auto lightPosition0-2 light_position 0-2), then the object usually gets the incorrect lights at startup.
However, if I move a light (any light in the scene, even far away from the object) the objects get their correct lights (the closest ones to them).
I am unable to find a function to fix this bug, as _notifyLightsDirty does nothing for this issue.

paroj
OGRE Team Member
OGRE Team Member
Posts: 1993
Joined: Sun Mar 30, 2014 2:51 pm
x 1073
Contact:

Re: Upgrading from 1.11.2 to 1.12.13

Post by paroj »

you could break here

to verify whether the light list is being updated, when one expect

rpgplayerrobin
Gnoll
Posts: 617
Joined: Wed Mar 18, 2009 3:03 am
x 353

Re: Upgrading from 1.11.2 to 1.12.13

Post by rpgplayerrobin »

It comes into MovableObject::queryLights, and it updates there every frame (because I force it through calling mBillboardSet->_notifyMoved() every frame).

Then, in SceneManager::_populateLightList it seems to check against how many shadow textures there are... I am using 2 shadow textures in my current setup.
But that has nothing to do with non-casting lights, so I find that kind of strange.
It does not check if the light in question is actually using shadows, but it just assumes that it does.
So in here it gets the 2 first lights in the list, which means my directional light (which is always first) and then a random light in the scene.
That makes it take an incorrect light because of that, and if I had 4 shadow textures active it would take 3 incorrect lights without sorting them (since it then assumes I am using 4 lights that is using shadows).

What it should do it probably to check if the first lights are actually casting shadows, otherwise it is pretty bad to assume that they are (as it is currently in the code).

But still... That does not explain why it gets fixed when I move a random light.
But SceneManager::findLightsAffectingFrustum explains it.
Because when I add all lights to the scene, it might be in a random order. But then when I move a light (any light), SceneManager::findLightsAffectingFrustum sees a change and sorts the lights based on the camera position. And incidentally in this case, it means the object I look at gets the right lights on it.

So in short, the whole bug could probably be solved by user code that sets the number of shadow textures to 0 if there are no lights that casts shadows in the scene, and then to update it each time it gets a new one and then needs to remake shaders/GPU params if needed. Or it could instead just be solved to do that automatically in SceneManager::-populateLightList to something like this (not tested):

Code: Select all

void SceneManager::_populateLightList(const Vector3& position, Real radius, 
                                      LightList& destList, uint32 lightMask)
{
    // Really basic trawl of the lights, then sort
    // Subclasses could do something smarter

// Pick up the lights that affecting frustum only, which should has been
// cached, so better than take all lights in the scene into account.
const LightList& candidateLights = _getLightsAffectingFrustum();

// Pre-allocate memory
destList.clear();
destList.reserve(candidateLights.size());

size_t lightIndex = 0;
size_t numShadowTextures = isShadowTechniqueTextureBased() ? getShadowTextureConfigList().size() : 0;

for (Light* lt : candidateLights)
{
    // check whether or not this light is suppose to be taken into consideration for the current light mask set for this operation
    if(!(lt->getLightMask() & lightMask))
        continue; //skip this light

    // Calc squared distance
    lt->_calcTempSquareDist(position);

    // only add in-range lights, but ensure texture shadow casters are there
    // note: in this case the first numShadowTextures canditate lights are casters
    if ((lt->getCastShadows() && lightIndex < numShadowTextures) || lt->isInLightRange(Sphere(position, radius)))
    {
        destList.push_back(lt);
    }

	lightIndex++;
}

auto start = destList.begin();
// if we're using texture shadows, we actually want to use
// the first few lights unchanged from the frustum list, matching the
// texture shadows that were generated
// Thus we only allow object-relative sorting on the remainder of the list
std::advance(start, std::min(numShadowTextures, destList.size()));
// Sort (stable to guarantee ordering on directional lights)
std::stable_sort(start, destList.end(), lightLess());

// Now assign indexes in the list so they can be examined if needed
lightIndex = 0;
for (auto lt : destList)
{
    lt->_notifyIndexInFrame(lightIndex++);
}
}

What do you think of that solution?
I basically only added "lt->getCastShadows()" and moved the "lightIndex++" to later.

paroj
OGRE Team Member
OGRE Team Member
Posts: 1993
Joined: Sun Mar 30, 2014 2:51 pm
x 1073
Contact:

Re: Upgrading from 1.11.2 to 1.12.13

Post by paroj »

It does not check if the light in question is actually using shadows, but it just assumes that it does.

SceneManager::findLightsAffectingFrustum ensures that the first lights are shadow casters here
https://github.com/OGRECave/ogre/blob/1 ... .cpp#L2977

so this should not be the problem

rpgplayerrobin
Gnoll
Posts: 617
Joined: Wed Mar 18, 2009 3:03 am
x 353

Re: Upgrading from 1.11.2 to 1.12.13

Post by rpgplayerrobin »

Actually it is still the same problem.
I knew the light list already sorted the lights after shadow casters first, but the issue in my previous post has nothing to do with this.

The issue is that if you have 4 shadow maps but not a single shadow casting light, that function you mention (SceneManager::findLightsAffectingFrustum) will sort them by only distance (from the camera). This is correct.

But the bug has to do with the function I mention (SceneManager::_populateLightList). That function believes your function (SceneManager::findLightsAffectingFrustum) has 4 shadow casting lights first in its list.
But this is very incorrect, since if you have 4 shadow maps you might currently not have 4 shadow-casting lights in the scene. That means that the 4 first lights in the list, which are sorted from the camera, will be first in the list for the object as well, even if those 4 lights are not shadow casters and that they are not that close to the object. This means that the 4 first lights for that object will not even be sorted in range to the object, because this function believes they must be shadow casting lights (which is incorrect).

Same problem arises with arbitrary amount of shadow maps if they are greater than the current amount of shadow caster lights in the scene.

If you read my last message again with this in mind, I think you will understand the problem completely.

rpgplayerrobin
Gnoll
Posts: 617
Joined: Wed Mar 18, 2009 3:03 am
x 353

Re: Upgrading from 1.11.2 to 1.12.13

Post by rpgplayerrobin »

Here is the full code that I am using, it fixes the bug completely and does not need any user-code to fix the issue:

Code: Select all

void SceneManager::_populateLightList(const Vector3& position, Real radius, 
                                      LightList& destList, uint32 lightMask)
{
    // Really basic trawl of the lights, then sort
    // Subclasses could do something smarter

// Pick up the lights that affecting frustum only, which should has been
// cached, so better than take all lights in the scene into account.
const LightList& candidateLights = _getLightsAffectingFrustum();

// Pre-allocate memory
destList.clear();
destList.reserve(candidateLights.size());

size_t numberOfLightsCastingShadows = 0;
size_t lightIndex = 0;
size_t numShadowTextures = isShadowTechniqueTextureBased() ? getShadowTextureConfigList().size() : 0;

for (Light* lt : candidateLights)
{
    // check whether or not this light is suppose to be taken into consideration for the current light mask set for this operation
    if(!(lt->getLightMask() & lightMask))
        continue; //skip this light

    // Calc squared distance
    lt->_calcTempSquareDist(position);

    // only add in-range lights, but ensure texture shadow casters are there
    // note: in this case the first numShadowTextures canditate lights are casters
	if (lt->getCastShadows() && lightIndex < numShadowTextures)
	{
		destList.push_back(lt);
		numberOfLightsCastingShadows++;
	}
    else if (lt->isInLightRange(Sphere(position, radius)))
    {
        destList.push_back(lt);
    }

	lightIndex++;
}

auto start = destList.begin();
// if we're using texture shadows, we actually want to use
// the first few lights unchanged from the frustum list, matching the
// texture shadows that were generated
// Thus we only allow object-relative sorting on the remainder of the list
std::advance(start, std::min(numberOfLightsCastingShadows, destList.size()));
// Sort (stable to guarantee ordering on directional lights)
std::stable_sort(start, destList.end(), lightLess());

// Now assign indexes in the list so they can be examined if needed
lightIndex = 0;
for (auto lt : destList)
{
    lt->_notifyIndexInFrame(lightIndex++);
}
}

Specifically look at "lightIndex", "lt->getCastShadows()" and "numberOfLightsCastingShadows".

paroj
OGRE Team Member
OGRE Team Member
Posts: 1993
Joined: Sun Mar 30, 2014 2:51 pm
x 1073
Contact:

Re: Upgrading from 1.11.2 to 1.12.13

Post by paroj »

rpgplayerrobin wrote: Fri May 06, 2022 9:35 pm

That means that the 4 first lights in the list, which are sorted from the camera, will be first in the list for the object as well, even if those 4 lights are not shadow casters and that they are not that close to the object.

ah.. thanks! got it now:
https://github.com/OGRECave/ogre/pull/2455/files

rpgplayerrobin
Gnoll
Posts: 617
Joined: Wed Mar 18, 2009 3:03 am
x 353

Re: Upgrading from 1.11.2 to 1.12.13

Post by rpgplayerrobin »

I have almost successfully upgrade to 1.12.13, however there are still some issues left.

The current issue is that the performance of the Ogre::InstanceManager seems to have changed (using HWInstancingBasic).

I thought there might be something in my user code that must have changed since the previous version, so I made a hybrid code that can run on both 1.11.2 and 1.12.13 with #ifdef here and there.
Then, with the exact same user-code (apart from function changes in the Ogre version of course) the performance is still different.

If I disable Instancing in my game completely, both versions run at almost the same speed.
However, when I enable Instancing on the newer 1.12.13, my FPS is almost cut in half in a normal game scene.
The older 1.11.2 is not affected by any FPS change at all (since this is a top-down game, Instancing rarely help with performance).

When I instead make a stress-test scene in 1.12.13 with 6000 of the same object, I get 53 FPS with Instancing on and 42 FPS without instancing on.
That same test on 1.11.2 has 107 FPS with instancing on and 42 FPS without instancing on.
That means instancing on the older 1.11.2 version has double the FPS than instancing on the newer 1.12.13 version.

But, the results are strange, since the newer 1.12.13 version gets higher FPS with instancing on in the stress test (though barely), but it gets much lower FPS when using a normal game scene.

I guess something big must have changed regarding instancing since the 1.11.2 version?

I checked through https://github.com/OGRECave/ogre/releases but I could not find anything except for these changes:
1.12.0: use Matrix3x4f TransformBase typedef in InstancedEntity
1.12.4: InstanceBatch: Initialize frame number in constructor.
1.12.12: InstanceManager - fixed invalid EdgeData after unshareVertices
1.12.13: Instances Make sure cloned vertex data is deleted.
1.12.13: InstanceManager - unshare LODs as well (#2000)
But could those small changes really affect something this big? I am not using LOD or Edgelists on any of my meshes.

paroj
OGRE Team Member
OGRE Team Member
Posts: 1993
Joined: Sun Mar 30, 2014 2:51 pm
x 1073
Contact:

Re: Upgrading from 1.11.2 to 1.12.13

Post by paroj »

But, the results are strange, since the newer 1.12.13 version gets higher FPS with instancing on in the stress test

but then

a stress-test scene in 1.12.13 with 6000 of the same object, I get 53 FPS (...) on 1.11.2 has 107 FPS

anyway.. you could bisect for the offending commit:

Code: Select all

$ git bisect start
$ git bisect good v1.11.2
$ git bisect bad v1.12.13

tells me it takes 10 steps until you find it.

rpgplayerrobin
Gnoll
Posts: 617
Joined: Wed Mar 18, 2009 3:03 am
x 353

Re: Upgrading from 1.11.2 to 1.12.13

Post by rpgplayerrobin »

But, the results are strange, since the newer 1.12.13 version gets higher FPS with instancing on in the stress test

but then

a stress-test scene in 1.12.13 with 6000 of the same object, I get 53 FPS (...) on 1.11.2 has 107 FPS

What I meant is that it still gets higher FPS with instancing on compared with not on, which was not true for a normal game scene, which I found strange. The comparison to the older was done a sentence before that ("That means instancing on the older 1.11.2 version has double the FPS than instancing on the newer 1.12.13 version.").

I have no idea how to do that with git, as I just downloaded the entire source of Ogre (and have made many changes to it as well to meet my needs) and then compiled it, so I don't think it is connected in any way to git.
But I will search on what that means and hopefully somehow try it and find the bug.
Or I could just compare the files with the 1.11.2 version.
Anyway, I will post my findings here when I am done! :D

Post Reply