Merge branch 'RB-10.5' into RB-10.6

Author: murraystevenson

Changes

@@ -1,7 +1,10 @@
 10.6.x.x (relative to 10.6.0.0)
 ========
 
+Fixes
+-----
 
+- USDScene : Worked around numerical imprecision when converting between time and UsdTimeCode.
 
 10.6.0.0 (relative to 10.5.15.2)
 ========
@@ -35,11 +38,19 @@ Breaking Changes
 - Removed support for `IECORE_RTLD_GLOBAL` environment variable.
 - SmoothSkinningData : Removed, along with all associated Ops and Parameters.
 
-10.5.x.x (relative to 10.5.15.2)
+10.5.x.x (relative to 10.5.15.3)
 ========
 
 
 
+10.5.15.3 (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

@@ -4607,5 +4607,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()