[osg-submissions] Collada plugin enhancements
Mattias Linde
linde at acc.umu.se
Fri Sep 28 08:23:08 PDT 2007
Hi Robert,
Nice, this almost gets the job done, one way communication into the plugin is possible.
I've done some additional small modification regarding constness in ReaderWriter and added
mutable on _pluginData so passing data back would be possible too.
Have updated the collada plugin (ReaderWriterDAE.cpp) to use the map to handle options and
have attached the changes.
The stuff in daeReader.h and daeWriter.h are just cosmetic changes to get rid of a warning.
CMakeLists.txt in src/osgPlugins/dae is changed to link with shared libs in !WIN32
instead of static libs (have proven to work better for me that way when several libs used by
the app link with the collada-libs).
/Mattias
On Fri, Sep 28, 2007 at 02:07:55PM +0100, Robert Osfield wrote:
> 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 <linde at acc.umu.se> 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 <bryan.thrall at flightsafety.com> 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 <linde at acc.umu.se> 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 <linde at acc.umu.se> 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
> > > > Bryan.Thrall at flightsafety.com
> > > > _______________________________________________
> > > > osg-submissions mailing list
> > > > osg-submissions at lists.openscenegraph.org
> > > > 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
> > > osg-submissions at lists.openscenegraph.org
> > > http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
> >
> > _______________________________________________
> > osg-submissions mailing list
> > osg-submissions at lists.openscenegraph.org
> > http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
> >
> _______________________________________________
> osg-submissions mailing list
> osg-submissions at lists.openscenegraph.org
> http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: collada-updatesV2.tar
Type: application/x-tar
Size: 40960 bytes
Desc: not available
Url : http://lists.openscenegraph.org/pipermail/osg-submissions-openscenegraph.org/attachments/20070928/cb1fa306/attachment-0001.tar
More information about the osg-submissions
mailing list