gdtiti / alembic

Automatically exported from code.google.com/p/alembic
0 stars 0 forks source link

OSchemaObjects don't support the wrap-existing constructor #150

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
This makes me a sad panda.

Original issue reported on code.google.com by ard...@gmail.com on 28 Apr 2011 at 2:19

GoogleCodeExporter commented 8 years ago
This is going to be slightly less trivial than I thought.  I think I've made 
the required change to Abc::OSchemaObject 
(http://code.google.com/r/ardent-embic/source/detail?r=311848ccff39787dd276179c9
031a618366008fa), but all the types in AbcGeom will need WrapExisting 
constructors that call a "reinit()" method that checks each of their child 
Properties for existence, and if they exist, calls those Properties' 
wrap-existing constructors.

Not impossible, just not super straightforward.

Original comment by ard...@gmail.com on 5 May 2011 at 12:00

GoogleCodeExporter commented 8 years ago

Original comment by ble...@gmail.com on 8 Jul 2011 at 1:44

GoogleCodeExporter commented 8 years ago
Per today's call, noted this as a P2 (so a nice to have) for Milestone 1 (aka 
1.0 release).

Original comment by scottmmo...@gmail.com on 11 Jul 2011 at 11:04

GoogleCodeExporter commented 8 years ago
After speaking with Shandra and Joe today, the current thought is to pass on 
this for Milestone 1. I'll verify with Sony here momentarily.

Original comment by scottmmo...@gmail.com on 14 Jul 2011 at 11:45

GoogleCodeExporter commented 8 years ago
If this issue isn't going to be resolved, I need a suggestion for an 
alternative workflow. Specifically, what I'm trying to do is:

- take a model in a custom format
- generate an Alembic representation
- take an animated vert cache
- apply the animation to the Alembic representation

But I don't want to have to worry about what type of geometry it is (in the 
last step). I am confident the schema will expose a P attribute, regardless 
whether it's a Poly, Subd, Nurbs, etc.

If I look at the Maya exporter, it looks like there are specialized cases for 
each type of schema object.

Does that make sense?

Original comment by robpi...@gmail.com on 26 Jul 2011 at 6:05

GoogleCodeExporter commented 8 years ago
Or, worst-case, if I do need to worry about the schema type, I still want to 
store the object in a generic way. For example:

genericAlembicClass obj = convertToAlembic( myFileInAFunkyFormat );
for( i = 1, 1000 )
  addAnimatedPositions( obj, myAnimatedPositions[ i ] )
end

I'm happy to worry about the different types in convertToAlembic(), and I'll 
grudgingly accept (if I absolutely have to) worrying about different types in 
addAnimatedPositions(), but I really do want obj to be a generic type, 
independent of which geo type I'm converting.

I see how the Maya exporter handles this with a generic MayaNode class, with 
inherited versions that each store specific Alembic pointers, but I'd really 
like to design my library such that it returns Alembic data types, not custom 
wrapper classes.

Cheers!

Original comment by robpi...@gmail.com on 26 Jul 2011 at 6:15

GoogleCodeExporter commented 8 years ago
You want to hold onto OPolyMeshSchema, OSubDSchema, etc.

Although it might be able technically feasible to hold onto the OArrayProperty 
"P" for each shape, you won't generate correct self bounds because you aren't 
doing your write via the schema.

Do it the harder way, other parts of your pipeline will thank you.

Original comment by miller.lucas on 26 Jul 2011 at 6:16

GoogleCodeExporter commented 8 years ago
I think in your example genericAlembicClass could be a 
boost::variant<OSubDSchema, OPolyMeshSchema, whatever other types you care 
about>

Original comment by miller.lucas on 26 Jul 2011 at 6:33

GoogleCodeExporter commented 8 years ago
I'd really like to hold onto it as an OGenericSchema (or something similar), 
not an OPolySchema. 

The object seems to advertise that it's a Poly/Subd via its properties, so I 
was really hoping I could "recast"/"rewrap" it as such.

I'll have a look at boost::variant.

I'm not trying to be picky. I'm just confused about the design of this aspect 
of Alembic. :)

Cheers!

Original comment by robpi...@gmail.com on 26 Jul 2011 at 6:50

GoogleCodeExporter commented 8 years ago
As part of the confusion, the following seems to fit the exposed API but 
crashes at runtime.

...
Alembic::Abc::OObject archiveTop = archive.getTop();
MPCGeo mpcGeo = mpcGeoLoad( "myGeo.geo" );
Alembic::Abc::OObject obj = OObjectFromMPCGeo()( mpcGeo, archiveTop, "geo1" );
Alembic::AbcGeom::OSubD subd( obj.getPtr(), Alembic::Abc::kWrapExisting );  // 
CRASH!

Original comment by robpi...@gmail.com on 28 Jul 2011 at 2:36

GoogleCodeExporter commented 8 years ago

Original comment by ble...@gmail.com on 24 Aug 2011 at 11:53

GoogleCodeExporter commented 8 years ago

Original comment by ble...@gmail.com on 26 Aug 2011 at 11:56

GoogleCodeExporter commented 8 years ago
Adding to Milestone 1.1

Original comment by scottmmo...@gmail.com on 6 Sep 2011 at 11:10

GoogleCodeExporter commented 8 years ago

Original comment by ble...@gmail.com on 19 Sep 2011 at 9:53

GoogleCodeExporter commented 8 years ago
The problem with kWrapExisting of OSchemaObject is that if the schema has gone 
out of scope all of it's properties which it would be holding on to will have 
gone out of scope.

As far as I can tell kWrapExisting is only useful when you are holding onto the 
OSchema elsewhere.

Original comment by miller.lucas on 25 Jun 2012 at 6:11

GoogleCodeExporter commented 8 years ago
Okay, so then how do I hold onto the schema in a generic way elsewhere? :)

Again, my original code looked something like:

Alembic::Abc::OObject OPolyMeshFromPolyGeo(PolyGeo &geo, Alembic::Abc::OObject 
parent, const std::string &name)
{
  Alembic::AbcGeom::OPolyMesh meshObj(parent, name);
  Alembic::AbcGeom::OPolyMeshSchema &mesh = meshObj.getSchema();
  ...
  return meshObj;
}

With similar versions for other types of geometry.

My current work-around (which just feels like an unnecessary layer) is:

class OGeomWrapper
{
public:
  OGeomWrapper() {}
  virtual ~OGeomWrapper() {}
};
class OPolyMeshWrapper : public OGeomWrapper
{
public:
  OPolyMeshWrapper(Alembic::AbcGeom::OPolyMesh *ptr) : m_ptr(ptr) {}
  virtual ~OPolyMeshWrapper() { delete m_ptr; }
  Alembic::AbcGeom::OPolyMesh *m_ptr;
};
...

typedef boost::shared_ptr< OGeomWrapper > OGeomWrapperPtr;
typedef boost::shared_ptr< OPolyMeshWrapper > OPolyMeshWrapperPtr;
...

OGeomWrapperPtr Alembic::Abc::OObject OPolyMeshFromPolyGeo(PolyGeo &geo, 
Alembic::Abc::OObject parent, const std::string &name)
{
  OGeomWrapperPtr geomPtr = OPolyMeshWrapperPtr(new OPolyMeshWrapper(new Alembic::AbcGeom::OPolyMesh(parent, name)));
  Alembic::AbcGeom::OPolyMesh *meshObj = boost::dynamic_pointer_cast<OPolyMeshWrapper>(geomPtr)->m_ptr;
  Alembic::AbcGeom::OPolyMeshSchema &mesh = meshObj->getSchema();
  ...
  return geomPtr;
}

Original comment by robpi...@gmail.com on 27 Jun 2012 at 4:57

GoogleCodeExporter commented 8 years ago
boost::variant is one way.

Another way to support this might be having a protected register on OObject (or 
OSchemaObject) that would hold onto certain properties defined by the schema 
while the object is in scope.

I'll dig a little deeper and see if there are any bad side effects to this 
approach.

Original comment by miller.lucas on 27 Jun 2012 at 5:31

GoogleCodeExporter commented 8 years ago
Ah yes, I remember the boost::variant suggestion from the mailing list. It's 
perhaps a thinner/simpler approach than the wrapper class I've written, but 
it's still a wrapper.

I shall cross my fingers and think happy thoughts about "extending" OObject :)

Original comment by robpi...@gmail.com on 27 Jun 2012 at 8:17

GoogleCodeExporter commented 8 years ago
Unfortunately the "extension" isn't going to work (some of the objects have 
state that goes beyond their properties).

I can however make OObject polymorphic so that everything that inherits from it 
is polymorphic.

I'll also create typedefs (if they aren't already there) for smart pointers 
around all the current O*Object types.

Is this reasonable enough (it allows you to toss out your wrapper stuff)?

OPolyMeshPtr mesh = Alembic::Util::dynamic_pointer_cast<OPolyMesh>(geom);

Original comment by miller.lucas on 6 Jul 2012 at 8:47

GoogleCodeExporter commented 8 years ago
This sounds very promising. If the patch is straight-forward, I'd be
happy to try it on my end before it's committed to provide feedback.

Original comment by robpi...@gmail.com on 6 Jul 2012 at 9:16

GoogleCodeExporter commented 8 years ago
All you really need to do is to make ~OObject virtual, the full commit with all 
the typedefs is here:

http://code.google.com/r/millerlucas-dev/source/detail?r=e2c8f804bd492f260210be4
94416ad3dbe950bf9

Original comment by miller.lucas on 7 Jul 2012 at 12:28

GoogleCodeExporter commented 8 years ago
We've removed kWrapExisting for OSchema.

The fix in comment 21 was released in Alembic 1.1

Original comment by miller.lucas on 2 Aug 2012 at 11:52