Skip to content

Commit

Permalink
Cycles SocketAlgo : Fix warning format arguments
Browse files Browse the repository at this point in the history
There was one too many, which caused `boost::format()` to throw, subsequently crashing Gaffer.

I broke this in 8454f37, where I confidently stated that we didn't need to support transform sockets. But the `texture_coordinate` shader does have such a socket, called `ob_tfm`, which is how this problem surfaced. But I'm still not convinced we need to support it; I'm not sure how a user would use `ob_tfm` - maybe it's more of an implementation detail for Blender?

Fixes GafferHQ#5147.
  • Loading branch information
johnhaddon committed Feb 16, 2023
1 parent 82a0b5f commit b4ac6ae
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 2 deletions.
5 changes: 5 additions & 0 deletions Changes.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
1.2.x.x (relative to 1.2.0.1)
=======

Fixes
-----

- Cycles : Fixed crash triggered by unsupported shader parameters (#5147).

1.2.0.1 (relative to 1.2.0.0)
=======

Expand Down
32 changes: 32 additions & 0 deletions python/GafferCyclesTest/IECoreCyclesPreviewTest/RendererTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -1791,5 +1791,37 @@ def testInvalidShaderEnumValue( self ) :
"Invalid enum value \"missing\" for socket `subsurface_method` on node .*"
)

def testUnsupportedShaderParameters( self ) :

renderer = GafferScene.Private.IECoreScenePreview.Renderer.create(
"Cycles",
GafferScene.Private.IECoreScenePreview.Renderer.RenderType.Batch,
)

with IECore.CapturingMessageHandler() as mh :

attributes = renderer.attributes(
IECore.CompoundObject( {
"cycles:surface" : IECoreScene.ShaderNetwork(
shaders = {
"output" : IECoreScene.Shader( "principled_bsdf", "cycles:surface" ),
"coordinates" : IECoreScene.Shader( "texture_coordinate", "cycles:shader", { "ob_tfm" : imath.M44f() } ),
},
connections = [
( ( "coordinates", "normal" ), ( "output", "normal" ) )
],
output = "output"
)
} )
)

self.assertEqual( len( mh.messages ), 1 )
self.assertEqual( mh.messages[0].context, "Cycles::SocketAlgo" )
self.assertEqual( mh.messages[0].level, IECore.Msg.Level.Warning )
self.assertRegex(
mh.messages[0].message,
"Unsupported socket type `transform` for socket `ob_tfm` on node .*"
)

if __name__ == "__main__":
unittest.main()
8 changes: 6 additions & 2 deletions src/GafferCycles/IECoreCyclesPreview/SocketAlgo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ IECORE_PUSH_DEFAULT_VISIBILITY
#include "util/vector.h"
IECORE_POP_DEFAULT_VISIBILITY

#include "fmt/format.h"

using namespace std;
using namespace Imath;
using namespace IECore;
Expand Down Expand Up @@ -448,8 +450,10 @@ void setSocket( ccl::Node *node, const ccl::SocketType *socket, const IECore::Da
default:
IECore::msg(
IECore::Msg::Warning, "Cycles::SocketAlgo",
boost::format( "Unsupported socket type `%1%` for socket `%2%` on node `%3%`." )
% ccl::SocketType::type_name( socket->type ) % value->typeName() % socket->name % node->name
fmt::format(
"Unsupported socket type `{}` for socket `{}` on node `{}`.",
ccl::SocketType::type_name( socket->type ), socket->name, node->name
)
);
break;
}
Expand Down

0 comments on commit b4ac6ae

Please sign in to comment.