-
Notifications
You must be signed in to change notification settings - Fork 207
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
Changes from all commits
96c8a02
de3d35a
047804f
9c590db
525aacf
6b2e7a4
31caf98
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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 ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the reason for taking There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. The |
||
|
||
m_layerHandle = DelightHandle( context, "outputLayer:" + name, ownership, "outputlayer", layerParams ); | ||
|
||
|
@@ -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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Completely agreed. Similar to 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's not very well documented, but any sets with names matching There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
@@ -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"; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" ), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
( "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 ), | ||
|
@@ -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" : "", | ||
} | ||
) | ||
) | ||
|
@@ -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" : "", | ||
} | ||
) | ||
) | ||
|
||
|
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.