RFC on new flexible ILightManager

[rogerborg]

Now committed to the SVN trunk - see example 20.ManagedLights.

This patch provides flexible management of lights during rendering, including per-node lighting changes. It allows (for example) turning on the nearest X lights to a given node, or in finding and turning on lights that are part of a 'zone' (e.g. a room) so that they don't light anything outside of that zone.

This means that you can add an arbitrary number of lights to your scene, and then turn them on and off during the scene rendering, exactly when you need them. Since the light manager works by getting callbacks before and after each render phase and scene node render, you can also use it to get up to other cunning hijinks during scene node rendering, given sufficient imagination.

A new example has been added which demonstrates two methods of light management. In this image, there are 45 lights in the scene. Normally, only the 8 lights nearest to the camera would be turned on and would light every node, but here, each cube is being lit by the 3 lights that are part of its local 'zone', with no spill-over to the other cubes. In effect, all 45 lights are being used.



Note that the device abstractions have had to be modified, from relatively minor changes to Null and Software2, to a slightly larger change in D3D8/D3D9, and a fairly big change in OpenGL to separate out requested lights from hardware lights, which fairly closely mimics the behaviour in D3D, where you can create an arbitrary number of device lights, with the limit being on the total number that are actually set active at one time.

I believe all of this is backwards compatible, with no change to the default behaviour. The extra testing in the scene rendering shouldn't introduce any measurable performance hit.

[rogerborg]

Whee, local lights. This is two CEmptySceneNodes, each containing one mesh, and one light (red and green respectively) which only light the mesh in their parent CEmptySceneNode (and any other meshes in it or its children, if there were any).

[bitplane]

I think we need to think about some kind of scene light management which turns lights on and off depending on type, brightness, distance from the camera, hirarchy, distance to surface, etc.
I haven't looked into this myself though so I haven't thought about a nice way to do it.
an optional material flag perhaps, along with a way to query the max lights supported by a material?

[rogerborg]

OK, here's my faltering start at it. It does this:

1) Culls out point lights whose bounding box (created from their radii) are outside the viewing frustrum. If they can't light anything that the camera can see, then they might as well be culled.
2) For other lights, set a priority based on their type and situation. Directional lights take top (and equal) priority - I've assumed that users will have only a few directional lights, and they'll always want them used. Point lights are then prioritised based on the rather arbitrary (total brightness * radius / distance from camera).

This is a very basic first cut at it. A user defined priority equation would be neat, but I think most users will initially be happy to be able to throw lots of lights in the scene and have something like a sane subset of them actually used. These changes should be backwards compatible; if you have 8 or fewer lights in total, you won't notice any difference. If you have more than 8 (visible) lights then the excess would have been turned off based purely on where they were encountered in the scene graph; while I'm not claiming to present an ideal solution, I can't see any way in which it devolves the current situation.

Please ignore (or incorporate ) the changes to createFlyStraightAnimator() - I am using them in my test program to send 50 small radius lights in sequence down a path past the camera. Under the existing system, the first 8 lights found in the scene graph would be active, leaving the model in the foreground un-illuminated much of the time; under the new system, the 8 (well, 7 plus a directional light) closest to the camera are active, which keeps the foreground model illuminated.

I'm particularly interested in coming up with a better default priority algorithm, that represents the sanest behaviour for picking the 8 active lights. The algorithm at the moment is rather naive.

Code: Select all

... old code elided...

[sio2]

Hmmm. I think there are instances where item 1) won't work. Example: imagine a triangle with one vertex outside the frustum. If that vertex is lit by a light whose sphere of influence is outside the frustum then incorrect interpolation will occur over the triangle. The triangle will "pop" as it comes into view.

What do you think?

[bitplane]

hmm yes, you'd need to know the bounding box that encompases every node that is being rendered (in registerNodeForRendering)... but this would grow the bounding box to encompas the entire scene for large meshes
a better approach would be to also turn lights on and off in relation to each node (by mesh buffer would be ideal, but that's gonna be much harder), but that might also slow things down lots (havent tested).. then there's shadow volumes to think about also.

[rogerborg]

Fair point. That breaks backwards compatibility, so that's a cull too far.

OK, I've removed that, and added an optional callback function to allow the application to decide light priorities. If one is not supplied, the default algorithm is used. I've updated the test app to demonstrate use of the callback.

How is this looking? Is adding yet another method to ISceneManager (for setting the light priority callback) acceptable?

Code: Select all

... old code elided...

[zenaku]

Is there a reason the solid node list is sorted by texture in the smgr->drawAll() routine?

[vitek]

Is there a reason the solid node list is sorted by texture in the smgr->drawAll() routine?

Yes, to avoid unnecessary texture changes.

I've removed that, and added an optional callback function

I wouldn't use a function. Instead I'd use an interface class. That way the user can add additional data without having to create global variables. One reason this would be good would be to store some previous light state information to avoid flickering or to fade lights in over time.

I'd also opt to kill the default algorithm as it breaks compatibility with the existing light system and it can cause lights to flicker/pop. You could put your default light management algorithm into a class derived from the interface suggested above. It would also clean up your code in the scene manager a little.

I'd still like to see it made possible for the lights to be optionally managed before each scene node is rendered. It doesn't seem like it would be difficult.

Code: Select all

class ILightManager : public IUnknown
{
public:
  // called before first scene node is rendered
  virtual void OnPreRender() = 0;

  // called after last scene node is rendered
  virtual void OnPostRender() = 0;

  // called before the given scene node is rendered
  virtual void OnPreRender(ISceneNode* n) = 0;

  // called after the given scene node is rendered
  virtual void OnPostRender(ISceneNode* n) = 0;
};

The scene manager keeps a pointer to one of these light managers. By default it would point to a derived manager that just configured the first few lights before anything was rendered. The scene manager would call the appropriate method of the light manager at the right times and the light manager would manipulate the light list and setup the lights.

The one outstanding issue is that there is no real good way for the light manager to get hold of a light list. I guess pre-rendering a light could just put it into a light list maintained by the scene manager. Then the light list could be passed to the light manager for prioritization.

[rogerborg]

I've removed that, and added an optional callback function

I wouldn't use a function. Instead I'd use an interface class.

Fair enough. I'm borderline delerious from lack of sleep at the moment, and a single callback was all I could manage.

I'd also opt to kill the default algorithm as it breaks compatibility with the existing light system and it can cause lights to flicker/pop.

That's deliberate (the "breakage", I mean), because I think the existing light system is... less than ideal. There aren't many situations when changing default behaviour is acceptable, but I'd argue that this may be one.

Just dropping the 9th and subsequence visible lights found in the scene graph on the floor is egregiously unfriendly. Note that if you've been careful to restrict yourself to 8 or fewer visible lights, you won't see any change, and if you have more than 8 lights visible, then I'm not really sure how this change can make the behaviour worse than it already is.

However, I do accept that not changing legacy behaviour is a Good Thing, so could certainly live with it.

You could...

We could.

I'd still like to see it made possible for the lights to be optionally managed before each scene node is rendered

It would be nice, yes. It would mean turning already-created device lights on and off, which needs some of the changes from my previous (incomplete) "local lights" patch.

The one outstanding issue is that there is no real good way for the light manager to get hold of a light list. I guess pre-rendering a light could just put it into a light list maintained by the scene manager.

CLightSceneNode::OnPreRender() already pushes lights onto CSceneManger::LightList, which could be made accessible.

It sounds like you have a good handle on this. Do you want to submit a patch? I don't mind what goes in, as long as it gets the job done.

[vitek]

Just dropping the 9th and subsequence visible lights found in the scene graph on the floor is egregiously unfriendly

Agreed, but it is the old behavior. Someone might be depending on that behavior and using setVisible() to control which lights are actually applied. Your change would break their code in a bad way. Their app would compile and link just fine, but it would not have the same runtime behavior. Granted this is not likely, but it is still a possibility.

The big reason that I'm trying to encourage you to move the light management code out of the scene manager is because that isn't really where it belongs. Since you are attempting to provide an external mechanism for managing lights, that is where the light management should happen. Period. It shouldn't happen in two different places based on a flag.

I have a pretty good handle on it, but I'm not really begging to write up a patch for submission... :)

already pushes lights onto CSceneManger::LightList, which could be made accessible.

Yes, the problem isn't unsurmountable. It is just an issue that I didn't think about until I had nearly finished my post. Just exposing the light list isn't good design IMO, but it would work.

[rogerborg]

Damn Open Sores peer review systems. All right, I give, an interface class it is, with all default behavior preserved. I'll have to type it one handed though, due to the wailing infant clinging to my right arm - and that's my coding arm. If you get bored waiting, feel free to jump in at any time.

...patch now in first message...

It occurs to me that this now allows "local" lights; in the node pre-render, you could check each light to see if it's in the scene heirarchy for the node (i.e. a direct child of an ancestor of the node), and turn it on or off on that basis.

The ILightManager implemenation shown in the example code is very simple; a real light manager could, as vitek says, hold more state data and make much smarter decisions about what lights to enable. Just for starters, the per-node pre render could create a sorted light list based on distance to the node and turn on the nearest 8 (or whatever) lights and the rest off, rather than just turn on the nearest one. I'm dizzy with the possibilities.

Many thanks to vitek for prodding this in the right direction.

[vitek]

I'm not seeing the patch at the site you provided in the original post. I'm also interested to see what the overhead of all of the light management is with varying numbers of lights and scene nodes.

Travis

[rogerborg]

I do apologise, I trimmed the filename off of the URL. That's it updated, and my latest version uploaded.

I haven't tested the performance hit, because the feature is still under development, and premature optimisation is your worst enemy. In the default case, it's a few cycles running if()'s in CSceneManager, plus a few more in COpenGLDriver.cpp to deal with the new light abstraction layer that I've had to add there. I hesitate to say negligable, but I only hesitate for a few cycles.

If you choose (note: choose) to register an ILightManager that does a lot of per node/ per light calculations, then it could become significant, but that's under the sole control of the developer.

One thing that I'm chewing over is whether to pass parameters (the LightList and the CurentRenderPass) to all the ILightManager methods on the basis that it's faster to do so if they're actually required, or to let the ILightManager query back for the ones that it wants, which will be faster if it tends not to use them in the pre/post node renders. Please let me know what you think.

[hybrid]

I did not try to compile it yet but I think the LightManager code is missing
I'd like to know how to set the relation of scene nodes to light scene nodes. Anyway, nice that someone finally started work on this topic!