HI John, As the Camera has Initial/Pre/Post and FinalDrawCallbacks having add/remove/getNum/get methods rather than just set/get will mean a lot of extra new methods expanding an already large API.
To avoid a big expansion in the API it would probably be best to push the list of callbacks onto DrawCallback, possibly as a linked list. Or have a CallbackList object that we get for each for the Intial/Pre/Post/Final draw callbacks. For backwards compatibility we'd need to main the old set/get methods, but for code that uses like osgShadow we can easily update them to use the list method. Robert. On 9 April 2014 22:22, PCJohn <[email protected]> wrote: > Hi Robert, > >> Providing similar support for Camera::DrawCallbacks would be possible, >> there isn't the same imperative for controlling traversal, but it'd >> keep the public Camera API simple - it's complicated enough as it is >> so am reluctant to add further API to it. > > Sorry, I am not native English. By the last sentence do you mean that the > multiple Camera::DrawCallback support must be as simple as possible or that > you would prefer to not put it into the API? > > I am not pushing the things, but it seems that shadow classes are already not > compatible with OcclusionQueryNode as they both register pre and post draw > callbacks. And a programmer might want to make his own callbacks for profiling > reasons, start various OpenGL queries, resize his own custom RenderBuffers > used > for HDR rendering whenever screen window resizes, etc. > > My idea was: > > struct OSG_EXPORT DrawCallback : virtual public Object > { > DrawCallback() {} > DrawCallback(const DrawCallback&,const CopyOp&) {} > META_Object(osg, DrawCallback); > virtual void operator () (osg::RenderInfo& renderInfo) const; > virtual void operator () (const osg::Camera& /*camera*/) const {} > > ref_ptr<DrawCallback> _nestedCallback; > }; > > inline void addPreDrawCallback(DrawCallback* dc) > { > if (dc != NULL) { > dc->_nestedCallback = _preDrawCallback; > _preDrawCallback = dc; > } > } > > inline void removePreDrawCallback(DrawCallback* dc) > { > if (dc != NULL) { > if (dc == _preDrawCallback) > _preDrawCallback = _preDrawCallback->_nestedCallback; > else > { > DrawCallback *c = _preDrawCallback; > while (c) { > if (c->_nestedCallback==dc) { > c->_nestedCallback = c->_nestedCallback- >>_nestedCallback; > dc->_nestedCallback = NULL; > return; > } > } > } > } > } > > Not sure if it is enough thread-safe. If you think that Initial-,Pre-,Post-, > and Final-DrawCallback functionality has too many methods, > we can make like see bellow, while possibly deprecating some of the current > methods. > > enum DrawCallbackEnum { INITIALIZE_DRAW_CALLBACK = 0, > PRE_DRAW_CALLBACK = 1, > POST_DRAW_CALLBACK = 2, > FINALIZE_DRAW_CALLBACK = 3 }; > > inline void addPreDrawCallback(DrawCallbackEnum which, DrawCallback* dc) > { > if( which < 4 ) { > ref_ptr<DrawCallback> &c = _drawCallbacks[which]; > .... > } > inline void removePreDrawCallback(DrawCallbackEnum which, DrawCallback* > dc) > { .... } > > ref_ptr<DrawCallback> _drawCallbacks[4]; // this replaces four variables > > > John > > > > On Wednesday 09 of April 2014 16:16:31 Robert Osfield wrote: >> Hi John, >> >> Original Node callbacks didn't support multiple callbacks, but I >> worked out that I could add the functionality relatively >> non-intrusively by nesting callbacks. The key advantage with nesting >> is that it enables a callback retain data on the stack within the >> local scope of the callback - which makes things easy to implement and >> gives you thread safety for free, and enables the callback to take >> complete controlling traversal. >> >> Providing similar support for Camera::DrawCallbacks would be possible, >> there isn't the same imperative for controlling traversal, but it'd >> keep the public Camera API simple - it's complicated enough as it is >> so am reluctant to add further API to it. The alternative would be to >> add a list of callbacks, but as you said this would break >> compatibility. >> >> Robert. >> >> On 9 April 2014 14:10, PCJohn <[email protected]> wrote: >> > Hi, >> > >> > I very like the capability of osg::Node to register any number of update, >> > event and cull callbacks. This allows for a nice modular approach with >> > each >> > module registering callbacks for anything they wish without disturbing >> > others. >> > >> > I came to a problem that Camera can register just one Initial-,Pre-,Post-, >> > and Final-DrawCallback. Surely, I can register my own "callback >> > container" that would call all my registered callback. But that is not >> > compatible with OSG- registered Callbacks, for instance >> > OcclusionQueryNode would overwrite my callback container. Could we >> > possibly make the similar approach for the camera as for osg::Node? I can >> > volunteer to make the first iteration for osg- submissions, if the >> > community and Robert thinks that it is a good idea. >> > >> > >> > My idea for implementation would be to append >> > ref_ptr<DrawCallback> _nestedCallback into the Camera::DrawCallback >> > structure, append Camera::add*Callback() and Camera::remove*Callback() >> > and make Camera::callCallbacks(DrawCallback *cb) with a parameter giving >> > pointer to the first DrawCallback of Initial-,Pre-,Post-, and Final- >> > DrawCallback list. >> > An alternative would be to let the user call nested callbacks from >> > DrawCallback::operator(), but as we would always want (probably) to call >> > all the callbacks and never stop in the middle of the work, I personally >> > like the first approach more and it is more compatibility-friendly, as >> > users will not need to modify their existing callbacks to get the new >> > multi-callback capability. But I am ok with any approach. >> > >> > >> > Do you think it is a good idea? >> > John >> > >> > _______________________________________________ >> > osg-users mailing list >> > [email protected] >> > http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org >> >> _______________________________________________ >> osg-users mailing list >> [email protected] >> http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org > _______________________________________________ > osg-users mailing list > [email protected] > http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org _______________________________________________ osg-users mailing list [email protected] http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
