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
