Skip to content

Prman Spline Experiments #1492

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 21, 2025
+278 −103
Merged

Prman Spline Experiments #1492

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e6960a8
IECoreUSD::ShaderAlgo : Omit shaderType prefix for some shaders in Rman
danieldresser-ie Sep 22, 2025
a00b37a
ShaderNetworkAlgo::expand/collapseSpline : Support Prman convention f…
danieldresser-ie Sep 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Changes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
10.6.x.x (relative to 10.6.0.2)
========

Improvements
------------

- ShaderNetworkAlgo : Added support for RenderMan's spline convention in `expandSplines()` and `collapseSplines()`.

Fixes
-----

- ConfigLoader : Fixed UnicodeDecodeErrors in non-UTF8 locales.
- USDScene : Fixed identifiers for exported RenderMan shaders, by omitting the `ri:` type prefix.

10.6.0.2 (relative to 10.6.0.1)
========
Expand Down
28 changes: 23 additions & 5 deletions contrib/IECoreUSD/src/IECoreUSD/ShaderAlgo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,15 +309,33 @@ pxr::UsdShadeConnectableAPI createShaderPrim( const IECoreScene::Shader *shader,
{
throw IECore::Exception( "Could not create shader at " + path.GetAsString() );
}

const std::string type = shader->getType();

std::string typePrefix;
size_t typeColonPos = type.find( ":" );
if( typeColonPos != std::string::npos )
if( boost::starts_with( shader->getName(), "Pxr" ) || boost::starts_with( shader->getName(), "Lama" ) )
{
typePrefix = type.substr( 0, typeColonPos ) + ":";
if( typePrefix == "ai:" )
// Leave the type prefix empty. This should be the default, but we are currently only doing this
// for a small number of shaders that we can be completely confident require it, in order to
// preserve backwards compatibility.
}
else
{
size_t typeColonPos = type.find( ":" );
if( typeColonPos != std::string::npos )
{
typePrefix = "arnold:";
// According to our current understanding, this is almost completely wrong. Renderer's like
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For future consideration, this would suggest that it works for Cycles, although it's not clear if it's what they prefer or just something they tolerate : https://github.com/blender/cycles/blob/main/src/hydra/material.cpp#L435-L437. I think long-term it's actually a bit unclear what the default should be, and I fear that different renderers are going to want different things for OSL shaders. Anyway, that's a battle for another day.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, does look like they will accept "cycles:". It still seems like we don't have any example of a renderer that actually wants an "osl:" prefix though, right? Once we're ready to break compatibility, there isn't much benefit in the "osl:" prefix if the only thing that is being done with it is running it through pipeline code that replaces it with "arnold:", which I think is what you said is currently happening?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we'll definitely need to do something with the OSL stuff, but I think "almost completely wrong" is overstating it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

although it's not clear if it's what they prefer or just something they tolerate

Thinking about this a bit more, I think the prefixing is essential for Cycles (and it is for Arnold too). Neither renderer uses any unique prefix in their shader names so the possibility of a clash is just far too high without a prefix added to the identifier in USD. Whereas Pixar seem to have gone the other way, and included the Pxr or Lama prefix in the shader name, so the identifier doesn't need an additional prefix.

// PRMan won't accept shaders with type prefixes, and Arnold apparently requires all shaders
// to be prefixed with "arnold:", including OSL. This code prefixes OSL shaders with "osl:",
// which fails in all renderers we're aware of - we're keeping this behaviour for now for
// backwards compatibility reasons.
typePrefix = type.substr( 0, typeColonPos ) + ":";

// This is the one case that actually works
if( typePrefix == "ai:" )
{
typePrefix = "arnold:";
}
}
}
usdShader.SetShaderId( pxr::TfToken( typePrefix + shader->getName() ) );
Expand Down
32 changes: 31 additions & 1 deletion contrib/IECoreUSD/test/IECoreUSD/USDSceneTest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3055,6 +3055,13 @@ def testShaders( self ) :
) )
manualComponentNetwork.setOutput( IECoreScene.ShaderNetwork.Parameter( "dest", "" ) )


rmanSurface = IECoreScene.Shader( "PxrFoo", "osl:surface", surface.parameters )
rmanNetwork = IECoreScene.ShaderNetwork()
rmanNetwork.addShader( "out", rmanSurface )
rmanNetwork.setOutput( IECoreScene.ShaderNetwork.Parameter( "out", "" ) )


writerRoot = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Write )
shaderLocation = writerRoot.createChild( "shaderLocation" )
shaderLocation.writeAttribute( "ai:surface", oneShaderNetwork, 0 )
Expand All @@ -3063,6 +3070,7 @@ def testShaders( self ) :
shaderLocation.writeAttribute( "complex:surface", complexNetwork, 0 )
shaderLocation.writeAttribute( "componentConnection:surface", componentConnectionNetwork, 0 )
shaderLocation.writeAttribute( "manualComponent:surface", manualComponentNetwork, 0 )
shaderLocation.writeAttribute( "ri:surface", rmanNetwork, 0 )

shaderLocation.writeAttribute( "volume", oneShaderNetwork, 0 ) # USD supports shaders without a prefix

Expand Down Expand Up @@ -3121,11 +3129,26 @@ def testShaders( self ) :
self.assertEqual( textureUsd.GetInput( "filename" ).Get(), "sometexture.tx" )




riShaderSource = mat.GetOutput( "ri:surface" ).GetConnectedSource()
self.assertEqual( riShaderSource[1], "DEFAULT_OUTPUT" )
riShaderUsd = pxr.UsdShade.Shader( riShaderSource[0].GetPrim() )
self.assertEqual( riShaderUsd.GetShaderId(), "PxrFoo" )
self.assertEqual( riShaderUsd.GetInput( "c" ).Get(), "42" )
self.assertEqual( riShaderUsd.GetInput( "a" ).Get(), 42.0 )
self.assertEqual( riShaderUsd.GetInput( "g" ).Get(), 5 )
self.assertEqual( riShaderUsd.GetInput( "g_Knots" ).Get(), pxr.Vt.FloatArray( [0, 0, 10, 20, 20] ) )
self.assertEqual( riShaderUsd.GetInput( "g_Colors" ).Get(), pxr.Vt.Vec3fArray(
[pxr.Gf.Vec3f( 1 ), pxr.Gf.Vec3f( 1 ), pxr.Gf.Vec3f( 2 ), pxr.Gf.Vec3f( 0 ), pxr.Gf.Vec3f( 0 )]
) )
self.assertEqual( riShaderUsd.GetInput( "g_Interpolation" ).Get(), 'linear' )

# Read via SceneInterface, and check that we've round-tripped successfully.

root = IECoreScene.SceneInterface.create( fileName, IECore.IndexedIO.OpenMode.Read )

self.assertEqual( set( root.child( "shaderLocation" ).attributeNames() ), set( ['ai:disp_map', 'ai:surface', 'complex:surface', 'volume', 'componentConnection:surface', 'manualComponent:surface' ] ) )
self.assertEqual( set( root.child( "shaderLocation" ).attributeNames() ), set( ['ai:disp_map', 'ai:surface', 'complex:surface', 'volume', 'componentConnection:surface', 'manualComponent:surface', "ri:surface" ] ) )

self.assertEqual( root.child( "shaderLocation" ).readAttribute( "ai:surface", 0 ).outputShader().parameters, oneShaderNetwork.outputShader().parameters )
self.assertEqual( root.child( "shaderLocation" ).readAttribute( "ai:surface", 0 ).outputShader(), oneShaderNetwork.outputShader() )
Expand All @@ -3150,6 +3173,13 @@ def testShaders( self ) :
self.assertEqual( root.child( "shaderLocation" ).readAttribute( "componentConnection:surface", 0 ), componentConnectionNetwork )
self.assertEqual( root.child( "shaderLocation" ).readAttribute( "manualComponent:surface", 0 ), manualComponentNetwork )

rmanSurfaceWithoutTypePrefix = IECoreScene.Shader( "PxrFoo", "surface", surface.parameters )
rmanNetworkWithoutTypePrefix = IECoreScene.ShaderNetwork()
rmanNetworkWithoutTypePrefix.addShader( "out", rmanSurfaceWithoutTypePrefix )
rmanNetworkWithoutTypePrefix.setOutput( IECoreScene.ShaderNetwork.Parameter( "out", "" ) )

self.assertEqual( root.child( "shaderLocation" ).readAttribute( "ri:surface", 0 ), rmanNetworkWithoutTypePrefix )

def testManyShaders( self ) :

# Write shaders
Expand Down
6 changes: 4 additions & 2 deletions include/IECoreScene/ShaderNetworkAlgo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,12 @@ IECORESCENE_API void collapseSplines( ShaderNetwork *network, std::string target
IECORESCENE_API void expandSplines( ShaderNetwork *network, std::string targetPrefix = "" );


/// \deprecated: Use collapseSplines on the whole network, which can handle input connections
/// \deprecated: Use collapseSplines on the whole network, which can handle input connections, and supports
/// different spline conventions for different renderers' shader libraries
IECORESCENE_API IECore::ConstCompoundDataPtr collapseSplineParameters( const IECore::ConstCompoundDataPtr& parametersData );

/// \deprecated: Use expandSplines on the whole network, which can handle input connections
/// \deprecated: Use expandSplines on the whole network, which can handle input connections, and supports
/// different spline conventions for different renderers' shader libraries
IECORESCENE_API IECore::ConstCompoundDataPtr expandSplineParameters( const IECore::ConstCompoundDataPtr& parametersData );


Expand Down
Loading