Hi, On Mon, 2011-01-10 at 10:52 -0600, Carsten Neumann wrote: > Hello Gerrit, > > On 01/10/2011 10:25 AM, Gerrit Voß wrote: > > On Fri, 2011-01-07 at 17:19 -0600, Carsten Neumann wrote: > >> I've a patch set (attached) that adds support for reading containers > >> (not just nodes) to the SceneFileHandler and all SceneFileTypes in trunk > >> (there was some talk about this a while back). > > > > + std::ifstream is(filename.c_str(), std::ios_base::in | > > + std::ios_base::binary); > > + > > + OSG::OSGSceneFileType::ContainerVecType fcVec = > > + OSG::OSGSceneFileType::the().readContainer( > > + SceneFileHandler::the(), is, filename, resolver); > > + > > + if(fcVec.empty() == false) > > + returnValue = fcVec.front(); > > > > ok, I'm not that happy with pushing this kind of stuff to the rest > > of the code (replicating it), e.g. why did the Char * signature have > > to go ? > > I assume you don't really care that much about the Char* part, but about > the SFT not taking a file name any more? Hm, I can add that back in and > have the SFT base class implement it in terms of the stream interface, ok? > > > Similar can't we keep the return one container variant in the > > SFH/SFT for convenience. For both reading and writing, it is a nuisance > > to deal with the vector wrapper in case one is only interested in one > > container. > > ok, I'll add a single container version. Again the SFT base class can > implement it in terms of the multi container version, that way it is > enough to implement the multi container read/write functions for a > loader and get the others for free.
in both cases it is more about the inconvenience of spreading these things all over the place. It seems easier to collect them at a central place. > >> I've taken the opportunity to also do some cleanup on the > >> SFHandler/SFType interface, the most notable change is that the handler > >> now gets passed down to the SFType. The motivation here is that > >> currently it is not possible to safely load more than one file in > >> parallel [1], because there is only one SFH and loaders access the > >> singleton whenever they need to pull in additional files (changing e.g. > >> the state of the PathHandler). > >> If we allow the SFH to be cloned, each clone can operate in parallel > >> with the others, as long as the loaders only operate on the instance > >> passed to them. Of course for convenience a global instance remains > >> accessible with SceneFileHandler::the(). > > > > hmm, in general I'm ok with the changes, but I'm not sure if it > > gets us all the way for all loaders, as quite some store stuff > > in the SFT (killing any par loading). > > Don't think so (after the patch that is). It also makes > read()/readContainer() const for SFT, so they either use function local > variables or instantiate some loader object to store stuff in. ok, I remembered wrongly the items store are global maps which are created during the constructor. > > And passing the SceneFileHandler > > around as a parameter gets kind of painful for complex loaders so one > > would be tempted to store it somewhere. Which, for complex loaders, is > > not that much of a problem as they instantiate something on the fly > > anyway. But for a more general solution for all loaders it seems the > > better way around it would be to get rid of the 'static' scene file > > types (only leave a small registration part) and let them be > > instantiated by the SFH, including passing the cloned SFH, during > > loading. > > ok, so the SFT would register something like a create() function with > the SFH, which gets passed a few parameters including the calling SFH > instance? Sounds good. > > >> Some things that I'd also like to add (so not in the patch yet): > >> > >> - For the parallel load to also work for images, I want to give the SFH > >> a pointer to an associated ImageFileHandler, that then gets used by the > >> loaders instead of going through the singleton. > > > > should be ok. > > > > A general question, as you currently pass the SFH down as 'this', > > later on this has to involve some form of locking so the cloning does > > not happen while something else modifies the global SFH from which you > > clone ? > > > > Another one, as IIRC it was mentioned/half discussed some time ago that > > the general search path handling is kind of scattered, would it be a > > good time (read do we have the time) to revised (or at least remember) > > it in this context ? > > not sure what you mean with scattered? It seems to be mostly contained > in SFH::initPathHandler(). Can you refresh my memory what the problem was? I guess I somehow remembered pieces of the following two discussions / pages: http://www.opensg.org/wiki/Dev/FileIO http://www.opensg.org/wiki/Dev/PluginsPathes > >> - Do we want to keep the read/write callback in SFH? What are they good > >> for anyway, it looks as if they hijack the loading process? > > > > A better way would maybe be to provide a generic callback SFT which > > can easily be registered for the corresponding suffixes instead of > > pushing this to the SFH ? > > well, the funny thing is that the callbacks get passed the SFT as an > argument... - Is anybody using this? What's is/was the purpose? IIRC that came from Andreas, not sure about the usage might be some custom compression/encryption hooks. > >> - Do we want to keep the progress callbacks? > > > > hmm, are they actually working with useful numbers for all loaders ? > > sort of, I guess. It does check the stream position and compares with > the end position to determine progress, so it does not necessarily need > much support from the loader implementations IIUC. ah, ok, have to check this. Anyway I would assume the outside to take care that the usage of the progress callbacks is thread safe. kind regards gerrit ------------------------------------------------------------------------------ Gaining the trust of online customers is vital for the success of any company that requires sensitive data to be transmitted over the Web. Learn how to best implement a security strategy that keeps consumers' information secure and instills the confidence they need to proceed with transactions. http://p.sf.net/sfu/oracle-sfdevnl _______________________________________________ Opensg-core mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/opensg-core
