USD PrimitiveAlgo : Don't load invalid primitive variables

This required some fixes to the existing tests, since they were accidentally using invalid variables. Also required a change to the PointsPrimitive reader to construct the primitive with the right point count, so that the primitive variable checks are using the right sizes.

Author: johnhaddon

Changes

@@ -3,8 +3,9 @@
 
 Fixes
 -----
-- ImageReader : Fixed compilation with versions of OIIO < 2.4
 
+- ImageReader : Fixed compilation with versions of OIIO < 2.4.
+- USDScene : Invalid primitive variables are now skipped during loading, with a warning being emitted instead.
 
 10.4.5.0 (relative to 10.4.4.0)
 ========

contrib/IECoreUSD/src/IECoreUSD/PointsAlgo.cpp

@@ -57,15 +57,9 @@ namespace
 
 IECore::ObjectPtr readPoints( pxr::UsdGeomPoints &points, pxr::UsdTimeCode time, const Canceller *canceller )
 {
-	IECoreScene::PointsPrimitivePtr newPoints = new IECoreScene::PointsPrimitive();
+	IECoreScene::PointsPrimitivePtr newPoints = new IECoreScene::PointsPrimitive( points.GetPointCount( time ) );
 	PrimitiveAlgo::readPrimitiveVariables( points, time, newPoints.get(), canceller );
 
-	Canceller::check( canceller );
-	if( auto *p = newPoints->variableData<V3fVectorData>( "P" ) )
-	{
-		newPoints->setNumPoints( p->readable().size() );
-	}
-
 	Canceller::check( canceller );
 	if( auto i = boost::static_pointer_cast<Int64VectorData>( DataAlgo::fromUSD( points.GetIdsAttr(), time ) ) )
 	{

contrib/IECoreUSD/src/IECoreUSD/PrimitiveAlgo.cpp

@@ -242,7 +242,14 @@ void readPrimitiveVariable( const pxr::UsdGeomPrimvar &primVar, pxr::UsdTimeCode
 		indices = DataAlgo::fromUSD( srcIndices );
 	}
 
-	primitive->variables[name] = IECoreScene::PrimitiveVariable( interpolation, data, indices );
+	const IECoreScene::PrimitiveVariable primitiveVariable( interpolation, data, indices );
+	if( !primitive->isPrimitiveVariableValid( primitiveVariable ) )
+	{
+		IECore::msg( IECore::MessageHandler::Level::Warning, "IECoreUSD::PrimitiveAlgo", boost::format( "Skipping invalid UsdGeomPrimvar \"%1%\"" ) % primVar.GetAttr().GetPath().GetAsString() );
+		return;
+	}
+
+	primitive->variables[name] = primitiveVariable;
 }
 
 pxr::UsdSkelCache *skelCache()

contrib/IECoreUSD/test/IECoreUSD/SceneCacheFileFormatTest.py

@@ -522,10 +522,12 @@ def testCustomPrimvars( self ):
 		box = m.createChild( "box" )
 		mesh = IECoreScene.MeshPrimitive.createBox( imath.Box3f( imath.V3f( 0 ), imath.V3f( 1 ) ) )
 		vertexSize = mesh.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Vertex )
+		uniformSize = mesh.variableSize( IECoreScene.PrimitiveVariable.Interpolation.Uniform )
 
-		v = IECore.V3fVectorData( [ imath.Color3f( 12, 12, 12 ) ], IECore.GeometricData.Interpretation.Point )
+		v = IECore.V3fVectorData( [ imath.Color3f( 12, 12, 12 ) ] * vertexSize, IECore.GeometricData.Interpretation.Point )
 		mesh["customPoint"] = IECoreScene.PrimitiveVariable( IECoreScene.PrimitiveVariable.Interpolation.Vertex, v )
-		mesh["customIndexedInt"] = IECoreScene.PrimitiveVariable( IECoreScene.PrimitiveVariable.Interpolation.Uniform, IECore.IntVectorData( [12] ), IECore.IntVectorData( [ 0 ] * vertexSize ) )
+		mesh["customIndexedInt"] = IECoreScene.PrimitiveVariable( IECoreScene.PrimitiveVariable.Interpolation.Uniform, IECore.IntVectorData( [12] ), IECore.IntVectorData( [ 0 ] * uniformSize ) )
+		self.assertTrue( mesh.arePrimitiveVariablesValid() )
 
 		# Test a shallow copy of a primvar which causes IECore.Object to write a reference into the IndexedIO file
 		mesh["Pref"] = mesh["P"]
@@ -549,12 +551,12 @@ def testCustomPrimvars( self ):
 		self.assertEqual( customPoint.GetMetadata( "interpolation" ), "vertex" )
 
 		# value
-		self.assertEqual( customPoint.Get( 24.0 ), pxr.Vt.Vec3fArray( 1, [ pxr.Gf.Vec3f( 12 ) ] ) )
+		self.assertEqual( customPoint.Get( 24.0 ), pxr.Vt.Vec3fArray( vertexSize, pxr.Gf.Vec3f( 12 ) ) )
 
 		# indices
 		indices = prim.GetAttribute( "primvars:customIndexedInt:indices" )
 		self.assertTrue( indices )
-		self.assertEqual( indices.Get( 24.0 ), pxr.Vt.IntArray( vertexSize, [ 0 ] * vertexSize ) )
+		self.assertEqual( indices.Get( 24.0 ), pxr.Vt.IntArray( [ 0 ] * uniformSize ) )
 
 		# uniform interpolation
 		customIndexedInt = prim.GetAttribute( "primvars:customIndexedInt" )
@@ -587,10 +589,10 @@ def testCustomPrimvars( self ):
 		self.assertEqual( customPoint.interpolation, IECoreScene.PrimitiveVariable.Interpolation.Vertex )
 
 		# value
-		self.assertEqual( customPoint.data, IECore.V3fVectorData( [ imath.V3f( 12.0 ) ], IECore.GeometricData.Interpretation.Point ) )
+		self.assertEqual( customPoint.data, IECore.V3fVectorData( [ imath.V3f( 12.0 ) ] * vertexSize, IECore.GeometricData.Interpretation.Point ) )
 
 		# indices
-		self.assertEqual( mesh["customIndexedInt"].indices, IECore.IntVectorData( [0] * vertexSize ) )
+		self.assertEqual( mesh["customIndexedInt"].indices, IECore.IntVectorData( [ 0 ] * uniformSize ) )
 
 		# copy
 		self.assertEqual( mesh["Pref"], mesh["P"] )

contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py

@@ -803,6 +803,7 @@ def testCanWriteVariousPrimVarTypes ( self ):
 		pointsPrimitive["c3f"] = IECoreScene.PrimitiveVariable( IECoreScene.PrimitiveVariable.Interpolation.Vertex, c3f)
 		pointsPrimitive["token"] = IECoreScene.PrimitiveVariable( IECoreScene.PrimitiveVariable.Interpolation.Vertex, intStr)
 		pointsPrimitive["quat"] = IECoreScene.PrimitiveVariable( IECoreScene.PrimitiveVariable.Interpolation.Vertex, quat)
+		self.assertTrue( pointsPrimitive.arePrimitiveVariablesValid() )
 
 		child.writeObject( pointsPrimitive, 0.0 )
 
@@ -2450,7 +2451,14 @@ def assertAttributesValues( sphere ):
 		self.assertEqual( bObj.keys(), ["bar", "barDeprecated", "withIndices"] )
 		self.assertEqual( bObj["bar"].data, IECore.StringData( "black" ) )
 		self.assertEqual( bObj["barDeprecated"].data, IECore.StringData( "black" ) )
-		self.assertEqual( bObj["withIndices"], IECoreScene.PrimitiveVariable( IECoreScene.PrimitiveVariable.Interpolation.Vertex, IECore.FloatVectorData([ 1, 2, 3 ]), IECore.IntVectorData( [1] ) ) )
+		self.assertEqual(
+			bObj["withIndices"],
+			IECoreScene.PrimitiveVariable(
+				IECoreScene.PrimitiveVariable.Interpolation.Vertex,
+				IECore.FloatVectorData( [ 1, 2 ] ),
+				IECore.IntVectorData( [ 0, 1, 0, 1 ] )
+			)
+		)
 
 		# check primvars from USD API
 		stage = pxr.Usd.Stage.Open(  os.path.dirname( __file__ ) + "/data/customAttribute.usda" )
@@ -3302,5 +3310,28 @@ def assertExpectedArrayInputs( network ) :
 		scene = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Read )
 		assertExpectedArrayInputs( scene.child( "sphere" ).readAttribute( "ai:surface", 0 ) )
 
+	def testInvalidPrimitiveVariables( self ) :
+
+		goodRoot = IECoreScene.SceneInterface.create( os.path.dirname( __file__ ) + "/data/cube.usda", IECore.IndexedIO.OpenMode.Read )
+		badRoot = IECoreScene.SceneInterface.create( os.path.dirname( __file__ ) + "/data/invalidCube.usda", IECore.IndexedIO.OpenMode.Read )
+
+		goodCube = goodRoot.child( "pCube1" ).readObject( 0 )
+		self.assertIn( "uv", goodCube )
+		self.assertTrue( goodCube.arePrimitiveVariablesValid() )
+
+		with IECore.CapturingMessageHandler() as mh :
+			badCube = badRoot.child( "pCube1" ).readObject( 0 )
+
+		self.assertEqual( len( mh.messages ), 1 )
+		self.assertEqual( mh.messages[0].level, IECore.Msg.Level.Warning )
+		self.assertEqual( mh.messages[0].message, "Skipping invalid UsdGeomPrimvar \"/pCube1.primvars:st\"" )
+
+		self.assertNotIn( "uv", badCube )
+		self.assertTrue( badCube.arePrimitiveVariablesValid() )
+
+		self.assertNotEqual( goodCube, badCube )
+		del goodCube["uv"]
+		self.assertEqual( goodCube, badCube )
+
 if __name__ == "__main__":
 	unittest.main()

contrib/IECoreUSD/test/IECoreUSD/data/customAttribute.usda

@@ -38,10 +38,10 @@ def Xform "a"
 		)
 		string primvars:user:baz = "white"
 		string primvars:notUserPrefixAttribute = "orange"
-		float[] primvars:withIndices = [ 1, 2, 3 ] (
+		float[] primvars:withIndices = [ 1, 2 ] (
 			interpolation = "vertex"
 		)
-		int[] primvars:withIndices:indices = [1]
+		int[] primvars:withIndices:indices = [0, 1, 0, 1]
 
 		def Sphere "c"
 		{

contrib/IECoreUSD/test/IECoreUSD/data/invalidCube.usda

@@ -0,0 +1,30 @@
+#usda 1.0
+(
+    defaultPrim = "pCube1"
+    endTimeCode = 1
+    startTimeCode = 1
+    upAxis = "Y"
+)
+
+def Mesh "pCube1" (
+    kind = "component"
+)
+{
+    uniform bool doubleSided = 1
+    float3[] extent = [(-0.5, -0.5, -0.5), (0.5, 0.5, 0.5)]
+    int[] faceVertexCounts = [4, 4, 4, 4, 4, 4]
+    int[] faceVertexIndices = [0, 1, 3, 2, 2, 3, 5, 4, 4, 5, 7, 6, 6, 7, 1, 0, 1, 7, 5, 3, 6, 0, 2, 4]
+    point3f[] points = [(-0.5, -0.5, 0.5), (0.5, -0.5, 0.5), (-0.5, 0.5, 0.5), (0.5, 0.5, 0.5), (-0.5, 0.5, -0.5), (0.5, 0.5, -0.5), (-0.5, -0.5, -0.5), (0.5, -0.5, -0.5)]
+    color3f[] primvars:displayColor = [(0.21763764, 0.21763764, 0.21763764)] (
+        customData = {
+            dictionary Maya = {
+                bool generated = 1
+            }
+        }
+    )
+    float2[] primvars:st = [] (
+        interpolation = "faceVarying"
+    )
+    int[] primvars:st:indices = [0, 1, 2, 3, 3, 2, 4, 5, 5, 4, 6, 7, 7, 6, 8, 9, 1, 10, 11, 2, 12, 0, 3, 13]
+}
+