Merge pull request #1482 from johnhaddon/usdTimeFix

USDScene : Work around imprecision converting time to UsdTimeCode

Author: murraystevenson

Changes

@@ -1,7 +1,10 @@
 10.5.x.x (relative to 10.5.15.2)
 ========
 
+Fixes
+-----
 
+- USDScene : Worked around numerical imprecision when converting between time and UsdTimeCode.
 
 10.5.15.2 (relative to 10.5.15.1)
 =========

contrib/IECoreUSD/src/IECoreUSD/USDScene.cpp

@@ -715,7 +715,28 @@ class USDScene::IO : public RefCounted
 
 		pxr::UsdTimeCode timeCode( double timeSeconds ) const
 		{
-			return timeSeconds * m_timeCodesPerSecond;
+			const double timeCode = timeSeconds * m_timeCodesPerSecond;
+
+			// It's common for `timeSeconds` to have been converted from a
+			// `frame` value (by Gaffer's SceneReader for example), and it's
+			// also common for USD's `timeCodesPerSecond` to match the FPS used
+			// in the conversion, meaning that integer timecodes correspond to
+			// integer frames.
+			//
+			// But numerical imprecision means that `timeCode` may no longer be
+			// the exact same integer `frame` we started with. Compute the
+			// integer version of the timecode, and if it is an equally
+			// plausible conversion of `timeSeconds`, then prefer it.
+			//
+			// This is important because timesamples and value clips are commonly
+			// placed on integer timecodes, and we want to hit them exactly.
+			const double integerTimeCode = std::round( timeCode );
+			if( integerTimeCode / m_timeCodesPerSecond == timeSeconds )
+			{
+				return integerTimeCode;
+			}
+
+			return timeCode;
 		}
 
 		// Tags

contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py

@@ -4595,5 +4595,44 @@ def testUsdVolVolumeWithEmptyField( self ) :
 		root = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Read )
 		self.assertIsNone( root.child( "volume" ).readObject( 0 ) )
 
+	def testTimeCodeClamping( self ) :
+
+		fileName = os.path.join( self.temporaryDirectory(), "test.usda" )
+		fileName = "test.usda"
+
+		# Create a stage with a fairly common timesampling setup.
+		# TimeCodesPerSecond and FramesPerSecond are equal, so that integer
+		# timecodes correspond to whole frames.
+
+		framesPerSecond = 30.0
+
+		stage = pxr.Usd.Stage.CreateNew( fileName )
+		stage.SetTimeCodesPerSecond( framesPerSecond )
+		stage.SetFramesPerSecond( framesPerSecond )
+
+		# Keyframe a boolean value, alternating on and off each frame.
+
+		prim = pxr.UsdGeom.Xform.Define( stage, "/child" )
+		primVar = pxr.UsdGeom.PrimvarsAPI( prim ).CreatePrimvar( "test", pxr.Sdf.ValueTypeNames.Bool )
+
+		frameRange = range( 1, 50000 )
+		for frame in frameRange :
+			primVar.Set( bool( frame % 2 ), frame )
+
+		stage.GetRootLayer().Save()
+		del stage
+
+		# Read back the values for each frame, asserting they are as expected.
+		# Because boolean values can't be interpolated, we have to hit the
+		# _exact_ timecode for the frame - if we're under, then we'll get the
+		# held value from the previous frame.
+
+		scene = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Read )
+		child = scene.child( "child" )
+
+		for frame in frameRange :
+			timeInSeconds = frame / framesPerSecond
+			self.assertEqual( child.readAttribute( "render:test", timeInSeconds ), IECore.BoolData( frame % 2 ) )
+
 if __name__ == "__main__":
 	unittest.main()