Robert Osfield wrote on Wednesday, September 26, 2007 11:10 AM:
> Hi Bryan,
>
> The threading issues are rather orthogonal to the general purpose API
> save perhaps for adding a mutex to getting/setting internal data in a
> single Options object, these will be a mute point if you simply pass
> in a new Options object for each call.
>
> The main thread problems are related to the plugins themselves and if
> they aren't thread safe then they need to use a serializer mutex to
> marshal access to the plugin. This side to the threading issues are
> very plugin specific so have to be treated on a case by case basis.
Right, I just wanted to make sure Mattias was aware of the potential
problem, and so it was described in the list archives in case it helps
future users of the feature :)
> On 9/26/07, Thrall, Bryan <[EMAIL PROTECTED]> wrote:
>> Robert Osfield wrote on Wednesday, September 26, 2007 9:19 AM:
>>> Hi Mattias,
>>>
>>> I don't really have any strong suggestions for the name.
>>> ExternalData/UserData/ExtraData/PluginData, I don't really know.
>>
>> I really like this idea; in my own work, paged data needs context
>> information about the already loaded scenegraph, and this would help
>> out a lot (I'm currently working around the Options limit by
>> subclassing and dynamic_casting).
>>
>> One thing I'm worried about is thread safety, though. If you modify
>> the ExternalData/whatever, then you have to worry about the (possibly
>> multiple) DatabasePager threads accessing it, and potentially the
>> (possibly multiple) render threads too if the data is used there. Not
>> that this is an argument against that design, but it's something to
>> think about (I don't even know how the Collada plugin works, so this
>> problem might not even crop up at all here).
>>
>>> On 9/26/07, Mattias Linde <[EMAIL PROTECTED]> wrote:
>>>> Hi Robert,
>>>>
>>>> When reading your reply I can agree that putting something like
>>>> ColladaOptions into osgDB might not be the best thing to do. My
>>>> thoughts when writing the code was to change as little as possible
>>>> and still be able to get access to plugin internal stuff and to
>>>> have the option to pass in another pointer to be used.
>>>>
>>>> I'd think adding a std::map to ReaderWriter::Options and some
>>>> methods to operate on it (set, get, remove - anything else
>>>> needed?) would be better than just a userdata object. But I'm not
>>>> sure what would be an appropriate name for it. I don't think
>>>> ExternalData really describes it since it can be used to pass back
>>>> data from the plugin.
>>>>
>>>> / Mattias
>>>>
>>>> On Wed, Sep 26, 2007 at 11:51:29AM +0100, Robert Osfield wrote:
>>>>> Hi Mattias,
>>>>>
>>>>> Thanks for the changes. I have done a quick review and am not
>>>>> happy with placing a plugin specific data structure like
>>>>> ColladaOptions into a general purpose library like osgDB. Once
>>>>> one starts going down the road in placing plugin specific data
>>>>> structures in osgDB you'll end up on a slippery slope of messy
>>>>> and difficult to maintain library.
>>>>>
>>>>> I don't have a solution to this problem yet. Perhaps a general
>>>>> purpose UserData object for passing in/out void pointers would be
>>>>> sufficient. Or perhaps one could provide a
>>>>> std::map<std::string,void*> in ReaderWriter::Options base class
>>>>> for passing in and out different types of data, i.e.
>>>>>
>>>>> options->setExternalData("DAE",dataPtr);
>>>>>
>>>>> DAE* daePtr = (DAE*)options->getExternalData("DAE");
>>>>>
>>>>>
>>>>> The aim must be to keep things with a very loose coupling in
>>>>> osgDB, and to make any extensions general purpose.
>>>>>
>>>>> Robert.
>>>>>
>>>>>
>>>>> On 9/26/07, Mattias Linde <[EMAIL PROTECTED]> wrote:
>>>>>> Hi,
>>>>>>
>>>>>> I've attached some suggested changes to the collada plugin:
>>>>>>
>>>>>> I've added a ColladaOptions class which can specify which DAE
>>>>>> instance should be used. If no options are used when
>>>>>> osgDB::readNodeFile is called the plugin will work just as
>>>>>> before. (include/osgDB/ColladaOptions and
>>>>>> src/osgDB/ColladaOptions.cpp in the tarball)
>>>>>>
>>>>>> With the colladaoptions one can use an "external" (as in outside
>>>>>> of osg) dae-instance which possibly already holds parsed data or
>>>>>> an internal instance (the plugin creates one unless it has
>>>>>> already done so). The options object also makes it possible to
>>>>>> get a pointer to the dae instance so it can be used outside of
>>>>>> osg.
>>>>>>
>>>>>> src/osgPlugins/dae/ReaderWriterDAE.cpp have been updated to use
>>>>>> ColladaOptions. daeReader.h and daeWriter.h have one minor change
>>>>>> each, a ; was removed after the namespace end } which caused a
>>>>>> warning.
>>>>>>
>>>>>> CMakeLists.txt have also been updated to link with
>>>>>> collada_dom_shared and collada_dae_shared (if not under win32)
>>>>>> instead of collada_dom and collada_dae. This is because of
>>>>>> collada-dom now supports building shared libs under linux (been
>>>>>> tested on OS X aswell). Linking with the static libs in the
>>>>>> plugin can (and have) cause problems if the application using
>>>>>> osg links with collada-dom libraries.
>>>>>>
>>>>>> / Mattias
--
Bryan Thrall
FlightSafety International
[EMAIL PROTECTED]
_______________________________________________
osg-submissions mailing list
[email protected]
http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
