From 08bc02471d20450e9f410b1ed5ab515bf9126a70 Mon Sep 17 00:00:00 2001
From: davidsminor <davidm@image-engine.com>
Date: Thu, 18 Sep 2014 10:30:07 -0700
Subject: [PATCH] SceneCache::hash() uses file name where available

SceneCache::hash() used to use a memory address. Ideally you'd want this method to give a stable result across different processes, so now we use the file name if we're reading a scene cache from a file, and fall back to a memory address if we're using something like a MemoryIndexedIO.
---
 src/IECore/SceneCache.cpp     | 17 ++++++++++++++---
 test/IECore/SceneCacheTest.py | 25 +++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/src/IECore/SceneCache.cpp b/src/IECore/SceneCache.cpp
index 78092a8c60..0dcb48084d 100644
--- a/src/IECore/SceneCache.cpp
+++ b/src/IECore/SceneCache.cpp
@@ -100,7 +100,12 @@ class SceneCache::Implementation : public RefCounted
 			}
 			throw Exception( "File name not available in scene cache!" );
 		}
-
+		
+		bool isFile() const
+		{
+			return m_indexedIO->typeId() == FileIndexedIOTypeId;
+		}
+		
 		bool hasObject() const
 		{
 			return m_indexedIO->hasEntry( objectEntry );
@@ -1020,8 +1025,14 @@ class SceneCache::ReaderImplementation : public SceneCache::Implementation
 
 		static void sceneHash( const ReaderImplementation *scene, MurmurHash &h )
 		{
-			// \todo Currently there's a chance of hash collision if the file is closed and others opened, if the shared data object happens to be allocated in the same address. Replace it by a more reliable mechanism for uniquely identifying the file. 
-			h.append( (uint64_t)scene->m_sharedData );
+			if( scene->isFile() )
+			{
+				h.append( scene->fileName() );
+			}
+			else
+			{
+				h.append( (uint64_t)scene->m_sharedData );
+			}
 			const ReaderImplementation *currScene = scene;
 			while( currScene->m_parent )
 			{
diff --git a/test/IECore/SceneCacheTest.py b/test/IECore/SceneCacheTest.py
index 1d77d8ef95..2aa61f00f8 100644
--- a/test/IECore/SceneCacheTest.py
+++ b/test/IECore/SceneCacheTest.py
@@ -875,6 +875,31 @@ def checkHash( hashType, scene, currTime, duplicates = 0 ):
 		t1 = checkHash( IECore.SceneInterface.HashType.HierarchyHash, m, 1 )
 		self.assertEqual( t0[0] + t1[0], len(t0[1].union(t1[1])) )		# all locations differ
 	
+	def testHashStability( self ) :
+		
+		m = IECore.SceneCache( "test/IECore/data/sccFiles/animatedSpheres.scc", IECore.IndexedIO.OpenMode.Read )
+
+		def collectHashes( scene, hashType, time, hashResults ) :
+			counter = 1
+			h = scene.hash( hashType, time ).toString()
+			hashResults.add( h )
+			for n in scene.childNames() :
+				counter += collectHashes( scene.child(n), hashType, time, hashResults )
+			return counter
+		
+		firstReadHashes = set()
+		collectHashes( m, IECore.SceneInterface.HashType.BoundHash, 0, firstReadHashes )
+		del m
+		
+		secondReadHashes = set()
+		m = IECore.SceneCache( "test/IECore/data/sccFiles/animatedSpheres.scc", IECore.IndexedIO.OpenMode.Read )
+		collectHashes( m, IECore.SceneInterface.HashType.BoundHash, 0, secondReadHashes )
+		del m
+		
+		self.assertEqual( firstReadHashes, secondReadHashes )
+		
+		
+		
 	
 if __name__ == "__main__":
 	unittest.main()