Hi,
On Fri, 2011-01-07 at 17:19 -0600, Carsten Neumann wrote:
> Hello all,
>
> 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 ? 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.
> 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). 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.
> 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 ?
> - 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 ?
> - Do we want to keep the progress callbacks?
hmm, are they actually working with useful numbers for all loaders ?
> - If we keep any of the above, can I convert them to use boost::function?
that should never be a problem, it's just something we did not come
around doing.
> - I would like to have some support for caching in the loader
> infrastructure [2]. My current thinking is to define an abstract
> ContainerCache:
>
> class ContainerCache
> {
> typedef std::vector<FieldContainerUnrecPtr> ContainerVecType;
>
> virtual Int32 insert(const ContainerVecType &containers,
> const std::string &fileName,
> const std::string &name = "",
> const FieldContainerType &type =
> FieldContainer::getClassType()) = 0;
>
> virtual Int32 lookup(const std::string &fileName,
> const std::string &name = "",
> const FieldContainerType &type =
> FieldContainer::getClassType()) const = 0;
>
> virtual ContainerVecType clone(Int32 handle) const = 0;
>
>
> // plus some bookkeeping stuff like clear(), empty()
> };
>
> and leave it up to applications to provide an implementation that meets
> their requirements. The name and type argument are meant to be more or
> less optional, just to give the cache implementation a better hint what
> is being asked for. SFH could then first query the cache if it has
> something suitable and then obtain a clone (shallow/deep up to the cache
> implementation/app) to use.
have to think about this a little ;), so far it seems ok.
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
