In ParaView 5.9, when ray tracing is enabled, we are present with group called Path Traced Environment. From what I see, it’s largely same as the Background and does indeed override whatever is set in the Background. In which case, any objections to removing this duplicate Path Traced Environment and consolidating with the Background?
Make sense
It is not the same thing and should not be removed, but there may be more intuitive ways to present it to users than what we have.
Background acts as a backplate for the ray tracer, it does not move when the camera moves. Path traced environment is more like a sky box, it has an orientation. Furthermore background does not show up in reflections and such on objects in the scene, whereas PTE does.
The “Background mode” control, it lets you switch between three behaviors - use only backplate, use only PTE, or both in which you see backplate on background pixels but still see PTE reflections on objects.
Here’s why the implementation is incorrect:
-
vtkPVRenderView::SetEnvironmentalBGTexture
andvtkPVRenderView::UpdateSkybox
both update the samethis->GetRenderer()->SetEnvironmentTexture(...)
. -
vtkPVRenderView::SetUseEnvironmentLighting
andvtkPVRenderView::SetTexturedEnvironmentalBG
both update the samethis->GetRenderer()->SetUseImageBasedLighting(...)
.
Two different properties setting the same thing under the covers is not robust and points to incorrect implementation. Someone needs to clean it up.
FYI @LfxPaul
We could disambiguate by moving skybox over to environment background.
Ie.
- rename path traced environment to environment
- make environment and background mode widgets not require ray tracing turned on
- move (with appropriate deprecation forwarding) sky box controls that are under background over to environment
- hide “both” mode of “background mode” control for rasterization, or add the feature there.
- update ospray version and get support for skybox (cube) formatted textures and environment support in the ray caster
Hey @utkarsh.ayachit, as @Dave_DeMarle said they may seem similar, but serve very different purposes when used together. The env maps needs to be very high res (at least 8k) to look as a background as well, which are very difficult and expensive to find. This feature allows us to use a low res or more generic map to get good lighting into the scene, and then put whatever static 1080p backplate we want into the scene as the background.
This was a key feature that my company paid to have implemented as part of our recent contract with Kitware, and we now use it all of the time. PLEASE do not remove it.
I would not be totally apposed to rearranging the functionality to make it clearer, but as Dave will remember there were quite a bit of issues getting this to work correctly so I ask that you verify the updated functionality with either Dave, @aron.helser, or I.
Please let me know if you have questions, thanks.
Gravedigging this thread a bit as we kind of had the same conversation on this issue : https://gitlab.kitware.com/paraview/paraview/-/issues/20889.
I understand how useful the feature of dissociating background and environment lighting is, however I still think the UI is not in a good state right now as we can have the same result with multiple UI configuration. Also it is possible to use the Both
background mode even when not using a pathtracer i.e. when using the GPU implementation of Principled instead (PBR lighting). So AFAIK it makes no sense to hide the section Path Traced Environment when ray tracing is not enabled.
Here’s a new UI proposition (bold are sections, italic are properties) :
-
Background
- [Color,Gradient,Image,Skybox] (dropdown menu)
- […] (properties relative to the background mode)
- Remove the current “Use as envrionment lighting”
-
Environment Lighting
- [Use background as environment] (checkbox, only shown when pair {backend,background mode} supports it. When enabled, hide the properties below)
- [Color,Gradient,Skybox] (dropdown menu)
- […] (properties relative to the envrionment mode)
-
Ray Traced Rendering
- Remove the current Background mode
Thoughts ?
I’ve opened a related issue: https://gitlab.kitware.com/paraview/paraview/-/issues/21270