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

gkocov · 2024-01-23T06:20:33Z

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

Support for exporting Gaffer output parameters as both 3Delight outputdriver and outputlayer attributes. This enables direct control of 3Delight's outputlayer attributes like scalarformat and colorprofile (to set the rendered image bit-depth format and color space without the limitations of the quantize Gaffer parameter), filter type, filter width size, etc.
Support for rendering to multilayer EXRs by using the customdrivername output parameter. The updated Cryptomatte presets showcase how this is used. Drag and dropping the Name parameter from one output into the customdrivername parameter of another output is the easiest way to get the multilayer EXRs set up.
Support for light groups (lightsets in 3Delight terminology) using the lightgroup output parameter. The lightgroup output parameter accepts a space separated list of light objects that will determine which lights will be used for the rendering of the given output. Drag and drop from the Hierarchy View to the lightgroup output parameter is the easiest way to populate it.
In order for the lightgroups to work I had to change the order when outputs are generated in src/GafferScene/Render.cpp by moving them from second place after options to last place after objects. This is because the NSI scene needs to have the light object node defined in order to create a connection between it and the outputlayer node.
Updated output presets to expose all of the above functionality, including fully functional multilayer Cryptomatte presets.

Breaking changes

The change to the src/GafferScene/Render.cpp is a potential breaking change, not sure. I tested Cycles and everything seemed to work as expected, but I don't have access to Arnold, so I can't test it unfortunately.

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.

gkocov · 2024-02-03T04:56:33Z

Synced to the latest main branch revision and updated the 3Delight outputs to have ${renderPass} in the file name.

gkocov · 2024-05-01T21:41:18Z

@johnhaddon since the current PR has conflicts with some of the recently added changes, would you prefer for me to close this one and split the intended changes/features from it into several smaller new PRs?

johnhaddon

Sorry for the extended delay in getting to this one @gkocov. In truth I've been avoiding it a little because outputs are a bit of a thorny area and I'm unsure how best to proceed.

Part of the problem is that the Outputs system is one of Gaffer's weakest areas, desperately needing better alignment among renderers and improved definition/management for users. This is coming to a head a bit as we continue push on the Render Pass Editor, and find the Output system lacking for what we want to do. Continuing to have the renderers diverge isn't ideal if we want to pull them all back into alignment soon. Through no fault of your own, this PR is somewhat caught up in that mess.

I have to confess that I also find the lightset implementation unconvincing in its current form - I can't see it scaling sufficiently to be useful in many production scenarios. Doing better would be a fair bit more work, but I think might be preferable.

would you prefer for me to close this one and split the intended changes/features from it into several smaller new PRs?

Yes please, I think that would be useful. It would also be great to include unit tests this time, so we have some coverage for when we hopefully start refactoring all the renderers into alignment.

Perhaps we could break it down in this order :

Custom layer params, scalarformat, quantize fix. Although these represent divergence from a theoretical future where we've defined standard cross-renderer names for everything, I don't think that's a good enough reason to hobble 3Delight when we've already done it for Cycles/Arnold. Of the three, I think 3Delight's parameterisation may be closer to something we might want to standardise on anyway.
Multipart, based on filename, along with output presets for cryptomatte.
Light groups, after a bit of head scratching about what that might look like in an ideal world.

Hopefully that sounds OK?

Cheers...
John

johnhaddon · 2024-05-02T09:21:08Z

src/GafferScene/Render.cpp

@@ -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() );


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?

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.

johnhaddon · 2024-05-02T09:24:36Z

src/IECoreDelight/Renderer.cpp

-			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 );


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

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.

johnhaddon · 2024-05-02T09:28:56Z

src/IECoreDelight/Renderer.cpp

+			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 );
+			}


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?

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.

johnhaddon · 2024-05-02T09:31:21Z

src/IECoreDelight/Renderer.cpp

+			if( lightGroup != "" )
+			{
+				for( string lightGroupToken : lightGroupTokens )
+				{
+					NSIConnect(
+						m_context,
+						lightGroupToken.c_str(), "",
+						m_layerHandle.name(), "lightset",
+						0, nullptr
+					);
+				}
+			}


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).

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.

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?

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

johnhaddon · 2024-05-02T09:33:27Z

startup/gui/outputs.py

-		( "diffuse.indirect", "Diffuse (indirect)", "shader", "color" ),
-		( "hair", "Hair and Fur", "shader", "color" ),
-		( "subsurface", "Subsurface Scattering", "shader", "color" ),
+		( "diffuse.direct", "Diffuse_(direct)", "shader", "color" ),


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

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.

gkocov · 2024-05-03T00:38:04Z

Thanks so much for looking into this @johnhaddon ! Everything you mentioned sounds good and I'll look into setting up individual smaller PRs along with tests for the changes. Replying inline on the specific code comments/questions.

johnhaddon · 2024-05-03T08:13:26Z

I'll look into setting up individual smaller PRs along with tests for the changes.

Great, thanks Goran! I'll close this PR now then, in anticipation of the future PRs...

gkocov added 3 commits January 23, 2024 00:39

3Delight extended outputs - outputlayers, lightsets, multilayer EXRs

96c8a02

Merge branch 'GafferHQ:main' into 3dl_outputs

de3d35a

adding renderPass to 3delight outputs file name

047804f

gkocov added 2 commits March 15, 2024 17:48

Merge branch 'GafferHQ:main' into 3dl_outputs

9c590db

Merge branch 'GafferHQ:main' into 3dl_outputs

525aacf

gkocov mentioned this pull request Apr 8, 2024

3Delight output not respecting scalarformat value #5776

Open

gkocov added 2 commits April 8, 2024 11:39

Merge branch 'GafferHQ:main' into 3dl_outputs

6b2e7a4

Merge branch 'GafferHQ:main' into 3dl_outputs

31caf98

johnhaddon reviewed May 2, 2024

View reviewed changes

johnhaddon closed this May 3, 2024

gkocov mentioned this pull request May 27, 2024

3Delight render sets and lightgroups support #5869

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

gkocov commented Jan 23, 2024

gkocov commented Feb 3, 2024

gkocov commented May 1, 2024

johnhaddon left a comment

johnhaddon May 2, 2024

gkocov May 3, 2024

johnhaddon May 2, 2024

gkocov May 3, 2024

johnhaddon May 2, 2024

gkocov May 3, 2024

johnhaddon May 2, 2024

gkocov May 3, 2024

johnhaddon May 3, 2024

gkocov May 3, 2024

johnhaddon May 2, 2024

gkocov May 3, 2024

gkocov commented May 3, 2024

johnhaddon commented May 3, 2024

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

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

Conversation

gkocov commented Jan 23, 2024

Breaking changes

Checklist

gkocov commented Feb 3, 2024

gkocov commented May 1, 2024

johnhaddon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkocov commented May 3, 2024

johnhaddon commented May 3, 2024