Skip to content
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.

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

3Delight extended outputs - outputlayers, lightsets, multilayer EXRs #5641

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion src/GafferScene/Render.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,6 @@ void Render::executeInternal( bool flushCaches ) const
Monitor::Scope performanceMonitorScope( performanceMonitor );

GafferScene::Private::RendererAlgo::outputOptions( renderOptions.globals.get(), renderer.get() );
GafferScene::Private::RendererAlgo::outputOutputs( inPlug(), renderOptions.globals.get(), renderer.get() );

{
// Using nested scope so that we free the memory used by `renderSets`
Expand All @@ -361,6 +360,8 @@ void Render::executeInternal( bool flushCaches ) const
GafferScene::Private::RendererAlgo::outputObjects( adaptedInPlug(), renderOptions, renderSets, &lightLinks, renderer.get() );
}

GafferScene::Private::RendererAlgo::outputOutputs( inPlug(), renderOptions.globals.get(), renderer.get() );
Copy link
Member

Choose a reason for hiding this comment

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

I'm a bit wary of this change, and any effects it might have on other renderer backends. It's also a bit of a departure from what I think is pretty logical - outputting all the globals before the scene itself.

If we were to keep this, it seems we'd need to do the same in InteractiveRender/RenderController to maintain feature parity?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll look into what other options we might have. Since this change is related only to the light groups implementation, not having it for the moment won't affect any of the other changes.


if( renderScope.sceneTranslationOnly() )
{
return;
Expand Down
54 changes: 46 additions & 8 deletions src/IECoreDelight/Renderer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,6 @@ class DelightOutput : public IECore::RefCounted
driverParams.add( { "drivername", &typePtr, NSITypeString, 0, 1, 0 } );
driverParams.add( { "imagefilename", &namePtr, NSITypeString, 0, 1, 0 } );

m_driverHandle = DelightHandle( context, "outputDriver:" + name, ownership, "outputdriver", driverParams );

// Layer

string variableName;
Expand Down Expand Up @@ -327,18 +325,45 @@ class DelightOutput : public IECore::RefCounted
layerName = IECore::CamelCase::join( layerTokens.begin(), layerTokens.end(), IECore::CamelCase::AllExceptFirst);
}

ParameterList layerParams;
ParameterList layerParams( output->parameters() );

layerParams.add( "variablename", variableName );
layerParams.add( "variablesource", variableSource );
layerParams.add( "layertype", layerType );
layerParams.add( "layername", layerName );
layerParams.add( { "withalpha", &withAlpha, NSITypeInteger, 0, 1, 0 } );

const string scalarFormat = this->scalarFormat( output );
const string colorProfile = scalarFormat == "float" ? "linear" : "sRGB";
layerParams.add( "scalarformat", scalarFormat );
layerParams.add( "colorprofile", colorProfile );
const double filterSize = parameter<float>( output->parameters(), "filtersize", 3.0 );
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for taking filtersize here when the NSI name is filterwidth?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TBH, I don't remember at the moment :) It probably was related to me trying what would be the best way to get a Gaffer float plug converted to a double type for the NSI filterwidth attribute. I'll double check once I revisit the code for the individual smaller PRs, but I'm pretty sure we can use filterwidth instead of filtersize here.

if( filterSize != 3.0 )
{
layerParams.add( { "filterwidth", &filterSize, NSITypeDouble, 0, 1, 0 } );
}

const string scalarFormatQuant = this->scalarFormat( output );
const string colorProfileQuant = scalarFormatQuant == "float" ? "linear" : "sRGB";
const string scalarFormat = parameter<string>( output->parameters(), "scalarformat", "" );
if( scalarFormat == "" )
{
layerParams.add( "scalarformat", scalarFormatQuant );
layerParams.add( "colorprofile", colorProfileQuant );
}

const string lightGroup = parameter<string>( output->parameters(), "lightgroup", "" );
vector<string> lightGroupTokens;
if( lightGroup != "" )
{
IECore::StringAlgo::tokenize( lightGroup, ' ', lightGroupTokens );
}

const string customDriverName = parameter<string>( output->parameters(), "customdrivername", "" );
if( customDriverName == "" )
{
m_driverHandle = DelightHandle( context, "outputDriver:" + name, ownership, "outputdriver", driverParams );
}
else
{
m_driverHandle = DelightHandle( context, "outputDriver:" + customDriverName, ownership, "outputdriver", driverParams );
}
Comment on lines +358 to +366
Copy link
Member

Choose a reason for hiding this comment

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

When talking about supporting multi-part for Arnold, I think we've concluded that we wanted to do it by automatically sharing drivers for outputs that had the same filename. Perhaps we could take the same approach here by just keeping a map of filename->driver?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good. The customdrivername was a first pass attempt to get the multipart output running and it just stuck for the PR. Completely agreed that the approach to use the filename plug value as a base for the multipart output setup would work better.


m_layerHandle = DelightHandle( context, "outputLayer:" + name, ownership, "outputlayer", layerParams );

Expand All @@ -348,6 +373,19 @@ class DelightOutput : public IECore::RefCounted
m_layerHandle.name(), "outputdrivers",
0, nullptr
);

if( lightGroup != "" )
{
for( string lightGroupToken : lightGroupTokens )
{
NSIConnect(
m_context,
lightGroupToken.c_str(), "",
m_layerHandle.name(), "lightset",
0, nullptr
);
}
}
Comment on lines +377 to +388
Copy link
Member

Choose a reason for hiding this comment

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

I fear that this won't scale to real production scenarios - there will just be too many lights for it to be practical to manage them as a list of individual locations. It's also really vulnerable to lights changing names, being pruned etc.

I think a production-proof solution would either have to use Gaffer sets to define light groups, or a sort of "virtual parameter" on the light to say what groups it was in (mimicking the Arnold approach).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Completely agreed. Similar to customdrivername this was a first pass setup to get light groups running with 3Delight and it definitely isn't an elegant solution.

I did want to see what it would take to get Gaffer sets translated to NSI sets, since that would not only help light groups, but any future attempt at light linking implementation as well. Once we have sets working well, I'll revisit light groups and any other potentially needed changes like the above mentioned GafferScene/Render.cpp outputs order.

Copy link
Member

Choose a reason for hiding this comment

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

It's not very well documented, but any sets with names matching render:* are exported to renderers as a string attribute with the list of sets an object belongs to. It would be possible to map that to connections to set nodes in the NSI backend. Maybe that's a starting point?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's great to know. Thanks for the info!

}

const DelightHandle &layerHandle() const
Expand All @@ -368,7 +406,7 @@ class DelightOutput : public IECore::RefCounted
{
return "uint8";
}
else if( quantize == vector<int>( { 0, 65536, 0, 65536 } ) )
else if( quantize == vector<int>( { 0, 65535, 0, 65535 } ) )
{
return "uint16";
}
Expand Down
238 changes: 208 additions & 30 deletions startup/gui/outputs.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,45 +185,209 @@
# https://gitlab.com/3Delight/3delight-for-houdini/-/blob/master/ui/aov.cpp
# See `contrib/scripts/3delightOutputs.py` in this repository for a helper script.

GafferScene.Outputs.registerOutput(
"Interactive/3Delight/Beauty_3Delight",
IECoreScene.Output(
"beauty",
"ieDisplay",
"rgba",
{
"catalogue:imageName" : "Image",
"driverType" : "ClientDisplayDriver",
"displayHost" : "localhost",
"displayPort" : "${image:catalogue:port}",
"remoteDisplayType" : "GafferImage::GafferDisplayDriver",
"scalarformat" : "half",
"colorprofile" : "linear",
"filter" : "blackman-harris",
"filtersize" : 3.0,
}
)
)

GafferScene.Outputs.registerOutput(
"Batch/3Delight/Beauty_3Delight",
IECoreScene.Output(
"${project:rootDirectory}/renders/${script:name}/${renderPass}/beauty/beauty.####.exr",
"exr",
"rgba",
{
"scalarformat" : "half",
"colorprofile" : "linear",
"filter" : "blackman-harris",
"filtersize" : 3.0,
}
)
)

GafferScene.Outputs.registerOutput(
"Batch/3Delight/Crypto/Surface_Shader_Cryptomatte_Header",
IECoreScene.Output(
"${project:rootDirectory}/renders/${script:name}/${renderPass}/id.surfaceshader/id.surfaceshader.####.exr",
"exr",
"color builtin:id.surfaceshader",
{
"scalarformat" : "half",
"sortkey" : 1,
"filter" : "cryptomatteheader",
}
)
)

GafferScene.Outputs.registerOutput(
"Batch/3Delight/Crypto/Surface_Shader_Cryptomatte_Layer0",
IECoreScene.Output(
"${project:rootDirectory}/renders/${script:name}/${renderPass}/id.surfaceshader/id.surfaceshader.####.exr",
"exr",
"quad builtin:id.surfaceshader",
{
"scalarformat" : "float",
"sortkey" : 2,
"filter" : "cryptomattelayer0",
"customdrivername" : "Batch/3Delight/Crypto/Surface_Shader_Cryptomatte_Header",
}
)
)

GafferScene.Outputs.registerOutput(
"Batch/3Delight/Crypto/Surface_Shader_Cryptomatte_Layer2",
IECoreScene.Output(
"${project:rootDirectory}/renders/${script:name}/${renderPass}/id.surfaceshader/id.surfaceshader.####.exr",
"exr",
"quad builtin:id.surfaceshader",
{
"scalarformat" : "float",
"sortkey" : 3,
"filter" : "cryptomattelayer2",
"customdrivername" : "Batch/3Delight/Crypto/Surface_Shader_Cryptomatte_Header",
}
)
)

GafferScene.Outputs.registerOutput(
"Batch/3Delight/Crypto/Geometry_Cryptomatte_Header",
IECoreScene.Output(
"${project:rootDirectory}/renders/${script:name}/${renderPass}/id.geometry/id.geometry.####.exr",
"exr",
"color builtin:id.geometry",
{
"scalarformat" : "half",
"sortkey" : 1,
"filter" : "cryptomatteheader",
}
)
)

GafferScene.Outputs.registerOutput(
"Batch/3Delight/Crypto/Geometry_Cryptomatte_Layer0",
IECoreScene.Output(
"${project:rootDirectory}/renders/${script:name}/${renderPass}/id.geometry/id.geometry.####.exr",
"exr",
"quad builtin:id.geometry",
{
"scalarformat" : "float",
"sortkey" : 2,
"filter" : "cryptomattelayer0",
"customdrivername" : "Batch/3Delight/Crypto/Geometry_Cryptomatte_Header",
}
)
)

GafferScene.Outputs.registerOutput(
"Batch/3Delight/Crypto/Geometry_Cryptomatte_Layer2",
IECoreScene.Output(
"${project:rootDirectory}/renders/${script:name}/${renderPass}/id.geometry/id.geometry.####.exr",
"exr",
"quad builtin:id.geometry",
{
"scalarformat" : "float",
"sortkey" : 3,
"filter" : "cryptomattelayer2",
"customdrivername" : "Batch/3Delight/Crypto/Geometry_Cryptomatte_Header",
}
)
)

GafferScene.Outputs.registerOutput(
"Batch/3Delight/Crypto/Scene_Path_Cryptomatte_Header",
IECoreScene.Output(
"${project:rootDirectory}/renders/${script:name}/${renderPass}/id.scenepath/id.scenepath.####.exr",
"exr",
"color builtin:id.scenepath",
{
"scalarformat" : "half",
"sortkey" : 1,
"filter" : "cryptomatteheader",
}
)
)

GafferScene.Outputs.registerOutput(
"Batch/3Delight/Crypto/Scene_Path_Cryptomatte_Layer0",
IECoreScene.Output(
"${project:rootDirectory}/renders/${script:name}/${renderPass}/id.scenepath/id.scenepath.####.exr",
"exr",
"quad builtin:id.scenepath",
{
"scalarformat" : "float",
"sortkey" : 2,
"filter" : "cryptomattelayer0",
"customdrivername" : "Batch/3Delight/Crypto/Scene_Path_Cryptomatte_Header",
}
)
)

GafferScene.Outputs.registerOutput(
"Batch/3Delight/Crypto/Scene_Path_Cryptomatte_Layer2",
IECoreScene.Output(
"${project:rootDirectory}/renders/${script:name}/${renderPass}/id.scenepath/id.scenepath.####.exr",
"exr",
"quad builtin:id.scenepath",
{
"scalarformat" : "float",
"sortkey" : 3,
"filter" : "cryptomattelayer2",
"customdrivername" : "Batch/3Delight/Crypto/Scene_Path_Cryptomatte_Header",
}
)
)

for name, displayName, source, dataType in [
( "Ci", "Ci", "shader", "color" ),
( "Ci.direct", "Ci (direct)", "shader", "color" ),
( "Ci.indirect", "Ci (indirect)", "shader", "color" ),
( "Ci.direct", "Ci_(direct)", "shader", "color" ),
( "Ci.indirect", "Ci_(indirect)", "shader", "color" ),
( "diffuse", "Diffuse", "shader", "color" ),
( "diffuse.direct", "Diffuse (direct)", "shader", "color" ),
( "diffuse.indirect", "Diffuse (indirect)", "shader", "color" ),
( "hair", "Hair and Fur", "shader", "color" ),
( "subsurface", "Subsurface Scattering", "shader", "color" ),
( "diffuse.direct", "Diffuse_(direct)", "shader", "color" ),
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for the " " -> "_" changes here? Is it to align with the other outputs, or is there a technical reason as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The customdrivername implementation didn't work with spaces in the output name, so I had to replace them with _. With multipart output getting changed from using customdrivername to using the file name, there is no need for this change any more.

( "diffuse.indirect", "Diffuse_(indirect)", "shader", "color" ),
( "hair", "Hair_and_Fur", "shader", "color" ),
( "subsurface", "Subsurface_Scattering", "shader", "color" ),
( "reflection", "Reflection", "shader", "color" ),
( "reflection.direct", "Reflection (direct)", "shader", "color" ),
( "reflection.indirect", "Reflection (indirect)", "shader", "color" ),
( "reflection.direct", "Reflection_(direct)", "shader", "color" ),
( "reflection.indirect", "Reflection_(indirect)", "shader", "color" ),
( "refraction", "Refraction", "shader", "color" ),
( "volume", "Volume Scattering", "shader", "color" ),
( "volume.direct", "Volume Scattering (direct)", "shader", "color" ),
( "volume.indirect", "Volume Scattering (indirect)", "shader", "color" ),
( "volume", "Volume_Scattering", "shader", "color" ),
( "volume.direct", "Volume_Scattering_(direct)", "shader", "color" ),
( "volume.indirect", "Volume_Scattering_(indirect)", "shader", "color" ),
( "incandescence", "Incandescence", "shader", "color" ),
( "toon_base", "Toon Base", "shader", "color" ),
( "toon_diffuse", "Toon Diffuse", "shader", "color" ),
( "toon_specular", "Toon Specular", "shader", "color" ),
( "toon_matte", "Toon Matte", "shader", "color" ),
( "toon_tint", "Toon Tint", "shader", "color" ),
( "toon_base", "Toon_Base", "shader", "color" ),
( "toon_diffuse", "Toon_Diffuse", "shader", "color" ),
( "toon_specular", "Toon_Specular", "shader", "color" ),
( "toon_matte", "Toon_Matte", "shader", "color" ),
( "toon_tint", "Toon_Tint", "shader", "color" ),
( "outlines", "Outlines", "shader", "quad" ),
( "albedo", "Albedo", "shader", "color" ),
( "z", "Z (depth)", "builtin", "float" ),
( "P.camera", "Camera Space Position", "builtin", "point" ),
( "N.camera", "Camera Space Normal", "builtin", "point" ),
( "P.world", "World Space Position", "builtin", "point" ),
( "N.world", "World Space Normal", "builtin", "point" ),
( "Pref", "Reference Position", "attribute", "point" ),
( "shadow_mask", "Shadow Mask", "shader", "color" ),
( "z", "Z_(depth)", "builtin", "float" ),
( "P.camera", "Camera_Space_Position", "builtin", "point" ),
( "N.camera", "Camera_Space_Normal", "builtin", "point" ),
( "P.world", "World_Space_Position", "builtin", "point" ),
( "N.world", "World_Space_Normal", "builtin", "point" ),
( "Pref", "Reference_Position", "attribute", "point" ),
( "shadow_mask", "Shadow_Mask", "shader", "color" ),
( "st", "UV", "attribute", "point" ),
( "id.geometry", "Geometry Cryptomatte", "builtin", "float" ),
( "id.scenepath", "Scene Path Cryptomatte", "builtin", "float" ),
( "id.surfaceshader", "Surface Shader Cryptomatte", "builtin", "float" ),
( "relighting_multiplier", "Relighting Multiplier", "shader", "color" ),
( "relighting_reference", "Relighting Reference", "shader", "color" ),
( "motionvector", "Motion Vector", "builtin", "point" ),
( "occlusion", "Ambient Occlusion", "shader", "color" ),
( "relighting_multiplier", "Relighting_Multiplier", "shader", "color" ),
( "relighting_reference", "Relighting_Reference", "shader", "color" ),
( "motionvector", "Motion_Vector", "builtin", "point" ),
( "occlusion", "Ambient_Occlusion", "shader", "color" ),
] :
GafferScene.Outputs.registerOutput(
"Interactive/3Delight/{}/{}".format( source.capitalize(), displayName ),
Expand All @@ -236,6 +400,12 @@
"displayHost" : "localhost",
"displayPort" : "${image:catalogue:port}",
"remoteDisplayType" : "GafferImage::GafferDisplayDriver",
"scalarformat" : "half",
"colorprofile" : "linear",
"filter" : "blackman-harris",
"filtersize" : 3.0,
"customdrivername" : "",
"lightgroup" : "",
}
)
)
Expand All @@ -246,6 +416,14 @@
"${project:rootDirectory}/renders/${script:name}/${renderPass}/%s/%s.####.exr" % ( name, name ),
"exr",
"{} {}:{}".format( dataType, source, name ),
{
"scalarformat" : "half",
"colorprofile" : "linear",
"filter" : "blackman-harris",
"filtersize" : 3.0,
"customdrivername" : "",
"lightgroup" : "",
}
)
)

Expand Down
Loading