Request: manage which passes are drawn by smgr->drawall

[hendu]

I mentioned that before, and my opinion was that instead of clearing the lists, place the flag check before each section. Hybrid, would that way be preferable to you?

[hybrid]

Yes, my first thought was also 'why is there no check in advance'. We even have the curly braces there, so probably the checks have been there before. But changing the test to that place would limit the applicability of the light manager. I thin kthat the call back should be called always, and additionally push the list of scene nodes to the call back method. This would allow for an easy check whether the light manager has something to do or not. But it would also allow users to implement functions happening if no scene node is available. As an example, what if you'd like to do something before any mesh node is rendered. Assuming that the solid nodes are rendered first is not possible, as you cannot be sure that the call back is called. So I guess we leave it as is in the patch, and just make sure that the light manager documents that the method is called regardless of any existing scene node. Disabling the light manager in some passes needs to happen in the light manager then.

[hendu]

Ping. This is wanted by STK too now.

[Granyte]

why didn't anything like this ever get implemented? what would be needed for such a thing to be implemented?


I just broke the Draw all into a couple method because I needed control over wich passes were drawn and over the onanimate call to every scenenodes

Code: Select all

 
//! Explicitly clear material, transforms and other states.
        virtual void SetupDraw() = 0;
 
        //! Explicitly Animate All Scenenodes.
        /** this should only be called when using custom render path and and you need to animate once befor doing custom rendering*/ 
        virtual void Animate() = 0;
 
        //! Explicitly Animate All Scenenodes.
        /** this should only be called when using custom render path and and you need to animate once befor doing custom rendering*/
        virtual void Animate(u32 timeMs) = 0;
 
        //!Explicitly render active Camera
        virtual void RenderCamera() = 0;
 
 
        //!Explicitly call scenenodes registering
        virtual void RegisterScenenodes() = 0;
 
        //!Explicitly call OnPreRender on the light manager
        virtual void PreRenderLightManager() = 0;
 
        //!Explicitly call OnPostRender on the light manager
        virtual void PostRenderLightManager() = 0;
 
        //!Explicitly Render all camera
        virtual void RenderCameraScenes() = 0;
 
        //!Explicitly Render all light
        virtual void RenderLightsScenes() = 0;
 
        //!Explicitly Render all Skyboxes
        virtual void RenderSkyboxes() = 0;
 
        //!Explicitly Render all Solid Scenenodes
        virtual void RenderSolid() = 0;
 
        //!Explicitly Render all Shadow Scenenodes
        virtual void RenderShadow() = 0;
 
        //!Explicitly Render transparent Scenenodes
        virtual void RenderTransparent() = 0;
 
        //!Explicitly Render transparent Effect Scenenodes
        virtual void RenderTransparentEffect() = 0;
 
        //! Explicitly clear material, transforms and other states.
        virtual void CleanupDraw() = 0;
 

and then the draw all simply call ll those methods in the order the calls were made befor so any one who just wanna cal drawall get the same simplicity as befor but those who need more complex behavior can have them without being forced to re do the work irrlicht is already doing

[hendu]

The patch exists, works well, and has existed for years. So basically the same as every other irr patch, the few devs are too busy to care.

[CuteAlien]

@hendu: As well as the problem that the only maintainer of that part of the code is obviously no longer active. So someone else has to work into that first - no idea right now who will do that. I know roughly what's going on there, but probably no more than you do. On first look I'd say having lot's of names which all do the same is a pretty good sign this should likely be implemented with each - one pre, render and post function which each take an enum for the stage. Then at least the interface will not get as large and not be broken again when new stages are added some day. And no - we likely don't want all users to call everything explicitely but make this a choice. So if there's a way to keep the old interface intact it should stay intact - meaning new functions should be optional. And while we are at that we should consider if any of those functions should maybe also have a parameter about custom sorting - as that is the other feature which is requested a lot in that place.

So on first view - and I really(!) have no time doing a longer view right now - I don't like the patch like it is. But maybe it is good and I just don't know it because I don't know that code well enough. But it's nothing I'm going to apply blind as my first reaction is against coding it like that.

[hendu]

The patch I mean is completely different to what Granyte posted above. It breaks no API, merely adds an optional parameter to drawAll.

edit: https://github.com/clbr/seirr/commit/0a ... 8ae3135b3f

[CuteAlien]

Yeah, but your patch doesn't have the pre and post render functions Granyte seems to need. So maybe it wouldn't work for him? I don't know...

[Granyte]

I guess I didn't explain clearly what I did I simply refactored the draw all to be composed of smaller methods
so the interphase is not broken the draw all call is still present it simply call all those methods in the order and fashion it used to so any older code can still be compiled and work as it always did

but the users who wan more control can call individual sub methods in the order they wish by ditching the still present drawall


Now I don't think my way of splitting the sceneManager drawall() call is perfect it does make the interphase fatter but I posted it here to obtain feed back but apparently reading a post to the end is so damn hard

and then the draw all simply call ll those methods in the order the calls were made befor so any one who just wanna cal drawall get the same simplicity as befor but those who need more complex behavior can have them without being forced to re do the work irrlicht is already doing

and if the damn 2 missing letters made the text impossible to read here is a corrected version

and then the drawall() simply call all those methods in the order the calls were made before so any one who just want to call drawall() get the same simplicity as before
but those who need more complex behavior can have them without being forced to re do the work irrlicht is already doing

while the other patch seem fine it still won't be any useful to me because it still call OnAnimate() on every draw pass messing the scene as soon as your fps dips a little to low and adding to the over all latency

On first look I'd say having lot's of names which all do the same is a pretty good sign this should likely be implemented with each - one pre, render and post function which each take an enum for the stage. Then at least the interface will not get as large and not be broken again when new stages are added some day.

I agree this was only a quick toss I mostly split the part that were already in braces but they could be merged and use an enum

And no - we likely don't want all users to call everything explicitely but make this a choice. So if there's a way to keep the old interface intact it should stay intact - meaning new functions should be optional.

do I need to quote my self a 3rd time?

And while we are at that we should consider if any of those functions should maybe also have a parameter about custom sorting.

how would custom sorting be implemented was there any previous patch proposal about this?

I don't like the patch like it is. But maybe it is good and I just don't know it because I don't know that code well enough. But it's nothing I'm going to apply blind as my first reaction is against coding it like that.

No one is asking for an application on faith but a quick look at the patch or the general idea to steer further improvements on it but generally we don't even get that

[CuteAlien]

d it here to obtain feed back but apparently reading a post to the end is so damn hard

Yeah, sorry - I do the forum reading/posting mostly in compile pauses (or right now while cooking/eating). So I just overfly it - in this case just looking at the code. Only way I manage to find enough time to read all forum posts and give several times per day some feedback ;-)

Just to make it clear - I will not work on this patch! It's just not my area and nothing I need myself and I already have too many tasks. If I have time left I work on bugs and not features as there are some open right now (and I don't have time because next weekends will all go into things like taxes as I won't survive next month otherwise). But if a patch looks sane and comes with actual examples that use it I can apply such patches. I usually don't apply patches which come without example - most of those tend to not work well anyway or do stuff no-one needs (otherwise people would have an example using it).

So figure out a good interface between yourself. You are the guys who need this and know what it's about - so you can discuss this. Then write some examples and give me a well tested patch and I'll apply it. Especially if it doesn't break the inteface or costs too much speed (several more virtual functions inside render are... probably fine... it's only called once per frame).

ok, I'm outa here again - and back to lunch before it's complete cold.

[hendu]

It was discussed in 2012, and we ended up with the solution I linked above...

[CuteAlien]

@hendu: I wasn't involved in the discussion back then as Hyrid still worked on Irrlicht in 2012 and this is his area. But from a quick view - your patch does not seem to have the same features. But hard to tell - as it comes without any examples. Maybe I find time in the evening to read through the whole thread. But likely not. Maybe on the weekend, but likely not. *sigh* - I really have no idea what to do without more core programmers in Irrlicht. This is impossible...

[mongoose7]

This topic is very complicated.

I can agree that the OnAnimate methods can be called first, as this is generally necessary and does not affect the other parts of the drawing. But I think Nadro was promising a change to make the draw function more flexible. Personally I'm thinking about shadow mapping.

Can we wait until the shader pipeline work is near completion before discussing changes? Or hear from Nadro?

[CuteAlien]

Yeah - no doubt that it should be possible to run render without OnAnimate and have a way to call that external. I can agree to that absolutely (I think I agree to it in another thread discussing that... which is also old).
edit: Did a quick check - that is currently on position 17 on my todo, but bugs and new things always go on top - so this is far down.

[CuteAlien]

OK, I took a 5-minute look at the other patch. I like the basic idea - no new functions, pre and post render are not needed because you can simply do stuff before and after calling drawAll. What I don't like on first view is: OnAnimate is not only still part of drawAll but would be called each time! Can't do that obviously, so that needs another parameter. And OnRegisterSceneNode will each pass add all nodes to their arrays - just to remove them again directly afterwards for those passes not needed. Couldn't this be avoided by passing on the new flags? With lot's of nodes that could otherwise already be a noticable speed cost. And last - patch for most central part of rendering loops without even the simplest example or test to check the new behavior - so someone (...?) would have to write it.

edit: One more thing. Seems pre-render still would make sense. As you can't access the arrays right now without drawing them. In case that kind of stuff could be needed - which I'm not sure about (I never did, but I also didn't need the other changes so far).