diff --git a/src/GafferCycles/IECoreCyclesPreview/Renderer.cpp b/src/GafferCycles/IECoreCyclesPreview/Renderer.cpp index 353e3c39818..6996c649098 100644 --- a/src/GafferCycles/IECoreCyclesPreview/Renderer.cpp +++ b/src/GafferCycles/IECoreCyclesPreview/Renderer.cpp @@ -821,7 +821,15 @@ class ShaderCache : public IECore::RefCounted } m_shaderAssignPairs.clear(); - // TODO: Optimise + /// \todo There are several problems here : + /// + /// - We're clobbering the `tex_mapping.rotation` parameter, which is exposed to users + /// but now has no effect for them. This also prevents us getting the orientation of USD + /// DomeLights correct - see ShaderNetworkAlgo. + /// - We're iterating through all N lights just to find the background light, and we're + /// doing it even when the transform hasn't changed. Can't we just do this in `CyclesLight::transform()`? + /// - The light shader was created via `ShaderCache::get()`, and could therefore be shared + /// between several lights, so we're not at liberty to clobber the shader anyway. for( ccl::Light *light : m_scene->lights ) { if( light->get_light_type() == ccl::LIGHT_BACKGROUND ) diff --git a/src/GafferCycles/IECoreCyclesPreview/ShaderNetworkAlgo.cpp b/src/GafferCycles/IECoreCyclesPreview/ShaderNetworkAlgo.cpp index 530a31f4e83..70f97e48d7f 100644 --- a/src/GafferCycles/IECoreCyclesPreview/ShaderNetworkAlgo.cpp +++ b/src/GafferCycles/IECoreCyclesPreview/ShaderNetworkAlgo.cpp @@ -985,6 +985,9 @@ void transferUSDTextureFile( ShaderNetwork *network, InternedString shaderHandle IECore::msg( IECore::Msg::Warning, "convertUSDShaders", fmt::format( "Unsupported value \"{}\" for DomeLight.format", format ) ); format = "equirectangular"; } + /// \todo We're missing a 90 degree rotation about Y here, which we could easily + // add as a `tex_mapping.rotation` parameter value. Except that it would get clobbered + // by `ShaderCache::updateShaders()` in `Renderer.cpp` - see additional comments there. imageShader->parameters()[g_projectionParameter] = new StringData( format ); imageShader->parameters()[g_texMappingScaleParameter] = new V3fData( V3f( -1.0f, 1.0f, 1.0f ) ); imageShader->parameters()[g_texMappingYMappingParameter] = new StringData( "z" ); @@ -1083,6 +1086,13 @@ void IECoreCycles::ShaderNetworkAlgo::convertUSDShaders( ShaderNetwork *shaderNe newShader = new Shader( "background_light", "cycles:light" ); transferUSDLightParameters( shaderNetwork, handle, shader.get(), newShader.get() ); transferUSDTextureFile( shaderNetwork, handle, shader.get(), newShader.get() ); + /// \todo This brings performance into line with a default-created `background_light` + /// using the CyclesLight node, which will define a resolution of 1024 to override + /// the auto-resolution-from-map behaviour of Cycles. If we don't do this, viewer + /// update gets very choppy when using high resolution maps. I suspect this may + /// indicate a problem whereby we are causing Cycles to rebuild importance maps + /// unnecessarily when moving the free camera. + newShader->parameters()["map_resolution"] = new IntData( 1024 ); } else if( shader->getName() == "RectLight" ) {