Hi Mattias,
I've checked in your changes and added a const getPluginData() as well
as a typedef for the map.
Thanks,
Robert.
On 9/28/07, Mattias Linde <[EMAIL PROTECTED]> wrote:
> Hi again,
>
> I just tested the suggested additions in a small standalone app and a thought
> struck me during my lunch break.. for the change to work in a plugin (which
> gets a const options pointer) the 3 suggested methods needs to be const and
> the _pluginData muteable.
>
> Sorry for sloppy testing.
>
> / Mattias
>
> On Fri, Sep 28, 2007 at 11:53:22AM +0200, Mattias Linde wrote:
> > Hi,
> >
> > Yes, I'm aware of possible threading issues but it's better to mention
> > such things once to often than never at all :)
> >
> > I've attached a modified ReaderWriter header which has some additions
> > to osgDB::ReaderWriter::Options to handle PluginData.
> >
> > I hope something like this could be useful for many plugins. I haven't
> > updated the collada plugin yet - and 'll wait with that until I know what
> > happens with this suggested change.
> >
> > / Mattias
> >
> > On Wed, Sep 26, 2007 at 11:32:22AM -0500, Thrall, Bryan wrote:
> > > 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
>
> > /* -*-c++-*- OpenSceneGraph - Copyright (C) 1998-2006 Robert Osfield
> > *
> > * This library is open source and may be redistributed and/or modified
> > under
> > * the terms of the OpenSceneGraph Public License (OSGPL) version 0.0 or
> > * (at your option) any later version. The full license is in LICENSE file
> > * included with this distribution, and on the openscenegraph.org website.
> > *
> > * This library is distributed in the hope that it will be useful,
> > * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > * OpenSceneGraph Public License for more details.
> > */
> >
> > #ifndef OSGDB_READERWRITER
> > #define OSGDB_READERWRITER 1
> >
> > #include <osg/Referenced>
> > #include <osg/Image>
> > #include <osg/Shape>
> > #include <osg/Node>
> >
> > #include <osgDB/Export>
> >
> > #include <deque>
> > #include <iosfwd>
> >
> > namespace osgDB {
> >
> > class Archive;
> >
> > /** list of directories to search through which searching for files. */
> > typedef std::deque<std::string> FilePathList;
> >
> > /** pure virtual base class for reading and writing of non native formats.
> > */
> > class OSGDB_EXPORT ReaderWriter : public osg::Object
> > {
> > public:
> >
> >
> > ReaderWriter():
> > osg::Object(true) {}
> >
> > ReaderWriter(const ReaderWriter& rw,const osg::CopyOp&
> > copyop=osg::CopyOp::SHALLOW_COPY):
> > osg::Object(rw,copyop) {}
> >
> > virtual ~ReaderWriter();
> >
> > META_Object(osgDB,ReaderWriter);
> >
> > virtual bool acceptsExtension(const std::string& /*extension*/)
> > const { return false; }
> >
> > /** Options base class used for passing options into plugins to
> > control their operation.*/
> > class Options : public osg::Object
> > {
> > public:
> >
> >
> > /// bit mask for setting up which object types get cached
> > by readObject/Image/HeightField/Node(filename) calls
> > enum CacheHintOptions
> > { /// do not cache objects of any type
> > CACHE_NONE = 0,
> >
> > /// cache nodes loaded via readNode(filename)
> > CACHE_NODES = 1,
> >
> > /// cache images loaded via readImage(filename)
> > CACHE_IMAGES = 2,
> >
> > /// cache heightfield loaded via
> > readHeightField(filename)
> > CACHE_HEIGHTFIELDS = 4,
> >
> > /// cache heightfield loaded via
> > readHeightField(filename)
> > CACHE_ARCHIVES = 8,
> >
> > /// cache objects loaded via readObject(filename)
> > CACHE_OBJECTS = 16,
> >
> > /// cache on all read*(filename) calls
> > CACHE_ALL = CACHE_NODES |
> > CACHE_IMAGES |
> > CACHE_HEIGHTFIELDS |
> > CACHE_ARCHIVES |
> > CACHE_OBJECTS
> > };
> >
> >
> > Options():
> > osg::Object(true),
> > _objectCacheHint(CACHE_ARCHIVES) {}
> > Options(const std::string& str):
> > osg::Object(true),
> > _str(str),
> > _objectCacheHint(CACHE_ARCHIVES) {}
> >
> > Options(const Options& options,const osg::CopyOp&
> > copyop=osg::CopyOp::SHALLOW_COPY):
> > osg::Object(options,copyop),
> > _str(options._str),
> > _databasePaths(options._databasePaths),
> > _objectCacheHint(options._objectCacheHint) {}
> >
> > META_Object(osgDB,Options);
> >
> > /** Set the general Options string.*/
> > void setOptionString(const std::string& str) { _str = str; }
> >
> > /** Get the general Options string.*/
> > const std::string& getOptionString() const { return _str; }
> >
> > /** Set the database path to use a hint of where to look
> > when loading models.*/
> > void setDatabasePath(const std::string& str) {
> > _databasePaths.clear(); _databasePaths.push_back(str); }
> >
> > /** Get the database path which is used a hint of where to
> > look when loading models.*/
> > FilePathList& getDatabasePathList() { return
> > _databasePaths; }
> >
> > /** Get the const database path which is used a hint of
> > where to look when loading models.*/
> > const FilePathList& getDatabasePathList() const { return
> > _databasePaths; }
> >
> > /** Set whether the Registry::ObjectCache should be used by
> > default.*/
> > void setObjectCacheHint(CacheHintOptions useObjectCache) {
> > _objectCacheHint = useObjectCache; }
> >
> > /** Get whether the Registry::ObjectCache should be used by
> > default.*/
> > CacheHintOptions getObjectCacheHint() const { return
> > _objectCacheHint; }
> >
> > /** Sets a plugindata value associated with a string */
> > void setPluginData(const std::string& s, void* v) {
> > _pluginData[s] = v; }
> >
> > /** Get a value from the plugindate */
> > void* getPluginData(const std::string& s) { return
> > _pluginData[s]; }
> >
> > /** Remove a value from the plugindata */
> > void removePluginData(const std::string& s) {
> > _pluginData.erase(s); }
> > protected:
> >
> > virtual ~Options() {}
> >
> > std::string _str;
> > FilePathList _databasePaths;
> > CacheHintOptions _objectCacheHint;
> >
> > std::map<std::string,void*> _pluginData;
> > };
> >
> >
> > class OSGDB_EXPORT ReadResult
> > {
> > public:
> >
> > enum ReadStatus
> > {
> > FILE_NOT_HANDLED,
> > FILE_NOT_FOUND,
> > FILE_LOADED,
> > FILE_LOADED_FROM_CACHE,
> > ERROR_IN_READING_FILE
> > };
> >
> > ReadResult(ReadStatus
> > status=FILE_NOT_HANDLED):_status(status) {}
> > ReadResult(const std::string&
> > m):_status(ERROR_IN_READING_FILE),_message(m) {}
> > ReadResult(osg::Object* obj, ReadStatus
> > status=FILE_LOADED):_status(status),_object(obj) {}
> >
> > ReadResult(const ReadResult&
> > rr):_status(rr._status),_message(rr._message),_object(rr._object) {}
> > ReadResult& operator = (const ReadResult& rr) { if
> > (this==&rr) return *this; _status=rr._status;
> > _message=rr._message;_object=rr._object; return *this; }
> >
> > osg::Object* getObject();
> > osg::Image* getImage();
> > osg::HeightField* getHeightField();
> > osg::Node* getNode();
> > osgDB::Archive* getArchive();
> >
> > bool validObject() { return _object.valid(); }
> > bool validImage() { return getImage()!=0; }
> > bool validHeightField() { return getHeightField()!=0; }
> > bool validNode() { return getNode()!=0; }
> > bool validArchive() { return getArchive()!=0; }
> >
> > osg::Object* takeObject();
> > osg::Image* takeImage();
> > osg::HeightField* takeHeightField();
> > osg::Node* takeNode();
> > osgDB::Archive* takeArchive();
> >
> > std::string& message() { return _message; }
> > const std::string& message() const { return _message; }
> >
> > ReadStatus status() const { return _status; }
> > bool success() const { return _status==FILE_LOADED ||
> > _status==FILE_LOADED_FROM_CACHE ; }
> > bool loadedFromCache() const { return
> > _status==FILE_LOADED_FROM_CACHE; }
> > bool error() const { return _status==ERROR_IN_READING_FILE;
> > }
> > bool notHandled() const { return _status==FILE_NOT_HANDLED;
> > }
> > bool notFound() const { return _status==FILE_NOT_FOUND; }
> >
> > protected:
> >
> > ReadStatus _status;
> > std::string _message;
> > osg::ref_ptr<osg::Object> _object;
> > };
> >
> > class WriteResult
> > {
> > public:
> >
> > enum WriteStatus
> > {
> > FILE_NOT_HANDLED,
> > FILE_SAVED,
> > ERROR_IN_WRITING_FILE
> > };
> >
> > WriteResult(WriteStatus
> > status=FILE_NOT_HANDLED):_status(status) {}
> > WriteResult(const std::string&
> > m):_status(ERROR_IN_WRITING_FILE),_message(m) {}
> >
> > WriteResult(const WriteResult&
> > rr):_status(rr._status),_message(rr._message) {}
> > WriteResult& operator = (const WriteResult& rr) { if
> > (this==&rr) return *this; _status=rr._status; _message=rr._message; return
> > *this; }
> >
> > std::string& message() { return _message; }
> > const std::string& message() const { return _message; }
> >
> > WriteStatus status() const { return _status; }
> > bool success() const { return _status==FILE_SAVED; }
> > bool error() const { return _status==ERROR_IN_WRITING_FILE;
> > }
> > bool notHandled() const { return _status==FILE_NOT_HANDLED;
> > }
> >
> > protected:
> >
> > WriteStatus _status;
> > std::string _message;
> > };
> >
> > enum ArchiveStatus
> > {
> > READ,
> > WRITE,
> > CREATE
> > };
> >
> > /** open an archive for reading, writing, or to create an empty
> > archive for writing to.*/
> > virtual ReadResult openArchive(const std::string&
> > /*fileName*/,ArchiveStatus, unsigned int =4096, const Options* =NULL) const
> > { return ReadResult(ReadResult::FILE_NOT_HANDLED); }
> >
> > /** open an archive for reading.*/
> > virtual ReadResult openArchive(std::istream& /*fin*/,const Options*
> > =NULL) const { return ReadResult(ReadResult::FILE_NOT_HANDLED); }
> >
> > virtual ReadResult readObject(const std::string& /*fileName*/,const
> > Options* =NULL) const { return ReadResult(ReadResult::FILE_NOT_HANDLED); }
> > virtual ReadResult readImage(const std::string& /*fileName*/,const
> > Options* =NULL) const { return ReadResult(ReadResult::FILE_NOT_HANDLED); }
> > virtual ReadResult readHeightField(const std::string&
> > /*fileName*/,const Options* =NULL) const { return
> > ReadResult(ReadResult::FILE_NOT_HANDLED); }
> > virtual ReadResult readNode(const std::string& /*fileName*/,const
> > Options* =NULL) const { return ReadResult(ReadResult::FILE_NOT_HANDLED); }
> >
> > virtual WriteResult writeObject(const osg::Object& /*obj*/,const
> > std::string& /*fileName*/,const Options* =NULL) const {return
> > WriteResult(WriteResult::FILE_NOT_HANDLED); }
> > virtual WriteResult writeImage(const osg::Image& /*image*/,const
> > std::string& /*fileName*/,const Options* =NULL) const {return
> > WriteResult(WriteResult::FILE_NOT_HANDLED); }
> > virtual WriteResult writeHeightField(const osg::HeightField&
> > /*heightField*/,const std::string& /*fileName*/,const Options* =NULL) const
> > {return WriteResult(WriteResult::FILE_NOT_HANDLED); }
> > virtual WriteResult writeNode(const osg::Node& /*node*/,const
> > std::string& /*fileName*/,const Options* =NULL) const { return
> > WriteResult(WriteResult::FILE_NOT_HANDLED); }
> >
> > virtual ReadResult readObject(std::istream& /*fin*/,const Options*
> > =NULL) const { return ReadResult(ReadResult::FILE_NOT_HANDLED); }
> > virtual ReadResult readImage(std::istream& /*fin*/,const Options*
> > =NULL) const { return ReadResult(ReadResult::FILE_NOT_HANDLED); }
> > virtual ReadResult readHeightField(std::istream& /*fin*/,const
> > Options* =NULL) const { return ReadResult(ReadResult::FILE_NOT_HANDLED); }
> > virtual ReadResult readNode(std::istream& /*fin*/,const Options*
> > =NULL) const { return ReadResult(ReadResult::FILE_NOT_HANDLED); }
> >
> > virtual WriteResult writeObject(const osg::Object&
> > /*obj*/,std::ostream& /*fout*/,const Options* =NULL) const { return
> > WriteResult(WriteResult::FILE_NOT_HANDLED); }
> > virtual WriteResult writeImage(const osg::Image&
> > /*image*/,std::ostream& /*fout*/,const Options* =NULL) const { return
> > WriteResult(WriteResult::FILE_NOT_HANDLED); }
> > virtual WriteResult writeHeightField(const osg::HeightField&
> > /*heightField*/,std::ostream& /*fout*/,const Options* =NULL) const { return
> > WriteResult(WriteResult::FILE_NOT_HANDLED); }
> > virtual WriteResult writeNode(const osg::Node&
> > /*node*/,std::ostream& /*fout*/,const Options* =NULL) const { return
> > WriteResult(WriteResult::FILE_NOT_HANDLED); }
> >
> > };
> >
> > }
> >
> > #endif // OSGDB_READERWRITER
>
> > _______________________________________________
> > osg-submissions mailing list
> > [email protected]
> > http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
>
> _______________________________________________
> osg-submissions mailing list
> [email protected]
> http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
>
_______________________________________________
osg-submissions mailing list
[email protected]
http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
