FIX : Don't throw new exceptions from ShaderNetworkAlgo

Author: danieldresser-ie

src/IECore/Ramp.cpp

@@ -458,19 +458,6 @@ void Ramp<X,Y>::toOSL( std::string &basis, std::vector<X> &positions, std::vecto
 template<typename X, typename Y>
 int Ramp<X,Y>::oslStartPointMultiplicity() const
 {
-	if( interpolation == RampInterpolation::MonotoneCubic )
-	{
-		// I guess the only way to handle this properly would be to do the end point duplication
-		// and conversion from monotone to bezier in our OSL shaders ( like the PRMan and 3delight
-		// shader libraries do ). Except that the PRMan approach has a potentially significant
-		// downside that if the inputs are varying, the array conversion code can't be constant
-		// folded, and you would end up doing the array processing per-shading point. 3delight
-		// appears to have implemented their own spline handling, which may not have this problem,
-		// but we don't want to do this ourselves. For now, not handling inputs to monotoneCubic
-		// ramps seems pretty reasonable.
-		throw IECore::Exception( "Cannot connect adaptors to ramp when using monotoneCubic interpolation" );
-	}
-
 	// The "multiplicity" is one greater than the number of duplicates: by default, without duplication,
 	// every control point has a multiplicity of 1, and after 1 duplicate, there is a multiplicity of 2.
 	return getOSLEndPointDuplication( interpolation ).first + 1;

src/IECoreScene/ShaderNetworkAlgo.cpp

@@ -1013,13 +1013,23 @@ struct RampInputAdapterParameters
 
 template< typename TypedRamp >
 RampInputAdapterParameters createRampInputAdapter(
-	ShaderNetwork *network, const TypedData<TypedRamp> *rampData,
+	ShaderNetwork *network, const TypedRamp &ramp,
 	const IECore::CompoundDataMap &newParameters, const IECore::InternedString &rampParameterName,
 	const ShaderNetwork::Parameter &destination
 )
 {
 	using ValueVectorData = TypedData< std::vector< typename TypedRamp::YType > >;
 
+	if( ramp.interpolation == RampInterpolation::MonotoneCubic )
+	{
+		IECore::msg(
+			Msg::Error, "ShaderNetworkAlgo",
+			"Cannot connect adaptors to ramp when using monotoneCubic interpolation: " +
+			destination.shader.string() + "." + destination.name.string()
+		);
+		return { "", 0, 0, 0 };
+	}
+
 	IECore::InternedString splineValuesName = rampParameterName.string() + "Values";
 	auto findValues = newParameters.find( splineValuesName );
 	const ValueVectorData *splineValuesData = findValues != newParameters.end() ? runTimeCast<const ValueVectorData>( findValues->second.get() ) : nullptr;
@@ -1032,12 +1042,13 @@ RampInputAdapterParameters createRampInputAdapter(
 
 	if( splineValues.size() > maxArrayInputAdapterSize )
 	{
-		throw IECore::Exception(
-			"Cannot handle input to " +
-			destination.shader.string() + "." + destination.name.string() +
+		IECore::msg(
+			Msg::Error, "ShaderNetworkAlgo",
+			"Cannot handle input to " + destination.shader.string() + "." + destination.name.string() +
 			" : expanded spline has " + std::to_string( splineValues.size() ) +
 			" control points, but max input adapter size is " + std::to_string( maxArrayInputAdapterSize )
 		);
+		return { "", 0, 0, 0 };
 	}
 
 	// Using this adapter depends on Gaffer being available, but I guess we don't really
@@ -1060,7 +1071,7 @@ RampInputAdapterParameters createRampInputAdapter(
 		{ destination.shader, splineValuesName }
 	) );
 
-	return { adapterHandle, rampData->readable().points.size(), splineValues.size(), rampData->readable().oslStartPointMultiplicity() - 1 };
+	return { adapterHandle, ramp.points.size(), splineValues.size(), ramp.oslStartPointMultiplicity() - 1 };
 }
 
 } // namespace
@@ -1129,28 +1140,40 @@ void ShaderNetworkAlgo::collapseRamps( ShaderNetwork *network, std::string targe
 			auto targetParameterIt = targetParameters.find( rampName );
 			if( targetParameterIt != targetParameters.end() )
 			{
-				if( const RampffData *findRampff = runTimeCast<const RampffData>( targetParameterIt->second.get() ) )
+				if( const RampffData *findRampffData = runTimeCast<const RampffData>( targetParameterIt->second.get() ) )
 				{
-					targetRampKnotOffset = findRampff->readable().oslStartPointMultiplicity() - 1;
-					targetRampSize = findRampff->readable().points.size();
+					const Rampff &ramp = findRampffData->readable();
+					if( ramp.interpolation != RampInterpolation::MonotoneCubic )
+					{
+						targetRampKnotOffset = ramp.oslStartPointMultiplicity() - 1;
+						targetRampSize = ramp.points.size();
+					}
 				}
-				else if( const RampfColor3fData *findRampfColor3f = runTimeCast<const RampfColor3fData>( targetParameterIt->second.get() ) )
+				else if( const RampfColor3fData *findRampfColor3fData = runTimeCast<const RampfColor3fData>( targetParameterIt->second.get() ) )
 				{
-					targetRampKnotOffset = findRampfColor3f->readable().oslStartPointMultiplicity() - 1;
-					targetRampSize = findRampfColor3f->readable().points.size();
+					const RampfColor3f &ramp = findRampfColor3fData->readable();
+					if( ramp.interpolation != RampInterpolation::MonotoneCubic )
+					{
+						targetRampKnotOffset = ramp.oslStartPointMultiplicity() - 1;
+						targetRampSize = ramp.points.size();
+					}
 				}
-				else if( const RampfColor4fData *findRampfColor4f = runTimeCast<const RampfColor4fData>( targetParameterIt->second.get() ) )
+				else if( const RampfColor4fData *findRampfColor4fData = runTimeCast<const RampfColor4fData>( targetParameterIt->second.get() ) )
 				{
-					targetRampKnotOffset = findRampfColor4f->readable().oslStartPointMultiplicity() - 1;
-					targetRampSize = findRampfColor4f->readable().points.size();
+					const RampfColor4f &ramp = findRampfColor4fData->readable();
+					if( ramp.interpolation != RampInterpolation::MonotoneCubic )
+					{
+						targetRampKnotOffset = ramp.oslStartPointMultiplicity() - 1;
+						targetRampSize = ramp.points.size();
+					}
 				}
 			}
 
 			if( targetRampKnotOffset == -1 )
 			{
 				IECore::msg(
 					Msg::Error, "ShaderNetworkAlgo",
-					"Invalid connection to spline parameter that doesn't exist \"" +
+					"Invalid connection to spline parameter that doesn't exist or can't accept connections \"" +
 					output.destination.shader.string() + "." + output.destination.name.string() + "\""
 				);
 				continue;
@@ -1256,7 +1279,7 @@ void ShaderNetworkAlgo::expandRamps( ShaderNetwork *network, std::string targetP
 			// if we remove the connection.
 			const ShaderNetwork::Connection connection = *it++;
 
-			const std::string &destName = connection.destination.name.string();
+			const std::string destName = connection.destination.name.string();
 			boost::smatch rampElementMatch;
 			if( !boost::regex_match( destName, rampElementMatch, g_rampElementRegex) )
 			{
@@ -1285,19 +1308,27 @@ void ShaderNetworkAlgo::expandRamps( ShaderNetwork *network, std::string targetP
 				if( colorRampData )
 				{
 					adapterIter->second = createRampInputAdapter(
-						network, colorRampData, *newParameters, parameterName, connection.destination
+						network, colorRampData->readable(), *newParameters, parameterName, connection.destination
 					);
 				}
 				else
 				{
 					adapterIter->second = createRampInputAdapter(
-						network, floatRampData, *newParameters, parameterName, connection.destination
+						network, floatRampData->readable(), *newParameters, parameterName, connection.destination
 					);
 				}
 			}
 
+			network->removeConnection( connection );
+
 			const RampInputAdapterParameters &adapterParms = adapterIter->second;
 
+			if( adapterParms.adapterHandle.string().size() == 0 )
+			{
+				// Can't form new connection, createRampInputAdapter should have already printed an error
+				continue;
+			}
+
 			int elementId;
 			std::string_view elementIdString( stringViewFromMatch( destName, rampElementMatch, 2 ) );
 			try
@@ -1322,8 +1353,6 @@ void ShaderNetworkAlgo::expandRamps( ShaderNetwork *network, std::string targetP
 			// specify unused duplicated end points for "consistency", but for simplicity, we always connect
 			// the first or last control point to the duplicated end points.
 
-			network->removeConnection( connection );
-
 			int outIndexMin, outIndexMax;
 
 			if( elementId == 0 )

test/IECoreScene/ShaderNetworkAlgoTest.py

@@ -407,7 +407,7 @@ def testRampConversion( self ):
 			IECoreScene.ShaderNetworkAlgo.collapseRamps( shaderNetworkBadRampConnection )
 		self.assertEqual( len( mh.messages ), 1 )
 		self.assertEqual( mh.messages[0].level, IECore.Msg.Level.Error )
-		self.assertEqual( mh.messages[0].message, 'Invalid connection to spline parameter that doesn\'t exist "test.notARampValues"' )
+		self.assertEqual( mh.messages[0].message, 'Invalid connection to spline parameter that doesn\'t exist or can\'t accept connections "test.notARampValues"' )
 
 		shaderNetworkBadRampConnection = shaderNetworkExpandedGood.copy()
 		shaderNetworkBadRampConnection.addConnection( ( ( "adapt", "out4" ), ( "test", "testffbSplineValues" ) ) )
@@ -593,9 +593,13 @@ def testRampInputs( self ):
 			output = "testRamps"
 		)
 
-		with self.assertRaisesRegex( Exception, r".*Cannot handle input to testRamps.fC3fcatmullRom\[0\].y.b : expanded spline has 35 control points, but max input adapter size is 32.*" ):
+		with IECore.CapturingMessageHandler() as mh :
 			IECoreScene.ShaderNetworkAlgo.convertToOSLConventions( n, 11000 )
 
+		self.assertEqual( len( mh.messages ), 1 )
+		self.assertEqual( mh.messages[0].level, IECore.Msg.Level.Error )
+		self.assertEqual( mh.messages[0].message, "Cannot handle input to testRamps.fC3fcatmullRom[0].y.b : expanded spline has 35 control points, but max input adapter size is 32" )
+
 		n = IECoreScene.ShaderNetwork(
 			shaders = {
 				"testRamps" : IECoreScene.Shader(