Instancer : Fix handling of unindexed primvars in RootPerVertex mode

GafferHQ · Mar 20, 2024 · 6a9547e · 6a9547e
1 parent bb50865
commit 6a9547e
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 8 deletions.
diff --git a/Changes.md b/Changes.md
@@ -1,6 +1,10 @@
 1.3.x.x (relative to 1.3.14.0)
 =======
 
+Fixes
+-----
+
+- Instancer : Fixed handling of unindexed primvars in RootPerVertex mode.
 
 
 1.3.14.0 (relative to 1.3.13.1)

diff --git a/python/GafferSceneTest/InstancerTest.py b/python/GafferSceneTest/InstancerTest.py
@@ -1325,6 +1325,11 @@ def updateRoots( roots, indices ) :
 		self.assertRootsMatchPrototypeSceneChildren( script )
 		self.assertEncapsulatedRendersSame( script["instancer"] )
 
+		# We should be able to get the same result with an un-indexed primvar
+		updateRoots( IECore.StringVectorData( [ "/foo", "/bar", "/bar", "/foo" ] ), None )
+		self.assertRootsMatchPrototypeSceneChildren( script )
+		self.assertEncapsulatedRendersSame( script["instancer"] )
+
 		updateRoots( IECore.StringVectorData( [ "/foo/bar", "/bar" ] ), IECore.IntVectorData( [ 0, 1, 1, 0 ] ) )
 		self.assertConflictingRootNames( script )
 		self.assertEncapsulatedRendersSame( script["instancer"] )
@@ -2200,18 +2205,10 @@ def quant( x, q ):
 		self.assertEncapsulatedRendersSame( instancer )
 
 		instancer["prototypeRoots"].setValue( "unindexedRoots" )
-		"""
-		# How things should work
 		self.assertEqual( uniqueCounts(), { "" : 3 } )
 		self.assertEqual( childNameStrings( "points/instances/cube" ), [ str(i) for i in range( 0, 34 ) ] )
 		self.assertEqual( childNameStrings( "points/instances/plane" ), [ str(i) for i in range( 34, 68 ) ] )
 		self.assertEqual( childNameStrings( "points/instances/sphere" ), [ str(i) for i in range( 68, 100 ) ] )
-		"""
-		# How things currently work
-		self.assertEqual( uniqueCounts(), { "" : 1 } )
-		self.assertEqual( childNameStrings( "points/instances/cube" ), [ str(i) for i in range( 100 ) ] )
-		self.assertEqual( childNameStrings( "points/instances/plane" ), [] )
-		self.assertEqual( childNameStrings( "points/instances/sphere" ), [] )
 
 		self.assertEncapsulatedRendersSame( instancer )
 

diff --git a/src/GafferScene/Instancer.cpp b/src/GafferScene/Instancer.cpp
@@ -816,6 +816,7 @@ class Instancer::EngineData : public Data
 		void initPrototypes( PrototypeMode mode, const std::string &prototypeIndex, const std::string &rootsVariable, const StringVectorData *rootsList, const ScenePlug *prototypes )
 		{
 			const std::vector<std::string> *rootStrings = nullptr;
+			std::vector<std::string> rootStringsAlloc;
 
 			switch( mode )
 			{
@@ -879,6 +880,28 @@ class Instancer::EngineData : public Data
 
 					m_prototypeIndices = view->indices();
 					rootStrings = &view->data();
+
+					if( !m_prototypeIndices )
+					{
+						std::unordered_map<std::string, int> duplicateRootMap;
+
+						m_prototypeIndicesAlloc.reserve( rootStrings->size() );
+						for( const std::string &i : *rootStrings )
+						{
+							auto insertResult = duplicateRootMap.try_emplace( i, rootStringsAlloc.size() );
+							if( insertResult.second )
+							{
+								m_prototypeIndicesAlloc.push_back( rootStringsAlloc.size() );
+								rootStringsAlloc.push_back( i );
+							}
+							else
+							{
+								m_prototypeIndicesAlloc.push_back( insertResult.first->second );
+							}
+						}
+						rootStrings = &rootStringsAlloc;
+						m_prototypeIndices = &m_prototypeIndicesAlloc;
+					}
 					break;
 				}
 			}
@@ -933,6 +956,7 @@ class Instancer::EngineData : public Data
 		Private::ChildNamesMapPtr m_names;
 		std::vector<ConstInternedStringVectorDataPtr> m_roots;
 		std::vector<int> m_prototypeIndexRemap;
+		std::vector<int> m_prototypeIndicesAlloc;
 		const std::vector<int> *m_prototypeIndices;
 		const std::vector<int> *m_ids;
 		const std::vector<Imath::V3f> *m_positions;