AutoParamDataSource "interface" changes

[gerds]

I've recently written my own scene manager for ogre, everything went very well, but I did have to make a couple minor modifications to the ogre source. I'm hopeing that I can provide a patch so that these changes are not required, and in future writing your own scene manager can be even easier.

My main problem was that I needed to do some special handling in the AutoParamDataSource class (which provides information about the scene to the shaders - used by GpuProgram).

Unfortunately AutoParamDataSource is not subclass-able, so I ended up having to write a "special mode" for it. When the "special mode" is enabled some extra calculations are done that are specific to my scene manager.

I propose that all "relevent" AutoParamDataSource methods are made virtual so that scene managers can include their own AutoParamDataSource class (which subclasses from the class provided with ogre). This removes the requirement to modify the ogre source for scene manager development.

Unfortunately the AutoParamDataSource instance is "contained" within the Ogre::SceneManager class and then passed through to the GpuProgram instance via a couple of "const AutoParamDataSource&" method parameters.

For AutoParamDataSource to be subclass-able the Ogre::SceneManager class would need to contain a reference (ie: pointer) to its associated AutoParamDataSource. Also the AutoParamDataSource class needs to be passed around using "const AutoParamDataSource*".
This way scene managers could "new" their own instance of a AutoParamDataSource class and everything would work correctly.

I do have one small implementation question: how would you prefer the base Ogre::SceneManager create its AutoParamDataSource instance?
1) It could be created by using "p = new AutoParamDataSource" in the constructor, but I personally am not a huge fan of "new's" in constructors.
- this would also mean that overwriting scene managers would actually have to "delete" the base classes AutoParamDataSource instance and "new" their own in their constructor. Also not a huge fan of this either.

2) It could be created through a overwritable method of Ogre::SceneManager, for example:

Code: Select all

virtual AutoParamDataSource* AutoParamDataSourceFactory() const
{
    return new AutoParamDataSource();
}

However this method would also have to be called from the constructor of Ogre::SceneManager, but at least it doesn't require subclasses to call "delete" and "new" again in their own constructor.

What other options do I have?

I also propose to add some more methods to the AutoParamDataSource class so that it acts more like an "interface" to the scene information for the GpuProgram class. Currently the GpuProgram class calls methods of Ogre::Light like getDerivedPosition and getAs4DVector which I also had to find work-arounds for when creating a new scene manager. I would prefer if GpuProgram called AutoParamDataSource for all of its information instead of digging around in the scenegraph. This would be solved by adding methods like getLightPostion(int i) and getLightPosition4D(int i) to AutoParamDataSource.

I just thought I'd ask for advice before submitting a patch, so if you have any thoughts let me know.

[sinbad]

Before considering 'how' you customise AutoParamDataSource, I'd like to know the 'why'. I'd be interested in knowing why you needed to customise it, beyond adding your own parameters. AutoParamDataSource is really just there as a cacheing mechanism for derived matrices and the like, it's not supposed to be very smart and if you're needing to put SceneManager-specific things in there I'd like to know why - perhaps there's another way this could be achieved.

[gerds]

The problem is described on this forum:
"Rendering large scene & floating point precision on hardware"
http://www.ogre3d.org/phpBB2/viewtopic.php?t=29427.

Basically we have some enormous scene's, much too large to represent accurately with single-precision floating-point numbers. Therefore we have to compile ogre (and our application) to use double-precision floats.

I've had to write a scene manager which translates all of the "object transforms" to "eye coordinates" to overcome the single-precision limitations of the GPU. This translatation is done at the last possible moment in the render cycle (eg: in renderSingleObject) so that the rest of ogre, and the application does not require any changes what-so-ever.

This very easy solution works very well, except for any objects that use shaders. My problem is that AutoParamDataSource doesn't convert objects to eye coordinates (which is the change I had to make to it), and GpuProgram uses calls like "getLight(i)->getDerivedPosition()" which breaks my code too.

If GpuProgram used calls like "getLightPosition(i)" and AutoParamDataSource was overwritable then I wouldn't have to modify the ogre source for my scene manager

If you think my requirements of the AutoParamDataSource and GpuProgram classes are too specialized, and you dont want me to apply a patch (which would add some "virtual method" overhead too), then that's no problem.

I'll just merge my changes in every time ogre gets an update.

Just one last question: how would the LGPL licensing work in this situation?
Our solution requires changes to the ogre source that I'm unable to roll back in to the repository.
Should I just include a directory with the release of our software including the changes, eg:
-Readme.txt ("Built against Ogre 1.4.3 with these additional changes")
-GpuProgram.h
-GpuProgram.cpp
-AutoParamDataSource.h
-AutoParamDataSource.cpp

Cheers,

[sinbad]

Aah, I see. Makes sense, this was one of those things I intended to address in Ogre v2.0, when I finally get time for it.

Go for the patch then. I suggest that AutoParamDataSource is just held by pointer and instantiated with an overrideable method createAutoParamDataSource() on SceneManager, which you can override. I have no particular issues with making the methods virtual.

For future reference, in future if there are changes you want to include in your app that we don't accept, just making them available to people who use your app is enough, either by a downloadable or packaged patch against a defined version or a complete modified source copy.

[gerds]

I found something strange in OgreGpuProgram.cpp in the way the spotlight parameters are handled for non-spotlight lights.

Essentially for getting the params for one light the following vector is used (see "case ACT_SPOTLIGHT_PARAMS:")

Code: Select all

// Use safe values which result in no change to point & dir light calcs
// The spot factor applied to the usual lighting calc is 
// pow((dot(spotDir, lightDir) - y) / (x - y), z)
// Therefore if we set z to 0.0f then the factor will always be 1
// since pow(anything, 0) == 1
// However we also need to ensure we don't overflow because of the division
// therefore set x = 1 and y = 0 so divisor doesn't change scale
if (l.getType() != Light::LT_SPOTLIGHT)
use: Vector4(1.0, 
                0.0, 
                0.0, // since the main op is pow(.., vec4.z), this will result in 1.0
                1.0)

Essentially for getting the params for multiple lights the following vector is used (see "case ACT_SPOTLIGHT_PARAMS_ARRAY:")

Code: Select all

// Set angles to full circle for generality, and
// reduce outer angle slightly to avoid divide by zero
if (l.getType() != Light::LT_SPOTLIGHT)
use: Vector4(1.0,
                 1.0 - 1e-5,
                 0.0,
                 0.0)

Shouldn't there be a consistent way of dealing with this? otherwise potentially weird things might happen with the rendering.
Which method should be used?

[sinbad]

The non-array version is correct, that's the one I wrote and always use.