Fix T71123: OptiX error in Cycles viewport when adding HDRI
ClosedPublic
Actions

Authored by Patrick Mours (pmoursnv) on Oct 28 2019, 4:01 PM.

Details

Reviewers

Brecht Van Lommel (brecht)
Sergey Sharybin (sergey)

Commits

rBb45828ebe9c6: Fix T71123: OptiX error in Cycles viewport when adding HDRI

Summary

Adding an environment texture to the background causes a SHADER_EVAL_BACKGROUND launch. The OptiX implementation only creates the pipeline for that launch when kernels were loaded with the "use_background_light" feature and that feature is only set if the background light is enabled. But during synchronization with Cycles the "is_enabled" flag is never updated again after it was initially set. It is set to "false" via "LightManager::disable_ineffective_light" while no background texture node existed, but never set to "true" again after one was added. As such kernels were not reloaded with the required feature and the launch failed.

This change sets the "is_enabled" flag back to its default value of "true" when lights are synced. It is then later updated in "LightManager::disable_ineffective_light" if necessary as before. But now it retains the correct value after a background node was added to the scene.

Diff Detail

Repository

rB Blender

Branch

cycles_viewport_hdri_fix (branched from master)

Build Status

Buildable 5467
Build 5467: arc lint + arc unit

Event Timeline

Patrick Mours (pmoursnv) created this revision.Oct 28 2019, 4:01 PM

Harbormaster completed remote builds in B5467: Diff 19225.Oct 28 2019, 4:01 PM

Patrick Mours (pmoursnv) mentioned this in T71123: 2.81 cycles viewport crash when adding hdri (optix).Oct 28 2019, 7:31 PM

That is a good catch!

I feel like the fix belongs to light.cpp: is not blender-integration specific, It is almost like a missing loop in there to enable background:

2index 3--- 4+++ 5@@ 6 */ 7 Shader 8 9- 10- 11- 12- 13- 14- 15- 16+ 17+ 18+ 19+ 20+ 21 } 22 } 23 }

but specific to optimization happening in light.cpp.
href="/P1147">P1147 (An Untitled Masterwork)

diff --git a/intern/cycles/render/light.cpp b/intern/cycles/render/light.cpp 8c7a21da561..dc3f7c8f8ac 100644 a/intern/cycles/render/light.cpp b/intern/cycles/render/light.cpp -221,13 +221,11 @@ void LightManager::disable_ineffective_light(Scene *scene) *shader = (scene->background->shader) ? scene->background->shader : scene->default_background; bool disable_mis = !(has_portal || shader->has_surface_spatial_varying); if (disable_mis) { VLOG(1) << "Background MIS has been disabled.\n"; foreach (Light *light, scene->lights) { if (light->type == LIGHT_BACKGROUND) { light->is_enabled = false; } const bool disable_mis = !(has_portal || shader->has_surface_spatial_varying); VLOG_IF(1, disable_mis) << "Background MIS has been disabled.\n"; foreach (Light *light, scene->lights) { if (light->type == LIGHT_BACKGROUND) { light->is_enabled = !disable_mis;

Otherwise every integration will need to have the fix on their side.

Good point. That along doesn't fix it though, because kernels are not reloaded between the calls to disable_ineffective_light and device_update_background (which is where the launch happens) in the light manager update routine. So the light needs to be enabled before the update already.

I think having both the light.cpp code + the blender_object.cpp code makes sense. Considering that the light is first created in blender_object.cpp via light_map.sync (which sets is_enabled to true in the constructor), it seems right for it to set is_enabled again during an update there too.