[2.1+] Is F.Clustered supposed to work with ortho camera?

[rujialiu]

Hi!

Although 99% of the time we're using perspective camera, in some special cases we need to switch the camera mode to orthogonal, but then the screen becomes "griddy": as we move our camera, some cells suddenly becomes darker/brighter, making the cell's border clearly visible (that's why I guessed it's related to F. Clustered). So my first question is: is F.Clustered INTENDED to work with ortho camera? If so, it should be a bug and I will look into it.

[dark_sylinc]

It should work, and it is likely a bug.

The best way to debug this is to set cluster width, height and slices to 1. If it still doesn't work, then go to ForwardClustered::collectLightForSlice and make sure that the mask in "ArrayMaskR mask" is always true so that it enters this statement:

Code: Select all

if( IS_BIT_SET( k, scalarMask ) )

If it still doesn't work, then it is likely a bug in the shader code. Could be how the projection matrix, the way the xy position in the grid is calculated, etc.

If it works, then it is likely a bug in the frustum culling code. Most likely this returns incorrect results:

Code: Select all

camera->getFrustumExtents( origFrustumLeft, origFrustumRight,
						   origFrustumTop, origFrustumBottom, FET_PROJ_PLANE_POS );

Another possibility is that ForwardPlusBase::getCachedGridFor should account for projection types, else it may think that two near-identical cameras, where the only difference is the projection type, are he same, and reuse the same result for both render passes. Which is obviously wrong.

[rujialiu]

Thanks for the info. First, I reduce the number of threads to 1, disabled pcc and planar reflections, then tried to alter the grid size. I found 4x1x1 works (I can't use 1x1x1 because I'm using SSE and width must be multiple of 4), 4x1x2 works, but 4x1x3 doesn't work. Then I changed

Code: Select all

if( IS_BIT_SET( k, scalarMask ) )

to

Code: Select all

if( true )

It works (so shader code should be correct).

Then I reverted the if(true) change and commented out the dotResult checks, and it doesn't work. So it should mean getFrustumExtents is wrong.

And after carefully debugged the codes, I found that the frustom extents for slice 2 are ridiculously large (so one of the frustums lie completely outside the point light range, which is obviously wrong for my scene). To my knowledge, at each slice, the extents of an ortho camera should be the same, but it's still calculated from the "keep the half tangent, set near/far clip distance" method, which should be only applicable to perspective cameras.

I wonder if the whole "calculate frustom for each cell in this slice" part should follow a different path for ortho camera...

[rujialiu]

Here is my patch. It's simply wrapped the old code with if (camera->getProjectionType() == PT_PERSPECTIVE) and get the extends from mCurrentCamera for ortho cameras. I don't know whether it's correct but it works for me

BTW: During the debugging I also changed nearDepthAtSlice a little bit, because in our case camera's near clip is larger than Forward Clustered's mMinDistance.

Code: Select all

OgreMain/src/OgreForwardClustered.cpp | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/OgreMain/src/OgreForwardClustered.cpp b/OgreMain/src/OgreForwardClustered.cpp
index 881ffa9..99f4a6e 100644
--- a/OgreMain/src/OgreForwardClustered.cpp
+++ b/OgreMain/src/OgreForwardClustered.cpp
@@ -314,8 +314,8 @@ namespace Ogre
         Real nearDepthAtSlice = -getDepthAtSlice( slice );
         Real farDepthAtSlice  = -getDepthAtSlice( slice + 1 );

-        if( slice == 0 )
-            nearDepthAtSlice = mCurrentCamera->getNearClipDistance();
+        if (slice == 0)
+            nearDepthAtSlice = Ogre::min(mCurrentCamera->getNearClipDistance(), nearDepthAtSlice);

         if( slice == mNumSlices - 1u )
             farDepthAtSlice = Ogre::max( mCurrentCamera->getFarClipDistance(), farDepthAtSlice );

@@ -338,16 +338,22 @@ namespace Ogre
             camera->enableReflection( mCurrentCamera->getReflectionPlane() ); 

         Real origFrustumLeft, origFrustumRight, origFrustumTop, origFrustumBottom;
-        mCurrentCamera->getFrustumExtents(origFrustumLeft, origFrustumRight,
-                                          origFrustumTop, origFrustumBottom, FET_TAN_HALF_ANGLES);
-        camera->setFrustumExtents(origFrustumLeft, origFrustumRight,
-                                          origFrustumTop, origFrustumBottom, FET_TAN_HALF_ANGLES);
-
-        camera->setNearClipDistance( nearDepthAtSlice );
-        camera->setFarClipDistance( farDepthAtSlice );
-
-        camera->getFrustumExtents( origFrustumLeft, origFrustumRight,
-                                   origFrustumTop, origFrustumBottom, FET_PROJ_PLANE_POS );
+        if (camera->getProjectionType() == PT_PERSPECTIVE) {
+            mCurrentCamera->getFrustumExtents(origFrustumLeft, origFrustumRight,
+                origFrustumTop, origFrustumBottom, FET_TAN_HALF_ANGLES);
+            camera->setFrustumExtents(origFrustumLeft, origFrustumRight,
+                origFrustumTop, origFrustumBottom, FET_TAN_HALF_ANGLES);
+
+            camera->setNearClipDistance(nearDepthAtSlice);
+            camera->setFarClipDistance(farDepthAtSlice);
+
+            camera->getFrustumExtents(origFrustumLeft, origFrustumRight,
+                origFrustumTop, origFrustumBottom, FET_PROJ_PLANE_POS);
+        }
+        else {
+            mCurrentCamera->getFrustumExtents(origFrustumLeft, origFrustumRight,
+                origFrustumTop, origFrustumBottom, FET_PROJ_PLANE_POS);
+        }

         const Real frustumHorizLength = (origFrustumRight - origFrustumLeft) / (Real)mWidth;
         const Real frustumVertLength = (origFrustumTop - origFrustumBottom) / (Real)mHeight;

[dark_sylinc]

BTW: During the debugging I also changed nearDepthAtSlice a little bit, because in our case camera's near clip is larger than Forward Clustered's mMinDistance.

That seems unnecessary. You're obtaining the minimum between nearDepthAtSlice and mCurrentCamera->getNearClipDistance(). Anything behind mCurrentCamera->getNearClipDistance() won't be visible. You will just be collecting more lights than you should.

[rujialiu]

That seems unnecessary. You're obtaining the minimum between nearDepthAtSlice and mCurrentCamera->getNearClipDistance(). Anything behind mCurrentCamera->getNearClipDistance() won't be visible. You will just be collecting more lights than you should.

Yeah... you're right. I just found near > far for slice 0 in my case, which doesn't seem right. But yes, that's not a problem actually.
What about the other parts of my fix?

[dark_sylinc]

Yeah, the rest looks alright.

Although I'm still a little worried about caching. I think the cache issue may cause bugs.

[rujialiu]

Although I'm still a little worried about caching. I think the cache issue may cause bugs.

Do you mean new cache issues introduced by my changes, or there were already some cache issues before I made changes?

[dark_sylinc]

I meant that there were already existing bugs before your changes.

To quote myself:

Another possibility is that ForwardPlusBase::getCachedGridFor should account for projection types, else it may think that two near-identical cameras, where the only difference is the projection type, are he same, and reuse the same result for both render passes. Which is obviously wrong.

[rujialiu]

@al2950

I saw your github issue about this one. Could you try my patch earlier in this thread?

[al2950]

Apologies but I took some extended leave over the summer, and so had quite a lot of work when I came back and I am still catching up. Once I am on top of work, I will sync my development streams with the new Github stuff, then I can have a look at this!

[dark_sylinc]

5 years later, this bug is finally fixed