Cycles : Update to 5.1.0

[boberfly]

Generally describe what this PR will do, and why it is needed

• Support for Cycles 5.1.0

Related issues

• N/A

Dependencies

• Cycles : Update to 5.1.0 with even sample motion blur patches dependencies#298

Breaking changes

• FaceVarying normals now correctly translate while before it would revert to hard-surfaced normals
• I didn't remove the workaround for even motion samples, I can do this in this PR if you'd like

Checklist

• I have read the contribution guidelines.
• I have updated the documentation, if applicable.
• I have tested my change(s) in the test suite, and added new test cases where necessary.
• My code follows the Gaffer project's prevailing coding style and conventions.

[murraystevenson]

Thanks @boberfly, I've updated this to use the new dependencies-11.0.0a5 package which includes your Cycles 5.1.0 additions. Doing so has exposed a fairly nasty crash. I've tracked the source of it down to automatic tangent generation for subdiv meshes.

I've added 73ee78c to work around the crash in our tests by preventing the principled_bsdf shader from requesting ATTR_STD_GENERATED by providing a dummy connection to the tangent parameter, but this will need some more investigation before we merge. Maybe you have some insight given your involvement in getting that feature implemented in Cycles itself? It's pretty easy to reproduce with the script below, disconnect the "attribute" node before rendering to crash.

import Gaffer
import GafferCycles
import GafferScene
import IECore
import imath

Gaffer.Metadata.registerValue( parent, "serialiser:milestoneVersion", 1, persistent=False )
Gaffer.Metadata.registerValue( parent, "serialiser:majorVersion", 7, persistent=False )
Gaffer.Metadata.registerValue( parent, "serialiser:minorVersion", 0, persistent=False )
Gaffer.Metadata.registerValue( parent, "serialiser:patchVersion", 0, persistent=False )

__children = {}

__children["Sphere"] = GafferScene.Sphere( "Sphere" )
parent.addChild( __children["Sphere"] )
__children["Sphere"].addChild( Gaffer.V2fPlug( "__uiPosition", defaultValue = imath.V2f( 0, 0 ), flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic, ) )
__children["MeshType"] = GafferScene.MeshType( "MeshType" )
parent.addChild( __children["MeshType"] )
__children["MeshType"].addChild( Gaffer.V2fPlug( "__uiPosition", defaultValue = imath.V2f( 0, 0 ), flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic, ) )
__children["ShaderAssignment"] = GafferScene.ShaderAssignment( "ShaderAssignment" )
parent.addChild( __children["ShaderAssignment"] )
__children["ShaderAssignment"].addChild( Gaffer.V2fPlug( "__uiPosition", defaultValue = imath.V2f( 0, 0 ), flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic, ) )
__children["PathFilter"] = GafferScene.PathFilter( "PathFilter" )
parent.addChild( __children["PathFilter"] )
__children["PathFilter"].addChild( Gaffer.V2fPlug( "__uiPosition", defaultValue = imath.V2f( 0, 0 ), flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic, ) )
__children["principled_bsdf"] = GafferCycles.CyclesShader( "principled_bsdf" )
parent.addChild( __children["principled_bsdf"] )
__children["principled_bsdf"].loadShader( "principled_bsdf" )
__children["principled_bsdf"].addChild( Gaffer.V2fPlug( "__uiPosition", defaultValue = imath.V2f( 0, 0 ), flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic, ) )
__children["attribute"] = GafferCycles.CyclesShader( "attribute" )
parent.addChild( __children["attribute"] )
__children["attribute"].loadShader( "attribute" )
__children["attribute"].addChild( Gaffer.V2fPlug( "__uiPosition", defaultValue = imath.V2f( 0, 0 ), flags = Gaffer.Plug.Flags.Default | Gaffer.Plug.Flags.Dynamic, ) )
__children["Sphere"]["__uiPosition"].setValue( imath.V2f( -16.9648685, 13.2230463 ) )
__children["MeshType"]["in"].setInput( __children["Sphere"]["out"] )
__children["MeshType"]["filter"].setInput( __children["PathFilter"]["out"] )
__children["MeshType"]["meshType"].setValue( 'catmullClark' )
__children["MeshType"]["__uiPosition"].setValue( imath.V2f( -16.9648705, 5.0589838 ) )
__children["ShaderAssignment"]["in"].setInput( __children["MeshType"]["out"] )
__children["ShaderAssignment"]["filter"].setInput( __children["PathFilter"]["out"] )
__children["ShaderAssignment"]["shader"].setInput( __children["principled_bsdf"]["out"]["BSDF"] )
__children["ShaderAssignment"]["__uiPosition"].setValue( imath.V2f( -16.9648705, -3.1050787 ) )
__children["PathFilter"]["paths"].setValue( IECore.StringVectorData( [ '/sphere' ] ) )
__children["PathFilter"]["__uiPosition"].setValue( imath.V2f( -3.96486807, 11.1410151 ) )
__children["principled_bsdf"]["parameters"]["subsurface_radius"].setValue( imath.V3f( 1, 0.200000003, 0.100000001 ) )
__children["principled_bsdf"]["parameters"]["tangent"].setInput( __children["attribute"]["out"]["vector"] )
Gaffer.Metadata.registerValue( __children["principled_bsdf"]["parameters"]["tangent"], 'noduleLayout:visible', True )
__children["principled_bsdf"]["__uiPosition"].setValue( imath.V2f( -33.5063705, -3.1050787 ) )
Gaffer.Metadata.registerValue( __children["attribute"], 'annotation:user:text', 'Disconnect to crash Cycles' )
Gaffer.Metadata.registerValue( __children["attribute"], 'annotation:user:color', imath.Color3f( 0.150000006, 0.25999999, 0.25999999 ) )
__children["attribute"]["__uiPosition"].setValue( imath.V2f( -60.5663795, -6.70507908 ) )


del __children

[boberfly]

On my phone atm but I saw https://projects.blender.org/blender/blender/pulls/156693 which I think didn't make it into 5.1 standalone I think, I'll do a test with this and see. I'll look for more related patches.

[boberfly]

Ok I remember a bit more how this works - Blender hands off a ccl::ATTR_STD_GENERATED which is more or less Pref to help generate tangents, so I had an:

else if( name == "Pref" )
{
    attr->std = ccl::ATTR_STD_GENERATED;
}

In there to test this and a shuffle primitive variables node to copy P to Pref and this prevents the crashing.
https://projects.blender.org/blender/blender/src/commit/9b81403e0d43ee42882a4313a6a567de32e70499/intern/cycles/blender/mesh.cpp#L680
We did work on a way for Cycles to just make its own when there isn't one to use, but Blender doesn't run into this as it always hands off one to Cycles:
https://projects.blender.org/blender/blender/src/commit/9b81403e0d43ee42882a4313a6a567de32e70499/intern/cycles/scene/geometry.cpp#L837
This call should happen before subdivision/tessellation otherwise it will only generate a Pref up to the size of the original un-diced mesh.

I'll test out a fix by moving that call.

[boberfly]

GafferHQ/dependencies#300 try this one out @murraystevenson I think this fixed it for me.

[murraystevenson]

GafferHQ/dependencies#300 try this one out @murraystevenson I think this fixed it for me.

Thanks boberfly, I've updated this to dependencies-11.0.0a6 which includes that patch and have dropped the previous workaround commit. I've also pushed 950f582 with a slight refactor of the GeometryAlgo changes and an additional test. f01bd5b adds resampling of uniform normals to face-varying, and 36bf089 reverts the workaround for even motion samples on the GPU.

[johnhaddon]

Thanks @murraystevenson and @boberfly for pushing this along. I don't know what has caused it, but I'm seeing some issues rendering UVs, with messages like this one popping up a lot :

WARNING : IECoreCyles::GeometryAlgo::convertPrimitiveVariable : Primitive variable "uv" has size 96 but Cycles allocated size 0.

Any ideas?

[johnhaddon]

Why are we doing linear search rather than mesh->variables.find()?

[johnhaddon]

I'm seeing some issues rendering UVs

Scratch that - seems I just had a stale build.

[johnhaddon]

Thanks for the update Murray - I have squashed and merged.