Instancer : Add omitDuplicateIds

include/GafferScene/Instancer.h

@@ -126,6 +126,9 @@ class GAFFERSCENE_API Instancer : public BranchCreator
 		Gaffer::StringPlug *idPlug();
 		const Gaffer::StringPlug *idPlug() const;
 
+		Gaffer::BoolPlug *omitDuplicateIdsPlug();
+		const Gaffer::BoolPlug *omitDuplicateIdsPlug() const;
+
 		Gaffer::StringPlug *positionPlug();
 		const Gaffer::StringPlug *positionPlug() const;
 

python/GafferSceneTest/InstancerTest.py

@@ -1592,32 +1592,69 @@ def testDuplicateIds( self ) :
 		points = IECoreScene.PointsPrimitive( IECore.V3fVectorData( [ imath.V3f( x, 0, 0 ) for x in range( 6 ) ] ) )
 		points["id"] = IECoreScene.PrimitiveVariable(
 			IECoreScene.PrimitiveVariable.Interpolation.Vertex,
-			IECore.IntVectorData( [ 0, 0, 2, 2, 4, 4 ] ),
+			IECore.IntVectorData( [ 0, 1, 2, 2, 3, 4 ] ),
 		)
 
 		objectToScene = GafferScene.ObjectToScene()
 		objectToScene["object"].setValue( points )
 
 		sphere = GafferScene.Sphere()
+		parent = GafferScene.Parent()
+		parent["parent"].setValue( "/" )
+		parent["in"].setInput( sphere["out"] )
+		parent["children"][0].setInput( sphere["out"] )
 
 		instancer = GafferScene.Instancer()
 		instancer["in"].setInput( objectToScene["out"] )
-		instancer["prototypes"].setInput( sphere["out"] )
+		instancer["prototypes"].setInput( parent["out"] )
 		instancer["parent"].setValue( "/object" )
 		instancer["id"].setValue( "id" )
 
+		instancer["omitDuplicateIds"].setValue( False )
+
+		with self.assertRaisesRegex( RuntimeError, 'Instancer.__engine : Instance id "2" is duplicated at index 2 and 3. This probably indicates invalid source data, if you want to hack around it, you can set "omitDuplicateIds"' ) :
+			self.assertSceneValid( instancer["out"] )
+
+		instancer["omitDuplicateIds"].setValue( True )
 		self.assertSceneValid( instancer["out"] )
 
-		self.assertEqual( instancer["out"].childNames( "/object/instances/sphere" ), IECore.InternedStringVectorData( [ "0", "2", "4" ] ) )
+		self.assertEqual( instancer["out"].childNames( "/object/instances/sphere" ), IECore.InternedStringVectorData( [ "0", "1", "3", "4" ] ) )
 
 		self.assertEqual( instancer["out"].transform( "/object/instances/sphere/0" ), imath.M44f().translate( imath.V3f( 0, 0, 0 ) ) )
-		self.assertEqual( instancer["out"].transform( "/object/instances/sphere/2" ), imath.M44f().translate( imath.V3f( 2, 0, 0 ) ) )
-		self.assertEqual( instancer["out"].transform( "/object/instances/sphere/4" ), imath.M44f().translate( imath.V3f( 4, 0, 0 ) ) )
+		self.assertEqual( instancer["out"].transform( "/object/instances/sphere/1" ), imath.M44f().translate( imath.V3f( 1, 0, 0 ) ) )
+		self.assertEqual( instancer["out"].transform( "/object/instances/sphere/3" ), imath.M44f().translate( imath.V3f( 4, 0, 0 ) ) )
+		self.assertEqual( instancer["out"].transform( "/object/instances/sphere/4" ), imath.M44f().translate( imath.V3f( 5, 0, 0 ) ) )
+
+		self.assertEncapsulatedRendersSame( instancer )
+
+		instancer["prototypeIndex"].setValue( "prototypeIndex" )
+
+		# Test duplicate ids between different prototypes - the handling of this is now pretty consistent, but
+		# it used to be treated quite differently
+
+		points["prototypeIndex"] = IECoreScene.PrimitiveVariable(
+			IECoreScene.PrimitiveVariable.Interpolation.Vertex,
+			IECore.IntVectorData( [ 0, 1, 0, 1, 0, 1 ] ),
+		)
+		objectToScene["object"].setValue( points )
+
+		instancer["omitDuplicateIds"].setValue( False )
+
+		with self.assertRaisesRegex( RuntimeError, 'Instancer.__engine : Instance id "2" is duplicated at index 2 and 3. This probably indicates invalid source data, if you want to hack around it, you can set "omitDuplicateIds"' ) :
+			self.assertSceneValid( instancer["out"] )
+
+		instancer["omitDuplicateIds"].setValue( True )
+
+		self.assertEqual( instancer["out"].childNames( "/object/instances" ), IECore.InternedStringVectorData( [ "sphere", "sphere1" ] ) )
+		self.assertEqual( instancer["out"].childNames( "/object/instances/sphere" ), IECore.InternedStringVectorData( [ "0", "3" ] ) )
+		self.assertEqual( instancer["out"].childNames( "/object/instances/sphere1" ), IECore.InternedStringVectorData( [ "1", "4" ] ) )
+
+		self.assertEqual( instancer["out"].transform( "/object/instances/sphere/0" ), imath.M44f().translate( imath.V3f( 0, 0, 0 ) ) )
+		self.assertEqual( instancer["out"].transform( "/object/instances/sphere/3" ), imath.M44f().translate( imath.V3f( 4, 0, 0 ) ) )
+
+		self.assertEqual( instancer["out"].transform( "/object/instances/sphere1/1" ), imath.M44f().translate( imath.V3f( 1, 0, 0 ) ) )
+		self.assertEqual( instancer["out"].transform( "/object/instances/sphere1/4" ), imath.M44f().translate( imath.V3f( 5, 0, 0 ) ) )
 
-		# TODO - test duplicate ids for different prototypes - this behaviour has changed.
-		# Currently, we take the first instance with the id, but John pointed out that if order
-		# changes during the shutter, this could cause weirdness. Perhaps the best solution
-		# is if two elements have the same id, both of them are omitted, and a warning is printed.
 
 	def testAttributes( self ) :
 

python/GafferSceneUI/InstancerUI.py

@@ -286,50 +286,50 @@ def __init__( self, headings, toolTipOverride = "" ) :
 
 	"layout:customWidget:seedColumnHeadings:widgetType", "GafferSceneUI.InstancerUI._SeedColumnHeadings",
 	"layout:customWidget:seedColumnHeadings:section", "Context Variations",
-	"layout:customWidget:seedColumnHeadings:index", 18,
+	"layout:customWidget:seedColumnHeadings:index", 19,
 
 	"layout:customWidget:idContextCountSpacer:widgetType", "GafferSceneUI.InstancerUI._SeedCountSpacer",
 	"layout:customWidget:idContextCountSpacer:section", "Context Variations",
-	"layout:customWidget:idContextCountSpacer:index", 19,
+	"layout:customWidget:idContextCountSpacer:index", 20,
 	"layout:customWidget:idContextCountSpacer:accessory", True,
 
 	"layout:customWidget:idContextCount:widgetType", "GafferSceneUI.InstancerUI._SeedCountWidget",
 	"layout:customWidget:idContextCount:section", "Context Variations",
-	"layout:customWidget:idContextCount:index", 19,
+	"layout:customWidget:idContextCount:index", 20,
 	"layout:customWidget:idContextCount:accessory", True,
 
 	"layout:customWidget:seedVariableSpacer:widgetType", "GafferSceneUI.InstancerUI._VariationSpacer",
 	"layout:customWidget:seedVariableSpacer:section", "Context Variations",
-	"layout:customWidget:seedVariableSpacer:index", 20,
+	"layout:customWidget:seedVariableSpacer:index", 21,
 	"layout:customWidget:seedVariableSpacer:accessory", True,
 
 	"layout:customWidget:seedsSpacer:widgetType", "GafferSceneUI.InstancerUI._VariationSpacer",
 	"layout:customWidget:seedsSpacer:section", "Context Variations",
-	"layout:customWidget:seedsSpacer:index", 21,
+	"layout:customWidget:seedsSpacer:index", 22,
 	"layout:customWidget:seedsSpacer:accessory", True,
 
 	"layout:customWidget:seedPermutationSpacer:widgetType", "GafferSceneUI.InstancerUI._VariationSpacer",
 	"layout:customWidget:seedPermutationSpacer:section", "Context Variations",
-	"layout:customWidget:seedPermutationSpacer:index", 22,
+	"layout:customWidget:seedPermutationSpacer:index", 23,
 	"layout:customWidget:seedPermutationSpacer:accessory", True,
 
 	"layout:customWidget:seedSpacer:widgetType", "GafferSceneUI.InstancerUI._SectionSpacer",
 	"layout:customWidget:seedSpacer:section", "Context Variations",
-	"layout:customWidget:seedSpacer:index", 23,
+	"layout:customWidget:seedSpacer:index", 24,
 
 	"layout:customWidget:timeOffsetHeadings:widgetType", "GafferSceneUI.InstancerUI._TimeOffsetColumnHeadings",
 	"layout:customWidget:timeOffsetHeadings:section", "Context Variations",
-	"layout:customWidget:timeOffsetHeadings:index", 24,
+	"layout:customWidget:timeOffsetHeadings:index", 25,
 	"layout:customWidget:timeOffsetHeadings:description", "Testing description",
 
 	"layout:customWidget:timeOffsetSpacer:widgetType", "GafferSceneUI.InstancerUI._SectionSpacer",
 	"layout:customWidget:timeOffsetSpacer:section", "Context Variations",
-	"layout:customWidget:timeOffsetSpacer:index", 25,
+	"layout:customWidget:timeOffsetSpacer:index", 26,
 	"layout:customWidget:timeOffsetSpacer:divider", True,
 
 	"layout:customWidget:totalSpacer:widgetType", "GafferSceneUI.InstancerUI._SectionSpacer",
 	"layout:customWidget:totalSpacer:section", "Context Variations",
-	"layout:customWidget:totalSpacer:index", 26,
+	"layout:customWidget:totalSpacer:index", 27,
 
 	plugs = {
 
@@ -483,6 +483,22 @@ def __init__( self, headings, toolTipOverride = "" ) :
 
 		],
 
+		"omitDuplicateIds" : [
+
+			"description",
+			"""
+			When false, having the same ids on multiple points is considered
+			an error. Setting to True will allow a render to proceed, with any
+			points that share an id all removed.
+			""",
+
+			"layout:section", "Settings.General",
+
+			# \todo : Uncomment for Gaffer 1.4
+			# "userDefault", False,
+
+		],
+
 		"position" : [
 
 			"description",