nightstyles / alembic

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

SubD's integer properties latch to non-zero #119

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The problem: the SubD definition in AbcGeom has three integer properties: 
faceVaryingInterpolateBoundary, faceVaryingPropagateCorners, and 
interpolateBoundary.  They default to the value '0'.  If at any point any of 
them are set to non-zero, they cannot currently be set back to 0.

How this happens: When setting a sample on the SubD, you are free to specify 
only the properties you wish to change at that sample, and Alembic will 
automatically use the previously set value for anything left unspecified.  It 
does this using a templated inline function in Foundation.h called 
"setPropUsePrevIfNull", defined thus:

template <class PROP, class SAMP>
inline void SetPropUsePrevIfNull( PROP iProp, SAMP iSamp,
                                  const Abc::OSampleSelector &iSS )
{
    if ( iSamp ) { iProp.set( iSamp, iSS ); }
    else { iProp.setFromPrevious( iSS ); }
}

If the value passed in as the SAMP parameter is 0, it will evaluate as false, 
and setFromPrevious() will be called.  If at any previous time it had been 
called with non-zero, then that value will persist, even if you explicitly want 
to set it to 0.

The solution: the default values for these parameters in SubDSchema::Sample 
should be some invalid value, such as -1, and then that number is used as the 
"null" value for testing if it is explicitly set.  Then, overload that function 
in OSubD.cpp to explicitly take an OInt32Property and an int32_t.  The init 
method will set them to 0 if they are not specified on setting of the first 
sample, and subsequent set()s will test against -1 for null-ness.

There's already a specialization of that function in Foundation.h for 
OStringProperty, added to support SetPropUsePrevIfNull() for SubdivisionScheme 
(since "" does not evaluate as false).  I think that should probably stay where 
it is, to support StringProperties in other Schemas, such as [potentially] 
Camera.  Additionally, a specialization for WStrings should be added for the 
sake of completeness.

This is not a very serious bug, I feel, as it is unlikely that those properties 
will be time-varying.  Still, it's a bug, and it's an easy fix.

Original issue reported on code.google.com by ard...@gmail.com on 17 Nov 2010 at 12:22

GoogleCodeExporter commented 9 years ago
updating metadata

Original comment by ard...@gmail.com on 17 Nov 2010 at 12:22