GafferUSD : Added Cycles-specific light parameters to USD Lux lights #6249
Conversation
There was a problem hiding this comment.
Thanks Alex - would be great to get this merged. I've noted a couple of issues inline, and it would also be great to get the new parameters registered in the LightEditor's USD section, in the same way the Arnold ones are.
When we introduced the Arnold parameters, we were a bit concerned about what the UI would look like when we added other renderers. And I'm about to add RenderMan too. So I think we'll need to do a bit of head-scratching about how to filter them for what the user most cares about. That can be done separately from this PR though.
usdSchemas/GafferCycles.usda
Outdated
|
|
||
| bool inputs:cycles:use_mis = true ( | ||
| displayGroup = "Refine" | ||
| displayName = "Multiple Importance (Cycles)" |
There was a problem hiding this comment.
This and several other labels are too wide for the NodeEditor in Gaffer. Could we shorten them to fit while keeping them intelligible?
There was a problem hiding this comment.
Done, maybe a little vague but it isn't too much different to Arnold now.
usdSchemas/GafferCycles.usda
Outdated
| bool inputs:cycles:use_camera = true ( | ||
| displayGroup = "Refine" | ||
| displayName = "Visible to Camera (Cycles)" | ||
| ) |
There was a problem hiding this comment.
I tested this with a RectLight in Gaffer's viewer, and it didn't seem to have any effect? Is there some plumbing we need in the Cycles backend to pass the values through?
There was a problem hiding this comment.
Oh silly me I forgot the important part in ShaderNetworkAlgo that takes away the namespace prefix, fixed and should work now.
boberfly
force-pushed
the
features/cyclesUSDLightSchema
branch
from
19f264d to
bda649a
Compare
Oh yeah I found that and added into this PR, I shuffled the USD Lux lights metadata to the top of the file also, as it looks like the metadata gets modified after for Arnold but Cycles was above the USD Lux declarations. |
| "use_mis", "use_camera", "use_diffuse", "use_glossy", "use_transmission", "use_scatter", "use_caustics", | ||
| "spread", "map_resolution", "max_bounces" | ||
| ] ) : | ||
| Gaffer.Metadata.registerValue( GafferUSD.USDLight, f"parameters.cycles:{parameter}", "layout:index", 2000 + i ) |
There was a problem hiding this comment.
I think it'd be worth respecting GAFFERCYCLES_HIDE_UI and hiding these plugs if the user has decided to opt out of Cycles in the UI. I think we could do this with a visibility activator doing the usual Cycles environment variable checks along the lines of:
Gaffer.Metadata.registerValue( GafferUSD.USDLight, "parameters", "layout:activator:cyclesUIEnabled", lambda x : os.environ.get( "CYCLES_ROOT" ) and os.environ.get( "GAFFERCYCLES_HIDE_UI", "" ) != "1" )
Gaffer.Metadata.registerValue( GafferUSD.USDLight, "parameters.cycles:*", "layout:visibilityActivator", "cyclesUIEnabled" )
usdSchemas/GafferCycles.usda
Outdated
| ) | ||
| { | ||
|
|
||
| float inputs:cycles:spread = 1.0 ( |
There was a problem hiding this comment.
This isn't matching the default spread value on a Cycles light, is that intentional?
There was a problem hiding this comment.
Good catch I forgot we went to degrees on this one.
usdSchemas/GafferCycles.usda
Outdated
|
|
||
| float inputs:cycles:spread = 1.0 ( | ||
| displayGroup = "Geometry" | ||
| displayName = "Spread" |
There was a problem hiding this comment.
This should be Spread (Cycles).
usdSchemas/GafferCycles.usda
Outdated
| ) | ||
| { | ||
|
|
||
| float inputs:cycles:spread = 1.0 ( |
There was a problem hiding this comment.
Like the above, this also isn't matching the Cycles default.
usdSchemas/GafferCycles.usda
Outdated
| ) | ||
| { | ||
|
|
||
| int inputs:cycles:map_resolution = 0 ( |
There was a problem hiding this comment.
This also isn't matching the Cycles default.
There was a problem hiding this comment.
I was checking the original source code and it has it set to 0 for automatic, I didn't realise it is set to 1024 on the Gaffer-end - best to not deviate away, and the user can always opt for 0.
Now that I think about it, I think John must've set this for speeding up the viewport from what I remember, so that Cycles doesn't generate ungodly-sized resolutions and slow the viewport up.
usdSchemas/GafferCycles.usda
Outdated
| { | ||
|
|
||
| int inputs:cycles:map_resolution = 0 ( | ||
| displayGroup = "Refine" |
There was a problem hiding this comment.
For Arnold, we've put this in "Sampling". Might be worth keeping them together?
boberfly
force-pushed
the
features/cyclesUSDLightSchema
branch
from
bda649a to
1b4ea61
Compare
boberfly
force-pushed
the
features/cyclesUSDLightSchema
branch
from
1b4ea61 to
3d5916c
Compare
Generally describe what this PR will do, and why it is needed
Related issues
Dependencies
Breaking changes
Checklist